-
Notifications
You must be signed in to change notification settings - Fork 29
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
Bulk registration of validators #287
Conversation
@@ -46,6 +46,7 @@ const config: HardhatUserConfig = { | |||
} as SSVNetworkConfig, | |||
hardhat: { | |||
allowUnlimitedContractSize: true, | |||
gas: 5000000, |
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.
I had problems using hardhat network when bulk registering validators. With less than 10 validators I reached 30M gas, which is not the real cost. I set this param to fix it.
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.
What does this do? Why was reaching so high? WIth this param that you added is the gas correct now?
Not related but the param above it "allowUnlimitedContractSize" i think maybe its best we put that back to true since were diamond now?
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.
Using default hardhat config, for some reason moving from bulk registering 8 validators (less than 1M gas) to 10 results in more than 30M gas. Specifying the gas in the network config results in real numbers close to holesky ones.
Agree on removing allowUnlimitedContractSize
@@ -65,21 +77,33 @@ const MAX_GAS_PER_GROUP: any = { | |||
[GasGroup.EXECUTE_OPERATOR_FEE]: 52000, | |||
[GasGroup.REDUCE_OPERATOR_FEE]: 51900, | |||
|
|||
[GasGroup.REGISTER_VALIDATOR_EXISTING_CLUSTER]: 202000, | |||
[GasGroup.REGISTER_VALIDATOR_EXISTING_CLUSTER]: 201200, |
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.
@andrew-blox Refactoring the code gets savings on the gas costs, around 1500 gas per GasGroup, but as this version uses ssv-keys
to generate the shares, all values are increased.
@@ -18,5 +18,45 @@ | |||
"id": 4, | |||
"privateKey": "0x52e4bdbdedd85e9e8a77c2f60d083a0693d6f7b2bd717f642a02514528db4278", | |||
"publicKey": "0x9150572051c3496a67207b4caa371dfba34f127318a7aef145ebdba6e0de506c292af31e20831b0c537ab7478508d3e9" | |||
}, |
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.
@Mohsen-T I had to add new keys to support bulk registration.
* event ValidatorExited -> indexed owner * fix: added a test case in balance.ts * fix: reviewed the balance calculation when an operator is removed * Update balances.ts * fix: added a test case * fix: removed only * fix: progressing * fix: updated test case * fix: removed log * [Feature] - Integration ssv-keys in ssv-network for generating keyshares (#276) * feat: updated validators & operators management using ssv-keys * [Update]: deployment tests and update task and documentation (#282) * fix: updated documentations * [Update] Added triggers for compilation in a few tasks (upgrading / updating) (#285) * feat: added trigger for compilation in tasks * fix: changed before test * bulk validator registration (init) * 2 register validator functions, all tests pass * refactor cluster and operators libraries * V1.0.2 deployment metadata (#283) * v1.0.2 deployment metadata * Bulk registration of validators (#287) * bulk validator registration * update CHANGELOG * bulk remove validators * upgrade goerli prod * bulk exit validator * hlsky dev metadata * add exit validator tests * update CHANGELOG * update gas usage * update roles doc * minor improvements * v1.1.0 goerli_prod metadata * remove abis from git * remove liquidation check (duplicate) * Fix: updated the test coverage Enhanced test coverage by incorporating additional test cases to address previously overlooked scenarios * fix: bulkRemoveValidators reduce validator counter * fix: bulkRemoveValidator all tests passed * update gas numbers * update NatSpec comments * add audit report * holesky deployment metadata --------- Co-authored-by: Mohsen-T <[email protected]> Co-authored-by: Mohsen-T <[email protected]>
bulkRegisterValidator
functionregisterValidator
andbulkRegisterValidator