feat: fully implement the injective bank precompile#1
Conversation
WalkthroughAdds a workspace dependency Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Precompile as BankPrecompile
participant Router as MethodRouter
participant Handler as OperationHandler
participant Storage as ThreadLocalStorage
Caller->>Precompile: call(injective_bank, calldata)
Precompile->>Router: decode selector
Router->>Handler: dispatch to specific handler
alt Mint
Handler->>Storage: get_balance(token, recipient)
Storage-->>Handler: balance
Handler->>Storage: set_balance(token, recipient, new_balance)
Handler->>Storage: set_supply(token, new_supply)
Handler-->>Precompile: encoded success
else Burn
Handler->>Storage: get_balance(token, account)
Handler->>Storage: set_balance(token, account, reduced)
Handler->>Storage: set_supply(token, new_supply)
Handler-->>Precompile: encoded success
else Transfer
Handler->>Storage: get_balance(token, sender)
Handler->>Storage: set_balance(token, sender, debited)
Handler->>Storage: set_balance(token, recipient, credited)
Handler-->>Precompile: encoded success
else Query (balanceOf/totalSupply/metadata)
Handler->>Storage: read requested value
Storage-->>Handler: value
Handler-->>Precompile: encoded value
end
Precompile-->>Caller: returndata / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
crates/evm/networks/Cargo.toml(1 hunks)crates/evm/networks/src/injective/bank.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/evm/networks/src/injective/bank.rs (1)
crates/evm/networks/src/lib.rs (1)
precompiles(122-133)
🔇 Additional comments (1)
crates/evm/networks/Cargo.toml (1)
25-25: Dependency addition looks correct.The workspace-scoped
alloy-sol-typesaddition aligns with the rest of the alloy stack already pulled from the workspace—no action needed.
| let current_supply = get_supply(token); | ||
| let new_supply = current_supply - amount; | ||
| set_supply(token, new_supply); |
There was a problem hiding this comment.
Prevent total supply underflow when burning.
After subtracting amount from current_supply, we use U256’s unchecked -, so calling burn with amount > current_supply will underflow (panic or wrap). Mirror the balance guard by checking the supply before mutating state.
- let current_supply = get_supply(token);
- let new_supply = current_supply - amount;
- set_supply(token, new_supply);
+ let current_supply = get_supply(token);
+ let new_supply = current_supply
+ .checked_sub(amount)
+ .ok_or_else(|| PrecompileError::Other("Insufficient supply to burn".into()))?;
+ set_supply(token, new_supply);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let current_supply = get_supply(token); | |
| let new_supply = current_supply - amount; | |
| set_supply(token, new_supply); | |
| let current_supply = get_supply(token); | |
| let new_supply = current_supply | |
| .checked_sub(amount) | |
| .ok_or_else(|| PrecompileError::Other("Insufficient supply to burn".into()))?; | |
| set_supply(token, new_supply); |
🤖 Prompt for AI Agents
In crates/evm/networks/src/injective/bank.rs around lines 203 to 205, the code
subtracts amount from current_supply using U256’s unchecked subtraction which
can underflow; add a guard that checks if current_supply >= amount before
mutating state and handle the failure the same way the balance guard does
(return an error/revert or early return) to prevent underflow, otherwise compute
new_supply = current_supply - amount and call set_supply(token, new_supply).
There was a problem hiding this comment.
It is not possible for an address to contain more than the total issued amount.
And we already check that the burn amount doesn't exceed the user's balance
There was a problem hiding this comment.
@arrivets You're absolutely right! Your reasoning is sound: since we verify current_balance >= amount and the invariant ensures total_supply >= current_balance, it follows that total_supply >= amount, so the subtraction cannot underflow.
I notice that handle_mint uses checked_add for both balance and supply updates as a defensive measure against overflow. Using checked_sub for the supply in handle_burn would maintain symmetry with that defensive programming approach, but given the mathematical guarantee from the balance check, it's indeed redundant under normal operation.
Thank you for the clarification!
✏️ Learnings added
Learnt from: arrivets
Repo: InjectiveLabs/foundry PR: 1
File: crates/evm/networks/src/injective/bank.rs:203-205
Timestamp: 2025-11-06T13:28:01.523Z
Learning: In crates/evm/networks/src/injective/bank.rs, the `handle_burn` function does not need checked_sub for total supply because the balance check (`current_balance >= amount`) combined with the invariant (total supply >= any individual balance) mathematically guarantees that `total_supply >= amount`, preventing underflow.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
maxim-inj
left a comment
maxim-inj
left a comment
There was a problem hiding this comment.
The limits there should match current Cosmos side:
if len(metadata.Name) > 256 {
return nil, errors.New("name is too long (max 256 characters)")
} else if len(metadata.Symbol) > 128 {
return nil, errors.New("symbol is too long (max 128 characters)")
}
| let symbol = call.symbol; | ||
| let decimals = call.decimals; | ||
|
|
||
| // Validate inputs |
There was a problem hiding this comment.
Supposed to leave comment there
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/evm/networks/src/injective/bank.rs (1)
203-205: Critical: Prevent total supply underflow when burning.After subtracting
amountfromcurrent_supply, the code usesU256's unchecked subtraction. Ifamount > current_supply, this will underflow and panic. While the balance is checked at lines 194-196, there's no guarantee thatcurrent_supply >= amount.Apply this diff to add underflow protection:
- let current_supply = get_supply(token); - let new_supply = current_supply - amount; - set_supply(token, new_supply); + let current_supply = get_supply(token); + let new_supply = current_supply + .checked_sub(amount) + .ok_or_else(|| PrecompileError::Other("Insufficient supply to burn".into()))?; + set_supply(token, new_supply);
🧹 Nitpick comments (2)
crates/evm/networks/src/injective/bank.rs (2)
306-340: Consider using Alloy's built-in tuple encoding.The manual ABI encoding is correct but complex and error-prone. Since other handlers use
.abi_encode()for return values, consider simplifying this implementation.Consider this refactor using Alloy's built-in encoding:
- // Manually encode the return tuple (string, string, uint8) for ABI compatibility - // This is the proper ABI encoding for a tuple with two dynamic types (strings) and one static type (uint8) - let mut result = Vec::new(); - - // Offsets for the two strings (both are dynamic) - // Offset to first string (name) - starts after the 3 header words (96 bytes) - result.extend_from_slice(&U256::from(96).to_be_bytes::<32>()); - - // Calculate offset to second string (symbol) - let name_bytes = name.as_bytes(); - let name_padded_len = ((name_bytes.len() + 31) / 32) * 32; - let symbol_offset = 96 + 32 + name_padded_len; // header (96) + name length word (32) + name content - result.extend_from_slice(&U256::from(symbol_offset).to_be_bytes::<32>()); - - // Decimals value (static, directly encoded) - let mut decimals_bytes = [0u8; 32]; - decimals_bytes[31] = decimals; - result.extend_from_slice(&decimals_bytes); - - // Name string: length + content + padding - result.extend_from_slice(&U256::from(name_bytes.len()).to_be_bytes::<32>()); - result.extend_from_slice(name_bytes); - if name_bytes.len() % 32 != 0 { - result.resize(result.len() + (32 - name_bytes.len() % 32), 0); - } - - // Symbol string: length + content + padding - let symbol_bytes = symbol.as_bytes(); - result.extend_from_slice(&U256::from(symbol_bytes.len()).to_be_bytes::<32>()); - result.extend_from_slice(symbol_bytes); - if symbol_bytes.len() % 32 != 0 { - result.resize(result.len() + (32 - symbol_bytes.len() % 32), 0); - } - - Ok(PrecompileOutput::new(METADATA_GAS_COST, Bytes::from(result))) + // Use Alloy's built-in tuple encoding + let result = (name, symbol, decimals).abi_encode(); + Ok(PrecompileOutput::new(METADATA_GAS_COST, Bytes::from(result)))Note: Verify that Alloy's
SolValue::abi_encode()for tuples produces the expected encoding for the return type.
357-364: Consider adding explanatory comment.A previous reviewer noted that a comment should be added here. While the validation code is self-explanatory, a brief comment explaining the length limits would improve readability.
Consider adding a comment:
+ // Validate metadata string lengths to prevent excessive storage usage // Validate inputs if name.len() > 256 { return Err(PrecompileError::Other("Name too long (max 256 characters)".into())); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/evm/networks/src/injective/bank.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/evm/networks/src/injective/bank.rs (1)
crates/evm/networks/src/lib.rs (1)
precompiles(122-133)
🔇 Additional comments (8)
crates/evm/networks/src/injective/bank.rs (8)
23-31: LGTM! Thread-local storage appropriate for testing.The use of thread-local storage with
RefCellis suitable for the testing context mentioned in the file documentation. This design provides isolated state per thread and runtime borrow checking.
33-44: LGTM! Interface definition is clear and complete.The
IBankModuleinterface properly defines all seven operations with appropriate function signatures and modifiers.
70-103: LGTM! Clean dispatcher with proper error handling.The routing logic correctly checks input length and dispatches to appropriate handlers based on selectors, with clear error messages for unknown methods.
142-174: LGTM! Mint operation with proper overflow protection.The implementation correctly uses
checked_addfor both balance and supply updates, preventing overflow. The gas check and error handling are appropriate.
211-229: LGTM! Balance query is straightforward.The implementation correctly retrieves and returns the balance with appropriate gas handling.
231-270: LGTM! Transfer implementation with proper safety checks.The transfer correctly validates sender balance and uses
checked_addfor the recipient to prevent overflow.
272-289: LGTM! Total supply query is correct.The implementation properly retrieves and returns the total supply.
343-374: LGTM! Metadata setter with proper input validation.The implementation correctly validates string lengths and stores metadata. The length limits (256 for name, 128 for symbol) provide reasonable bounds.
2 participants
Full implementation of the Injective Bank precompile
For tests, have a look at this PR in the solidity-contracts repo:
InjectiveLabs/solidity-contracts#25
Summary by CodeRabbit
New Features
Improvements