refactor: add natspec documentation #58
Conversation
We are using, in documentation as well in code, a distinction between the role of an operator and that of a validator. The set of validators is a subset of the set of operators; in other words, every validator is considered as an operator. However, not every operator is a validator. A validator is an entity registered to validate the Exocore chain. An operator is an entity registered to validate any AVS, including the dogfood AVS which represents the Exocore chain. Before the network is considered bootstrapped, the only way to register as an operator is to register as a validator as well; hence, we rename within the (exclusively) Bootstrap context `operator` as `validator`. Any entity registering itself as a validator in the Bootstrap contract will automatically get registered as an operator when the chain is started, as a consequence.
|
Warning Rate limit exceeded@MaxMustermann2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 51 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 recent changes mark a significant transition from an operator-centric approach to a validator-focused system. This shift reflects a comprehensive enhancement across configuration files, contract interfaces, and script modifications, all aimed at improving the management and functionality of validators within the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ValidatorRegistry
participant Bootstrap
User->>Bootstrap: Request to register validator
Bootstrap->>ValidatorRegistry: registerValidator()
ValidatorRegistry-->>Bootstrap: Confirmation of registration
Bootstrap-->>User: Validator registration successful
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 (
|
MaxMustermann2
requested review from
bwhour,
wewecalibrate and
adu-web3
and removed request for
bwhour and
wewecalibrate
![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 (1)
docs/client-chain-contracts-design.md (1)
247-247: Inconsistency Found: Documentation Reference to Removed FunctionThe
docs/client-chain-contracts-design.mdfile still contains a detailed description of theupdateUsersBalancefunction, which was supposed to be removed. Please update the documentation to reflect the current state of the codebase and remove any references toupdateUsersBalance.
docs/client-chain-contracts-design.md: Lines around the mention ofupdateUsersBalance.Analysis chain
LGTM!
The removal of
TokenBalanceUpdateInfo,UserBalanceUpdateInfostructures, andupdateUsersBalancefunction simplifies theIControllerinterface, indicating a shift in how user balances are managed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of the removal of `TokenBalanceUpdateInfo`, `UserBalanceUpdateInfo`, and `updateUsersBalance` on other documentation or code. # Test: Search for the usage of removed elements. Expect: No usage of removed elements. rg --type markdown --type solidity 'TokenBalanceUpdateInfo|UserBalanceUpdateInfo|updateUsersBalance'Length of output: 135
Script:
#!/bin/bash # Description: Verify the impact of the removal of `TokenBalanceUpdateInfo`, `UserBalanceUpdateInfo`, and `updateUsersBalance` on other documentation or code. # Test: Search for the usage of removed elements. Expect: No usage of removed elements. rg 'TokenBalanceUpdateInfo|UserBalanceUpdateInfo|updateUsersBalance' --glob '*.md' --glob '*.sol'Length of output: 162
Script:
#!/bin/bash # Description: Extract the section of `docs/client-chain-contracts-design.md` that mentions `updateUsersBalance`. # Extract lines around the mention of `updateUsersBalance` rg 'updateUsersBalance' docs/client-chain-contracts-design.md -A 10 -B 10Length of output: 1422
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
docs/architecture.svgis excluded by!**/*.svg
Files selected for processing (38)
- .env.example (1 hunks)
- .github/workflows/slither.yml (1 hunks)
- docs/client-chain-contracts-design.md (2 hunks)
- docs/native_deposit_workflow.wsd (1 hunks)
- script/8_RegisterOperatorsAndDelegate.s.sol (5 hunks)
- script/integration/1_DeployBootstrap.s.sol (10 hunks)
- src/core/BaseRestakingController.sol (6 hunks)
- src/core/BeaconProxyBytecode.sol (1 hunks)
- src/core/Bootstrap.sol (13 hunks)
- src/core/BootstrapLzReceiver.sol (3 hunks)
- src/core/ClientChainGateway.sol (6 hunks)
- src/core/ClientGatewayLzReceiver.sol (9 hunks)
- src/core/CustomProxyAdmin.sol (1 hunks)
- src/core/ExoCapsule.sol (14 hunks)
- src/core/ExocoreGateway.sol (22 hunks)
- src/core/LSTRestakingController.sol (4 hunks)
- src/core/NativeRestakingController.sol (5 hunks)
- src/core/Vault.sol (4 hunks)
- src/interfaces/IBaseRestakingController.sol (1 hunks)
- src/interfaces/IClientChainGateway.sol (2 hunks)
- src/interfaces/ICustomProxyAdmin.sol (1 hunks)
- src/interfaces/IExoCapsule.sol (2 hunks)
- src/interfaces/IExocoreGateway.sol (2 hunks)
- src/interfaces/ILSTRestakingController.sol (1 hunks)
- src/interfaces/INativeRestakingController.sol (1 hunks)
- src/interfaces/ITokenWhitelister.sol (1 hunks)
- src/interfaces/IValidatorRegistry.sol (1 hunks)
- src/interfaces/IVault.sol (1 hunks)
- src/interfaces/precompiles/IAssets.sol (1 hunks)
- src/interfaces/precompiles/IClaimReward.sol (1 hunks)
- src/interfaces/precompiles/IDelegation.sol (1 hunks)
- src/storage/BootstrapStorage.sol (6 hunks)
- src/storage/ClientChainGatewayStorage.sol (3 hunks)
- src/storage/ExoCapsuleStorage.sol (3 hunks)
- src/storage/ExocoreGatewayStorage.sol (1 hunks)
- src/storage/GatewayStorage.sol (2 hunks)
- src/storage/VaultStorage.sol (1 hunks)
- test/foundry/unit/Bootstrap.t.sol (15 hunks)
Files skipped from review due to trivial changes (10)
- .github/workflows/slither.yml
- docs/native_deposit_workflow.wsd
- src/core/BaseRestakingController.sol
- src/core/BeaconProxyBytecode.sol
- src/core/LSTRestakingController.sol
- src/interfaces/IBaseRestakingController.sol
- src/interfaces/IClientChainGateway.sol
- src/interfaces/ITokenWhitelister.sol
- src/interfaces/precompiles/IAssets.sol
- src/interfaces/precompiles/IClaimReward.sol
Additional context used
Learnings (4)
src/interfaces/ICustomProxyAdmin.sol (1)
Learnt from: MaxMustermann2 PR: ExocoreNetwork/exocore-contracts#16 File: src/core/CustomProxyAdmin.sol:20-22 Timestamp: 2024-06-03T11:27:03.714Z Learning: The `CustomProxyAdmin` contract is designed for a `TransparentUpgradeableProxy` to upgrade itself. The `require(msg.sender == proxy)` condition in the `changeImplementation` function ensures that the proxy is the one initiating the upgrade, preventing unauthorized upgrades. This is a security measure to ensure that only the designated proxy can upgrade itself, and it can only do so once during its lifetime.src/core/CustomProxyAdmin.sol (1)
Learnt from: MaxMustermann2 PR: ExocoreNetwork/exocore-contracts#16 File: src/core/CustomProxyAdmin.sol:20-22 Timestamp: 2024-06-03T11:27:03.714Z Learning: The `CustomProxyAdmin` contract is designed for a `TransparentUpgradeableProxy` to upgrade itself. The `require(msg.sender == proxy)` condition in the `changeImplementation` function ensures that the proxy is the one initiating the upgrade, preventing unauthorized upgrades. This is a security measure to ensure that only the designated proxy can upgrade itself, and it can only do so once during its lifetime.src/storage/BootstrapStorage.sol (1)
Learnt from: MaxMustermann2 PR: ExocoreNetwork/exocore-contracts#17 File: src/storage/BootstrapStorage.sol:393-396 Timestamp: 2024-06-05T04:51:12.825Z Learning: Multiple modifiers in `BootstrapStorage.sol` are used in different contexts to ensure that validations are only applied where necessary, providing flexibility in contract logic.src/core/ExocoreGateway.sol (2)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#23 File: src/core/ExocoreGateway.sol:74-87 Timestamp: 2024-06-18T02:35:50.005Z Learning: User: adu-web3 PR: ExocoreNetwork/exocore-contracts#23 File: src/core/ExocoreGateway.sol:74-87 Timestamp: 2024-06-18T02:33:17.212Z Learning: An issue was created to track the optimization of the governance model by removing `exocoreValidatorSetAddress` and using a multi-sig wallet as the contract owner. This issue can be found at https://github.com/ExocoreNetwork/exocore-contracts/issues/30.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 (171)
.env.example (1)
15-15: Verify the usage of the renamed environment variable.The environment variable
OPERATOR_KEYShas been renamed toVALIDATOR_KEYS. Ensure that all references toOPERATOR_KEYSin the codebase are updated toVALIDATOR_KEYS.Verification successful
Environment variable renaming verified successfully.
The environment variable
OPERATOR_KEYShas been renamed toVALIDATOR_KEYS, and all references toOPERATOR_KEYShave been updated accordingly.
- No instances of
OPERATOR_KEYSfound.VALIDATOR_KEYScorrectly referenced inscript/8_RegisterOperatorsAndDelegate.s.sol.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the renamed environment variable. # Test: Search for the old and new environment variable names. Expect: Only occurances of the new variable name. rg --type text --fixed-strings --word-regexp 'OPERATOR_KEYS' rg --type text --fixed-strings --word-regexp 'VALIDATOR_KEYS'Length of output: 180
Script:
#!/bin/bash # Description: Verify all references to the renamed environment variable. # Test: Search for the old and new environment variable names. Expect: Only occurrences of the new variable name. rg --fixed-strings --word-regexp 'OPERATOR_KEYS' rg --fixed-strings --word-regexp 'VALIDATOR_KEYS'Length of output: 201
src/interfaces/ICustomProxyAdmin.sol (1)
1-17: LGTM! Verify the usage of the new interface.The new interface
ICustomProxyAdminand its documentation are well-implemented.Ensure that the new interface is correctly integrated and used in the codebase.
Verification successful
The new interface
ICustomProxyAdminis correctly integrated and used in the codebase.The interface is imported and utilized in functions that handle proxy implementation changes, consistent with its intended purpose.
src/core/CustomProxyAdmin.sol../lzApp/OAppCoreUpgradeable.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new interface `ICustomProxyAdmin`. # Test: Search for the usage of the new interface. Expect: Correct integration and usage. rg --type solidity --word-regexp 'ICustomProxyAdmin'Length of output: 85
Script:
#!/bin/bash # Description: Verify the usage of the new interface `ICustomProxyAdmin`. # Find all Solidity files and search for the usage of the new interface `ICustomProxyAdmin`. fd -e sol -x rg 'ICustomProxyAdmin' {}Length of output: 467
Script:
#!/bin/bash # Description: Extract context around the usage of `ICustomProxyAdmin`. # Find all Solidity files and search for the usage of the new interface `ICustomProxyAdmin`, including context lines. fd -e sol -x rg 'ICustomProxyAdmin' -A 5 -B 5 {}Length of output: 2917
src/core/CustomProxyAdmin.sol (1)
7-30: LGTM! Verify the usage of the updated contract.The enhanced documentation and the
changeImplementationfunction are well-implemented.Ensure that the updated contract is correctly integrated and used in the codebase.
Verification successful
The
CustomProxyAdmincontract is correctly integrated and used in the codebase.The search results indicate that
CustomProxyAdminis imported and instantiated in several deployment and test scripts, confirming its usage.
- Files with
CustomProxyAdminusage:
test/foundry/unit/CustomProxyAdmin.t.soltest/foundry/unit/Bootstrap.t.solscript/integration/1_DeployBootstrap.s.solscript/7_DeployBootstrap.s.solscript/12_RedeployClientChainGateway.s.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the updated `CustomProxyAdmin` contract. # Test: Search for the usage of the updated contract. Expect: Correct integration and usage. rg --type solidity --word-regexp 'CustomProxyAdmin'Length of output: 84
Script:
#!/bin/bash # Description: Verify the usage of the updated `CustomProxyAdmin` contract. # Step 1: Find all Solidity files. solidity_files=$(fd --extension sol) # Step 2: Search for the usage of the `CustomProxyAdmin` contract within the found Solidity files. rg --word-regexp 'CustomProxyAdmin' $solidity_filesLength of output: 7567
src/core/BootstrapLzReceiver.sol (3)
7-11: Natspec documentation looks good!The added Natspec documentation provides clarity on the contract's purpose and usage.
Line range hint
14-20:
Security enhancement withonlyCalledFromThismodifier.The
onlyCalledFromThismodifier enhances security by restricting function calls to the contract itself.
Line range hint
23-45:
Annotations with@inheritdocimprove clarity.The annotations with
@inheritdoc OAppReceiverUpgradeableimprove the understanding of the contract's behavior in relation to its parent class.src/interfaces/IVault.sol (7)
4-7: Natspec documentation looks good!The added Natspec documentation provides clarity on the interface's purpose and usage.
10-13: Natspec documentation forwithdrawfunction looks good!The added Natspec documentation provides clarity on the function's parameters and purpose.
16-18: Natspec documentation fordepositfunction looks good!The added Natspec documentation provides clarity on the function's parameters and purpose.
21-23: Natspec documentation forupdatePrincipalBalancefunction looks good!The added Natspec documentation provides clarity on the function's parameters and purpose.
26-28: Natspec documentation forupdateRewardBalancefunction looks good!The added Natspec documentation provides clarity on the function's parameters and purpose.
31-34: Natspec documentation forupdateWithdrawableBalancefunction looks good!The added Natspec documentation provides clarity on the function's parameters and purpose.
38-39: Natspec documentation forgetUnderlyingTokenfunction looks good!The added Natspec documentation provides clarity on the function's purpose and return value.
src/interfaces/precompiles/IDelegation.sol (1)
1-1: SPDX license change to MIT is appropriate.The change in the license identifier to "MIT" reflects a shift to a more permissive license, which is appropriate.
src/storage/VaultStorage.sol (10)
12-13: Natspec documentation forprincipalBalanceslooks good.The comment is clear and accurately describes the mapping.
15-16: Natspec documentation forrewardBalanceslooks good.The comment is clear and accurately describes the mapping.
18-19: Natspec documentation forwithdrawableBalanceslooks good.The comment is clear and accurately describes the mapping.
21-22: Natspec documentation fortotalDepositedPrincipalAmountlooks good.The comment is clear and accurately describes the mapping.
24-25: Natspec documentation fortotalUnlockPrincipalAmountlooks good.The comment is clear and accurately describes the mapping.
33-36: Natspec documentation forPrincipalBalanceUpdatedevent looks good.The comment is clear and accurately describes the event parameters.
38-41: Natspec documentation forRewardBalanceUpdatedevent looks good.The comment is clear and accurately describes the event parameters.
43-47: Natspec documentation forWithdrawableBalanceUpdatedevent looks good.The comment is clear and accurately describes the event parameters.
49-53: Natspec documentation forWithdrawalSuccessevent looks good.The comment is clear and accurately describes the event parameters.
55-56: Natspec documentation for__gaplooks good.The comment is clear and accurately describes the purpose of the storage gap.
src/storage/GatewayStorage.sol (8)
Line range hint
9-21:
Natspec documentation forActionenum looks good.The comment is clear and accurately describes the enum.
22-23: Natspec documentation for_whiteListFunctionSelectorslooks good.The comment is clear and accurately describes the mapping.
25-26: Natspec documentation forinboundNoncelooks good.The comment is clear and accurately describes the mapping.
31-35: Natspec documentation forMessageSentevent looks good.The comment is clear and accurately describes the event parameters.
38-39: Natspec documentation forUnsupportedRequesterror looks good.The comment is clear and accurately describes the error parameters.
42-43: Natspec documentation forUnexpectedSourceChainerror looks good.The comment is clear and accurately describes the error parameters.
46-48: Natspec documentation forUnexpectedInboundNonceerror looks good.The comment is clear and accurately describes the error parameters.
51-55: Natspec documentation for_verifyAndUpdateNoncefunction looks good.The comment is clear and accurately describes the function parameters and behavior.
src/interfaces/ILSTRestakingController.sol (4)
14-18: Natspec documentation fordepositfunction looks good.The comment is clear and accurately describes the function parameters and behavior.
21-26: Natspec documentation forwithdrawPrincipalFromExocorefunction looks good.The comment is clear and accurately describes the function parameters and behavior.
29-31: Natspec documentation forwithdrawRewardFromExocorefunction looks good.The comment is clear and accurately describes the function parameters and behavior.
34-40: Natspec documentation fordepositThenDelegateTofunction looks good.The comment is clear and accurately describes the function parameters and behavior.
src/interfaces/INativeRestakingController.sol (5)
8-12: LGTM! Documentation improves readability.The added documentation provides a clear overview of the interface and follows the Natspec format.
15-21: LGTM! Documentation enhances clarity.The added documentation for the
stakefunction is clear and follows the Natspec format, improving the understanding of the function.
24-26: LGTM! Documentation enhances clarity.The added documentation for the
createExoCapsulefunction is clear and follows the Natspec format, improving the understanding of the function.
29-35: LGTM! Documentation enhances clarity.The added documentation for the
depositBeaconChainValidatorfunction is clear and follows the Natspec format, improving the understanding of the function.
41-52: LGTM! Documentation enhances clarity.The added documentation for the
processBeaconChainWithdrawalfunction is clear and follows the Natspec format, improving the understanding of the function.src/storage/ExoCapsuleStorage.sol (9)
8-10: LGTM! Documentation improves readability.The added documentation provides a clear overview of the contract and follows the Natspec format.
Line range hint
13-21:
LGTM! Documentation enhances clarity.The added documentation for the
VALIDATOR_STATUSenum is clear and follows the Natspec format, improving the understanding of the enum.
22-30: LGTM! Documentation enhances clarity.The added documentation for the
Validatorstruct is clear and follows the Natspec format, improving the understanding of the struct.
39-41: LGTM! Documentation enhances clarity.The added documentation for the
BEACON_ROOTS_ADDRESSconstant is clear and follows the Natspec format, improving the understanding of the constant.
42-44: LGTM! Documentation enhances clarity.The added documentation for the
BEACON_CHAIN_GENESIS_TIMEconstant is clear and follows the Natspec format, improving the understanding of the constant.
45-47: LGTM! Documentation enhances clarity.The added documentation for the
VERIFY_BALANCE_UPDATE_WINDOW_SECONDSconstant is clear and follows the Natspec format, improving the understanding of the constant.
48-50: LGTM! Documentation enhances clarity.The added documentation for the
GWEI_TO_WEIconstant is clear and follows the Natspec format, improving the understanding of the constant.
51-56: LGTM! Documentation enhances clarity.The added documentation for the
MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATORconstant andprincipalBalancevariable is clear and follows the Natspec format, improving the understanding of the constant and variable.
61-84: LGTM! Documentation enhances clarity.The added documentation for the
withdrawableBalance,nonBeaconChainETHBalance,capsuleOwner,gateway,beaconOracle,_capsuleValidators,_capsuleValidatorsByIndex,provenWithdrawal, and__gapvariables is clear and follows the Natspec format, improving the understanding of the variables.src/interfaces/IValidatorRegistry.sol (7)
1-3: LGTM! Standard file header and pragma statement.The file header and pragma statement are standard and necessary for Solidity files.
4-17: LGTM! Documentation enhances clarity.The added documentation for the
Commissionstruct is clear and follows the Natspec format, improving the understanding of the struct.
19-29: LGTM! Documentation enhances clarity.The added documentation for the
Validatorstruct is clear and follows the Natspec format, improving the understanding of the struct.
31-44: LGTM! Documentation enhances clarity.The added documentation for the
registerValidatorfunction is clear and follows the Natspec format, improving the understanding of the function.
46-48: LGTM! Documentation enhances clarity.The added documentation for the
replaceKeyfunction is clear and follows the Natspec format, improving the understanding of the function.
50-56: LGTM! Documentation enhances clarity.The added documentation for the
updateRatefunction is clear and follows the Natspec format, improving the understanding of the function.
58-79: LGTM! Documentation enhances clarity.The added documentation for the
ValidatorRegistered,ValidatorKeyReplaced, andValidatorCommissionUpdatedevents is clear and follows the Natspec format, improving the understanding of the events.src/interfaces/IExoCapsule.sol (9)
Line range hint
12-19:
LGTM! Documentation update forValidatorContainerProof.The documentation provides clear and concise information about the struct and its fields.
21-27: LGTM! Addition ofWithdrawalContainerProofstruct.The new struct and its documentation are clear and well-defined.
28-33: LGTM! Documentation update forinitializefunction.The documentation clearly describes the function's purpose and parameters.
34-43: LGTM! Documentation update forverifyDepositProoffunction.The documentation provides a detailed description of the function, its parameters, and return value.
45-62: LGTM! Documentation update forverifyWithdrawalProoffunction.The documentation provides a detailed description of the function, its parameters, and return values.
63-68: LGTM! Documentation update forwithdrawfunction.The documentation clearly describes the function's purpose and parameters.
69-72: LGTM! Documentation update forupdatePrincipalBalancefunction.The documentation clearly describes the function's purpose and parameters.
73-76: LGTM! Documentation update forupdateWithdrawableBalancefunction.The documentation clearly describes the function's purpose and parameters.
77-80: LGTM! Documentation update forcapsuleWithdrawalCredentialsfunction.The documentation clearly describes the function's purpose and return value.
script/8_RegisterOperatorsAndDelegate.s.sol (5)
5-5: LGTM! Import statement update forIValidatorRegistry.The import statement correctly reflects the new focus on validators.
Line range hint
13-24:
LGTM! Contract name and registration data update.The contract name and registration data correctly reflect the new focus on validators.
Line range hint
37-48:
LGTM! Updates tosetUpfunction for validator registration data.The
setUpfunction correctly initializes the validator registration data and performs necessary validation checks.
60-76: LGTM! Updates torunfunction for validator registration and delegation.The
runfunction correctly registers validators, performs necessary transactions, and handles delegation.
92-100: LGTM! Updates to delegation loop inrunfunction.The delegation loop correctly handles the delegation process for validators.
src/interfaces/IExocoreGateway.sol (4)
15-19: LGTM! Addition ofquotefunction.The new function and its documentation are clear and well-defined.
22-37: LGTM! Documentation update and parameter name change forregisterOrUpdateClientChainfunction.The documentation provides a detailed description of the function, its parameters, and purpose. The parameter name change improves clarity.
44-51: LGTM! Addition ofaddWhitelistTokensfunction.The new function and its documentation are clear and well-defined.
61-76: LGTM! Addition ofupdateWhitelistedTokensfunction.The new function and its documentation are clear and well-defined.
src/core/Vault.sol (8)
19-22: LGTM!The
onlyGatewaymodifier is correctly implemented and enhances security by restricting access to critical functions.
25-27: LGTM!The constructor correctly disables initialization to align with the proxy pattern.
Line range hint
30-37:
LGTM!The
initializefunction is correctly implemented with necessary checks to prevent zero addresses and ensure robustness.
46-49: LGTM!The
getWithdrawableBalancefunction is correctly implemented and well-documented.
Line range hint
52-63:
LGTM!The
withdrawfunction is correctly implemented with necessary checks and event emission.
74-77: LGTM!The
updatePrincipalBalancefunction is correctly implemented and follows best practices for state updates and event emission.
81-84: LGTM!The
updateRewardBalancefunction is correctly implemented with necessary checks and event emission.
Line range hint
88-101:
LGTM!The
updateWithdrawableBalancefunction is correctly implemented with necessary checks and event emission.src/core/ClientChainGateway.sol (6)
Line range hint
34-49:
LGTM!The constructor is correctly implemented and follows best practices for initializing state variables and disabling initializers.
Line range hint
60-74:
LGTM!The
initializefunction is correctly implemented with necessary checks and initialization steps.
Line range hint
79-93:
LGTM!The
_clearBootstrapDatafunction is correctly implemented and ensures that bootstrap data is cleared.
96-104: LGTM!The
pauseandunpausefunctions are correctly implemented and follow best practices for pausing and unpausing contracts.
106-109: LGTM!The
getWhitelistedTokensCountfunction is correctly implemented and well-documented.
Line range hint
121-126:
LGTM!The
oAppVersionfunction is correctly implemented and follows best practices for returning version information.src/core/NativeRestakingController.sol (5)
27-30: LGTM!The
nativeRestakingEnabledmodifier is correctly implemented and enhances security by ensuring that only whitelisted tokens can utilize native restaking.
Line range hint
35-56:
LGTM!The
stakefunction is correctly implemented with necessary checks and follows best practices for staking operations.
Line range hint
58-83:
LGTM!The
createExoCapsulefunction is correctly implemented with necessary checks and follows best practices for contract creation.
Line range hint
86-100:
LGTM!The
depositBeaconChainValidatorfunction is correctly implemented with necessary checks and follows best practices for deposit verification.
Line range hint
102-118:
LGTM!The
processBeaconChainWithdrawalfunction is correctly implemented with necessary checks and follows best practices for withdrawal verification.src/storage/ClientChainGatewayStorage.sol (6)
10-16: Natspec documentation looks good!The Natspec documentation provides clear descriptions of the contract and its purpose.
19-52: State variables and mappings are well-defined.The state variables and mappings are correctly defined and documented, enhancing the contract's ability to handle cross-chain messaging and state management.
58-87: Events are well-defined and documented.The events provide detailed feedback on contract operations, which is essential for monitoring and logging contract activity.
94-94: Error definition is clear and useful.The
CapsuleNotExisterror provides clear feedback when operations involving non-existent capsules are attempted.
Line range hint
97-122: Constructor initialization is correct.The constructor parameters are correctly defined, and the initialization logic ensures that essential addresses are set up properly.
Line range hint
123-130: Internal function_getCapsuleis well-implemented.The function correctly retrieves the ExoCapsule for a given owner and handles the case where the capsule does not exist.
src/storage/ExocoreGatewayStorage.sol (4)
6-31: Natspec documentation and constants are well-defined.The Natspec documentation provides clear descriptions of the contract and its purpose. The constants standardize the data structure used for various operations.
42-51: Mappings are well-defined and documented.The mappings enhance the contract's ability to track which chains are bootstrapped and registered, as well as which tokens are permitted for transactions.
54-116: Events are well-defined and documented.The events provide detailed feedback on contract operations, which is essential for monitoring and logging contract activity.
121-163: Error definitions are clear and useful.The errors provide clear feedback on the nature of failures, aiding in debugging and user interaction.
script/integration/1_DeployBootstrap.s.sol (5)
Line range hint
17-35: Imports and variable renaming are consistent.The import statement for
IValidatorRegistryis correct, and the variable renaming from operators to validators is consistent throughout the file.
Line range hint
58-81: Setup logic adjustments are correct.The setup function correctly initializes the validators, ensuring that the contract is set up with the necessary addresses.
88-93: Token deployment logic is correct.The
deployTokensfunction correctly handles the new validators array, ensuring that tokens are deployed to the appropriate addresses.
Line range hint
167-194: Validator registration logic is correct.The function
registerValidatorscorrectly registers the validators, ensuring that the contract supports validator registrations.
Line range hint
200-250: Delegation logic update is correct.The delegation process correctly handles the new validators array, ensuring that delegations are structured and processed appropriately.
src/core/ClientGatewayLzReceiver.sol (11)
10-13: Natspec comments added.The Natspec comments provide a clear and concise description of the contract, its author, and its purpose.
16-32: Natspec comments for errors added.The Natspec comments for the errors provide clear descriptions of the conditions under which the errors are thrown and their parameters.
35-37: Natspec comments for eventWithdrawFailedOnExocoreadded.The Natspec comments for the event provide clear descriptions of the event and its parameters.
Line range hint
49-75:
Enhanced error handling and response processing in_lzReceive.The function now includes additional error handling and response processing, improving robustness and clarity. Ensure that all new error types and response handlers are tested.
Do you want me to generate unit tests for these changes or open a GitHub issue to track this task?
Line range hint
87-135:
Enhanced response handling in_handleResponse.The function now handles different types of responses and updates states accordingly. The logic appears sound, but ensure that all response types are adequately tested.
Do you want me to generate unit tests for these changes or open a GitHub issue to track this task?
Line range hint
136-144:
Function_getCachedRequestForResponseadded.The function retrieves cached request details for a response, with appropriate error handling for unexpected responses.
153-206: Utility functions for action checks added.The utility functions improve code readability and maintainability by encapsulating action checks. The functions appear correct.
Line range hint
211-232:
Function_decodeCachedRequestadded.The function decodes the cached request based on the action type, with appropriate error handling for unsupported requests.
Line range hint
234-255:
Functions_decodeBalanceResponseand_decodeBasicResponseadded.The functions decode balance and basic responses, respectively, improving code readability and maintainability.
Line range hint
258-287:
Function_updatePrincipalAssetStateadded.The function updates the principal asset state based on the action type, with appropriate handling for different token types.
Line range hint
288-311:
FunctionafterReceiveAddWhitelistTokensRequestadded.The function processes add-whitelist-tokens requests, with appropriate error handling for invalid requests.
src/storage/BootstrapStorage.sol (10)
12-15: Natspec comments added.The Natspec comments provide a clear and concise description of the contract, its author, and its purpose.
23-99: Updated state variables and mappings.The updates reflect the new validator-centric model, improving code clarity and maintainability. Ensure that all references to the old variables are updated accordingly.
147-233: Updated events.The updates reflect the new validator-centric model, improving code clarity and maintainability. Ensure that all references to the old events are updated accordingly.
239-250: Updated errors.The updates reflect the new validator-centric model, improving code clarity and maintainability. Ensure that all references to the old errors are updated accordingly.
253-260: StructTokenInfoadded.The struct provides detailed information about a token, improving code clarity and maintainability.
270-270: Storage gap added.The storage gap allows for future upgrades, improving code maintainability.
273-300: Updated modifiers.The updates reflect the new validator-centric model, improving code clarity and maintainability. Ensure that all references to the old modifiers are updated accordingly.
Line range hint
301-317:
Constructor updated.The updates reflect the new validator-centric model, improving code clarity and maintainability.
Line range hint
319-328:
Function_getVaultadded.The function retrieves the vault associated with a given token, with appropriate error handling for non-existent vaults.
331-340: FunctionisValidExocoreAddressadded.The function checks if a provided Exocore address is valid, with appropriate error handling for invalid addresses.
src/core/ExoCapsule.sol (8)
15-17: Natspec comments added.The Natspec comments provide a clear and concise description of the contract, its author, and its purpose.
25-69: Updated and new events.The updates include parameter names and new events, improving code clarity and maintainability.
72-128: Updated errors.The updates provide more precise feedback and additional context, improving error handling and debugging capabilities.
130-135: ModifieronlyGatewayupdated.The update uses the new
InvalidCallererror, improving code clarity and maintainability.
Line range hint
149-160:
Functioninitializeupdated.The function includes additional parameters and emits an event, improving functionality and clarity.
Line range hint
162-208:
FunctionverifyDepositProofupdated.The function includes additional checks and error handling, improving functionality and robustness.
Line range hint
209-263:
FunctionverifyWithdrawalProofupdated.The function includes additional checks and error handling, improving functionality and robustness.
Line range hint
264-273:
Functionwithdrawupdated.The function includes additional checks and error handling,
src/core/ExocoreGateway.sol (13)
Line range hint
43-47:
LGTM!The
onlyCalledFromThismodifier enhances security by restricting function calls to those initiated from within the contract itself.
52-55: LGTM!The constructor initializes the contract with a given endpoint address and disables initializers, enhancing clarity and security.
Line range hint
60-71:
LGTM!The
initializefunction includes robust input validation and clear documentation, ensuring proper contract setup.
Line range hint
72-82:
LGTM!The
_initializeWhitelistFunctionSelectorsfunction is clear and well-documented, ensuring proper setup of whitelist function selectors.
84-90: LGTM!The
pauseandunpausefunctions are straightforward and include appropriate access control.
94-98: LGTM!The
markBootstrapOnAllChainsfunction includes detailed documentation and robust error handling, ensuring proper execution.
Line range hint
119-145:
LGTM!The
registerOrUpdateClientChainfunction is well-structured, includes comprehensive input validation, and emits appropriate events.
Line range hint
146-162:
LGTM!The
setPeerfunction is clear, includes necessary checks, and properly overrides the parent method.
Line range hint
164-186:
LGTM!The
addWhitelistTokensandupdateWhitelistedTokensfunctions are well-documented, include robust input validation, and effectively reduce redundancy by using a common internal function.
Line range hint
188-245:
LGTM!The
_addOrUpdateWhitelistTokensfunction is well-structured, includes detailed input validation, and ensures proper handling of token registration.
Line range hint
249-278:
LGTM!The
_validateWhitelistTokensInputfunction is clear, includes robust validation checks, and ensures input data integrity.
Line range hint
281-297:
LGTM!The
_registerClientChainfunction is straightforward, includes necessary validation, and ensures proper error handling.
Line range hint
300-322:
LGTM!The
_lzReceivefunction is well-structured, includes robust validation, and ensures proper handling of incoming messages.src/core/Bootstrap.sol (12)
41-51: LGTM!The constructor initializes the contract with parameters for validator management, enhancing clarity and functionality.
Line range hint
53-91:
LGTM!The
initializefunction includes robust input validation and clear documentation, ensuring proper contract setup.
93-99: LGTM!The
isLockedfunction is straightforward and includes clear documentation.
108-117: LGTM!The
pauseandunpausefunctions are straightforward and include appropriate access control.
Line range hint
120-132:
LGTM!The
setSpawnTimefunction includes robust input validation and clear documentation.
Line range hint
134-143:
LGTM!The
setOffsetDurationfunction includes robust input validation and clear documentation.
Line range hint
147-174:
LGTM!The
addWhitelistTokensfunction is well-structured, includes robust validation, and ensures proper handling of token whitelisting.
181-206: LGTM!The
registerValidatorfunction is well-structured, includes comprehensive input validation, and emits appropriate events.
208-222: LGTM!The
consensusPublicKeyInUsefunction is clear, includes necessary validation, and ensures the uniqueness of the consensus public key.
Line range hint
225-235:
LGTM!The
isCommissionValidfunction is clear, includes robust validation, and ensures the integrity of the commission data.
238-252: LGTM!The
nameInUsefunction is clear, includes necessary validation, and ensures the uniqueness of the validator name.
254-259: LGTM!The
replaceKeyfunction is well-structured, includes necessary validation, and ensures proper handling of key replacement.test/foundry/unit/Bootstrap.t.sol (8)
9-9: LGTM! Import statement update.The import statement has been correctly updated to reflect the new validator-centric model.
69-71: LGTM! Address initialization update.The addresses have been correctly updated to reflect the new validator-centric model.
309-333: LGTM! Function update to register validators.The function name and logic have been correctly updated to reflect the new validator-centric model.
341-354: LGTM! Function update for ETH address already registered.The function name and error message have been correctly updated to reflect the new validator-centric model.
358-372: LGTM! Function update for Exocore address already registered.The function name and error message have been correctly updated to reflect the new validator-centric model.
376-391: LGTM! Function update for consensus key already in use.The function name and error message have been correctly updated to reflect the new validator-centric model.
395-410: LGTM! Function update for name already in use.The function name and error message have been correctly updated to reflect the new validator-centric model.
901-926: LGTM! Function update for commission validation.The function has been correctly updated to use the
IValidatorRegistry.Commissionstructure.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
src/core/ExocoreGateway.sol (1)
Line range hint
95-105:
LGTM!The
markBootstrapOnAllChainsfunction is well-documented and correctly implements the logic for marking the bootstrap on all chains. The TODO comment indicates that further improvements are planned.Would you like me to generate the code for automatically calling this function or open a GitHub issue to track this task?
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- src/core/BaseRestakingController.sol (7 hunks)
- src/core/BeaconProxyBytecode.sol (1 hunks)
- src/core/Bootstrap.sol (13 hunks)
- src/core/BootstrapLzReceiver.sol (3 hunks)
- src/core/ClientChainGateway.sol (7 hunks)
- src/core/ClientGatewayLzReceiver.sol (10 hunks)
- src/core/CustomProxyAdmin.sol (1 hunks)
- src/core/ExoCapsule.sol (15 hunks)
- src/core/ExocoreGateway.sol (23 hunks)
- src/core/LSTRestakingController.sol (5 hunks)
- src/core/NativeRestakingController.sol (6 hunks)
- src/core/Vault.sol (5 hunks)
Files skipped from review due to trivial changes (2)
- src/core/BeaconProxyBytecode.sol
- src/core/LSTRestakingController.sol
Files skipped from review as they are similar to previous changes (8)
- src/core/BaseRestakingController.sol
- src/core/Bootstrap.sol
- src/core/BootstrapLzReceiver.sol
- src/core/ClientChainGateway.sol
- src/core/ClientGatewayLzReceiver.sol
- src/core/CustomProxyAdmin.sol
- src/core/ExoCapsule.sol
- src/core/Vault.sol
Additional context used
Learnings (1)
src/core/ExocoreGateway.sol (2)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#23 File: src/core/ExocoreGateway.sol:74-87 Timestamp: 2024-06-18T02:35:50.005Z Learning: User: adu-web3 PR: ExocoreNetwork/exocore-contracts#23 File: src/core/ExocoreGateway.sol:74-87 Timestamp: 2024-06-18T02:33:17.212Z Learning: An issue was created to track the optimization of the governance model by removing `exocoreValidatorSetAddress` and using a multi-sig wallet as the contract owner. This issue can be found at https://github.com/ExocoreNetwork/exocore-contracts/issues/30.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 (18)
src/core/NativeRestakingController.sol (5)
28-32: LGTM!The
nativeRestakingEnabledmodifier is correctly implemented and enhances security by ensuring only whitelisted tokens can engage in native restaking.
Line range hint
36-56:
LGTM!The
stakefunction is well-documented and correctly implements the staking logic. The use of thenativeRestakingEnabledmodifier ensures security.
Line range hint
59-84:
LGTM!The
createExoCapsulefunction is well-documented and correctly implements the logic for creating an ExoCapsule contract. The use of thenativeRestakingEnabledmodifier ensures security.
Line range hint
87-101:
LGTM!The
depositBeaconChainValidatorfunction is well-documented and correctly implements the logic for verifying deposit proofs. The use of thenativeRestakingEnabledmodifier ensures security.
Line range hint
103-119:
LGTM!The
processBeaconChainWithdrawalfunction is well-documented and correctly implements the logic for verifying withdrawal proofs. The use of thenativeRestakingEnabledmodifier ensures security.src/core/ExocoreGateway.sol (13)
Line range hint
44-49:
LGTM!The
onlyCalledFromThismodifier is correctly implemented and enhances security by ensuring that certain function calls can only be initiated from the contract itself.
Line range hint
61-71:
LGTM!The
initializefunction is well-documented and correctly implements the initialization logic. The requirement that the owner address cannot be zero enhances security.
Line range hint
120-145:
LGTM!The
registerOrUpdateClientChainfunction is well-documented and correctly implements the logic for registering or updating a client chain. The input validation ensures robustness.
Line range hint
165-175:
LGTM!The
addWhitelistTokensfunction is well-documented and correctly implements the logic for adding whitelist tokens. The use of the internal function_addOrUpdateWhitelistTokensenhances maintainability.
Line range hint
177-187:
LGTM!The
updateWhitelistedTokensfunction is well-documented and correctly implements the logic for updating whitelisted tokens. The use of the internal function_addOrUpdateWhitelistTokensenhances maintainability.
Line range hint
189-247:
LGTM!The
_addOrUpdateWhitelistTokensfunction is well-documented and correctly implements the logic for adding or updating whitelist tokens. The input validation ensures robustness.
Line range hint
250-279:
LGTM!The
_validateWhitelistTokensInputfunction is well-documented and correctly implements the input validation logic. The validation ensures that lengths are equal and within expected limits.
Line range hint
324-343:
LGTM!The
requestDepositfunction is well-documented and correctly implements the logic for responding to deposit requests. The use of theonlyCalledFromThismodifier enhances security.
Line range hint
346-374:
LGTM!The
requestWithdrawPrincipalfunction is well-documented and correctly implements the logic for responding to withdraw-principal requests. The use of theonlyCalledFromThismodifier enhances security.
Line range hint
378-405:
LGTM!The
requestWithdrawRewardfunction is well-documented and correctly implements the logic for responding to withdraw-reward requests. The use of theonlyCalledFromThismodifier enhances security.
Line range hint
408-432:
LGTM!The
requestDelegateTofunction is well-documented and correctly implements the logic for responding to delegate requests. The use of theonlyCalledFromThismodifier enhances security.
Line range hint
435-462:
LGTM!The
requestUndelegateFromfunction is well-documented and correctly implements the logic for responding to undelegate requests. The use of theonlyCalledFromThismodifier enhances security.
Line range hint
466-504:
LGTM!The
requestDepositThenDelegateTofunction is well-documented and correctly implements the logic for responding to deposit-then-delegate requests. The use of theonlyCalledFromThismodifier enhances security.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- script/8_RegisterValidatorsAndDelegate.s.sol (5 hunks)
Additional comments not posted (10)
script/8_RegisterValidatorsAndDelegate.s.sol (10)
5-5: Import statement is necessary and correct.The import statement for
IValidatorRegistryis necessary for the changes related to validators.
13-13: Contract name change is appropriate.The contract name change from
RegisterOperatorsAndDelegatetoRegisterValidatorsAndDelegateaccurately reflects the shift from managing operators to managing validators.
16-17: Variable name change is appropriate.The variable name change from
operatorKeystovalidatorKeysis consistent with the shift from operators to validators.
37-37: Environment variable change is appropriate.The change from
OPERATOR_KEYStoVALIDATOR_KEYSin thesetUpfunction is necessary to reflect the new environment variable for validators.
46-48: Validation logic is correct.The validation logic in the
setUpfunction ensures that the lengths ofvalidatorKeys,exoAddresses,names, andconsKeysmatch, which is crucial for consistent data handling.
60-60: Commission structure change is appropriate.The change from
IOperatorRegistry.CommissiontoIValidatorRegistry.Commissionin therunfunction is necessary to reflect the new commission structure for validators.
64-65: Variable name change in loop is appropriate.The change from
operatorKeystovalidatorKeysin the loop is consistent with the shift from operators to validators.
72-73: Function call change is appropriate.The change from
registerOperatortoregisterValidatorin therunfunction is necessary to reflect the new function for registering validators.
92-93: Variable name change in loop is appropriate.The change from
operatorKeystovalidatorKeysin the loop is consistent with the shift from operators to validators.
95-100: Validation logic in loop is correct.The validation logic in the loop ensures that the correct amounts are delegated to each validator.
There was a problem hiding this comment.
This looks great! @bwhour i think we need to finish natspecs for all of the other contracts as well.
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)
- docs/client-chain-contracts-design.md (3 hunks)
- src/core/ExoCapsule.sol (14 hunks)
- src/libraries/ValidatorContainer.sol (1 hunks)
- src/libraries/WithdrawalContainer.sol (1 hunks)
Files skipped from review due to trivial changes (2)
- src/libraries/ValidatorContainer.sol
- src/libraries/WithdrawalContainer.sol
Files skipped from review as they are similar to previous changes (1)
- docs/client-chain-contracts-design.md
Additional comments not posted (21)
src/core/ExoCapsule.sol (21)
29-29: LGTM! Improved readability.The event
PrincipalBalanceUpdatednow includes parameter names, enhancing clarity.
34-34: LGTM! Improved readability.The event
WithdrawableBalanceUpdatednow includes parameter names, enhancing clarity.
40-40: LGTM! Improved readability.The event
WithdrawalSuccessnow includes parameter names, enhancing clarity.
129-129: LGTM! Improved specificity and clarity.The error
InvalidGatewayhas been renamed toInvalidCallerand now includes parameter names.
265-265: LGTM! Documentation added.The function
withdrawnow includes documentation, improving clarity and understanding.
280-280: LGTM! Documentation added.The function
withdrawNonBeaconChainETHBalancenow includes documentation, improving clarity and understanding.
293-293: LGTM! Documentation added.The function
updatePrincipalBalancenow includes documentation, improving clarity and understanding.
300-300: LGTM! Documentation added.The function
updateWithdrawableBalancenow includes documentation, improving clarity and understanding.
320-320: LGTM! Documentation added.The function
getBeaconBlockRootnow includes documentation, improving clarity and understanding.
333-333: LGTM! Documentation added.The function
getRegisteredValidatorByPubkeynow includes documentation, improving clarity and understanding.
346-346: LGTM! Documentation added.The function
getRegisteredValidatorByIndexnow includes documentation, improving clarity and understanding.
358-358: LGTM! Documentation added.The function
_sendETHnow includes documentation, improving clarity and understanding.
369-369: LGTM! Documentation added.The function
_verifyValidatorContainernow includes documentation, improving clarity and understanding.
390-390: LGTM! Documentation added.The function
_verifyWithdrawalContainernow includes documentation, improving clarity and understanding.
405-405: LGTM! Documentation added.The function
_isActivatedAtEpochnow includes documentation, improving clarity and understanding.
419-419: LGTM! Documentation added.The function
_isStaleProofnow includes documentation, improving clarity and understanding.
427-427: LGTM! Documentation added.The function
_hasFullyWithdrawnnow includes documentation, improving clarity and understanding.
437-437: LGTM! Documentation added.The function
_timestampToEpochnow includes documentation, improving clarity and understanding.
46-49: LGTM! Event added.The event
PartialWithdrawalRedeemedimproves tracking of partial withdrawals.
55-58: LGTM! Event added.The event
FullWithdrawalRedeemedimproves tracking of full withdrawals.
62-62: LGTM! Event added.The event
RestakingActivatedimproves tracking of restaking activities.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- src/core/Bootstrap.sol (14 hunks)
- src/core/BootstrapLzReceiver.sol (2 hunks)
- src/core/ClientChainGateway.sol (7 hunks)
- src/core/ClientGatewayLzReceiver.sol (9 hunks)
- src/core/CustomProxyAdmin.sol (1 hunks)
- src/core/ExocoreGateway.sol (22 hunks)
- src/core/NativeRestakingController.sol (6 hunks)
- src/core/Vault.sol (5 hunks)
- src/libraries/Errors.sol (1 hunks)
- test/foundry/unit/Bootstrap.t.sol (15 hunks)
Files skipped from review as they are similar to previous changes (8)
- src/core/Bootstrap.sol
- src/core/BootstrapLzReceiver.sol
- src/core/ClientChainGateway.sol
- src/core/ClientGatewayLzReceiver.sol
- src/core/CustomProxyAdmin.sol
- src/core/ExocoreGateway.sol
- src/core/NativeRestakingController.sol
- src/core/Vault.sol
Additional comments not posted (13)
src/libraries/Errors.sol (4)
70-72: LGTM!The error message
BootstrapValidatorAlreadyHasAddresscorrectly reflects the shift from "operator" to "validator".
75-77: LGTM!The error message
BootstrapValidatorAlreadyRegisteredcorrectly reflects the shift from "operator" to "validator".
85-87: LGTM!The error message
BootstrapValidatorNameAlreadyUsedcorrectly reflects the shift from "operator" to "validator".
95-97: LGTM!The error message
BootstrapValidatorNotExistcorrectly reflects the shift from "operator" to "validator".test/foundry/unit/Bootstrap.t.sol (9)
9-9: LGTM!The import statement for
IValidatorRegistrycorrectly reflects the shift from "operator" to "validator".
71-73: LGTM!The address names
VALIDATOR1,VALIDATOR2, andVALIDATOR3correctly reflect the shift from "operator" to "validator".
311-335: LGTM!The function name
test03_RegisterValidatorand the internal logic correctly reflect the shift from "operator" to "validator".
343-356: LGTM!The function name
test03_RegisterValidator_EthAlreadyRegisteredand the internal logic correctly reflect the shift from "operator" to "validator".
360-374: LGTM!The function name
test03_RegisterValidator_ExoAlreadyRegisteredand the internal logic correctly reflect the shift from "operator" to "validator".
378-393: LGTM!The function name
test03_RegisterValidator_ConsensusKeyInUseand the internal logic correctly reflect the shift from "operator" to "validator".
397-412: LGTM!The function name
test03_RegisterValidator_NameInUseand the internal logic correctly reflect the shift from "operator" to "validator".
419-419: LGTM!The internal logic correctly reflects the shift from "operator" to "validator".
435-448: LGTM!The internal logic correctly reflects the shift from "operator" to "validator".
There was a problem hiding this comment.
Actionable comments posted: 12
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/client-chain-contracts-design.md (4 hunks)
Additional context used
LanguageTool
docs/client-chain-contracts-design.md
[uncategorized] ~5-~5: Possible missing preposition found.
Context: ...nt chain smart contracts refer to a set smart contracts that are deployed on multiple...(AI_HYDRA_LEO_MISSING_OF)
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...s architecture: 1.ClientChainGateway: This is the entry point where client ch...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...s from Exocore validator set. 2.Vault: This is where user funds are taken into...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Possible missing article found.
Context: ...teway. Every specific asset should have standaloneVault. 3.LSTRestakingController: T...(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...oneVault. 3.LSTRestakingController: The controller is responsible for manag...(UNLIKELY_OPENING_PUNCTUATION)
[locale-violation] ~22-~22: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...torage layout compatible after upgrade. Afterwards, by replacing the old logic contract ad...(AFTERWARDS_US)
[grammar] ~26-~26: The usual preposition is ‘at’.
Context: ...ct, we need to retain some unused slots in the end of storage contract so that we can add new...(IN_THE_END_OF)
[style] ~47-~47: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...nor versions. In most of the cases, we can not directly use implementations linked abo...(CAN_NOT_PREMIUM)
[uncategorized] ~50-~50: Possible missing comma found.
Context: ...value to state variables when declaring them exceptimmutableandconstant. 3. D...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~108-~108: Do not mix variants of the same word (‘upgradable’ and ‘upgradeable’) within a single text.
Context: ...have madeClientChainGatewaycontract upgradable so that the state can be retained while...(EN_WORD_COHERENCY)
[grammar] ~128-~128: The word ‘withdraw’ is not a noun. Did you mean “withdrawal”?
Context: ... an outgoing message. For example, if a withdraw request is initiated by a user, and sen...(PREPOSITION_VERB)
[uncategorized] ~174-~174: Possible missing comma found.
Context: ... the Exocore chain or on another client chain while the user principal is taken in cu...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~186-~186: The word ‘withdraw’ is not a noun. Did you mean “withdrawal”?
Context: ...eClientChainGatewayin response to a withdraw request, after receiving a response fro...(PREPOSITION_VERB)
[style] ~257-~257: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...pon receiving the message, Exocore will take into account the deposit, and must respond that the ...(EN_WORDINESS_PREMIUM_TAKE_INTO_ACCOUNT)
[style] ~272-~272: ‘on the basis of’ might be wordy. Consider a shorter alternative.
Context: ...bonding period is determined by Exocore on the basis of all of the AVSs in which the operator w...(EN_WORDINESS_PREMIUM_ON_THE_BASIS_OF)
[style] ~272-~272: Consider removing “of” to be more concise
Context: ...s determined by Exocore on the basis of all of the AVSs in which the operator was particip...(ALL_OF_THE)
Additional comments not posted (3)
docs/client-chain-contracts-design.md (3)
14-14: Remove loose punctuation mark.The colon after "architecture" should be removed.
- We have these components included in Exocore client chain smart contracts architecture: + We have these components included in Exocore client chain smart contracts architectureSkipped due to learnings
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: src/core/ExocoreGateway.sol:105-105 Timestamp: 2024-07-01T09:18:36.140Z Learning: In the Exocore contracts, the design decision to avoid receiving responses from the client chain due to cost considerations means that operations assume success on the client chain.Tools
LanguageTool
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...s architecture: 1.ClientChainGateway: This is the entry point where client ch...(UNLIKELY_OPENING_PUNCTUATION)
16-16: Remove loose punctuation mark.The colon after "Vault" should be removed.
- standalone `Vault`. 3. `LSTRestakingController`: The controller is responsible for managing multiple `Vault`s. + standalone `Vault` 3. `LSTRestakingController`: The controller is responsible for managing multiple `Vault`s.Likely invalid or redundant comment.
Tools
LanguageTool
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...oneVault. 3.LSTRestakingController: The controller is responsible for manag...(UNLIKELY_OPENING_PUNCTUATION)
15-15: Remove loose punctuation mark.The colon after "set" should be removed.
- Exocore validator set: 2. `Vault`: This is where user funds are taken into custody and managed. + Exocore validator set 2. `Vault`: This is where user funds are taken into custody and managed.Likely invalid or redundant comment.
Tools
LanguageTool
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...s from Exocore validator set. 2.Vault: This is where user funds are taken into...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Possible missing article found.
Context: ...teway. Every specific asset should have standaloneVault. 3.LSTRestakingController: T...(AI_HYDRA_LEO_MISSING_A)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/client-chain-contracts-design.md (4 hunks)
Additional context used
LanguageTool
docs/client-chain-contracts-design.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...s architecture: 1.ClientChainGateway: This is the entry point where client ch...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...s from Exocore validator set. 2.Vault: This is where user funds are taken into...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...oneVault. 3.LSTRestakingController: The controller is responsible for manag...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~276-~276: Possible missing article found.
Context: ...balance. If this process is successful, user should be able to claim the correspondi...(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~281-~281: Possible missing article found.
Context: ...cipalBalance` and claimable balance. If response indicates failure, no user balance shou...(AI_HYDRA_LEO_MISSING_THE)
Additional comments not posted (8)
docs/client-chain-contracts-design.md (8)
139-143: LGTM! Improved function signature.The new function signature improves clarity by specifying the withdrawer and recipient addresses.
160-165: LGTM! Improved function signature.The new function signature improves clarity by specifying the principal and reward amounts to be unlocked.
167-169: LGTM! Improved function signature.The new function signature improves clarity by specifying the function as
view.
108-108: Use "upgradeable" consistently.Do not mix variants of the same word.
- We have made `ClientChainGateway` contract upgradable so that the state can be retained while adding or removing some features in the future. + We have made `ClientChainGateway` contract upgradeable so that the state can be retained while adding or removing some features in the future.Likely invalid or redundant comment.
14-14: Add missing article.The sentence is missing the article "a" before "standalone
Vault".- Every specific asset should have standalone `Vault`. + Every specific asset should have a standalone `Vault`.Skipped due to learnings
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#39 File: src/core/ExocoreGateway.sol:105-105 Timestamp: 2024-07-01T09:18:36.140Z Learning: In the Exocore contracts, the design decision to avoid receiving responses from the client chain due to cost considerations means that operations assume success on the client chain.Tools
LanguageTool
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...s architecture: 1.ClientChainGateway: This is the entry point where client ch...(UNLIKELY_OPENING_PUNCTUATION)
272-272: Consider a shorter alternative."On the basis of" might be wordy. Consider "based on" instead.
- The unbonding period is determined by Exocore on the basis of all of the AVSs in which the operator was participating at the time of undelegation. + The unbonding period is determined by Exocore based on all the AVSs in which the operator was participating at the time of undelegation.Likely invalid or redundant comment.
47-47: Use "cannot" instead of "can not"."Cannot" is more common and preferred over "can not".
- In most of the cases, we can not directly use implementations linked above and need to implement our own upgradeable contracts. + In most of the cases, we cannot directly use implementations linked above and need to implement our own upgradeable contracts.Likely invalid or redundant comment.
26-26: Use correct preposition.Change "in the end" to "at the end".
- we need to retain some unused slots in the end of storage contract so that we can add new state variables and override the unused slots. + we need to retain some unused slots at the end of storage contract so that we can add new state variables and override the unused slots.Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- README.md (1 hunks)
- docs/client-chain-contracts-design.md (4 hunks)
- script/3_Setup.s.sol (3 hunks)
- src/interfaces/IBaseRestakingController.sol (1 hunks)
- src/interfaces/ILSTRestakingController.sol (1 hunks)
- src/libraries/Errors.sol (1 hunks)
- src/storage/BootstrapStorage.sol (6 hunks)
- test/foundry/ExocoreDeployer.t.sol (1 hunks)
Files skipped from review due to trivial changes (3)
- README.md
- script/3_Setup.s.sol
- test/foundry/ExocoreDeployer.t.sol
Files skipped from review as they are similar to previous changes (3)
- src/interfaces/IBaseRestakingController.sol
- src/interfaces/ILSTRestakingController.sol
- src/libraries/Errors.sol
Additional context used
Learnings (1)
docs/client-chain-contracts-design.md (1)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#23 File: src/core/ExocoreGateway.sol:74-87 Timestamp: 2024-06-18T02:33:17.317Z Learning: The `exocoreValidatorSetAddress` was initially intended to be derived from the aggregated TSS public key of the Exocore validator set, but this design was not implemented. The governance model now uses a multi-sig wallet as the contract owner, removing the role of `exocoreValidatorSetAddress`.
LanguageTool
docs/client-chain-contracts-design.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...s architecture: 1.ClientChainGateway: This is the entry point where client ch...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...s from Exocore validator set. 2.Vault: This is where user funds are taken into...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...oneVault. 3.LSTRestakingController: The controller is responsible for manag...(UNLIKELY_OPENING_PUNCTUATION)
Additional comments not posted (16)
src/storage/BootstrapStorage.sol (7)
36-39: Terminology update: ChangedregisteredOperatorstoregisteredValidators.The array now stores the Ethereum addresses of all validators instead of operators. This change aligns with the shift in terminology.
47-47: Terminology update: Changedoperatorstovalidators.The mapping now associates Exocore addresses with validator details instead of operator details. This change aligns with the shift in terminology.
59-59: Terminology update: ChangeddelegationstodelegationsByValidator.The mapping now tracks delegations by validator instead of operator. This change aligns with the shift in terminology.
176-182: Terminology update: ChangedoperatortodelegateeinDelegateResultevent.The event now reflects delegations to validators instead of operators. Ensure that all references to
operatorare updated accordingly.
198-204: Update event parameter names for clarity.The
DepositThenDelegateResultevent now usesdelegateSuccessanddelegatedAmountfor clarity. Ensure that these changes are reflected in the event handling logic.
12-15: Improved documentation.The documentation for the
BootstrapStoragecontract has been significantly improved, enhancing clarity and readability.
81-85: Ensure consistency in mapping names.The
delegationsmapping still uses the termoperatorin its nested mapping. Consider updating it tovalidatorfor consistency.- mapping(address delegator => mapping(string exoAddress => mapping(address tokenAddress => uint256 amount))) public delegations; + mapping(address delegator => mapping(string exoAddress => mapping(address tokenAddress => uint256 amount))) public delegationsByValidator;Likely invalid or redundant comment.
docs/client-chain-contracts-design.md (9)
5-5: Add missing preposition.The sentence is missing the preposition "of" after "set".
- Exocore Client chain smart contracts refer to a set smart contracts that are deployed on multiple chains + Exocore Client chain smart contracts refer to a set of smart contracts that are deployed on multiple chains
22-22: Use "afterward" instead of "afterwards".In American English, "afterward" is preferred over "afterwards".
- Afterwards, by replacing the old logic contract address with the address of new logic contract, we can upgrade a contract without violating its storage. + Afterward, by replacing the old logic contract address with the address of new logic contract, we can upgrade a contract without violating its storage.
26-26: Use correct preposition.Change "in the end" to "at the end".
- we need to retain some unused slots in the end of storage contract so that we can add new state variables and override the unused slots. + we need to retain some unused slots at the end of storage contract so that we can add new state variables and override the unused slots.
106-106: Clarify ownership.The documentation should clarify that the Exocore validator set should eventually own the
ClientChainGateway.
108-108: Use "upgradeable" consistently.Do not mix variants of the same word.
- We have made `ClientChainGateway` contract upgradable so that the state can be retained while adding or removing some features in the future. + We have made `ClientChainGateway` contract upgradeable so that the state can be retained while adding or removing some features in the future.
128-128: Use "withdrawal" instead of "withdraw".The word "withdraw" is not a noun. Use "withdrawal" instead.
- For example, if a withdraw request is initiated by a user, and sent by the `ClientChainGateway` to Exocore, a response is received indicating whether the withdrawal is valid. + For example, if a withdrawal request is initiated by a user, and sent by the `ClientChainGateway` to Exocore, a response is received indicating whether the withdrawal is valid.
174-174: Add missing comma.A comma is missing after "chain".
- the Exocore chain or on another client chain while the user principal is taken in custody on this chain. + the Exocore chain or on another client chain, while the user principal is taken in custody on this chain.
16-16: Loose punctuation mark.There is a loose punctuation mark in the documentation.
- 3. `LSTRestakingController`: The controller is responsible for managing multiple `Vault`s. It should be the entry point for operations on `Vault`, as well as the entry point for user’s interactions with the gateway. It is inherited / implemented by the `Gateway`. + 3. `LSTRestakingController`: The controller is responsible for managing multiple `Vault`s. It should be the entry point for operations on `Vault`, as well as the entry point for users’ interactions with the gateway. It is inherited/implemented by the `Gateway`.Likely invalid or redundant comment.
Tools
LanguageTool
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...oneVault. 3.LSTRestakingController: The controller is responsible for manag...(UNLIKELY_OPENING_PUNCTUATION)
15-15: Loose punctuation mark.There is a loose punctuation mark in the documentation.
- 2. `Vault`: This is where user funds are taken into custody and managed. Within `Vault`, user balance is updated periodically by Exocore validator set through cross-chain message to reveal user’s real position (after slashing, rewarding and other impact). Users can withdraw from `Vault` based on grant from the gateway. Every specific asset should have a standalone `Vault`. + 2. `Vault`: This is where user funds are taken into custody and managed. Within `Vault`, user balance is updated periodically by the Exocore validator set through cross-chain messages to reveal the user’s real position (after slashing, rewarding, and other impacts). Users can withdraw from `Vault` based on grants from the gateway. Every specific asset should have a standalone `Vault`.Likely invalid or redundant comment.
Tools
LanguageTool
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...s from Exocore validator set. 2.Vault: This is where user funds are taken into...(UNLIKELY_OPENING_PUNCTUATION)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/client-chain-contracts-design.md (4 hunks)
Additional context used
LanguageTool
docs/client-chain-contracts-design.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...s architecture: 1.ClientChainGateway: This is the entry point where client ch...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...om the Exocore validator set. 2.Vault: This is where user funds are taken into...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...oneVault. 3.LSTRestakingController: The controller is responsible for manag...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~281-~281: Possible missing article found.
Context: ...cipalBalance` and claimable balance. If response indicates failure, no user balance shou...(AI_HYDRA_LEO_MISSING_THE)
Additional comments not posted (5)
docs/client-chain-contracts-design.md (5)
139-143: LGTM!The updated function signature improves clarity.
162-165: LGTM!The updated function signature improves clarity.
167-169: LGTM!The updated function signature improves clarity.
108-108: Use "upgradeable" consistently.Do not mix variants of the same word.
- We have made `ClientChainGateway` contract upgradable so that the state can be retained while adding or removing some features in the future. + We have made `ClientChainGateway` contract upgradeable so that the state can be retained while adding or removing some features in the future.Likely invalid or redundant comment.
22-22: Use "afterward" instead of "afterwards".In American English, "afterward" is preferred over "afterwards".
- Afterwards, by replacing the old logic contract address with the address of new logic contract, we can upgrade a contract without violating its storage. + Afterward, by replacing the old logic contract address with the address of new logic contract, we can upgrade a contract without violating its storage.Likely invalid or redundant comment.
operatorwas used exclusively in the context of Bootstrap, replace it withvalidatorsrc/core,src/interfaces(but notsrc/interfaces/precompiles) andsrc/storageExocoreGatewaySummary by CodeRabbit
New Features
RestakingActivatedandWithdrawalSuccess.Improvements
ClientChainGatewayand structured user fund management in theVault.Bug Fixes
Documentation