Raunak/dispatcher agnostic fee implementation #114
Conversation
|
Warning Rate limit exceeded@RnkSngh has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 27 minutes and 11 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe updates across various smart contracts involve introducing and updating fee management mechanisms. Key additions include the Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
RnkSngh
force-pushed
the
raunak/dispatcher-agnostic-fee-implementation
branch
from
36538d5 to
7b89331
Compare
RnkSngh
force-pushed
the
raunak/dispatcher-agnostic-fee-implementation
branch
from
7b89331 to
18e531a
Compare
|
Note: we're still missing the last implementation detail of how we can identify channel open fees |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range and nitpick comments (4)
test/FeeVault.t.sol (2)
17-19: Consider adding a comment explaining whygreetMarsWithFee()is called within thesetUp()function. This could help future maintainers understand its role in setting up the test environment.
121-123: The comment about the multisig fallback is a bit unclear. It might be helpful to specify if this is a requirement or a recommendation, and why it's relevant to theFeeVaultTestcontract.test/upgradeableProxy/DispatcherRC4.upgrade.t.sol (1)
113-113: Add comments explaining the use ofvm.loadfor accessing specific storage slots to improve code readability and maintainability.Also applies to: 120-120, 128-128
test/Dispatcher/Dispatcher.t.sol (1)
320-341: Add detailed comments in the_doFeeSendPacketmethod to explain the steps and calculations, especially how fees are handled and verified, to improve code readability and maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (28)
- contracts/base/GeneralMiddleware.sol (3 hunks)
- contracts/core/Dispatcher.sol (5 hunks)
- contracts/core/FeeVault.sol (1 hunks)
- contracts/core/UniversalChannelHandler.sol (4 hunks)
- contracts/examples/Earth.sol (2 hunks)
- contracts/examples/Mars.sol (4 hunks)
- contracts/implementation_templates/FeeSender.sol (1 hunks)
- contracts/implementation_templates/IbcReceiverUpgradeable.sol (1 hunks)
- contracts/interfaces/IDispatcher.sol (3 hunks)
- contracts/interfaces/IFeeVault.sol (1 hunks)
- contracts/interfaces/IbcDispatcher.sol (2 hunks)
- contracts/interfaces/IbcMiddleware.sol (2 hunks)
- test/Dispatcher.gasGriefing.t.sol (1 hunks)
- test/Dispatcher/Dispatcher.client.t.sol (1 hunks)
- test/Dispatcher/Dispatcher.closeChannel.t.sol (3 hunks)
- test/Dispatcher/Dispatcher.dappHandlerRevert.t.sol (1 hunks)
- test/Dispatcher/Dispatcher.multiclient.sol (1 hunks)
- test/Dispatcher/Dispatcher.proof.t.sol (2 hunks)
- test/Dispatcher/Dispatcher.t.sol (5 hunks)
- test/FeeVault.t.sol (1 hunks)
- test/VirtualChain.sol (4 hunks)
- test/universal.channel.t.sol (5 hunks)
- test/upgradeableProxy/Dispatcher.upgrade.t.sol (5 hunks)
- test/upgradeableProxy/DispatcherRC4.upgrade.t.sol (10 hunks)
- test/upgradeableProxy/DispatcherUUPS.accessControl.t.sol (4 hunks)
- test/upgradeableProxy/upgrades/DispatcherV2Initializable.sol (2 hunks)
- test/utils/Dispatcher.base.t.sol (4 hunks)
- test/utils/TestUtils.t.sol (3 hunks)
Additional context used
GitHub Check: lint
contracts/base/GeneralMiddleware.sol
[warning] 48-48:
Variable name must be in mixedCase
[warning] 66-66:
Named parameters missing. MIN unnamed argumenst is 4
[warning] 76-76:
Code contains empty blocks
[warning] 86-86:
Named parameters missing. MIN unnamed argumenst is 4
[warning] 89-89:
All public or external methods in a contract must override a definition from an interface
[warning] 105-105:
Named parameters missing. MIN unnamed argumenst is 4
[warning] 134-134:
Named parameters missing. MIN unnamed argumenst is 4
[warning] 151-151:
Named parameters missing. MIN unnamed argumenst is 4
[warning] 164-164:
Named parameters missing. MIN unnamed argumenst is 4
[warning] 184-184:
Code contains empty blocks
Additional comments not posted (56)
test/upgradeableProxy/upgrades/DispatcherV2Initializable.sol (2)
22-22: Import statement forIFeeVaultis correctly added to support fee functionality.
33-37: Modification toinitializefunction correctly integratesIFeeVault. Ensure_feeVaultis not null before assignment to prevent issues.#!/bin/bash # Description: Verify that there's a null check for `_feeVault` before assignment. # Test: Search for null checks in the initialize function. Expect: Null check before assignment. ast-grep --lang solidity --pattern $'function initialize($_, $_) { $$$ if ($_ != null) { $$$ } $$$ }'contracts/implementation_templates/FeeSender.sol (2)
24-32: Function_depositSendPacketFeecorrectly handles the deposit logic for sending packet fees.
34-41: Function_depositOpenChannelFeeis implemented correctly to handle the deposit logic for opening channel fees.test/Dispatcher/Dispatcher.client.t.sol (1)
34-34: Correctly modified setup function to includefeeVault. Ensure integration tests cover scenarios usingfeeVault.#!/bin/bash # Description: Verify integration tests cover `feeVault` usage. # Test: Search for test cases involving `feeVault`. Expect: At least one test case using `feeVault`. rg --type solidity $'test.*feeVault'test/Dispatcher.gasGriefing.t.sol (1)
25-25: Setup function correctly modified to includefeeVaultin the dispatcher deployment. Verify thatfeeVaultis utilized correctly in gas griefing scenarios.#!/bin/bash # Description: Verify `feeVault` usage in gas griefing scenarios. # Test: Search for `feeVault` usage in gas griefing test cases. Expect: Correct usage of `feeVault`. rg --type solidity $'test.*feeVault'test/utils/TestUtils.t.sol (2)
6-6: Addition ofIFeeVaultimport is consistent with the PR's objectives to integrate fee vault functionality.
Line range hint
19-28: ThedeployDispatcherProxyAndImplfunction now accepts an additionalIFeeVault feeVaultparameter and passes it to theDispatcher.initializefunction. This change is necessary for initializing theDispatcherwith a fee vault, aligning with the PR's objectives.#!/bin/bash # Description: Verify the `Dispatcher.initialize` function correctly initializes the fee vault. ast-grep --lang solidity --pattern $'function initialize($_, $_) { $$$ }'contracts/implementation_templates/IbcReceiverUpgradeable.sol (2)
21-21: Addition ofIbcDispatcherimport ensures that theIbcReceiverBaseUpgradeablecan interact with the IBC dispatcher, necessary for the intended functionality.
27-27: Introduction of theinvalidAddresserror provides clearer error handling for address validation processes within the contract.contracts/interfaces/IFeeVault.sol (2)
20-28: Definition ofGasFeeandSendpacketFeeDepositedstructs provides structured data storage for gas fees, which is crucial for managing fee-related operations efficiently.
30-77: The interface methods added for depositing and withdrawing fees, along with the related events and errors, are well-defined and align with the PR's objectives to manage fee transactions securely and transparently.contracts/core/FeeVault.sol (3)
32-50: The implementation ofdepositSendPacketFeecorrectly calculates the total fee and validates it against the sent value (msg.value), ensuring that the correct fee amount is deposited. This is crucial for maintaining financial integrity within the protocol.
52-71: ThedepositOpenChannelFeefunction follows a similar pattern todepositSendPacketFee, ensuring consistency and correctness in handling channel opening fees. The use ofnonReentrantmodifier is appropriate for preventing reentrancy attacks.
73-75: ThewithdrawFeesToOwnerfunction allows the contract owner to withdraw accumulated fees. It's simple and uses thetransfermethod, which is appropriate for sending Ether. However, consider checking for successful transfer to handle potential failures.- payable(owner()).transfer(address(this).balance); + bool sent = payable(owner()).transfer(address(this).balance); + require(sent, "Transfer failed");Likely invalid or redundant comment.
test/upgradeableProxy/DispatcherUUPS.accessControl.t.sol (5)
8-8: Ensure theIFeeVaultimport is utilized correctly in the tests.
45-45: Check the correct retrieval offeeVaultafter the upgrade.
61-61: Ensure that theinitializemethod inDispatcherUUPSAccessControlprevents reinitialization attacks effectively.
25-25: Check the deployment ofdispatcherProxywithfeeVaultfor correct integration.#!/bin/bash # Description: Verify the correct integration of `feeVault` in `dispatcherProxy` deployment. # Test: Search for the deployment code. Expect: Correct usage of `feeVault`. rg --type solidity $'deployDispatcherProxyAndImpl'
39-39: Confirm that theupgradeToAndCallwithinitializecorrectly integrates the newfeeVault.#!/bin/bash # Description: Ensure the correct use of `feeVault` in `initialize` during `upgradeToAndCall`. # Test: Search for the `initialize` function usage with `feeVault`. Expect: Proper initialization. rg --type solidity $'initialize\(string,address\)'Also applies to: 41-41
contracts/interfaces/IbcDispatcher.sol (2)
22-22: Ensure that theIFeeVaultinterface is correctly integrated into theIbcDispatcherinterface.
53-53: Confirm that thefeeVaultfunction is correctly defined and returns the expected type.contracts/interfaces/IDispatcher.sol (2)
24-24: Ensure theIFeeVaultimport is utilized correctly in theIDispatcherinterface.
109-109: Check that thefeeVaultfunction is correctly defined in theIDispatcherinterface.contracts/examples/Earth.sol (2)
23-23: Confirm the proper use of theIUniversalChannelHandlerinterface in theEarthcontract.
66-77: Review the implementation of thegreetWithFeefunction to ensure it handles fees correctly and integrates well with the middleware.test/Dispatcher/Dispatcher.multiclient.sol (1)
27-27: The deployment of the dispatcher proxy and implementation insetUp()now includes thefeeVaultparameter. Ensure that all calls todeployDispatcherProxyAndImplacross the test suite are consistent with this change to avoid deployment errors.#!/bin/bash # Description: Verify consistency of dispatcher deployment across tests. # Test: Search for the deployment function usage. Expect: Consistent usage of feeVault parameter. rg --type solidity $'deployDispatcherProxyAndImpl'test/Dispatcher/Dispatcher.closeChannel.t.sol (2)
24-30: Ensure that thevm.startPrankandvm.stopPrankare used consistently to simulate actions from different users. It's good practice to wrap these calls around all actions that require user-specific context to prevent state leakage between tests.
61-74: The tests for sending packets after channel closure should explicitly check for the correct error messages. This would help ensure that the system behaves as expected under these edge cases and improves the test's reliability.#!/bin/bash # Description: Check for specific error messages in packet sending tests post-channel closure. # Test: Search for the error message assertions. rg --type solidity $'expectRevert.*channelNotOwnedBySender'test/Dispatcher/Dispatcher.dappHandlerRevert.t.sol (1)
24-24: The setup method inDappHandlerRevertTestsnow includes thefeeVaultparameter when deploying the dispatcher proxy and implementation. This is consistent with the changes in the PR, ensuring that fee handling is integrated into the dispatcher setup.contracts/base/GeneralMiddleware.sol (2)
64-66: Ensure that the_sendPacketfunction is implemented properly as it's being called here.#!/bin/bash # Description: Verify the implementation of `_sendPacket` function. ast-grep --lang solidity --pattern $'function _sendPacket($_, $_, $_, $_, $_, $_) { $$$ }' contracts/base/GeneralMiddleware.solTools
GitHub Check: lint
[warning] 66-66:
Named parameters missing. MIN unnamed argumenst is 4
85-86: Ensure that the_sendPacketfunction is implemented properly as it's being called here.#!/bin/bash # Description: Verify the implementation of `_sendPacket` function. ast-grep --lang solidity --pattern $'function _sendPacket($_, $_, $_, $_, $_, $_) { $$$ }' contracts/base/GeneralMiddleware.solTools
GitHub Check: lint
[warning] 86-86:
Named parameters missing. MIN unnamed argumenst is 4test/Dispatcher/Dispatcher.proof.t.sol (1)
35-67: This test methodtest_ibc_channel_open_init_WithFeeeffectively checks the fee handling during channel initialization. Ensure that thefeeVaultcontract is properly handling the fee deposits and calculations as expected.#!/bin/bash # Description: Verify the fee handling in `feeVault` contract. ast-grep --lang solidity --pattern $'function depositOpenChannelFee($_, $_, $_, $_) { $$$ }' contracts/interfaces/IFeeVault.solcontracts/core/UniversalChannelHandler.sol (1)
94-117: The functionsendUniversalPacketWithFeeeffectively handles the sending of packets with associated fees. Ensure that the_depositSendPacketFeefunction is implemented correctly to handle fee deposits.#!/bin/bash # Description: Verify the implementation of `_depositSendPacketFee` function. ast-grep --lang solidity --pattern $'function _depositSendPacketFee($_, $_, $_, $_, $_) { $$$ }' contracts/core/UniversalChannelHandler.solcontracts/examples/Mars.sol (2)
56-68: ThetriggerChannelInitWithFeefunction correctly initializes channel with fees. Ensure that the_depositOpenChannelFeefunction is implemented correctly to handle the deposit of fees.#!/bin/bash # Description: Verify the implementation of `_depositOpenChannelFee` function. ast-grep --lang solidity --pattern $'function _depositOpenChannelFee($_, $_, $_, $_) { $$$ }' contracts/examples/Mars.sol
126-140: ThegreetWithFeefunction effectively sends a greeting message with associated fees. Ensure that the_depositSendPacketFeefunction is implemented correctly to handle the deposit of fees.#!/bin/bash # Description: Verify the implementation of `_depositSendPacketFee` function. ast-grep --lang solidity --pattern $'function _depositSendPacketFee($_, $_, $_, $_, $_) { $$$ }' contracts/examples/Mars.soltest/upgradeableProxy/Dispatcher.upgrade.t.sol (4)
27-28: New imports ofIFeeVaultandFeeVaulthave been added to support fee management.
47-53: This function upgrades the dispatcher and initializes it with the newfeeVault. Ensure that theinitializefunction inDispatcherV2Initializableproperly handles the new parameter.#!/bin/bash # Description: Verify the `initialize` function in `DispatcherV2Initializable` handles `feeVault` correctly. # Test: Search for the `initialize` function definition. Expect: Proper handling of `feeVault`. ast-grep --lang solidity --pattern $'contract DispatcherV2Initializable { function initialize($_, $_) { $$$ } }'
64-69: The deployment ofIbcDispatcherRc4usingDispatcherRc4seems correct. Ensure that theinitializemethod ofDispatcherRc4is compatible with the provided arguments.#!/bin/bash # Description: Verify the `initialize` method of `DispatcherRc4`. # Test: Search for the `initialize` method in `DispatcherRc4`. Expect: Compatibility with provided arguments. ast-grep --lang solidity --pattern $'contract DispatcherRc4 { function initialize($_, $_) { $$$ } }'
Line range hint
177-192: The deployment and upgrade of the dispatcher proxy with a newFeeVaultinstance are tested here. It's crucial to ensure theupgradeDispatcherfunction handles the upgrade process correctly, particularly the initialization with the newfeeVault.#!/bin/bash # Description: Verify the upgrade process in `upgradeDispatcher`. # Test: Search for the `upgradeDispatcher` function. Expect: Correct handling of the upgrade and initialization. ast-grep --lang solidity --pattern $'function upgradeDispatcher($_, $_, $_) { $$$ }'contracts/interfaces/IbcMiddleware.sol (2)
59-59: ThesendMWPacketfunction now returns auint64 sequence, aligning it with the new packet handling requirements that include sequence tracking.
34-43: The addition ofsendUniversalPacketWithFeewith fee parametersgasLimitsandgasPricesextends the functionality to handle fee-based packet sending. Ensure that all middleware contracts that implement this interface handle these parameters correctly.#!/bin/bash # Description: Verify handling of `gasLimits` and `gasPrices` in implementing contracts. # Test: Search for implementations of `sendUniversalPacketWithFee`. Expect: Correct handling of `gasLimits` and `gasPrices`. ast-grep --lang solidity --pattern $'contract $_ implements IbcUniversalPacketSender { function sendUniversalPacketWithFee($_, $_, $_, $_, $_, $_) { $$$ } }'test/utils/Dispatcher.base.t.sol (4)
17-17: TheFeeVaultimport is added to support fee handling in the test environment. This aligns with the PR's objective to integrate fee management.
40-40: The new eventOpenChannelFeeDepositedis added to handle the logging of fee deposits during channel opening. This is a good practice for tracking operations and debugging.
51-67: The introduction of fee management variables and theFeeVaultinstance setup in the test environment is crucial for testing the new fee functionalities. Ensure that these settings are utilized appropriately in the test cases.#!/bin/bash # Description: Verify the use of fee management settings in test cases. # Test: Search for usage of `feeVault` and fee-related variables in test functions. Expect: Appropriate utilization. ast-grep --lang solidity --pattern $'contract $_ { IFeeVault feeVault; $$$ function $_() { $$$ } }'
109-145: ThechannelOpenInitWithFeefunction is added to handle channel openings with fee requirements. This function should correctly handle fee transactions and state changes, including the verification of balance adjustments.#!/bin/bash # Description: Verify the handling of fees in `channelOpenInitWithFee`. # Test: Search for the `channelOpenInitWithFee` function. Expect: Correct fee handling and state changes. ast-grep --lang solidity --pattern $'function channelOpenInitWithFee($_, $_, $_, $_) { $$$ }'test/VirtualChain.sol (3)
9-16: The introduction ofFeeVaultandIFeeVaultimports and their usage in the virtual chain setup is essential for testing fee management in a simulated blockchain environment. This aligns with the PR's objective to integrate fee handling.
40-48: ThefeeVaultis now part of theVirtualChainDatastruct and is used in the constructor to set up the environment. This is crucial for ensuring that fee management is integrated into the virtual chain environment from the start.
Line range hint
67-90: The setup process in the constructor where thefeeVaultis used to initialize the dispatcher and universal channel handler proxies is correctly implemented. This ensures that the fee handling capabilities are integrated from the beginning of the chain's operation.test/universal.channel.t.sol (3)
15-15: Ensure that theIFeeVaultinterface is correctly implemented and used in the tests as expected.#!/bin/bash # Description: Verify the correct implementation and usage of IFeeVault interface. ast-grep --lang solidity --pattern $'interface IFeeVault { $$$ }' ../contracts/interfaces/IFeeVault.sol
133-133: The addition of theSendPacketFeeDepositedevent is crucial for tracking and auditing fee transactions. Ensure that this event is emitted correctly in the context where fees are deposited.#!/bin/bash # Description: Verify that SendPacketFeeDeposited event is emitted correctly. ast-grep --lang solidity --pattern $'emit SendPacketFeeDeposited($_, $_, $_, $_)' test/universal.channel.t.sol
467-495: The alternating test for non-fee and fee-based packet sending is an excellent approach to ensure both scenarios are handled correctly. However, ensure that the fee calculations and the assertions of balance changes are thoroughly tested to prevent any discrepancies in fee handling.#!/bin/bash # Description: Verify the correct handling and calculations of fees in fee-based packet sending scenarios. ast-grep --lang solidity --pattern $'assertEq($_, $_ + $_)' test/universal.channel.t.solcontracts/core/Dispatcher.sol (4)
31-31: Ensure theIFeeVaultimport is used appropriately throughout the contract.
68-68: Introduction of a publicfeeVaultvariable to store the fee management contract.
80-87: Modification to theinitializefunction to accept anIFeeVaultinstance. This change is crucial for integrating the fee functionality right from the contract's initialization.#!/bin/bash # Description: Verify that the `initialize` function is only called once or appropriately controlled across upgrades. # Search for calls to `initialize` in the blockchain's transaction history for this contract. # This script is a placeholder and assumes access to blockchain transaction data. echo "Search blockchain transaction data for calls to the 'initialize' function."
Line range hint
480-494: ThesendPacketfunction now returns auint64sequence number, which is a significant change to support tracking packet sequences with fees. Ensure that this new return value is handled correctly by all calling functions.#!/bin/bash # Description: Check all calls to `sendPacket` to ensure they handle the return value correctly. # Search for `sendPacket` function calls in the codebase to ensure they use the sequence number. rg --type sol $'sendPacket'
RnkSngh
force-pushed
the
raunak/dispatcher-agnostic-fee-implementation
branch
from
92c3e52 to
a817989
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- contracts/core/FeeVault.sol (1 hunks)
- contracts/examples/Mars.sol (4 hunks)
- contracts/implementation_templates/FeeSender.sol (1 hunks)
- contracts/interfaces/IFeeVault.sol (1 hunks)
- test/Dispatcher/Dispatcher.proof.t.sol (2 hunks)
- test/Dispatcher/Dispatcher.t.sol (5 hunks)
- test/FeeVault.t.sol (1 hunks)
- test/universal.channel.t.sol (4 hunks)
- test/utils/Dispatcher.base.t.sol (4 hunks)
Files not reviewed due to errors (1)
- test/utils/Dispatcher.base.t.sol (no review received)
Files skipped from review as they are similar to previous changes (7)
- contracts/core/FeeVault.sol
- contracts/implementation_templates/FeeSender.sol
- contracts/interfaces/IFeeVault.sol
- test/Dispatcher/Dispatcher.proof.t.sol
- test/Dispatcher/Dispatcher.t.sol
- test/FeeVault.t.sol
- test/universal.channel.t.sol
Additional comments not posted (2)
contracts/examples/Mars.sol (2)
23-24: The addition ofIFeeVaultandFeeSenderimports, and their use in the contract inheritance, correctly aligns with the PR's objectives to integrate fee handling. Ensure that all imported modules are utilized and no unnecessary imports remain.Also applies to: 32-32
56-66: The implementation oftriggerChannelInitWithFeecorrectly handles fee integration with channel initialization. Consider adding a comment explaining the use of the_dispatcherlocal variable for gas optimization.
There was a problem hiding this comment.
would be good to break large PRs like this into smaller ones in future. for example, changes for adding sequence number return could be separate from fee vault changes. in this case fee fault addition alone would be rather large, probably stackable prs would be perfect here.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
test/utils/Dispatcher.base.t.sol (1)
52-52: Instantiation of FeeVaultThe direct instantiation of
FeeVaultas a state variable is critical for handling fee operations. However, consider ifFeeVaultshould be passed as an argument or set via a setter method to enhance modularity and testability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- contracts/core/FeeVault.sol (1 hunks)
- contracts/core/UniversalChannelHandler.sol (3 hunks)
- contracts/examples/Mars.sol (3 hunks)
- contracts/interfaces/IFeeVault.sol (1 hunks)
- test/Dispatcher/Dispatcher.proof.t.sol (2 hunks)
- test/upgradeableProxy/DispatcherRC4.upgrade.t.sol (10 hunks)
- test/utils/Dispatcher.base.t.sol (4 hunks)
Additional comments not posted (15)
contracts/interfaces/IFeeVault.sol (3)
20-23: New structGasFeeadded for fee management.The struct is well-defined and encapsulates the
gasLimitandgasPrice, which are essential for calculating transaction fees.
25-28: New structSendpacketFeeDepositedfor event data encapsulation.This struct will be useful for logging the details of fees deposited for sending packets, enhancing traceability of transactions.
30-49: InterfaceIFeeVaultadditions are comprehensive.The addition of methods for fee management (
depositSendPacketFee,depositOpenChannelFee,withdrawFeesToOwner) and associated events and errors are consistent with the PR objectives to enhance fee handling in the protocol.contracts/core/FeeVault.sol (1)
24-51: Implementation ofFeeVaultcontract.The implementation of the
FeeVaultcontract includes appropriate use of theReentrancyGuardto prevent re-entrant attacks, which is crucial for contract security. The methods for depositing fees and withdrawing them to the owner are correctly implemented with checks for correct fee amounts and non-zero fees.test/Dispatcher/Dispatcher.proof.t.sol (1)
35-51: New test for channel opening with fees.The test
test_ibc_channel_open_init_WithFeecorrectly handles the setup for testing channel openings with fees, including balance checks and event emissions. This ensures that the fee functionality is working as expected.contracts/core/UniversalChannelHandler.sol (1)
Line range hint
35-117: Integration of fee handling inUniversalChannelHandler.The methods
sendUniversalPacketandsendUniversalPacketWithFeeare well-implemented, with the latter including fee handling logic. This integration aligns with the PR objectives to enhance fee management across the protocol.contracts/examples/Mars.sol (1)
Line range hint
54-136: Fee handling inMarscontract.The methods
triggerChannelInitWithFeeandgreetWithFeeare correctly implemented to include fee handling. This is consistent with the PR's goal of integrating a fee vault system and ensures that the example contractMarsdemonstrates the intended functionality.test/utils/Dispatcher.base.t.sol (3)
17-17: New import added for FeeVaultThe import of
FeeVaultaligns with the PR's objective to implement a fee vault system. This will help in testing the fee functionalities in the Dispatcher base tests.
40-41: New events for fee handlingThe addition of
SendPacketFeeDepositedandOpenChannelFeeDepositedevents is crucial for tracking fee deposits related to packet sending and channel opening. This is a good practice as it enhances transparency and traceability in fee management.
117-143: New function for channel opening with feesThe new
channelOpenInitWithFeefunction integrates fee handling into the channel opening process, which is a significant part of the fee vault implementation. Ensure that the balance checks and fee deposits are correctly handled to prevent any discrepancies.test/upgradeableProxy/DispatcherRC4.upgrade.t.sol (5)
28-34: Import and Usage of Upgraded ContractsThe import statements for
MarsRc4,EarthRc4, and related contracts indicate the preparation for testing upgraded contract functionalities. This is essential for ensuring that the upgraded contracts integrate well with the existing system.
Line range hint
57-104: Setup Function EnhancementsThe
setUpfunction has been significantly enhanced to handle multiple scenarios including dispatcher and UCH upgrades. It's crucial to ensure that all new contracts and functionalities are correctly initialized and integrated.
136-145: Test for Sending Packet Post-UpgradeThis test checks the state conservation post-upgrade, which is critical for ensuring that upgrades do not disrupt the ongoing operations. It's good to see comprehensive checks on packet commitments.
157-177: Open Channel State Conservation TestThis segment tests the conservation of channel states post-upgrade. It's crucial to validate that the channel states, such as sequence numbers, are correctly managed during and after the upgrade.
261-261: Consider adding more assertionsGiven the significant actions being tested, it's important to ensure that each step is verified thoroughly to prevent issues post-deployment.
Also applies to: 276-276
RnkSngh
force-pushed
the
raunak/dispatcher-agnostic-fee-implementation
branch
from
936e194 to
42fcfb0
Compare
RnkSngh
force-pushed
the
raunak/dispatcher-agnostic-fee-implementation
branch
from
6fc7939 to
35e7481
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (28)
- contracts/base/GeneralMiddleware.sol (3 hunks)
- contracts/core/Dispatcher.sol (5 hunks)
- contracts/core/FeeVault.sol (1 hunks)
- contracts/core/UniversalChannelHandler.sol (3 hunks)
- contracts/examples/Earth.sol (2 hunks)
- contracts/examples/Mars.sol (3 hunks)
- contracts/implementation_templates/FeeSender.sol (1 hunks)
- contracts/implementation_templates/IbcReceiverUpgradeable.sol (1 hunks)
- contracts/interfaces/IDispatcher.sol (3 hunks)
- contracts/interfaces/IFeeVault.sol (1 hunks)
- contracts/interfaces/IbcDispatcher.sol (2 hunks)
- contracts/interfaces/IbcMiddleware.sol (2 hunks)
- test/Dispatcher.gasGriefing.t.sol (1 hunks)
- test/Dispatcher/Dispatcher.client.t.sol (1 hunks)
- test/Dispatcher/Dispatcher.closeChannel.t.sol (3 hunks)
- test/Dispatcher/Dispatcher.dappHandlerRevert.t.sol (1 hunks)
- test/Dispatcher/Dispatcher.multiclient.sol (1 hunks)
- test/Dispatcher/Dispatcher.proof.t.sol (2 hunks)
- test/Dispatcher/Dispatcher.t.sol (5 hunks)
- test/FeeVault.t.sol (1 hunks)
- test/VirtualChain.sol (4 hunks)
- test/universal.channel.t.sol (4 hunks)
- test/upgradeableProxy/Dispatcher.upgrade.t.sol (5 hunks)
- test/upgradeableProxy/DispatcherRC4.upgrade.t.sol (9 hunks)
- test/upgradeableProxy/DispatcherUUPS.accessControl.t.sol (4 hunks)
- test/upgradeableProxy/upgrades/DispatcherV2Initializable.sol (2 hunks)
- test/utils/Dispatcher.base.t.sol (4 hunks)
- test/utils/TestUtils.t.sol (3 hunks)
Files skipped from review as they are similar to previous changes (5)
- contracts/core/FeeVault.sol
- contracts/core/UniversalChannelHandler.sol
- contracts/examples/Mars.sol
- contracts/interfaces/IFeeVault.sol
- test/Dispatcher/Dispatcher.proof.t.sol
Additional context used
GitHub Check: lint
contracts/base/GeneralMiddleware.sol
[warning] 48-48:
Variable name must be in mixedCase
[warning] 50-50:
GC: [source] on Event [UCHPacketSent] could be Indexed
[warning] 59-59:
Function sendUniversalPacket() must match Foundry test naming convention
[warning] 66-66:
Named parameters missing. MIN unnamed argumenst is 4
[warning] 69-69:
Function sendUniversalPacketWithFee() must match Foundry test naming convention
[warning] 76-76:
Code contains empty blocks
[warning] 78-78:
Function sendMWPacket() must match Foundry test naming convention
[warning] 86-86:
Named parameters missing. MIN unnamed argumenst is 4
[warning] 89-89:
All public or external methods in a contract must override a definition from an interface
[warning] 89-89:
Function onRecvMWPacket() must match Foundry test naming convention
Additional comments not posted (45)
test/upgradeableProxy/upgrades/DispatcherV2Initializable.sol (1)
22-22: Approval of the import statement forIFeeVault.The import is correctly placed and follows standard Solidity conventions.
contracts/implementation_templates/FeeSender.sol (2)
23-33: Review of_depositSendPacketFeemethod.This method correctly interacts with the
feeVaultto deposit fees for sending packets. It uses the Solidityvaluekeyword to forward the transaction value, which is a good practice for handling payable functions.
35-45: Review of_depositOpenChannelFeemethod.The method is correctly implemented to handle fee deposits for channel openings. It uses the
valuekeyword appropriately.test/Dispatcher/Dispatcher.client.t.sol (1)
34-34: Ensure correct setup in test environment.The modification in the
setUpmethod to includefeeVaultensures that the testing environment mirrors the new contract structure. This is crucial for accurate and comprehensive testing.test/Dispatcher.gasGriefing.t.sol (1)
25-25: Correct setup of testing environment for gas griefing scenarios.The inclusion of
feeVaultin thesetUpmethod is essential for testing the dispatcher under scenarios that include fee handling. This ensures the tests are relevant to the updated contract functionalities.test/utils/TestUtils.t.sol (1)
Line range hint
19-28: Review ofdeployDispatcherProxyAndImplmethod.The method is correctly updated to include
feeVaultduring the dispatcher deployment. This change is necessary to ensure that the deployed dispatcher instance is initialized with the newfeeVaultfunctionality.contracts/implementation_templates/IbcReceiverUpgradeable.sol (1)
21-21: Ensure consistency in import paths.The import statement for
IbcDispatcheruses relative paths, which is consistent with other imports in the project. This helps maintain a clean and organized codebase.test/upgradeableProxy/DispatcherUUPS.accessControl.t.sol (4)
8-8: Good addition of theIFeeVaultinterface.Importing the
IFeeVaultinterface aligns with the changes made in the Dispatcher contracts to handle fee vault functionalities. This ensures that the tests will cover these new functionalities.
25-25: Validate the setup of the testing environment.The setup function correctly deploys the Dispatcher proxy and implementation while incorporating the
feeVault, ensuring that the test environment mirrors the new contract structure.
39-45: Check the upgrade and initialization process in UUPS.The test case
test_Dispatcher_Allows_Upgrade_To_And_Calleffectively verifies the upgrade and call functionality of the dispatcher using the newFeeVault. This is crucial for ensuring that upgrades are handled correctly in a live environment.
61-61: Ensure reinitialization protection is effective.The test
test_Dispatcher_Prevents_Reinit_Attackschecks for reinitialization, which is a critical security aspect in upgradeable contracts. This helps prevent potential vulnerabilities associated with reinitialization.test/FeeVault.t.sol (2)
30-46: Validate the fuzz testing logic for fee handling.The function
testFuzz_sendPacket_FeesAddUpToValueuses fuzz testing to ensure that the contract handles fees correctly under various conditions. This is a robust way to test the functionality under unpredictable inputs.
52-64: Check the logic for withdrawing fees to the owner.The function
test_withdrawAlwaysGoesToOwnerverifies that only the owner can withdraw fees, which is a critical security feature. This helps prevent unauthorized access to the funds.contracts/interfaces/IbcDispatcher.sol (2)
22-22: Good integration of theIFeeVaultinterface.The import of the
IFeeVaultinterface is crucial for enabling fee handling in the IBC dispatcher. This aligns with the changes across the contracts to support fee functionalities.
53-53: Validate thefeeVaultfunction implementation.The addition of the
feeVaultfunction to theIbcDispatcherinterface is essential for accessing the fee vault from other contracts. This ensures that the fee handling functionality is integrated throughout the system.contracts/interfaces/IDispatcher.sol (2)
24-24: Consistent import of theIFeeVaultinterface.The import of the
IFeeVaultinterface is consistent with the changes made in other related contracts, ensuring that the fee vault functionality is accessible in the dispatcher.
109-109: Ensure accessibility of thefeeVaultmethod.The addition of the
feeVaultmethod to theIDispatcherinterface allows for consistent access to the fee vault across different components of the system, which is crucial for integrated fee management.contracts/examples/Earth.sol (2)
23-23: New interface import noted: IUniversalChannelHandler.This import supports the new fee functionality, ensuring that
Earthcan interact with the updated middleware handling fees.
66-77: New functiongreetWithFeeproperly implements fee handling.This function is a new addition that handles sending packets with associated fees. The implementation correctly uses the
IUniversalChannelHandlerinterface and passes all necessary parameters, including gas limits and prices, which are crucial for fee calculations.test/Dispatcher/Dispatcher.multiclient.sol (1)
27-27: Updated deployment to include fee vault.The
setUpfunction now includes the deployment of the dispatcher proxy and implementation with the fee vault, which is essential for handling fees in the test environment.test/Dispatcher/Dispatcher.closeChannel.t.sol (1)
24-30: Proper handling ofvm.startPrankandvm.stopPrankin channel close tests.The use of
vm.startPrankandvm.stopPrankensures that state changes are correctly reverted if not committed, which is crucial for accurately testing channel closure scenarios, especially with the inclusion of fee handling.Also applies to: 61-74
test/Dispatcher/Dispatcher.dappHandlerRevert.t.sol (1)
24-24: Updated setup function to include fee vault in deployments.The inclusion of the fee vault in the
setUpfunction is crucial for ensuring that the fee handling functionality is correctly integrated into the test environment.test/upgradeableProxy/Dispatcher.upgrade.t.sol (5)
27-28: Approval of New ImportsThe inclusion of
IFeeVaultandFeeVaultis consistent with the PR's focus on fee handling mechanisms. These imports are necessary for the new functionality.
34-35: Approval of New Imports for RC4 UpgradesThe import of
DispatcherRc4and related contracts from theupgradesdirectory is appropriate for handling version-specific logic and maintaining compatibility.
64-69: Correctness of Proxy Deployment for Dispatcher RC4The method correctly encodes initialization data and creates a new proxy instance for
DispatcherRc4. This pattern is standard for upgradeable contracts and is implemented correctly here.
47-53: Ensure Proper Initialization During UpgradeThe
upgradeDispatcherfunction now includes afeeVaultparameter, which is crucial for initializing the upgraded Dispatcher contract with fee handling capabilities. The use ofencodeWithSignaturefor initialization data ensures that the contract is initialized correctly. However, ensure that thefeeVaultaddress provided is valid and has the expected contract deployed.Verification successful
Ensure Proper Initialization During Upgrade
The
upgradeDispatcherfunction now includes afeeVaultparameter, which is crucial for initializing the upgraded Dispatcher contract with fee handling capabilities. The use ofencodeWithSignaturefor initialization data ensures that the contract is initialized correctly. The test file initializesnewFeeVaultas a new instance ofFeeVault, which is passed to theupgradeDispatcherfunction. This confirms that thefeeVaultaddress is valid and points to a deployed contract.
test/upgradeableProxy/Dispatcher.upgrade.t.sol:IFeeVault newFeeVault = new FeeVault(); upgradeDispatcher("adfsafsa", newFeeVault, address(dispatcherProxy));Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the provided feeVault address points to a valid contract echo "Check feeVault address validity"Length of output: 72
Script:
#!/bin/bash # Locate the Dispatcher.upgrade.t.sol file file_path=$(fd Dispatcher.upgrade.t.sol) # Extract the feeVault address from the test script fee_vault_address=$(grep -oP 'address feeVault\s*=\s*0x[a-fA-F0-9]{40}' "$file_path" | awk -F'=' '{print $2}' | tr -d ' ') # Verify if the feeVault address points to a valid contract by checking if there is code at the address if [ -n "$fee_vault_address" ]; then if [ $(seth code "$fee_vault_address") != "0x" ]; then echo "Valid contract at feeVault address: $fee_vault_address" else echo "No contract found at feeVault address: $fee_vault_address" fi else echo "feeVault address not found in the test script" fiLength of output: 366
Script:
#!/bin/bash # Locate the Dispatcher.upgrade.t.sol file file_path=$(fd Dispatcher.upgrade.t.sol) # Search for the assignment or initialization of the feeVault address in the test file grep -E 'feeVault\s*=\s*0x[a-fA-F0-9]{40}' "$file_path"Length of output: 181
Script:
#!/bin/bash # Locate the Dispatcher.upgrade.t.sol file file_path=$(fd Dispatcher.upgrade.t.sol) # Search for any reference to feeVault in the test file to understand how it is being set or used grep -E 'feeVault' "$file_path"Length of output: 472
Script:
#!/bin/bash # Locate the Dispatcher.upgrade.t.sol file file_path=$(fd Dispatcher.upgrade.t.sol) # Search for calls to the upgradeDispatcher function in the test file and check the feeVault parameter grep -E 'upgradeDispatcher\(' "$file_path"Length of output: 348
Script:
#!/bin/bash # Locate the Dispatcher.upgrade.t.sol file file_path=$(fd Dispatcher.upgrade.t.sol) # Search for the definition or assignment of newFeeVault in the test file grep -E 'newFeeVault' "$file_path"Length of output: 284
Line range hint
177-192: Review of Dispatcher Upgrade ProcessThe upgrade process for the Dispatcher contract is initiated correctly, and a new
FeeVaultinstance is created and passed to theupgradeDispatcherfunction. Ensure that thenewFeeVaultis correctly integrated and that the upgrade does not disrupt existing functionalities.Verification successful
Review of Dispatcher Upgrade Process
The integration of
newFeeVaultand the upgrade process fordispatcherProxyhave been correctly implemented. The functionsdoChannelHandshakeandsendPacketare defined and appear to be intact post-upgrade. Therefore, the upgrade process does not disrupt existing functionalities.
newFeeVaultis instantiated in multiple test files, includingDispatcher.upgrade.t.sol.- The
upgradeDispatcherfunction is defined inDispatcher.upgrade.t.sol.- The
doChannelHandshakeandsendPacketfunctions are defined and used in the relevant files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the integration of newFeeVault and check for disruptions in existing functionalities echo "Verify newFeeVault integration and functionality continuity"Length of output: 128
Script:
#!/bin/bash # Verify the deployment of newFeeVault and its address ast-grep --lang solidity --pattern 'new FeeVault()' --jsonLength of output: 177
Script:
#!/bin/bash # Verify the proper upgrade of dispatcherProxy using newFeeVault ast-grep --lang solidity --pattern 'function upgradeDispatcher($_, $_, $_) { $$$ }' --jsonLength of output: 209
Script:
#!/bin/bash # Check if existing functionalities are intact after the upgrade ast-grep --lang solidity --pattern 'function doChannelHandshake($_, $_) { $$$ }' --json ast-grep --lang solidity --pattern 'function sendPacket($_) { $$$ }' --jsonLength of output: 401
Script:
#!/bin/bash # Verify the deployment of newFeeVault and its address rg 'new FeeVault()' -t solidityLength of output: 66
Script:
#!/bin/bash # Verify the proper upgrade of dispatcherProxy using newFeeVault rg 'function upgradeDispatcher' -t solidityLength of output: 78
Script:
#!/bin/bash # Check if existing functionalities are intact after the upgrade rg 'function doChannelHandshake' -t solidity rg 'function sendPacket' -t solidityLength of output: 151
Script:
#!/bin/bash # Verify the deployment of newFeeVault and its address grep -r 'new FeeVault()' --include \*.solLength of output: 375
Script:
#!/bin/bash # Verify the proper upgrade of dispatcherProxy using newFeeVault grep -r 'function upgradeDispatcher' --include \*.solLength of output: 205
Script:
#!/bin/bash # Check if existing functionalities are intact after the upgrade grep -r 'function doChannelHandshake' --include \*.sol grep -r 'function sendPacket' --include \*.solLength of output: 2256
contracts/interfaces/IbcMiddleware.sol (2)
34-43: Approval of New Universal Packet Sending Function with Fee HandlingThe addition of
sendUniversalPacketWithFeeis in line with the PR's objective to introduce fee handling mechanisms. The method signature is clear, and the use ofpayablekeyword is appropriate for functions that handle Ether transactions.
59-59: Approval of Changes to sendMWPacketThe return type
uint64 sequenceadded tosendMWPacketis appropriate and provides a clear sequence number for tracking packets, enhancing the function's usability in middleware operations.test/utils/Dispatcher.base.t.sol (4)
17-18: Approval of New Imports for Fee HandlingThe addition of
FeeVaultandIFeeVaultimports is necessary for the new fee handling functionalities in the test utilities.
41-50: Approval of New Events for Fee HandlingThe introduction of
SendPacketFeeDepositedandOpenChannelFeeDepositedevents is a positive change. These events enhance the transparency and traceability of fee-related actions within the system.
60-60: Creation of FeeVault InstanceThe instantiation of
FeeVaultdirectly in the test setup is appropriate for testing fee handling functionalities. Ensure that thefeeVaultis used consistently throughout the tests.
117-155: Review of Channel Opening with Fee HandlingThe
channelOpenInitWithFeefunction properly integrates fee handling into the channel opening process. The use ofvm.expectEmitandvm.startPrank/vm.stopPrankis correctly implemented. Ensure that the fee transactions are validated and that the balance checks are accurate.test/VirtualChain.sol (6)
9-9: Approved addition of IFeeVault import.This import is necessary for integrating the new fee functionality within the virtual chain setup.
16-16: Approved addition of FeeVault import.This aligns with the PR's objective to handle fee management within the virtual chain context.
40-40: Approved the addition of the feeVault variable in the VirtualChainData struct.This ensures that fee management functionality is encapsulated within the virtual chain data, aligning with the contract's enhanced responsibilities.
48-48: Approved the declaration of feeVault in the VirtualChain contract.This public variable declaration is crucial for integrating fee management directly accessible within the virtual chain setup.
67-69: Approved the instantiation of FeeVault and its integration in the constructor.The instantiation ensures that the
feeVaultis properly initialized before being used, which is critical for the fee management functionality of the contract.
90-90: Approved the return of feeVault in getVirtualChainData.This ensures that external entities can access the fee vault configuration as part of the virtual chain's state, which is essential for transparency and interaction with the fee management functionality.
test/upgradeableProxy/DispatcherRC4.upgrade.t.sol (1)
13-13: Comprehensive review of DispatcherRC4.upgrade.t.sol changes.
Import and Structure Changes:
- The import of new structures and types from
Ibc.solandEarthRc4.solsupports the updated testing requirements for the RC4 upgrade scenario.Test Utility Modifications:
- The introduction of
DispatcherRC4TestUtilsand the methods within it (sendOnePacketRc4,sendPacketRc4) are crucial for simulating packet sending in a controlled testing environment, ensuring that packet sequence and state are correctly managed and emitted.Upgrade Test Implementation:
- The
DispatcherRC4UpgradeTestcontract includes comprehensive testing for state preservation during upgrades. This includes testing packet sequence conservation (test_SentPacketState_Conserved_RC4_Upgrade), channel handshake state, and packet sending post-upgrade.State Preservation Assertions:
- The extensive use of
assertstatements to verify the state of packet sequences and channel states post-upgrade is critical for ensuring the robustness of the upgrade process.Midway Upgrade Testing:
- The
DispatcherRC4MidwayUpgradeTestcontract tests the upgrade process during active channel handshakes and packet transmissions, which is essential for validating the upgrade logic under dynamic conditions.Overall, these changes are aligned with the PR's objectives to enhance the testing framework for upgrades, ensuring that state transitions are handled correctly and that the system remains robust across contract version changes.
Also applies to: 28-34, 36-53, 56-312
test/universal.channel.t.sol (1)
15-15: Good addition of theIFeeVaultinterface.Integrating the
IFeeVaultinterface is crucial for handling fee-related functionalities in universal channel tests. This change aligns with the overall PR objectives.contracts/core/Dispatcher.sol (4)
31-31: Addition of FeeVault import is appropriate.The import statement for
IFeeVaultis correctly placed and necessary for the integration of fee management functionalities.
68-68: Newly added FeeVault variable.The
feeVaultvariable is correctly declared as public, allowing external visibility which is essential for interactions from other contracts or for verification purposes.
80-87: Review of theinitializefunction with added feeVault parameter.The addition of the
IFeeVault _feeVaultparameter to theinitializefunction is consistent with the PR's objective to integrate fee management. The function now properly initializes thefeeVaultvariable, ensuring that it is set at contract initialization. Ensure that all calls to this function in the upgrade scripts or deployment scripts have been updated to pass the appropriateIFeeVaultinstance.Verification successful
Ensure all calls to the
initializefunction include the newfeeVaultparameter.The search results indicate that there are multiple
initializefunctions across different files. However, only the relevant calls to theinitializefunction inDispatcher.solshould include theIFeeVaultparameter. The following lines confirm that the newinitializesignature is being used correctly:
test/upgradeableProxy/DispatcherUUPS.accessControl.t.sol: dispatcherImplementation.initialize("IIpolyibc.eth.", IFeeVault(vm.addr(1)));contracts/core/Dispatcher.sol: function initialize(string memory initPortPrefix, IFeeVault _feeVault) public virtual initializer nonReentrant {Other
initializefunctions without thefeeVaultparameter appear to belong to different contracts or versions.Conclusion: The update to the
initializefunction to include thefeeVaultparameter is correctly reflected in the relevant places.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all calls to `initialize` include the new `feeVault` parameter. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type solidity $'initialize'Length of output: 64
Script:
#!/bin/bash # Description: Verify that all calls to `initialize` include the new `feeVault` parameter. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg 'initialize\(' --glob '*.sol'Length of output: 1834
482-486: Modification in sendPacket function to return sequence number.The change in the
sendPacketfunction to return auint64 sequenceis aligned with the requirement to handle packet sequence numbers linked to fee depositing. This is a crucial update for the fee management system. Ensure that all invocations ofsendPackethandle the returned sequence number appropriately.Verification successful
Modification in sendPacket function to return sequence number.
The change in the
sendPacketfunction to return auint64 sequenceis aligned with the requirement to handle packet sequence numbers linked to fee depositing. This is a crucial update for the fee management system. Ensure that all invocations ofsendPackethandle the returned sequence number appropriately.
- contracts/examples/Mars.sol:
- Lines: 24, 28
- The returned sequence number is being assigned to the
sequencevariable.- contracts/core/UniversalChannelHandler.sol:
- Lines: 50, 51
- The returned sequence number is being assigned to the
sequencevariable.These instances confirm that the return value is being handled correctly in the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `sendPacket` handle the new return value. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type solidity $'sendPacket'Length of output: 64
Script:
#!/bin/bash # Description: Verify all function calls to `sendPacket` handle the new return value. # Test: Search for the function usage. Expect: Only occurrences of the new signature. # Correcting the file type to search for .sol files specifically. rg 'sendPacket' --glob '*.sol'Length of output: 9173
RnkSngh
force-pushed
the
raunak/dispatcher-agnostic-fee-implementation
branch
from
a461679 to
3a47f35
Compare
RnkSngh
force-pushed
the
raunak/dispatcher-agnostic-fee-implementation
branch
from
3a47f35 to
55b7c8c
Compare
PR to add a simple fee vault integration:
Summary by CodeRabbit
New Features
sendUniversalPacketWithFeemethod for fee-based packet sending.Bug Fixes
Tests
Refactor
Chores