-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(generate.js): update for compatibility #91
feat(generate.js): update for compatibility #91
Conversation
WalkthroughThe changes introduce a new dependency, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script
participant Blockchain
User->>Script: Request to generate genesis file
Script->>Script: Retrieve validator addresses and assets
Script->>Script: Calculate withdrawable amounts
Script->>Blockchain: Update genesis file with structured data
Blockchain-->>Script: Confirmation of update
Script-->>User: Provide updated genesis file
Possibly related PRs
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- script/generate.js (12 hunks)
Additional comments not posted (10)
script/generate.js (10)
49-50
: LGTM!The code changes are approved.
51-56
: LGTM!The code changes are approved.
58-61
: LGTM!The code changes are approved.
177-179
: LGTM!The code changes are approved.
233-235
: LGTM!The code changes are approved.
278-339
: LGTM!The code changes are approved.
363-370
: LGTM!The code changes are approved.
Line range hint
382-484
: LGTM!The code changes are approved.
497-546
: LGTM!The code changes are approved.
Line range hint
593-663
: LGTM!The code changes are approved.
9f5993a
to
996fd30
Compare
Previously, we had used the `SEPOLIA_RPC` variable, which is now changed to `CLIENT_CHAIN_RPC`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .env.example (2 hunks)
- script/integration/1_DeployBootstrap.s.sol (2 hunks)
Additional comments not posted (3)
.env.example (1)
20-20
: LGTM!The comment update accurately reflects the inclusion of
CLIENT_CHAIN_RPC
among the variables used bygenerate.js
, enhancing code clarity and maintainability.script/integration/1_DeployBootstrap.s.sol (2)
51-51
: LGTM!The change from
Vault[]
toIVault[]
for thevaults
array is a good design decision. It allows the contract to interact with any implementation of theIVault
interface, promoting flexibility and loose coupling.
148-152
: LGTM!The new loop that populates the
vaults
array based on thewhitelistTokens
is a great addition. It improves the contract's operational capabilities by automatically associating vaults with whitelisted tokens, reducing manual effort and ensuring synchronization between thevaults
andwhitelistTokens
arrays.
- Use `opAddressHex` and `opAddressExo` so that they fit in the same font width when using monospace. - Calculate total deposit amount first before setting token information
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on localnet and generated the genesis.json
from the integration test. Ran a network of 3 operators, and all good.
Made some minor changes in the last 4 commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
Not required anymore, after ExocoreNetwork/exocore#193
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
script/generate.js (2)
Line range hint
574-574
: Convertvalidators
to the top N validatorsThe
validators.slice
method returns a new array but does not modifyvalidators
. To limit the validators array, assign the result back tovalidators
.Apply this diff:
- validators.slice(0, genesisJSON.app_state.dogfood.params.max_validators); + validators = validators.slice(0, genesisJSON.app_state.dogfood.params.max_validators);
Line range hint
588-642
: Optimize nested loops for efficiencyThe nested loops over depositors, tokens, and operators can lead to performance issues, especially with large datasets. Consider refactoring to reduce complexity.
- Fetch all required data beforehand and use maps to reduce the number of iterations.
- Parallelize asynchronous calls where appropriate.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- script/generate.js (11 hunks)
Additional comments not posted (3)
script/generate.js (3)
560-560
: Assign the result ofvalidators.slice
to limit the array sizeThe
slice
method returns a new array and does not modify the original array in place. To limit thevalidators
array to the top N validators, you need to assign the result back tovalidators
.Apply this diff to fix the problem:
- validators.slice(0, genesisJSON.app_state.dogfood.params.max_validators); + validators = validators.slice(0, genesisJSON.app_state.dogfood.params.max_validators);Likely invalid or redundant comment.
586-587
: Ensure totalPower calculation is accurateAfter truncating
val.power
usingtoFixed(0)
,val.power
becomes a string. When adding tototalPower
, ensure you're converting it back to a number.Modify the addition to correctly parse the integer value:
- totalPower += parseInt(val.power, 10); + totalPower += Number(val.power);Likely invalid or redundant comment.
373-393
: Validate existence of operator informationEnsure that the
operatorInfo
retrieved from the contract contains all necessary fields before using them. Missing or undefined properties can lead to runtime errors.Run the following script to check for missing
operatorInfo
fields:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good.
description
This PR updates the
generate.js
to be compatible with the PR regarding genesis export. Please take a look at the related PR for more details.genesis exporting PR
This PR hasn't been tested so it might have some issues.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation