Skip to content

Conversation

@arrivets
Copy link
Contributor

@arrivets arrivets commented Nov 5, 2025

This PR adds a test for the new Injective Bank precompile added to our fork of foundry:
https://github.com/InjectiveLabs/foundry/tree/CP-698/complete-bank-precompile

Summary by CodeRabbit

Release Notes

  • Tests

    • Added integration tests for Bank precompile operations including mint, burn, transfer, and metadata management.
  • Documentation

    • Added setup and execution guide for Bank precompile integration tests.

@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Walkthrough

A new Bank precompile test suite is introduced with an interface definition mapping the Injective Bank module API (mint, burn, transfer, balanceOf, totalSupply, metadata operations) and a test contract executing a multi-step validation flow including token minting, transfers, burns, and balance verification.

Changes

Cohort / File(s) Change Summary
Bank Precompile Test Suite
test/BankPrecompile.t.sol
Added IBankModule interface defining the precompile contract surface (mint, burn, transfer, balanceOf, totalSupply, metadata, setMetadata). Added BankPrecompileTest contract instantiating the precompile at address 0x64 and declaring test actor addresses. Implemented run() function sequencing metadata setup, minting to Alice and Bob, token transfers, burns, and balance/supply assertions.
Test Documentation
test/README.md
Added guide documenting Bank precompile integration test prerequisites, setup instructions, execution commands, test scenario (7-step flow), and expected output. Specifies Foundry fork usage with chain-id 1776.

Sequence Diagram(s)

sequenceDiagram
    participant Test as BankPrecompileTest
    participant Precompile as Bank (0x64)
    
    Test->>Precompile: setMetadata(name, symbol, decimals)
    Note over Test,Precompile: Setup phase
    
    Test->>Precompile: balanceOf(token, alice)
    Test->>Precompile: totalSupply(token)
    Note over Test,Precompile: Verify initial state (zeros)
    
    Test->>Precompile: mint(alice, amount₁)
    Test->>Precompile: balanceOf(token, alice)
    Test->>Precompile: totalSupply(token)
    Note over Test,Precompile: Mint & verify alice
    
    Test->>Precompile: mint(bob, amount₂)
    Test->>Precompile: balanceOf(token, bob)
    Note over Test,Precompile: Mint & verify bob
    
    Test->>Precompile: transfer(alice, charlie, amount)
    Test->>Precompile: balanceOf(token, alice)
    Test->>Precompile: balanceOf(token, charlie)
    Note over Test,Precompile: Transfer & verify
    
    Test->>Precompile: burn(bob, amount)
    Test->>Precompile: totalSupply(token)
    Note over Test,Precompile: Burn & verify supply
    
    Test->>Precompile: transfer(bob, alice, amount)
    Note over Test,Precompile: Final transfers & assertions
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • The test file introduces new public entities (interface and contract) with straightforward method signatures and a linear test flow
  • Consistent patterns throughout: vm.prank calls, assertTrue/assertEq assertions, and console logging reduce cognitive load
  • No complex logic, conditional branching, or error handling paths
  • Documentation file is informational with no code to evaluate

Poem

🐰 A new Bank test hops into place,
With mint, burn, and transfer grace,
Alice, Bob, and Charlie dance,
While balances balance and supply rants—
The precompile's fortunes now traced! 💰

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a test for the Foundry bank precompile, which is clearly demonstrated by the new test file and documentation.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch CP-698/test-foundry-bank-precompile

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

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.

Actionable comments posted: 2

🧹 Nitpick comments (1)
test/BankPrecompile.t.sol (1)

40-146: Consider adding negative test cases and edge case coverage.

The current test comprehensively validates the happy path, which is excellent for an initial integration test. To strengthen the test suite, consider adding scenarios for:

  • Insufficient balance: Attempt to burn or transfer more tokens than available
  • Zero amounts: Test operations with amount=0
  • Zero addresses: Test operations with address(0) as recipient/sender
  • Unauthorized callers: Verify that non-token addresses cannot call mint/burn/transfer (should revert)
  • Boundary conditions: Test with max uint256 values

These additions would verify that the precompile properly enforces invariants and handles edge cases.

Example negative test structure:

// Test insufficient balance
vm.prank(token);
vm.expectRevert(); // or specific error
bank.burn(alice, 999999);

// Test unauthorized caller
vm.prank(alice); // not token
vm.expectRevert();
bank.mint(bob, 1000);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b152129 and cd271fb.

📒 Files selected for processing (2)
  • test/BankPrecompile.t.sol (1 hunks)
  • test/README.md (1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
test/README.md

12-12: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


48-48: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (3)
test/BankPrecompile.t.sol (3)

6-29: LGTM! Well-documented interface.

The IBankModule interface is cleanly defined with proper NatSpec documentation and appropriate function signatures for a bank precompile. The use of payable on mutating operations and view functions for read operations is correct.


31-37: LGTM! Clean contract setup.

The contract initialization is straightforward with the bank precompile at the conventional address 0x64 and simple addresses for testing. This approach is appropriate for integration tests.


45-46: Verify the authorization model using vm.prank(token) for all operations.

All mutating operations (mint, burn, transfer) are called with vm.prank(token), meaning the token address itself authorizes these operations rather than the token holders. This differs from standard ERC20 patterns where users call transfer/approve directly.

While this may be intentional for the Injective Bank precompile design, please confirm:

  1. Is the token address expected to be the sole authorized caller for these operations?
  2. How do actual users initiate transfers, mints, or burns if they can't call the precompile directly?
  3. Should the test include scenarios where unauthorized addresses attempt these operations (expected to fail)?

Consider adding a comment in the test explaining this authorization model for future maintainers.

Also applies to: 64-65, 77-78, 90-91, 106-107, 119-120

### Prerequisites

**This test requires the Injective Foundry fork.** You must build and install Foundry from:
```
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add language identifiers to fenced code blocks.

Markdown linters expect fenced code blocks to specify a language for proper syntax highlighting.

Apply this diff:

-```
+```bash
 https://github.com/InjectiveLabs/foundry/tree/CP-698/complete-bank-precompile

(and similarly for line 48)

- +text
=== Injective Bank Precompile Integration Test ===

Also applies to: 48-48

🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

12-12: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
In test/README.md around lines 12 and 48, the fenced code blocks are missing
language identifiers; update the opening fences accordingly: change the fence at
line 12 to ```bash for the GitHub URL block and change the fence at line 48 to
```text for the "=== Injective Bank Precompile Integration Test ===" block so
markdown linters and syntax highlighting recognize the languages.

Comment on lines +16 to +23
To build and install:
```bash
git clone https://github.com/InjectiveLabs/foundry.git
cd foundry
git checkout CP-698/complete-bank-precompile
cargo build --release --target aarch64-apple-darwin
cp target/aarch64-apple-darwin/release/forge ~/.foundry/bin/forge
```
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Make build instructions platform-agnostic.

The build command hardcodes --target aarch64-apple-darwin (macOS ARM), which won't work on Linux, Windows, or Intel-based systems. Consider providing generic instructions or documenting alternatives for other platforms.

Apply this diff to make the instructions more portable:

 To build and install:
 ```bash
 git clone https://github.com/InjectiveLabs/foundry.git
 cd foundry
 git checkout CP-698/complete-bank-precompile
-cargo build --release --target aarch64-apple-darwin
-cp target/aarch64-apple-darwin/release/forge ~/.foundry/bin/forge
+cargo build --release
+# On Unix-like systems:
+cp target/release/forge ~/.foundry/bin/forge
+# On Windows:
+# copy target\release\forge.exe %USERPROFILE%\.foundry\bin\forge.exe

<details>
<summary>🤖 Prompt for AI Agents</summary>

test/README.md around lines 16 to 23: the build instructions hardcode the macOS
ARM target (--target aarch64-apple-darwin) which is not portable; change to a
generic cargo build --release and document platform-specific install commands
(e.g., cp target/release/forge ~/.foundry/bin/forge for Unix-like systems and
copy target\release\forge.exe %USERPROFILE%.foundry\bin\forge.exe for Windows)
so users on Linux, Intel macs, and Windows have correct steps.


</details>

<!-- This is an auto-generated comment by CodeRabbit -->

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