Skip to content

fix: Leading 0 in Token Addresses#310

Open
jonas089 wants to merge 5 commits intoopenintentsframework:mainfrom
celestiaorg:jonas/fix-address-truncate
Open

fix: Leading 0 in Token Addresses#310
jonas089 wants to merge 5 commits intoopenintentsframework:mainfrom
celestiaorg:jonas/fix-address-truncate

Conversation

@jonas089
Copy link
Contributor

@jonas089 jonas089 commented Mar 13, 2026

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

  • Bug Fixes
    • Fixed address normalization behavior for non-standard hex formats. Modified padding logic for certain address input patterns to ensure consistent and correct address handling throughout the system.

jonas089 and others added 5 commits March 4, 2026 00:23
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
@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

📝 Walkthrough

Walkthrough

Modified the parse_address function in the conversion utilities to adjust padding logic for non-standard hex inputs. Inputs between 40 and 64 hex characters are now padded to 40 characters (20 bytes) instead of 64 characters (32 bytes), changing how such inputs are normalized before decoding.

Changes

Cohort / File(s) Summary
Address Parsing Logic
crates/solver-types/src/utils/conversion.rs
Updated padding threshold in parse_address from 64 hex characters to 40 hex characters for non-standard length inputs, affecting Address normalization behavior.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested reviewers

  • shahnami
  • pepebndc

Poem

🐰 A padding shift, so small yet keen,
From sixty-four to forty seen,
Address parsing, now refined,
Leaves odd-length inputs well-aligned! 🎯

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description explains the bug fix and context, but does not follow the provided template structure with the required 'Summary', 'Testing Process', and 'Checklist' sections. Restructure the description to follow the template with explicit sections for Summary, Testing Process, and Testing Checklist with checkboxes for issue references and unit tests.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing an issue where leading zeros in token addresses were being dropped during parsing.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can generate a title for your PR based on the changes with custom instructions.

Set the reviews.auto_title_instructions setting to generate a title for your PR based on the changes in the PR with custom instructions.

@codecov
Copy link

codecov bot commented Mar 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🟡 Minor

Fix 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9f145 and f2ea0ab.

📒 Files selected for processing (1)
  • crates/solver-types/src/utils/conversion.rs

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! makes sense:

I will approve it, but additionally, can you reject empty input explicitly and add unit tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants