fix: deep stack call mocks removed #383
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
contracts/test/integration/ArbToGnosis.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@typescript-eslint/parser' declared in 'contracts/.eslintrc.js': Cannot find module '@typescript-eslint/parser'
WalkthroughThis pull request introduces modifications to the Arbitrum to Gnosis bridge testing infrastructure. The changes focus on enhancing mock contracts for Changes
Sequence DiagramsequenceDiagram
participant User
participant VeaInboxMock
participant MockArbSys
participant Router
User->>VeaInboxMock: sendSnapshot(epoch, claim)
VeaInboxMock->>MockArbSys: sendTxToL1
MockArbSys-->>VeaInboxMock: Generate ticket ID
VeaInboxMock->>Router: Encode route call data
VeaInboxMock->>VeaInboxMock: Emit SnapshotSent event
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
❌ Deploy Preview for veascan failed. Why did it fail? →
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
contracts/test/integration/ArbToGnosis.ts (1)
422-432: Refactor repeated code into a helper functionThe logic for getting call data and executing L1 messages is repeated across tests. To improve maintainability, consider abstracting this into a helper function.
Apply this diff to create a helper function:
+ async function executeL1Message(epoch: number, claim: any) { + const callData = await veaInbox.connect(bridger).getCallData(epoch, 100000, claim); + const routerAddress = await router.getAddress(); + await bridgeMock.connect(bridger).executeL1Message(routerAddress, callData); + }Then replace repeated code with:
- const callData = await veaInbox.connect(bridger).getCallData(epoch, 100000, claim); - const routerAddress = await router.getAddress(); - await bridgeMock.connect(bridger).executeL1Message(routerAddress, callData); + await executeL1Message(epoch, claim);contracts/src/test/bridge-mocks/arbitrum/ArbSysMockWithBridge.sol (1)
21-24: Handle unused parametercalldataForL1insendTxToL1The
calldataForL1parameter is not used in the function. To avoid compiler warnings and clarify intent, consider prefixing it with an underscore or emitting it in the event.Option 1 - Prefix with underscore:
- function sendTxToL1(address destination, bytes calldata calldataForL1) external payable returns (uint256) { + function sendTxToL1(address destination, bytes calldata _calldataForL1) external payable returns (uint256) { emit L2toL1Transaction(msg.sender, destination); return 1; }Option 2 - Include in the event:
- event L2toL1Transaction(address caller, address destination); + event L2toL1Transaction(address caller, address destination, bytes calldataForL1); function sendTxToL1(address destination, bytes calldata calldataForL1) external payable returns (uint256) { emit L2toL1Transaction(msg.sender, destination, calldataForL1); return 1; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
contracts/src/test/ArbToGnosis/VeaInboxArbToGnosisMock.sol(1 hunks)contracts/src/test/bridge-mocks/arbitrum/ArbSysMockWithBridge.sol(1 hunks)contracts/test/integration/ArbToGnosis.ts(8 hunks)
🔇 Additional comments (9)
contracts/test/integration/ArbToGnosis.ts (7)
15-16: Imports for mock contracts are correctly added
The imports for BridgeMock and VeaInboxArbToGnosisMock are appropriately added to utilize the mock contracts in the integration tests.
28-29: Hardcoded TICKET_ID is acceptable for testing purposes
The hardcoded TICKET_ID simplifies event assertions in the tests and is suitable for the mock environment.
40-40: Updated veaInbox to use the mock contract
The veaInbox variable is now using VeaInboxArbToGnosisMock, which aligns with the introduction of the mock contract for testing.
46-46: Declared bridgeMock for bridge interactions in tests
The bridgeMock variable is introduced to simulate bridge interactions within the testing environment.
63-66: Simulated dispute resolution using getCallData and bridgeMock
The simulateDisputeResolution function now retrieves call data using veaInbox.getCallData and executes the L1 message via bridgeMock.executeL1Message, which correctly reflects the new mock setup.
120-120: Initialized bridgeMock with the correct contract instance
The bridgeMock variable is correctly assigned the BridgeMock contract instance for testing purposes.
420-420: Added event assertion for SnapshotSent with TICKET_ID
The test now asserts that the SnapshotSent event is emitted with the correct epoch and TICKET_ID, enhancing the verification of snapshot sending.
contracts/src/test/bridge-mocks/arbitrum/ArbSysMockWithBridge.sol (1)
16-16: Added L2toL1Transaction event for testing
The L2toL1Transaction event is introduced to simulate and test Layer 2 to Layer 1 transactions within the mock environment.
contracts/src/test/ArbToGnosis/VeaInboxArbToGnosisMock.sol (1)
40-42: Added getCallData function to retrieve encoded data
The getCallData function allows external callers to retrieve the encoded call data for IRouterToGnosis.route, facilitating testing without executing transactions.
PR-Codex overview
This PR focuses on enhancing the
ArbToGnosisbridge functionality by adding a new method to retrieve call data and modifying thesendTxToL1function to emit an event. It also updates test cases to utilize the new mock contract and ensure proper integration.Detailed summary
getCallDatafunction inVeaInboxArbToGnosisMockto encode call data.L2toL1Transactionevent inArbSysMockWithBridge.sendTxToL1to emitL2toL1Transactionand return a constant value.VeaInboxArbToGnosisMockinstead ofVeaInboxArbToGnosis.TICKET_IDin tests.TICKET_ID.Summary by CodeRabbit
New Features
getCallDatafor retrieving encoded call data.L2toL1Transactionto log transactions from Layer 2 to Layer 1.Bug Fixes
sendTxToL1functionality to emit the new event instead of interacting with the bridge.Tests