-
Notifications
You must be signed in to change notification settings - Fork 7
feat(precompiles): add Token Duality precompile #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @AndeLabs, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant new feature by implementing a "Token Duality" precompile, inspired by the Celo blockchain. This precompile allows native tokens to be transferred using an ERC-20-like interface, streamlining integration with smart contracts without requiring native tokens to be wrapped. It includes robust security features such as admin and allowlist-based access control, as well as per-call and per-block rate limiting to manage token flow. The configuration for this precompile, including its administrator and transfer limits, is now fully manageable through the chainspec. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new TokenDuality precompile, enabling native tokens to function as ERC-20 compatible tokens, following the Celo pattern. The implementation is comprehensive, with thorough documentation, extensive unit tests, and proper integration into the EVM factory and node configuration. The code is of high quality. I've provided a couple of minor suggestions to improve consistency and readability in the precompile implementation.
| fn add_balance( | ||
| internals: &mut EvmInternals<'_>, | ||
| addr: Address, | ||
| amount: U256, | ||
| ) -> Result<(), PrecompileError> { | ||
| let account = internals | ||
| .load_account(addr) | ||
| .map_err(Self::map_internals_error)?; | ||
|
|
||
| // SECURITY: Verify addition won't overflow before mutating state. | ||
| // The checked_add result is used for validation, then balance_incr | ||
| // performs the actual mutation (which is safe after this check). | ||
| let _new_balance = account | ||
| .info | ||
| .balance | ||
| .checked_add(amount) | ||
| .ok_or_else(|| PrecompileError::Other("balance overflow".to_string()))?; | ||
|
|
||
| internals | ||
| .balance_incr(addr, amount) | ||
| .map_err(Self::map_internals_error)?; | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency and clarity, this function can be implemented similarly to sub_balance. Using set_balance with the pre-checked new_balance avoids a potential double-load of the account that balance_incr might perform and makes the overflow protection more explicit within the state mutation logic.
fn add_balance(
internals: &mut EvmInternals<'_>,
addr: Address,
amount: U256,
) -> Result<(), PrecompileError> {
let account = internals
.load_account(addr)
.map_err(Self::map_internals_error)?;
// SECURITY: Verify addition won't overflow before mutating state.
let new_balance = account
.info
.balance
.checked_add(amount)
.ok_or_else(|| PrecompileError::Other("balance overflow".to_string()))?;
internals
.set_balance(addr, new_balance)
.map_err(Self::map_internals_error)?;
Ok(())
}| fn allowlist_key(addr: Address) -> U256 { | ||
| U256::from_be_bytes(addr.into_word().into()) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alloy_primitives::U256 type implements From<Address>, which provides a more idiomatic and readable way to convert an Address into a U256 for use as a storage key. This simplifies the code and relies on the tested conversion logic from the alloy library.
fn allowlist_key(addr: Address) -> U256 {
U256::from(addr)
}51880de to
ee1fc87
Compare
Implements Celo-style token duality pattern allowing native tokens to function as ERC-20 compatible tokens without wrapping. Features: - Native token transfers via precompile at 0x00..00FD - Admin + allowlist authorization model - Per-call and per-block transfer rate limiting - Chainspec configuration for admin, caps and activation height The precompile provides the transfer mechanism; full ERC-20 compatibility requires a wrapper contract that calls this precompile and provides standard ERC-20 views and events.
ee1fc87 to
0060389
Compare
Description
Implements Celo-style token duality pattern allowing native tokens to function as ERC-20 compatible tokens without wrapping.
Type of Change
Features
0x00..00FDThe precompile provides the transfer mechanism; full ERC-20 compatibility requires a wrapper contract that calls this precompile and provides standard ERC-20 views and events.
Checklist
Testing
References