OZ - Native Staking - M-01 All Addresses Are Registered Validators by Default #2081
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
naddison36
requested review from
sparrowDom,
DanielVF and
shahthepro
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## sparrowDom/nativeStaking #2081 +/- ##
============================================================
+ Coverage 62.99% 63.00% +0.01%
============================================================
Files 65 65
Lines 3243 3244 +1
Branches 839 840 +1
============================================================
+ Hits 2043 2044 +1
Misses 1197 1197
Partials 3 3 ☔ View full report in Codecov by Sentry. |
Merged
sparrowDom
approved these changes
May 31, 2024
Added new Holesky deploy script
I've added assertions confirming |
OZ - Native Staking - M-01 All Addresses Are Registered Validators by Default
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
naddison36
added a commit
that referenced
this pull request
Jun 8, 2024
* add fuse interval logic and deploy script * add accounting logic * add manually fixing accounting * implement the collect rewards function and add tests for it * implement and test checkBalance * add functions to register and exit/remove the ssv validator * Implemented depositSSV * Moved MAX_STAKE on ValidatorAccountant to a constant * Removed strategist from strategy as its already maintained in the Vault * Native staking changes (#2024) * Added OETH process diagram with functions calls for native staking * Native Staking Strategy now hold consensus rewards at ETH FeeAccumulator now holds execution rewards as ETH Removed WETH immutable from FeeAccumulator Converted custom errors back to require with string collect rewards now converts ETH to WETH at harvest checkBalance is now validators * 32 plus WETH balance from deposits Renamed beaconChainRewardsWETH to consensusRewards Fixed bug in stakeETH that was converting all WETH to ETH * Native staking changes and unit tests (#2029) * Fixed native staking deployment since the strategist is got from the vault * Refactor of some Native Staking events * Refactor of Native Staking unit tests * Renamed AccountingBeaconChainRewards to AccountingConsensusRewards * Accounting updated to handle zero ETH from the beacon chain * fixed bug not accounting for previous consensus rewards * Blow fuse if ETH balance < previous consensus rewards * Pause collectRewardTokens and doAccounting on accounting failure. Validated asset on deposit to Native Staking Strategy. Moved depositSSV from NativeStakingSSVStrategy to ValidatorRegistrator moved onlyStrategist modified and VAULT_ADDRESS immutable from ValidatorAccountant to ValidatorRegistrator manuallyFixAccounting changed to use whenPaused modifier made fuseIntervalEnd inclusive * allow for WETH to send ETH to the contract * Holesky deploy (#2026) * add basic steps to deploy OETH to holesky * add holesky deployment files * make the fork tests run on Holesky * testing SSV staking on Holesky * add deposit to validator deployment files * SSV cluster info (#2036) * add ability to fetch SSV cluster information * manuallyFixAccounting changes (#2034) * manuallyFixAccounting now uses delta values and only callable by the strategist manuallyFixAccounting calls doAccounting to check the fuse is still not blown Removed accountingGovernor * Added pauseOnFail param to internal _doAccounting Increased the allowed delta values of manuallyFixAccounting * mainnet native staking fork tests (#2037) * manuallyFixAccounting now uses delta values and only callable by the strategist manuallyFixAccounting calls doAccounting to check the fuse is still not blown Removed accountingGovernor * Added pauseOnFail param to internal _doAccounting Increased the allowed delta values of manuallyFixAccounting * ran prettier * Added Defender Relayer for validator registrator Added ssv utils to get cluster data Added native staking fork tests * Removed now redundant IWETH9 import * moved more logic into native staking fixture * Removed unused imports * fix native staking unit tests * Fail accounting if activeDepositedValidators < fullyWithdrawnValidators Changed Harvester to transfer WETH to dripper Added more mainnet fork tests for native staking * Updated the OETH value flows * Added governable Hardhat tasks Created a resolveContract util * deconstruct params for Hardhat tasks * WIP Hardhat tasks for validator registration * Added depositSSV HH task * Updated OETH contract dependency diagram * Update to diagrams * mini fixes * fix bug and minor test improvement * update yarn fulie * unify the holesky and the mainnet fork tests * re-deploy holesky native staking strategy (#2046) * also re-deploy the harvester * upgrade harvester as well * fix upgrade script and correct the bug in deploy actions * Deployed new Native Staking strategy including the proxy * Added Hardhat tasks for generic strategy functions * remove nativeStakingSSVStrategyProxy from js addresses file --------- Co-authored-by: Domen Grabec <grabec@gmail.com> * fix unit tests * fix global hooks filter * add github workflow * another workflow fix * another workflow fix * fix holesky tests * remove the .only * prettier * Sparrow dom/native staking defender action (#2051) * adding defender action task * fix unit tests setup * update the registrator address of the native staking contract * fix up the operate validators script so that it works for native staking on holesky * also add the gitignore * add ability to exit if staking contract is paused * Fix fork tests (#2050) * Fixed resolveAsset * Fixed validator fork tests Added error func sigs to ISSVNetwork * Fixed harvester behaviour tests * Fix global hooks * Fix tooling --------- Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com> * add payable to fee accumulator (#2053) * add payable to fee accumulator * remove zero initializers * some gas savings * fix linter * Fix balancer fork test * Fixed resolveContract for none proxied contracts * simplified woethCcipZapperFixture * Regenerated latest contract diagrams * Resolved Slither divide-before-multiply warning * Native staking deployment to reduceQueueTime * Fixed fork test harvesting CRV rewards * gas optimisation * make verification automatic * removed wethToVault from manuallyFixAccounting * native staking strategy redeployed * adjust the comment * add rate limit to calling manually fix accounting (#2057) * add rate limit to calling manually fix accounting * fix comment * Correct deployName in Native Staking deploy script * Fix linter * Remove approveAssets on Swapper contract in 095_ogn_buyback script as it has already been done * Fix Holesky fork tests * Fix mainnet fork tests * skip deploy 095_ogn_buyback for now * fix some Slither errors * Fix Slither warnings * Upgrade the CI to use node.js 20.x * Fix CI * Fix CI * Upgrade to node.js 20 * Still upgrade the buyback contracts but not approveAssets on the swapper * Upgrade actions so they don't use node.js 16 * attempt to fix holesky fork run * Generated latest contract diagram * fix codecov upload * Fix OETH process diagram for native staking * add a withdrawal event when withdrawing WETH to vault (#2059) * Regenerated latest native staking diagram * add a util contract that is able to recalculate a valid deposit data root with an invalid signature * Prettier native staking test * Renamed native staking deploy script * Fix event values (#2060) * add withdrawal events to the tests * add WETH accounting * add better documentation * add sending WETH to the Vault back to fix manual accounting function * actually wrap the ETH to WETH before sending it to the Vault when manually fixing accounting * add withdrawal event to manually fix accounting * fix bugs and add tests with WETH accounting * add test to verify correct deposits * Gas changes to Native Staking's depositAll --------- Co-authored-by: Nicholas Addison <nick@addisonbrown.com.au> * Deploy latest Native Staking Strategy to Holesky (#2073) * Deployed new NativeStakingSSVStrategy * Generated latest Native Staking Strategy diagrams * Deploy native staking Proxy via Relayer (#2066) * deploy native staking proxy and add auxiliary functions to transfer its governance * Shortened error message in InitializeGovernedUpgradeabilityProxy.initialize --------- Co-authored-by: Nicholas Addison <nick@addisonbrown.com.au> * Setup basic defender action (#2072) * deploy native staking proxy and add auxiliary functions to transfer its governance * Setup basic defender action * update defender-sdk version * add defender client as an external export * configuration update --------- Co-authored-by: Nicholas Addison <nick@addisonbrown.com.au> * Output more contract details in 097 deploy script * Fix Native Staking fork tests * Changed ssv util getClusterInfo to use the SSV API instead of the ssv-scanner Moved duplicate getClusterInfo in tasks to utils * Prettier * ValidatorRegistrator.stakeEth gas improvement when multiple validators * Corrected stakeEth call in OETH processes diagram * Added fork test for registering a validator twice * Stake funds with confirmations for front-running protection of Beacon Deposits (#2074) * contract changes and tests for gated protection against front running * Update off-chain validator registration process to also consider stake threshold * front run protection changes (#2076) * Added Holesky deploy script * Fixed OUSD metapool fork test * Generated latest NativeStakingSSVStrategy contract diagrams * Deployed new NativeStakingSSVStrategy to Holesky --------- Co-authored-by: Nicholas Addison <nick@addisonbrown.com.au> * Updated Buyback value flow for OETH * Updated Native staking process diagram * Updated OETH Vault diagrams * Add process for pausing the Native Staking Strategy * Removed the redundant condition in the fuse interval check (#2082) * OZ - Native Staking - M-01 All Addresses Are Registered Validators by Default (#2081) * Added default NON_REGISTERED to VALIDATOR_STATE * Added explicit check that a validator has not already been registered * Added new Holesky deploy script * OZ - Native Staking - N-07 Lack of Indexed Event Parameters (#2083) * Indexed the RegistratorChanged and StakingMonitorChanged events * Added indexed pubKeyHash to validator events * Deployed new Native Staking Strategy * generated latest NativeStakingSSVStrategy docs * P2P API changes (#2084) * Updated Natspec * Updated native staking process diagrams * Fixed signer for Holesky * split operateValidators into registerValidators and stakeValidators * Fix vault mint HH task. It now waits for the approve tx to be mined * resolveAsset to handle local Holesky fork * upgrade signer to use new defender-relay-client package * added approve option to vault mint HH task * Added stakeValidators HH task * update git ignore file * Added depositWETH and withdrawWETH HH tasks * Added resetStakeETHTally and setStakeETHThreshold HH tasks * generated latest Native Staking Strategy docs * Hardhat tasks for Native Staking (#2085) * Fixed depositSSV HH tasks * Added exitValidator and removeValidator HH tasks * Added doAccounting Defender Action * Added harvest HH task * Added fixAccounting and pauseStaking HH tasks * Fixed stakeValidators when run without uuid option * Cap the validators a Native Staking Strategy can hold (#2087) * Added max validators check to Native Staking Strategy * don't format Defender Action code in dist folder * Deployed latest NativeStakingSSVStrategy contract to Holesky * Capitalised error messages in Native Staking Strategy * Fix hot deploy of Native Staking Strategy * moved when event it emitted * remove unchecked when calculating fullyWithdrawnValidators * Renamed MAX_STAKE to FULL_STAKE * Renamed withdrawal_credentials to withdrawalCredentials * Renamed WETH_TOKEN_ADDRESS to WETH * renamed SSV_TOKEN_ADDRESS to SSV_TOKEN renamed SSV_NETWORK_ADDRESS to SSV_NETWORK * replaced 32 ether with FULL_STAKE * moved the internal function to the bottom of NativeStakingSSVStrategy * fixed formatting of if statements * Moved emits to after state changes * Removed validatorsLength variable as we aren't dealing with a storage array * removed unchecked iteration in for loop * consistent check of currentState * Added new Holesky deploy script * Removed Governable from FeeAccumulator --------- Co-authored-by: Nick Addison <nick@addisonbrown.com.au> Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contract Changes
NON_REGISTEREDas the default value in theVALIDATOR_STATEenum.Issue
In Solidity, an enum variable is automatically set to its first enumerated value if it is initialized without specifying a particular value. In
ValidatorRegistrator.sol,REGISTEREDis the default value of theVALIDATOR_STATEenum, meaning that it is possible to stake with any address even ifregisterSsvValidatorwere not called. Since the address would not beregistered as a validator in the SSV Network, it would not perform validations correctly which could result in funds being lost due to slashing.