fix(generate): update for chain addition #130
Conversation
WalkthroughThe changes in this pull request primarily focus on updating the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
✅ The Slither Analysis workflow has completed successfully. Check the workflow run for details. (89db9ee) |
|
✅ The Forge CI workflow has completed successfully. Check the workflow run for details. (89db9ee) |
|
✅ The Solhint workflow has completed successfully. Check the workflow run for details. (89db9ee) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
script/generate.mjs (1)
738-746: Refactor Validators and Operators InitializationMapping and initializing
validators,operators, andassociationsenhance code readability and maintainability. This refactoring makes the codebase easier to understand and modify in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
script/generate.mjs(13 hunks)
🔇 Additional comments (15)
script/generate.mjs (15)
6-9: Update clientChainInfo for Holesky Testnet
The updates to clientChainInfo correctly reflect the change to the Holesky testnet with the new name, meta_info, and layer_zero_chain_id. Ensure that all references to clientChainInfo in the codebase are updated accordingly.
14-14: Verify Token Metadata Alignment
The tokenMetaInfos array now includes 'Exocore Holesky ETH'. Confirm that the order of tokens in tokenMetaInfos matches the order in tokenNamesForOracle to maintain correct token associations.
23-23: Confirm Oracle Token Names Mapping
The tokenNamesForOracle array has been updated to ['ETH', 'nstETH']. Ensure that these names correctly correspond to the tokens defined in tokenMetaInfos and that they are accurately used in the oracle configurations.
164-166: Add Error Handling for Already Bootstrapped Contract
Throwing an error when the contract is already bootstrapped prevents redundant operations and potential overwriting of critical information.
232-233: Initialize supportedTokens and assetIds
Initializing supportedTokens and assetIds before their use ensures that they are correctly populated and prevents potential undefined errors later in the execution.
254-254: Append Cleaned Token to Supported Tokens
Adding tokenCleaned to supportedTokens ensures that the new token information is included in the genesis file for accurate asset representation.
Line range hint 259-263: Initialize Oracle Token Feeder Correctly
The oracleTokenFeeder object is properly initialized with the correct token_id, start_base_block, and other parameters. Verify that start_base_block is accurately calculated based on height.
291-303: Prevent Duplicates in Oracle Tokens
The code effectively checks for existing token names in oracleTokens to avoid duplicates, appending asset IDs if the token exists or adding a new token if not.
347-355: Update Start Base Block for Oracle Token Feeders
Updating the start_base_block for each oracleTokenFeeder ensures that the feeders commence at the correct block height, improving the reliability of oracle data feeds.
374-374: Initialize Deposits Array
Initializing deposits in genesisJSON.app_state.assets ensures that deposit data can be correctly accumulated without encountering undefined errors.
644-644: Initialize Operator Assets Array
The initialization of operator_assets prepares the array to store operator asset states, facilitating accurate tracking of operator holdings.
749-752: Initialize Operator-Related Arrays
Initializing operator_records, opt_states, avs_usd_values, and operator_usd_values ensures that all necessary data structures are prepared for storing operator information.
806-812: Validate Exchange Rates Against Supported Tokens
The check ensures the number of exchange rates matches the supportedTokensCount. This validation is crucial to prevent calculation errors when converting token amounts to USD values.
931-942: Update AVS USD Values Accurately
Updating avs_usd_values by checking if an existing item exists prevents duplicate entries and ensures that the USD values are aggregated correctly.
1000-1008: Initialize Delegation States and Staker Mapping
Initializing delegation_states and stakers_by_operator, and mapping stakers to operators, enhances data organization for delegation tracking.
|
✅ The Compare Storage Layouts workflow has completed successfully. Check the workflow run for details. (89db9ee) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
script/generate.mjs (6)
Line range hint
232-303: Consider modularizing token management logicThe token management logic is complex and handles multiple responsibilities. Consider extracting the token processing logic into separate functions for better maintainability.
Example refactor:
+ function processOracleToken(token, tokenName, height) { + return { + name: tokenName, + chain_id: 1, + contract_address: token.tokenAddress, + active: true, + asset_id: token.tokenAddress.toLowerCase() + clientChainSuffix, + decimal: 8, + }; + } + function processTokenFeeder(tokenId, height) { + return { + token_id: tokenId, + rule_id: "1", + start_round_id: "1", + start_base_block: (height + 20).toString(), + interval: "30", + end_block: "0", + }; + }
Line range hint
644-752: Consider migrating to TypeScript for better type safetyThe operator and validator handling logic deals with complex data structures. TypeScript would help catch potential type-related errors early and improve code maintainability.
Example type definitions:
interface OperatorRecord { operator_address: string; chains: { chain_id: string; consensus_key: string; }[]; } interface ValidatorState { public_key: string; power: Decimal; }
806-812: Improve error message clarityThe error message for exchange rates validation could be more descriptive.
- throw new Error( - `The number of exchange rates (${exchangeRates.length}) - does not match the number of supported tokens (${supportedTokensCount}).` - ); + throw new Error( + `Exchange rates count mismatch: expected ${supportedTokensCount} rates for supported tokens, ` + + `but received ${exchangeRates.length} rates. Please ensure all token rates are provided.` + );
931-942: Add documentation for AVS USD value calculationThe AVS USD value update logic would benefit from comments explaining the business rules and calculation methodology.
+ // Update or create AVS USD value entry + // If an entry exists for the AVS address, add the new USD value to the existing amount + // Otherwise, create a new entry with the calculated USD value
1000-1001: Use more descriptive variable namesConsider using more descriptive names for better code readability.
- const delegation_states = genesisJSON.app_state.delegation.delegation_states; - const stakers_by_operator = genesisJSON.app_state.delegation.stakers_by_operator; + const delegatorStates = genesisJSON.app_state.delegation.delegation_states; + const stakersGroupedByOperator = genesisJSON.app_state.delegation.stakers_by_operator;
1101-1116: Extract native chain handling into a separate functionConsider extracting the native chain handling logic into a dedicated function for better code organization.
+ function addNativeChainIfNotExists(genesisJSON, nativeChain, nativeAsset) { + const nativeChainExists = genesisJSON.app_state.assets.client_chains.some( + chain => chain.layer_zero_chain_id === nativeChain.layer_zero_chain_id + ); + + if (!nativeChainExists) { + genesisJSON.app_state.assets.client_chains.push(nativeChain); + genesisJSON.app_state.assets.tokens.push(nativeAsset); + genesisJSON.app_state.dogfood.params.asset_ids.push( + nativeAsset.asset_basic_info.address.toLowerCase() + '_0x' + + nativeAsset.asset_basic_info.layer_zero_chain_id.toString(16) + ); + } + return genesisJSON; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
script/generate.mjs(13 hunks)
🔇 Additional comments (2)
script/generate.mjs (2)
6-23: LGTM: Configuration updates for Holesky testnet
The changes correctly update the testnet configuration from Sepolia to Holesky, including the appropriate layer_zero_chain_id and token configurations.
164-164: LGTM: Improved error handling for bootstrap state
Clear error message when attempting to modify an already bootstrapped contract.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/forge-ci.yml (1)
177-177: LGTM! Consider adding JSON validation step.The addition of the
--jsonflag is consistent with the previous change and improves the output format for all contracts.Consider adding a validation step after generating the JSON files to ensure they are well-formed. Here's a suggested addition after the current line:
forge inspect --json src/core/${{ matrix.contract }}.sol:${{ matrix.contract }} storage-layout > ${{ matrix.contract }}.compiled.json; +jq empty ${{ matrix.contract }}.compiled.json || (echo "Invalid JSON generated for ${{ matrix.contract }}" && exit 1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/forge-ci.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/forge-ci.yml (1)
141-141: LGTM! Verify JSON output format.
The addition of the --json flag improves machine readability of the storage layout output.
MaxMustermann2
requested review from
magj2006,
bwhour,
adu-web3 and
cloud8little
Summary by CodeRabbit
New Features
Bug Fixes