Note tag 32bit routing(#2309)#2329
Conversation
| let len = 32; | ||
|
|
||
| let tag = NoteTag::with_custom_account_target(account_id, len)?; | ||
|
|
||
| let prefix = account_id.prefix().as_u64(); | ||
| let expected = (prefix >> (64 - len)) as u32; | ||
|
|
||
| assert_eq!(tag.as_u32(), expected, "full 32-bit tag should match account prefix"); |
There was a problem hiding this comment.
Same here regarding using the previous test setup as it's harder to understand now. All we need here is a simple assertion:
assert_eq!(
(account_id.prefix().as_u64() >> 32) as u32,
tag.as_u32(),
"32 most significant bits should match"
);There was a problem hiding this comment.
I think this comment wasn't addressed.
…or tag length handling
| let len = 32; | ||
|
|
||
| let tag = NoteTag::with_custom_account_target(account_id, len)?; | ||
|
|
||
| let prefix = account_id.prefix().as_u64(); | ||
| let expected = (prefix >> (64 - len)) as u32; | ||
|
|
||
| assert_eq!(tag.as_u32(), expected, "full 32-bit tag should match account prefix"); |
There was a problem hiding this comment.
I think this comment wasn't addressed.
swaploard
force-pushed
the
note-tag-32bit-routing(#2309)
branch
from
9e70508 to
5c53c17
Compare
PhilippGackstatter
left a comment
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks!
We usually try to avoid force pushing after review have started. The latest force push has rolled back a few suggestions I committed earlier. I have now pushed some other fixes, like for build_note_tag_for_network_account.
Let's wait for another review before merging this.
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you. I left some comments inline - but we can probably address them in a follow-up.
3 participants
Refactor NoteTag account targets to contain up to 32 bits (#2309)
This PR completes the refactor discussed after #2285 by allowing NoteTag account targets to use the full 32-bit space, while keeping RoutingParameters constrained to a representable format.