test: add bridge call origin token unit test #941
Conversation
WalkthroughThe pull request introduces modifications to the cross-chain bridge call functionality across multiple files. The primary change is the addition of a Changes
Sequence DiagramsequenceDiagram
participant Sender
participant CrosschainContract
participant DestinationChain
Sender->>CrosschainContract: bridgeCall(value, tokens, data)
CrosschainContract-->>DestinationChain: Transfer tokens + value
DestinationChain-->>Sender: Confirm transaction
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🧹 Nitpick comments (3)
precompiles/crosschain/crosschain_test.go (1)
32-73: Consider adding more assertions for comprehensive testing.The test setup and flow look good, but consider adding assertions for:
- The emitted event parameters
- The exact contents of the transaction logs
- The state of the contract before and after the crosschain operation
precompiles/crosschain/contract_test.go (1)
131-155: Parameterize hardcoded values in quote input.The function uses hardcoded values that might not be suitable for all test cases:
- Gas limit of 21000 (Ethereum base transaction cost)
- Amount of 1
Consider parameterizing these values to make the function more flexible.Apply this diff to improve flexibility:
-func (suite *CrosschainPrecompileTestSuite) Quote(denom string) { +func (suite *CrosschainPrecompileTestSuite) Quote(denom string, gasLimit uint64, amount *big.Int) { quoteQuoteInput := contract.IBridgeFeeQuoteQuoteInput{ Cap: 0, - GasLimit: 21000, + GasLimit: gasLimit, Expiry: uint64(time.Now().Add(time.Hour).Unix()), ChainName: contract.MustStrToByte32(suite.chainName), TokenName: contract.MustStrToByte32(denom), - Amount: big.NewInt(1), + Amount: amount, }precompiles/crosschain/bridge_call_test.go (1)
225-249: Add assertions for transaction logs.The test verifies the presence of logs but doesn't validate their content. Consider adding assertions to verify the log data matches the expected values.
Example assertion to add:
suite.NotNil(txResponse) suite.Len(txResponse.Logs, 1) + // Verify log content + log := txResponse.Logs[0] + suite.Equal(value, log.Amount) + suite.Equal(suite.signer.Address(), log.From)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
contract/crosschain_precompile.go(1 hunks)precompiles/crosschain/bridge_call_test.go(3 hunks)precompiles/crosschain/contract_test.go(5 hunks)precompiles/crosschain/crosschain_test.go(3 hunks)solidity/contracts/test/CrosschainTest.sol(2 hunks)tests/contract/crosschain_test.sol.go(2 hunks)testutil/helpers/crosschain_precompile_suite.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (9)
testutil/helpers/crosschain_precompile_suite.go (1)
61-64: LGTM! Test helper methods updated correctly.The BridgeCall method has been properly updated to include the value parameter and correctly propagates it to the underlying CrosschainPrecompileKeeper.
contract/crosschain_precompile.go (1)
Line range hint
57-66: LGTM! Core implementation updated correctly.The BridgeCall method has been properly updated to:
- Accept the value parameter for Ether transactions
- Pass the value to ApplyContract
- Maintain proper error handling and return values
precompiles/crosschain/crosschain_test.go (1)
Line range hint
75-95: LGTM! Origin token test case looks good.The test properly verifies:
- Balance changes before and after the operation
- Correct module account handling
- Transaction response and logs
solidity/contracts/test/CrosschainTest.sol (2)
67-67: LGTM! Function correctly marked as payable.The bridgeCall function is properly marked as payable to accept Ether transactions.
Line range hint
81-91: Verify value handling in production code.The implementation correctly forwards the msg.value to the ICrosschain contract. However, ensure that the production contract properly validates the msg.value against expected amounts to prevent accidental value transfers.
precompiles/crosschain/contract_test.go (2)
66-67: Verify the purpose of magic numbers in block height setup.The values
100and10are used without explanation. Consider using named constants or adding comments to explain their significance in the test setup.
85-87: LGTM!Clean implementation of the ERC20TokenKeeper getter.
precompiles/crosschain/bridge_call_test.go (1)
183-223: LGTM!Well-structured test that verifies:
- Token setup and minting
- Bridge call execution
- Balance checks after the operation
tests/contract/crosschain_test.sol.go (1)
331-345: LGTM!The generated code correctly reflects the contract changes making
bridgeCallpayable.
Summary by CodeRabbit
New Features
Improvements
Technical Updates