feat(staking-cli): add mainnet support and metadata validation#3922
feat(staking-cli): add mainnet support and metadata validation#3922
Conversation
sveitser
requested review from
bfish713,
imabdulbasit,
jbearer,
lukaszrzasik,
rob-maron and
ss-es
as code owners
Mainnet support: - Add config.mainnet.toml with mainnet contract addresses - Add Network enum (Mainnet, Decaf, Local) for init command - Update init command to require --network parameter - Update README with mainnet contract addresses and Docker usage Metadata validation: - Add NodeMetadataContent schema types for metadata validation - Add validate_metadata_uri function with 5s timeout - Add --skip-metadata-validation flag to bypass validation - Add --consensus-public-key parameter for update-metadata-uri - Add parse_bls_pub_key helper function - Include --skip-metadata-validation hint in error messages Contract deployer: - Add configurable deploy cooldown via --post-deployment-cooldown - Refactor Contracts struct to use explicit fields instead of Deref Tests: - Add metadata validation tests for register-validator and update-metadata-uri - Remove unnecessary spawn_blocking from metadata validation tests - Add bls_public_key_str helper to TestSystemExt Documentation: - Major README update with improved formatting and examples - Document metadata validation and schema requirements - Add calldata export documentation
sveitser
force-pushed
the
ma/staking-cli-mainnet
branch
from
d5d82e2 to
4192044
Compare
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces metadata validation for the staking-cli's register-validator and update-metadata-uri commands, which is a great addition for improving the security and reliability of validator metadata. The implementation correctly fetches and validates the metadata against the validator's public key. The new functionality is well-tested, and the documentation in the README has been significantly improved. I also appreciate the related refactorings, such as improving the init command to support multiple networks and enhancing test hygiene. I have one minor suggestion to make the metadata fetching more robust.
| addresses: HashMap<Contract, Address>, | ||
| // TODO: having the cooldown field here is a bit hacky but we postpone a better solution to | ||
| // avoid a large refactor. | ||
| deploy_cooldown: Duration, |
There was a problem hiding this comment.
what is this used for?
There was a problem hiding this comment.
To wait after deploying each contract, it was hardcoded to 1s making every test in the staking CLI that deployes contracts take 16 seconds. I just made it configurable so we can set it to zero for tests.
There was a problem hiding this comment.
Code Review
This pull request introduces metadata validation for the staking-cli, enhancing security and data integrity for validator registrations and updates. It also adds mainnet support with new configuration files and network selection options. The contract deployer has been refactored for better clarity by removing Deref magic and making the deployment cooldown configurable. The changes are well-supported by a significant number of new tests and extensive updates to the README documentation.
My review found a minor code duplication in the test helpers. Overall, this is a high-quality contribution that significantly improves the staking CLI's functionality and usability.
sveitser
changed the title
Move generate_bls_pub_key to module level with #[cfg(test)] so both test and validation_tests modules can share it.
Verify that running a command without providing any signer (--mnemonic, --private-key, or --ledger) fails with an appropriate error message.
…xists Previously the config command would print "Config file at X" and show default values even when no config file existed. Now it correctly detects missing files and suggests running init.
Add section explaining how to use the -c flag to maintain separate config files for different networks (mainnet, decaf). Also clarify that config files are optional and what happens when none exists.
2 participants
Mainnet support:
Metadata validation:
Contract deployer:
Tests:
Documentation: