Expose native UNIT as an ERC20 precompile - #86
Merged
Conversation
motoZ-crypto
added
T-enhancement
📝 WalkthroughWalkthroughThis PR introduces a complete ERC-20 precompile that exposes the chain's native balance token through standard ERC20 methods and EIP-2612 gasless approvals, integrates it into the runtime's EVM execution layer, and provides comprehensive unit and integration tests. ChangesERC-20 Balances Precompile with EIP-2612
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Rust |  |
May 14, 2026 7:46p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
precompiles/balances-erc20/src/lib.rs (1)
318-349: ⚡ Quick winZero-value handling is inconsistent across operations.
The
deposit()function rejects zero-value calls (lines 328-330) to help catch mistakes, butwithdraw()(lines 294-297) anddo_transfer()(lines 395-398) both silently allow zero-value operations. This creates an inconsistency:
- Standard ERC20 allows zero-value transfers
- WETH9 allows zero-value deposits
- This precompile rejects zero-value deposits but allows zero-value withdrawals/transfers
Users expecting WETH9-compatible behavior may be surprised. Consider either:
- Allowing zero-value deposits for consistency, or
- Documenting this deliberate deviation prominently in user-facing docs
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@precompiles/balances-erc20/src/lib.rs` around lines 318 - 349, The deposit() function currently reverts on zero-value while withdraw() and do_transfer() allow it; make deposit() behavior consistent by removing the zero-value revert in deposit() (the branch checking value.is_zero()) and instead treat a zero-value deposit as a no-op (return Ok(()) or let execution proceed without transferring funds but still record logs if desired), aligning deposit() with withdraw() and do_transfer(); reference deposit(), withdraw(), and do_transfer() when updating the logic so tests and docs can be adjusted if needed.precompiles/balances-erc20/src/eip2612.rs (1)
54-59: 💤 Low valueLarge deadline values become effectively non-expiring.
If a permit is signed with
deadline > u64::MAX(line 56), the conversion usesunwrap_or(u64::MAX), making the permit valid until the year ~584 billion. While this is far-future and safe (doesn't panic), it could be surprising if tooling accidentally generates oversized deadlines.Consider documenting this behavior or capping deadlines more explicitly if non-expiring permits are not intended.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@precompiles/balances-erc20/src/eip2612.rs` around lines 54 - 59, The current permit deadline conversion silently maps out-of-range values to u64::MAX (see the conversion of `deadline` into `deadline_secs` near `now_ms`/`now_secs` using `unwrap_or(u64::MAX)`), which makes oversized deadlines effectively non-expiring; change this to either (a) explicitly reject overflowed/invalid deadlines by returning an error instead of using `unwrap_or`, or (b) clamp the converted deadline to a well-documented MAX_DEADLINE constant and add a comment documenting that behavior; update the conversion logic where `deadline.try_into().unwrap_or(u64::MAX)` is used (and any related checks before `revert("Permit expired")`) and ensure unit tests reflect the chosen behavior.precompiles/balances-erc20/src/tests.rs (1)
698-700: ⚡ Quick winUse an independent EIP-712 vector for at least one permit path assertion.
These expectations are derived from the same helper functions used by runtime logic, so a shared hashing bug can still pass. Add one fixed external vector (known domain separator + digest/signature outcome) to decouple the test oracle.
Also applies to: 725-732
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@precompiles/balances-erc20/src/tests.rs` around lines 698 - 700, The test currently computes the domain separator and digest using the same helpers as the runtime (compute_domain_separator, compute_permit_struct_hash, compute_eip712_digest with PRECOMPILE_ADDRESS and NativeErc20Metadata::NAME), so add at least one assertion that uses an independent, fixed EIP‑712 test vector: hardcode a known domain separator and expected digest/signature outcome for a specific owner/spender/value/nonce/deadline and assert equality against the computed values; update the same pattern used around the other permit assertions to use this external vector to ensure a hashing bug in the helpers cannot make the test pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@precompiles/balances-erc20/src/eip2612.rs`:
- Around line 54-59: The current permit deadline conversion silently maps
out-of-range values to u64::MAX (see the conversion of `deadline` into
`deadline_secs` near `now_ms`/`now_secs` using `unwrap_or(u64::MAX)`), which
makes oversized deadlines effectively non-expiring; change this to either (a)
explicitly reject overflowed/invalid deadlines by returning an error instead of
using `unwrap_or`, or (b) clamp the converted deadline to a well-documented
MAX_DEADLINE constant and add a comment documenting that behavior; update the
conversion logic where `deadline.try_into().unwrap_or(u64::MAX)` is used (and
any related checks before `revert("Permit expired")`) and ensure unit tests
reflect the chosen behavior.
In `@precompiles/balances-erc20/src/lib.rs`:
- Around line 318-349: The deposit() function currently reverts on zero-value
while withdraw() and do_transfer() allow it; make deposit() behavior consistent
by removing the zero-value revert in deposit() (the branch checking
value.is_zero()) and instead treat a zero-value deposit as a no-op (return
Ok(()) or let execution proceed without transferring funds but still record logs
if desired), aligning deposit() with withdraw() and do_transfer(); reference
deposit(), withdraw(), and do_transfer() when updating the logic so tests and
docs can be adjusted if needed.
In `@precompiles/balances-erc20/src/tests.rs`:
- Around line 698-700: The test currently computes the domain separator and
digest using the same helpers as the runtime (compute_domain_separator,
compute_permit_struct_hash, compute_eip712_digest with PRECOMPILE_ADDRESS and
NativeErc20Metadata::NAME), so add at least one assertion that uses an
independent, fixed EIP‑712 test vector: hardcode a known domain separator and
expected digest/signature outcome for a specific
owner/spender/value/nonce/deadline and assert equality against the computed
values; update the same pattern used around the other permit assertions to use
this external vector to ensure a hashing bug in the helpers cannot make the test
pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a61d7046-5102-4ea0-bbf9-a4f816dcd11f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.tomlprecompiles/balances-erc20/Cargo.tomlprecompiles/balances-erc20/src/eip2612.rsprecompiles/balances-erc20/src/lib.rsprecompiles/balances-erc20/src/mock.rsprecompiles/balances-erc20/src/tests.rsruntime/Cargo.tomlruntime/src/configs/evm.rsruntime/tests/evm.rs
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
1 participant
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.
Summary
0x0000000000000000000000000000000000000802AccountId32, and EIP-2612 permit flowsSummary by CodeRabbit