refactor(precompiles): chain as source of truth #66
Conversation
The `ExocoreGateway` allows the contract owner to call precompiles which allow the registration of a client chain or a token. It also enables updated of these items. To check whether a request is an update or an addition, it relies on the ExocoreGateway keeping a copy of registered items. However, this does not work for items which were registered at genesis. This PR works around that problem by requiring that the single source of truth be the precompiles, whose responsibility is to report if a request was an update or an addition.
WalkthroughThe recent updates to the contract system significantly enhance token management by consolidating functionalities for adding and updating whitelist tokens. Key changes include renaming functions for clarity, removing redundant elements, and streamlining processes. These improvements create a more efficient, user-friendly system for managing tokens and client chains, ultimately simplifying the interaction for contract owners. Changes
Sequence Diagram(s)sequenceDiagram
participant Owner
participant ExocoreGateway
participant ClientChain
participant Token
Owner->>ExocoreGateway: addOrUpdateWhitelistTokens()
ExocoreGateway->>Token: Validate tokens
Token->>ExocoreGateway: Return validation status
ExocoreGateway->>ClientChain: Register or update chain
ClientChain->>ExocoreGateway: Return registration status
ExocoreGateway->>Owner: Emit events based on action
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 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, codebase verification and nitpick comments (2)
docs/contracts-owner-manual.md (2)
20-20: Add the missing article "the".The section title should include the article "the" before "whitelist" for grammatical correctness.
-## add or update tokens to whitelist +## add or update tokens to the whitelistTools
LanguageTool
[uncategorized] ~20-~20: You might be missing the article “the” here.
Context: ...r contract. ## add or update tokens to whitelist Another important task before restakin...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
22-22: Add the missing article "the".The description should include the article "the" before "whitelist" for grammatical correctness.
-Another important task before restaking being activated is to add tokens to whitelist to mark them as stake-able on both Exocore and client chain. +Another important task before restaking being activated is to add tokens to the whitelist to mark them as stake-able on both Exocore and client chain.Tools
LanguageTool
[uncategorized] ~22-~22: You might be missing the article “the” here.
Context: ...ing being activated is to add tokens to whitelist to mark them as stake-able on both Exoc...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- docs/contracts-owner-manual.md (1 hunks)
- script/3_Setup.s.sol (1 hunks)
- src/core/ExocoreGateway.sol (4 hunks)
- src/interfaces/IExocoreGateway.sol (2 hunks)
- src/interfaces/precompiles/IAssets.sol (1 hunks)
- src/libraries/Errors.sol (1 hunks)
- src/storage/ExocoreGatewayStorage.sol (2 hunks)
- test/foundry/ExocoreDeployer.t.sol (1 hunks)
- test/foundry/unit/ExoCapsule.t.sol (1 hunks)
- test/foundry/unit/ExocoreGateway.t.sol (15 hunks)
- test/mocks/AssetsMock.sol (1 hunks)
- test/mocks/ExocoreGatewayMock.sol (3 hunks)
Additional context used
Learnings (4)
src/interfaces/IExocoreGateway.sol (1)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: test/foundry/unit/ExocoreGateway.t.sol:360-366 Timestamp: 2024-07-02T01:45:58.683Z Learning: The `updateWhitelistedTokens` function in `ExocoreGateway` interacts solely with the trusted `ASSET_CONTRACT` precompile, mitigating the need for a reentrancy guard.test/mocks/ExocoreGatewayMock.sol (2)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#23 File: test/mocks/ExocoreGatewayMock.sol:0-0 Timestamp: 2024-06-17T07:43:43.986Z Learning: User adu-web3 suggests that the `chainToBootstrapped[clientChainId]` marking in `ExocoreGatewayMock.sol` has no significant effects beyond setting a state and that synchronization issues are currently not a concern. They are open to implementing a response check in the future if necessary.Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: test/foundry/unit/ExocoreGateway.t.sol:360-366 Timestamp: 2024-07-02T01:45:58.683Z Learning: The `updateWhitelistedTokens` function in `ExocoreGateway` interacts solely with the trusted `ASSET_CONTRACT` precompile, mitigating the need for a reentrancy guard.test/foundry/ExocoreDeployer.t.sol (1)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: test/foundry/unit/ExocoreGateway.t.sol:360-366 Timestamp: 2024-07-02T01:45:58.683Z Learning: The `updateWhitelistedTokens` function in `ExocoreGateway` interacts solely with the trusted `ASSET_CONTRACT` precompile, mitigating the need for a reentrancy guard.test/foundry/unit/ExocoreGateway.t.sol (1)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: test/foundry/unit/ExocoreGateway.t.sol:360-366 Timestamp: 2024-07-02T01:45:58.683Z Learning: The `updateWhitelistedTokens` function in `ExocoreGateway` interacts solely with the trusted `ASSET_CONTRACT` precompile, mitigating the need for a reentrancy guard.
LanguageTool
docs/contracts-owner-manual.md
[uncategorized] ~20-~20: You might be missing the article “the” here.
Context: ...r contract. ## add or update tokens to whitelist Another important task before restakin...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~22-~22: You might be missing the article “the” here.
Context: ...ing being activated is to add tokens to whitelist to mark them as stake-able on both Exoc...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Additional comments not posted (25)
src/interfaces/IExocoreGateway.sol (1)
Line range hint
52-57:
LGTM! The function renaming is appropriate.The renaming of
addWhitelistTokenstoaddOrUpdateWhitelistTokensaccurately reflects the combined functionality of adding and updating tokens.test/mocks/AssetsMock.sol (2)
53-65: LGTM! The function renaming and return type update are appropriate.The renaming of
registerClientChaintoregisterOrUpdateClientChainand the updated return type accurately reflect the combined functionality and provide useful information about the operation's outcome.
68-84: LGTM! The function renaming and return type update are appropriate.The renaming of
registerTokentoregisterOrUpdateTokenand the updated return type accurately reflect the combined functionality and provide useful information about the operation's outcome.src/interfaces/precompiles/IAssets.sol (5)
20-26: LGTM! The parameter renaming improves clarity.The renaming of
clientChainLzIDtoclientChainIDaligns with LayerZero's conventions and enhances the parameter's clarity.
32-39: LGTM! The parameter renaming improves clarity.The renaming of
clientChainLzIDtoclientChainIDaligns with LayerZero's conventions and enhances the parameter's clarity.
45-56: LGTM! The function renaming and return type update are appropriate.The renaming of
registerClientChaintoregisterOrUpdateClientChainand the updated return type accurately reflect the combined functionality and provide useful information about the operation's outcome.
58-77: LGTM! The function renaming and return type update are appropriate.The renaming of
registerTokentoregisterOrUpdateTokenand the updated return type accurately reflect the combined functionality and provide useful information about the operation's outcome.
79-81: No changes required.The function
getClientChainsremains unchanged and does not require any modifications.script/3_Setup.s.sol (1)
121-121: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
addOrUpdateWhitelistTokensmatch the new functionality.Verification successful
Function usage verified!
The function
addOrUpdateWhitelistTokensis used consistently with the new functionality across the codebase.
- Multiple instances of the function call with
{value: nativeFee}.- Multiple instances of the function call with parameters
clientChainId, whitelistTokens, decimals, tvlLimits, names, metaData.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `addOrUpdateWhitelistTokens` match the new functionality. # Test: Search for the function usage. Expect: Only occurrences of the new function name. rg --type solidity -A 5 $'addOrUpdateWhitelistTokens'Length of output: 85
Script:
#!/bin/bash # Description: Verify all function calls to `addOrUpdateWhitelistTokens` match the new functionality. # Find all Solidity files and search for the function usage within those files. fd -e sol -x rg 'addOrUpdateWhitelistTokens' {}Length of output: 1730
src/libraries/Errors.sol (1)
Line range hint
1-1:
LGTM! But verify the error handling logic in the codebase.The code changes are approved.
However, ensure that the removal of
ExocoreGatewayNotRegisteredClientChainIdis consistent with the updated error handling logic.Verification successful
The removal of
ExocoreGatewayNotRegisteredClientChainIdis consistent with the updated error handling logic. No occurrences of this error were found in the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the removal of `ExocoreGatewayNotRegisteredClientChainId` is consistent with the updated error handling logic. # Test: Search for the error usage. Expect: No occurrences of the removed error. rg --type solidity -A 5 $'ExocoreGatewayNotRegisteredClientChainId'Length of output: 99
Script:
#!/bin/bash # Description: Verify the removal of `ExocoreGatewayNotRegisteredClientChainId` is consistent with the updated error handling logic. # Test: Search for the error usage. Expect: No occurrences of the removed error. rg -A 5 'ExocoreGatewayNotRegisteredClientChainId'Length of output: 50
src/storage/ExocoreGatewayStorage.sol (3)
Line range hint
1-1:
Verify the impact of removingisRegisteredClientChain.The removal of the
isRegisteredClientChainmapping aligns with the PR objective of using precompiles as the single source of truth. Ensure that all references to this mapping are updated accordingly.Verification successful
Verification successful: No references to
isRegisteredClientChainfound.The removal of the
isRegisteredClientChainmapping aligns with the PR objective of using precompiles as the single source of truth. All references to this mapping have been successfully removed or updated.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `isRegisteredClientChain` have been removed or updated. # Test: Search for the mapping usage. Expect: No occurrences of `isRegisteredClientChain`. rg --type solidity -A 5 $'isRegisteredClientChain'Length of output: 82
Script:
#!/bin/bash # Description: Verify all references to `isRegisteredClientChain` have been removed or updated. # Test: Search for the mapping usage. Expect: No occurrences of `isRegisteredClientChain`. rg --type-add 'solidity:*.sol' -A 5 'isRegisteredClientChain'Length of output: 61
Line range hint
1-1:
Verify the impact of removingClientChainIDNotRegisteredBeforeerror declaration.The removal of the
ClientChainIDNotRegisteredBeforeerror declaration aligns with the PR objective of using precompiles as the single source of truth. Ensure that all references to this error are updated accordingly.Verification successful
Verification successful: The removal of the
ClientChainIDNotRegisteredBeforeerror declaration has been confirmed. No references to this error were found in the codebase, aligning with the PR objective of using precompiles as the single source of truth.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `ClientChainIDNotRegisteredBefore` have been removed or updated. # Test: Search for the error usage. Expect: No occurrences of `ClientChainIDNotRegisteredBefore`. rg --type solidity -A 5 $'ClientChainIDNotRegisteredBefore'Length of output: 91
Script:
#!/bin/bash # Description: Verify all references to `ClientChainIDNotRegisteredBefore` have been removed or updated. # Find all Solidity files and search for the error usage. Expect: No occurrences of `ClientChainIDNotRegisteredBefore`. fd -e sol -x rg 'ClientChainIDNotRegisteredBefore' {}Length of output: 55
Line range hint
1-1:
Verify the impact of removingisWhitelistedToken.The removal of the
isWhitelistedTokenmapping aligns with the PR objective of using precompiles as the single source of truth. Ensure that all references to this mapping are updated accordingly.test/mocks/ExocoreGatewayMock.sol (4)
246-246: Verify function name change and functionality for_registerOrUpdateClientChain.The function
_registerClientChainhas been renamed to_registerOrUpdateClientChain, reflecting its new capability to both register and update client chains. Ensure that the function name change is consistent throughout the codebase and that the new functionality is correctly implemented.Verification successful
Function name change verified for
_registerOrUpdateClientChain.The function
_registerClientChainhas been successfully renamed to_registerOrUpdateClientChainthroughout the codebase, and the new functionality is correctly referenced in the relevant files.
test/mocks/ExocoreGatewayMock.solsrc/core/ExocoreGateway.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `_registerClientChain` have been updated to `_registerOrUpdateClientChain`. # Test: Search for the function usage. Expect: No occurrences of `_registerClientChain`. rg --type solidity -A 5 $'_registerClientChain'Length of output: 79
Script:
#!/bin/bash # Description: Verify all references to `_registerClientChain` have been updated to `_registerOrUpdateClientChain`. # Test: Search for the function usage. Expect: No occurrences of `_registerClientChain`. rg '_registerClientChain' # Test: Search for the new function usage to ensure it is implemented correctly. rg '_registerOrUpdateClientChain'Length of output: 497
202-205: Verify control flow and event emissions inaddOrUpdateWhitelistTokens.The
addOrUpdateWhitelistTokensfunction now emitsWhitelistTokenAddedif a token was newly added, orWhitelistTokenUpdatedif it was updated. Ensure that the control flow correctly differentiates between newly added and updated tokens.Verification successful
Control flow and event emissions in
addOrUpdateWhitelistTokensare correctly implemented.The function correctly differentiates between newly added and updated tokens and emits the appropriate events based on the
successandupdatedflags.
test/mocks/ExocoreGatewayMock.sollines 202-205Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the control flow and event emissions in `addOrUpdateWhitelistTokens`. # Test: Search for the function implementation. Expect: Correct differentiation between newly added and updated tokens. rg --type solidity -A 10 $'function addOrUpdateWhitelistTokens'Length of output: 97
Script:
#!/bin/bash # Description: Verify the control flow and event emissions in `addOrUpdateWhitelistTokens`. # Test: Search for the function implementation. Expect: Correct differentiation between newly added and updated tokens. rg -A 10 $'function addOrUpdateWhitelistTokens'Length of output: 2194
Script:
#!/bin/bash # Description: Extract the full implementation of the `addOrUpdateWhitelistTokens` function from `test/mocks/ExocoreGatewayMock.sol`. # Test: Extract the function implementation. Expect: Correct differentiation between newly added and updated tokens. rg -A 50 'function addOrUpdateWhitelistTokens' test/mocks/ExocoreGatewayMock.solLength of output: 2046
157-163: Verify control flow and event emissions inregisterOrUpdateClientChain.The
registerOrUpdateClientChainfunction now emitsClientChainUpdatedif the client chain was updated, orClientChainRegisteredif it was newly registered. Ensure that the control flow correctly differentiates between newly registered and updated client chains.Verification successful
Verified control flow and event emissions in
registerOrUpdateClientChain.The
registerOrUpdateClientChainfunction correctly differentiates between newly registered and updated client chains, and emits the appropriate events (ClientChainUpdatedorClientChainRegistered). The control flow is implemented as expected.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the control flow and event emissions in `registerOrUpdateClientChain`. # Test: Search for the function implementation. Expect: Correct differentiation between newly registered and updated client chains. rg --type solidity -A 10 $'function registerOrUpdateClientChain'Length of output: 98
Script:
#!/bin/bash # Description: Verify the control flow and event emissions in `registerOrUpdateClientChain`. # Test: Search for the function implementation in Solidity files. fd -e sol -x rg 'function registerOrUpdateClientChain' -A 10 {}Length of output: 2330
Script:
#!/bin/bash # Description: Extract the full implementation of the `registerOrUpdateClientChain` function to verify control flow and event emissions. # Test: Search for the full implementation of the `registerOrUpdateClientChain` function. fd -e sol -x rg -A 50 'function registerOrUpdateClientChain' {}Length of output: 8193
179-179: Verify function name change and functionality foraddOrUpdateWhitelistTokens.The function
addWhitelistTokenshas been renamed toaddOrUpdateWhitelistTokens, reflecting its new capability to both add and update tokens. Ensure that the function name change is consistent throughout the codebase and that the new functionality is correctly implemented.test/foundry/ExocoreDeployer.t.sol (1)
186-186: Verify function call change and functionality foraddOrUpdateWhitelistTokens.The function call to
addWhitelistTokenshas been replaced withaddOrUpdateWhitelistTokens, suggesting an enhancement in functionality. Ensure that the function call change is consistent with the updated function name and that the new functionality is correctly utilized.test/foundry/unit/ExoCapsule.t.sol (1)
Line range hint
144-144:
Removal of assertions may reduce test robustness.The assertions checking
validator.mostRecentBalanceUpdateTimestampagainstvalidatorProof.beaconBlockTimestampandvalidator.restakedBalanceGweiagainst the result of_getEffectiveBalance(validatorContainer)have been removed. This may reduce the robustness of the validation checks performed during the deposit proof verification process.Ensure that the removal of these assertions does not impact the reliability of the contract's functionality.
src/core/ExocoreGateway.sol (3)
145-153: LGTM! Streamlined event emission logic.The use of the
updatedflag to streamline the control flow and event emission logic is a good improvement. It simplifies the code and reduces redundancy.
171-215: LGTM! Enhanced input validation and event emission.The restructuring of the
addOrUpdateWhitelistTokensfunction to handle both adding and updating tokens is a good improvement. The added input validation and tracking of token registration success enhance the robustness of the function.
277-295: LGTM! Improved control flow with boolean return value.The modification of
_registerOrUpdateClientChainto return a boolean indicating whether the client chain was updated is a good improvement. It streamlines the control flow in the calling function.test/foundry/unit/ExocoreGateway.t.sol (3)
Line range hint
360-366:
LGTM! Updated to useregisterOrUpdateClientChain.The function
test_Success_RegisterClientChainhas been successfully updated to use the newregisterOrUpdateClientChainfunction.
Line range hint
368-373:
LGTM! Updated to useregisterOrUpdateClientChain.The function
test_Success_UpdateClientChainhas been successfully updated to use the newregisterOrUpdateClientChainfunction.
624-624: LGTM! Updated to useaddOrUpdateWhitelistTokens.The function
test_Success_AddWhiteListTokenshas been successfully updated to use the newaddOrUpdateWhitelistTokensfunction.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- src/core/ExocoreGateway.sol (4 hunks)
- src/interfaces/precompiles/IAssets.sol (1 hunks)
- src/libraries/Errors.sol (1 hunks)
- test/foundry/unit/ExocoreGateway.t.sol (14 hunks)
- test/mocks/AssetsMock.sol (2 hunks)
- test/mocks/ExocoreGatewayMock.sol (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/interfaces/precompiles/IAssets.sol
- src/libraries/Errors.sol
Additional context used
Learnings (3)
test/mocks/ExocoreGatewayMock.sol (2)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#23 File: test/mocks/ExocoreGatewayMock.sol:0-0 Timestamp: 2024-06-17T07:43:43.986Z Learning: User adu-web3 suggests that the `chainToBootstrapped[clientChainId]` marking in `ExocoreGatewayMock.sol` has no significant effects beyond setting a state and that synchronization issues are currently not a concern. They are open to implementing a response check in the future if necessary.Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: test/foundry/unit/ExocoreGateway.t.sol:360-366 Timestamp: 2024-07-02T01:45:58.683Z Learning: The `updateWhitelistedTokens` function in `ExocoreGateway` interacts solely with the trusted `ASSET_CONTRACT` precompile, mitigating the need for a reentrancy guard.src/core/ExocoreGateway.sol (1)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: test/foundry/unit/ExocoreGateway.t.sol:360-366 Timestamp: 2024-07-02T01:45:58.683Z Learning: The `updateWhitelistedTokens` function in `ExocoreGateway` interacts solely with the trusted `ASSET_CONTRACT` precompile, mitigating the need for a reentrancy guard.test/foundry/unit/ExocoreGateway.t.sol (1)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: test/foundry/unit/ExocoreGateway.t.sol:360-366 Timestamp: 2024-07-02T01:45:58.683Z Learning: The `updateWhitelistedTokens` function in `ExocoreGateway` interacts solely with the trusted `ASSET_CONTRACT` precompile, mitigating the need for a reentrancy guard.
Additional comments not posted (17)
test/mocks/AssetsMock.sol (3)
99-101: LGTM!The code changes are approved.
53-65: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
registerOrUpdateClientChainmatch the new signature.
68-84: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
registerOrUpdateTokensmatch the new signature.Verification successful
LGTM! But verify the function usage in the codebase.
The code changes are approved.
All function calls to
registerOrUpdateTokensmatch the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateTokens` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type solidity -A 5 $'registerOrUpdateTokens'Length of output: 81
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateTokens` match the new signature. # Find all Solidity files fd -e sol -e solidity -x rg 'registerOrUpdateTokens' {} +Length of output: 3942
Script:
#!/bin/bash # Description: Verify the context of `registerOrUpdateTokens` function calls to ensure they match the new signature. # Check the context of the function call in `test/mocks/ExocoreGatewayMock.sol` rg --type solidity -A 5 'registerOrUpdateTokens' test/mocks/ExocoreGatewayMock.sol # Check the context of the function call in `src/core/ExocoreGateway.sol` rg --type solidity -A 5 'registerOrUpdateTokens' src/core/ExocoreGateway.sol # Check the context of the function declaration in `src/interfaces/precompiles/IAssets.sol` rg --type solidity -A 5 'registerOrUpdateTokens' src/interfaces/precompiles/IAssets.solLength of output: 346
Script:
#!/bin/bash # Description: Verify the context of `registerOrUpdateTokens` function calls to ensure they match the new signature. # Check the context of the function call in `test/mocks/ExocoreGatewayMock.sol` rg -A 5 'registerOrUpdateTokens' test/mocks/ExocoreGatewayMock.sol # Check the context of the function call in `src/core/ExocoreGateway.sol` rg -A 5 'registerOrUpdateTokens' src/core/ExocoreGateway.sol # Check the context of the function declaration in `src/interfaces/precompiles/IAssets.sol` rg -A 5 'registerOrUpdateTokens' src/interfaces/precompiles/IAssets.solLength of output: 890
test/mocks/ExocoreGatewayMock.sol (3)
258-270: LGTM!The code changes are approved.
181-220: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
addOrUpdateWhitelistTokensmatch the new signature.
158-164: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
registerOrUpdateClientChainmatch the new signature.Verification successful
LGTM! But verify the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to
registerOrUpdateClientChainmatch the new signature. The provided context shows that the new signature is being used correctly in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateClientChain` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type solidity -A 5 $'registerOrUpdateClientChain'Length of output: 86
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateClientChain` match the new signature. # Find all Solidity files and search for the function usage within them. fd -e sol -x rg 'registerOrUpdateClientChain' {}Length of output: 1708
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateClientChain` match the new signature. # Find all Solidity files and search for the function usage within them, displaying context around each match. fd -e sol -x rg 'registerOrUpdateClientChain' -A 5 -B 5 {}Length of output: 11543
src/core/ExocoreGateway.sol (3)
303-315: LGTM!The code changes are approved.
146-153: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
registerOrUpdateClientChainmatch the new signature.Verification successful
Verification Successful!
The function calls to
registerOrUpdateClientChainmatch the new signature across the codebase.
- All instances of
registerOrUpdateClientChainadhere to the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateClientChain` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type solidity -A 5 $'registerOrUpdateClientChain'Length of output: 86
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateClientChain` match the new signature. # Find all Solidity files and search for the function usage. fd -e sol -x rg 'registerOrUpdateClientChain' {}Length of output: 1708
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateClientChain` match the new signature. # Find all Solidity files and search for the function usage with more context lines. fd -e sol -x rg -A 10 -B 2 'registerOrUpdateClientChain' {}Length of output: 12418
175-220: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
addOrUpdateWhitelistTokensmatch the new signature.Verification successful
Function usage verified.
All instances of
addOrUpdateWhitelistTokensmatch the new function signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `addOrUpdateWhitelistTokens` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type solidity -A 5 $'addOrUpdateWhitelistTokens'Length of output: 85
Script:
#!/bin/bash # Description: Verify all function calls to `addOrUpdateWhitelistTokens` match the new signature. # Find all Solidity files and search for the function usage within those files. fd -e sol -x rg 'addOrUpdateWhitelistTokens' {}Length of output: 1730
Script:
#!/bin/bash # Description: Verify all function calls to `addOrUpdateWhitelistTokens` match the new signature with context. # Find all Solidity files and search for the function usage within those files, including more context lines. fd -e sol -x rg -A 10 'addOrUpdateWhitelistTokens' {}Length of output: 8326
test/foundry/unit/ExocoreGateway.t.sol (8)
Line range hint
1-83: Ensure the setup supports new functionality.The
SetUpcontract initializes the state for the tests. Ensure that the setup correctly supports the new functionality introduced by the renamed functionaddOrUpdateWhitelistTokens.
Line range hint
85-135: Ensure pause and unpause functionality works with new function.The
Pausablecontract tests the pause and unpause functionality. Ensure that the pause and unpause functionality works correctly with the new functionaddOrUpdateWhitelistTokens.
Line range hint
137-283: Ensure message handling works with new function.The
LzReceivecontract tests the message handling functionality. Ensure that the message handling works correctly with the new functionaddOrUpdateWhitelistTokens.
Line range hint
285-358: Ensure client chain registration and update works with new function.The
RegisterOrUpdateClientChaincontract tests the client chain registration and update functionality. Ensure that the client chain registration and update functionality works correctly with the new functionaddOrUpdateWhitelistTokens.
Line range hint
360-405: Ensure peer setting works with new function.The
SetPeercontract tests the peer setting functionality. Ensure that the peer setting functionality works correctly with the new functionaddOrUpdateWhitelistTokens.
Line range hint
407-630: Ensure tests validate new functionality.The
AddWhitelistTokenscontract contains tests for adding whitelist tokens. Ensure that these tests correctly validate the new functionality introduced by the renamed functionaddOrUpdateWhitelistTokens.
Line range hint
632-786: Ensure tests validate new functionality.The
UpdateWhitelistTokenscontract contains tests for updating whitelist tokens. Ensure that these tests correctly validate the new functionality introduced by the renamed functionaddOrUpdateWhitelistTokens.
Line range hint
788-859: Ensure operator association works with new function.The
AssociateOperatorWithEVMStakercontract tests the operator association functionality. Ensure that the operator association functionality works correctly with the new functionaddOrUpdateWhitelistTokens.
After the precompiles for assets and client chains were merged, the convoluted mechanism of calling the `getClientChains` function via the selector is not necessary.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/core/ExocoreGateway.sol (5 hunks)
- src/libraries/Errors.sol (1 hunks)
- src/storage/ExocoreGatewayStorage.sol (3 hunks)
- test/foundry/unit/ExocoreGateway.t.sol (15 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/libraries/Errors.sol
- src/storage/ExocoreGatewayStorage.sol
Additional context used
Learnings (2)
src/core/ExocoreGateway.sol (1)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: test/foundry/unit/ExocoreGateway.t.sol:360-366 Timestamp: 2024-07-02T01:45:58.683Z Learning: The `updateWhitelistedTokens` function in `ExocoreGateway` interacts solely with the trusted `ASSET_CONTRACT` precompile, mitigating the need for a reentrancy guard.test/foundry/unit/ExocoreGateway.t.sol (1)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: test/foundry/unit/ExocoreGateway.t.sol:360-366 Timestamp: 2024-07-02T01:45:58.683Z Learning: The `updateWhitelistedTokens` function in `ExocoreGateway` interacts solely with the trusted `ASSET_CONTRACT` precompile, mitigating the need for a reentrancy guard.
Additional comments not posted (15)
src/core/ExocoreGateway.sol (6)
111-121: Verify the bootstrap marking logic.The function marks chains as bootstrapped upon sending an interchain message. Consider marking them only upon receiving a response to ensure reliability.
181-218: Ensure comprehensive testing for token management.The function now handles both adding and updating tokens. Ensure that thorough tests cover all scenarios, including edge cases.
Line range hint
266-276:
LGTM! Ensure thorough testing of input validation.The function now focuses on validating input lengths. Ensure that tests cover various input scenarios to confirm correctness.
301-313: LGTM! But verify the function usage in the codebase.The code changes are approved. Ensure that all function calls to
_registerOrUpdateClientChainare correctly implemented.Verification successful
Function usage verified successfully.
The function
_registerOrUpdateClientChainis correctly implemented in the codebase.
src/core/ExocoreGateway.soltest/mocks/ExocoreGatewayMock.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_registerOrUpdateClientChain` are correctly implemented. # Test: Search for the function usage. Expect: Only occurrences of the new logic. rg --type solidity -A 5 $'_registerOrUpdateClientChain'Length of output: 87
Script:
#!/bin/bash # Description: Verify all function calls to `_registerOrUpdateClientChain` are correctly implemented. # Test: Search for the function usage. Expect: Only occurrences of the new logic. rg '_registerOrUpdateClientChain' -A 5Length of output: 1743
141-149: LGTM! But verify the function usage in the codebase.The code changes are approved. Ensure that all function calls to
registerOrUpdateClientChainmatch the new logic.Verification successful
Let's re-run the verification using a different approach to locate the function usage in Solidity files.
To ensure the function calls match the new logic, let's extract the surrounding lines of each occurrence to verify the context.
Verified: The function calls to
registerOrUpdateClientChainmatch the new logic.The code changes are correctly implemented, and all occurrences of the function calls follow the updated pattern.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateClientChain` match the new logic. # Test: Search for the function usage. Expect: Only occurrences of the new logic. rg --type solidity -A 5 $'registerOrUpdateClientChain'Length of output: 86
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateClientChain` match the new logic. # Find all Solidity files and search for function usage within them. fd -e sol -x rg 'registerOrUpdateClientChain' {}Length of output: 1760
Script:
#!/bin/bash # Description: Verify all function calls to `registerOrUpdateClientChain` match the new logic. # Find all Solidity files and search for function usage within them, extracting surrounding lines for context. fd -e sol -x rg -A 10 -B 10 'registerOrUpdateClientChain' {}Length of output: 19017
164-168: LGTM! But verify the function usage in the codebase.The code changes are approved. Ensure that all function calls to
setPeerinclude the new registration check.Verification successful
Function usage verified and includes the necessary registration check.
The
setPeerfunction insrc/core/ExocoreGateway.solcorrectly includes the_validateClientChainIdRegisteredcheck, ensuring that the client chain ID is validated before proceeding.
src/core/ExocoreGateway.sol: The_validateClientChainIdRegisteredcheck is included in thesetPeerfunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `setPeer` include the new registration check. # Test: Search for the function usage. Expect: Only occurrences of the new logic. rg --type solidity -A 5 $'setPeer'Length of output: 66
Script:
#!/bin/bash # Description: Verify all function calls to `setPeer` include the new registration check. # Test: Search for the function usage. Expect: Only occurrences of the new logic. rg 'setPeer' --glob '*.sol' -A 5Length of output: 8438
Script:
#!/bin/bash # Description: Verify that the `_validateClientChainIdRegistered` check is included in the `setPeer` function implementation. # Test: Search for the `_validateClientChainIdRegistered` function call within the `setPeer` function in `src/core/ExocoreGateway.sol`. rg '_validateClientChainIdRegistered' --glob 'src/core/ExocoreGateway.sol' -A 5Length of output: 932
test/foundry/unit/ExocoreGateway.t.sol (9)
535-537: LGTM! Ensure comprehensive testing for ownership checks.The test ensures that only the contract owner can call the
addOrUpdateWhitelistTokensfunction. Ensure that similar tests cover other functions requiring ownership checks.
549-551: LGTM! Ensure comprehensive testing for paused state.The test ensures that the
addOrUpdateWhitelistTokensfunction cannot be called when the contract is paused. Ensure that similar tests cover other functions requiring the contract to be unpaused.
562-564: LGTM! Ensure comprehensive testing for input length.The test ensures that the
addOrUpdateWhitelistTokensfunction reverts when the token list is too long. Ensure that similar tests cover other functions requiring input length validation.
576-578: LGTM! Ensure comprehensive testing for input length matching.The test ensures that the
addOrUpdateWhitelistTokensfunction reverts when the input lengths do not match. Ensure that similar tests cover other functions requiring input length matching.
596-598: LGTM! Ensure comprehensive testing for zero address validation.The test ensures that the
addOrUpdateWhitelistTokensfunction reverts when a token address is zero. Ensure that similar tests cover other functions requiring zero address validation.
610-612: LGTM! Ensure comprehensive testing for TVL limit validation.The test ensures that the
addOrUpdateWhitelistTokensfunction reverts when a TVL limit is zero. Ensure that similar tests cover other functions requiring TVL limit validation.
630-632: LGTM! Ensure comprehensive testing for successful token addition.The test ensures that the
addOrUpdateWhitelistTokensfunction successfully adds tokens to the whitelist. Ensure that similar tests cover other functions requiring successful operation validation.
761-763: LGTM! Ensure comprehensive testing for successful token updates.The test ensures that the
addOrUpdateWhitelistTokensfunction successfully updates tokens in the whitelist. Ensure that similar tests cover other functions requiring successful operation validation.
783-785: LGTM! Ensure comprehensive testing for token addition logic.The function now calls
addOrUpdateWhitelistTokensto add tokens to the whitelist. Ensure that tests cover various scenarios to confirm correctness.
The
ExocoreGatewayallows the contract owner to call precompiles which allow the registration of a client chain or a token. It also enables updates of these items. To check whether a request is an update or an addition, it relies on the ExocoreGateway keeping a copy of registered items. However, this does not work for items which were registered at genesis.This PR works around that problem by requiring that the single source of truth be the precompiles, whose responsibility is to report if a request was an update or an addition.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Refactor
Chores