fix(parser): BytesToHexWithPrefix drops leading zero bytes from addresses - #216
Open
boleklebovski wants to merge 1 commit into
Open
fix(parser): BytesToHexWithPrefix drops leading zero bytes from addresses#216boleklebovski wants to merge 1 commit into
BytesToHexWithPrefix drops leading zero bytes from addresses#216boleklebovski wants to merge 1 commit into
Conversation
TrimLeft takes a cutset, so every leading zero nibble was removed and distinct addresses collapsed onto the same key. rollytics already has the correct form of this function. Signed-off-by: boleklebovski <160799963+boleklebovski@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Walkthrough
ChangesHexadecimal prefix encoding
Estimated code review effort: 1 (Trivial) | ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
pkg/parser/accounts.go:60:strings.TrimLefttakes a cutset, not a prefix — it removes every leading'0'character from the encoded string. Any address with leading zero bytes comes back truncated, and the result is not merely shorter, it is a different value:0x00…010x10x00000000000000000000000000000000000000010x00…010x10x0000…0001(64 hex chars)0x000a00…0xa0000000000000000000000000000000000000x000a0000000000000000000000000000000000000xdeadbeef0xdeadbeef0xdeadbeefTwo things stand out. A 20-byte and a 32-byte address that both end in
01collapse onto the same string, so they collide as lookup keys. And in the third row the remaining characters shift left, producing a well-formed but entirely different address.That string is used as a query key throughout the API layer —
api/handlers/module.go(6 call sites) andapi/handlers/nft.go(4 call sites) pass it straight intoGetModuleById,GetCollectionsByAccountAddress,GetCollectionActivitiesand friends. Any account, module or collection whose address starts with a zero byte is looked up under the wrong key, and the lookup simply returns nothing.Fix
This is exactly what the sister repo already does —
initia-labs/rollyticsutil/util.go:49:Same function name, same purpose, no
TrimLeft. That looked like the strongest evidence of intent, so I matched it exactly.Scope
One line.
stringsis still used elsewhere in the file, so the import stays;gofmtis clean.I deliberately did not touch
AccAddressFromStringabove it, which also callsTrimLeft. There the"0x"prefix is removed first withTrimPrefixand the zero-strip is followed by re-padding to 40 or 64 characters, so it is deliberate normalisation rather than the same mistake. There is arguably an edge case when a 32-byte address strips down to ≤40 characters and gets padded back to 20 bytes, but that depends on address-width conventions I am not in a position to judge — happy to open a separate issue if it is worth a look.Verification
I could not run the package's tests here —
pkg/mqand themovevmdependency need cgo libraries I do not have in this environment — so I verified the semantics with a standalone program on Go 1.25.8 that runs both the old and new expression over the cases in the table above. The outputs are as shown.Summary by CodeRabbit
0xprefix.