fix: Leading 0 in Token Addresses#310
fix: Leading 0 in Token Addresses#310jonas089 wants to merge 5 commits intoopenintentsframework:mainfrom
Conversation
Add [lib] target to solver-service so it can be used as a library dependency. lib.rs already exists — this just declares it in Cargo.toml. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat: expose solver-service as a library crate
📝 WalkthroughWalkthroughModified the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/solver-types/src/utils/conversion.rs (1)
147-156:⚠️ Potential issue | 🟡 MinorFix correctly restores leading zeros for truncated address inputs.
The catch-all match arm now pads short inputs (< 40 chars) to 40 characters directly, properly handling addresses that were serialized without leading zeros (e.g., a 39-char hex string from a U256 serialization of an address starting with
0x0...).However, unit tests should be added to cover this edge case and prevent regression. Consider adding tests for:
- 39-char input (the reported bug case)
- Other short inputs (< 40 chars)
- Empty string input
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/solver-types/src/utils/conversion.rs` around lines 147 - 156, Add unit tests for the hex-padding logic in crates/solver-types/src/utils/conversion.rs that exercise the hex_clean -> padded_hex behavior: create tests that pass a 39-char hex string, several other short strings (<40 chars), and an empty string into the conversion function (the module containing the hex_clean/padded_hex logic) and assert the returned padded string matches expected lengths and leading-zero restoration (expect 40 chars for padded 20-byte addresses and 64 chars when code branches to 32-byte padding). Place tests as #[cfg(test)] unit tests near the conversion code or in the crate's tests module and use assert_eq!/assert! checks to ensure no regression for these edge cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@crates/solver-types/src/utils/conversion.rs`:
- Around line 147-156: Add unit tests for the hex-padding logic in
crates/solver-types/src/utils/conversion.rs that exercise the hex_clean ->
padded_hex behavior: create tests that pass a 39-char hex string, several other
short strings (<40 chars), and an empty string into the conversion function (the
module containing the hex_clean/padded_hex logic) and assert the returned padded
string matches expected lengths and leading-zero restoration (expect 40 chars
for padded 20-byte addresses and 64 chars when code branches to 32-byte
padding). Place tests as #[cfg(test)] unit tests near the conversion code or in
the crate's tests module and use assert_eq!/assert! checks to ensure no
regression for these edge cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1089e365-cb5e-46ee-b319-ac7479b781e7
📒 Files selected for processing (1)
crates/solver-types/src/utils/conversion.rs
There was a problem hiding this comment.
thanks! makes sense:
I will approve it, but additionally, can you reject empty input explicitly and add unit tests?
This PR fixes a bug where leading 0s in Token addresses are dropped.
I noticed this because the USDC deployment on our EVM chain starts with a "0" e.g. 0x0... and was truncated to 39 digits instead of 40.
This one-line fix should prevent issues like this from happening again in the future.
Summary by CodeRabbit