improve(AcrossV3): Drop gas introspection in handler contract #888
Conversation
The Across message handler function does not require any gas introspection. Cross-chain intents under Across instead outsource gas considerations on the destination chain to the relayer function.
WalkthroughThe changes in this pull request focus on simplifying the gas handling logic within the Changes
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
test/solidity/Periphery/ReceiverAcrossV3.t.sol (1)
Line range hint
1-140: Architecture improvement: Clear separation of concernsThe removal of gas introspection from the handler contract is a good architectural decision because:
- It properly delegates gas management to the relayer on the destination chain
- Reduces contract complexity
- Maintains focus on core bridge and swap functionality
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/Periphery/ReceiverAcrossV3.sol(3 hunks)test/solidity/Periphery/ReceiverAcrossV3.t.sol(3 hunks)
🔇 Additional comments (5)
src/Periphery/ReceiverAcrossV3.sol (3)
35-40: LGTM: Constructor simplified by removing gas parameter
The removal of the _recoverGas parameter aligns with the PR objective of dropping gas introspection, while maintaining essential contract initialization.
95-96: Documentation accurately reflects gas handling removal
The updated comment correctly indicates that native token handling isn't needed since Across always sends wrapped native tokens.
111-116: LGTM: Simplified swap execution with proper error handling
The removal of gas checks simplifies the code while maintaining the essential error recovery path. The function properly:
- Attempts the swap
- Falls back to returning tokens on failure
- Manages token approvals correctly
test/solidity/Periphery/ReceiverAcrossV3.t.sol (2)
37-37: LGTM: Test setup aligned with contract changes
The test initialization and setup verification have been properly updated to reflect the removal of gas introspection.
Also applies to: 48-48
Line range hint 198-203: LGTM: Error recovery test maintained
The test case properly verifies that funds are returned to the user on swap failure, maintaining coverage of critical recovery paths despite the removal of gas-related tests.
|
Changes have been incorporated in this PR #897 |
Which Jira task belongs to this PR?
n/a
Why did I implement it this way?
The Across message handler function does not require any gas introspection. Cross-chain intents under Across instead outsource gas considerations on the destination chain to the relayer function.
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)