Skip to content
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

SSV Native staking #2015

Open
wants to merge 97 commits into
base: master
Choose a base branch
from
Open

SSV Native staking #2015

wants to merge 97 commits into from

Conversation

sparrowDom
Copy link
Member

@sparrowDom sparrowDom commented Apr 4, 2024

Contracts

Pull Requests to this branch

Changes

  • Added new NativeStakingSSVStrategy that takes WETH from the Vault and stakes ETH into SSV validators.
  • Added FeeAccumulator to receive ETH execution rewards.
  • Changes BaseHarvester so it can collect WETH and send it straight to the Dripper

oethContracts

Diagrams

NativeStakingSSVStrategy

NativeStakingSSVStrategyHierarchy

NativeStakingSSVStrategySquashed

FeeAccumulator

FeeAccumulatorHierarchy

FeeAccumulatorSquashed

Processes

Value Flows

The flow of native currency (ETH) and tokens between the wallets and contracts.

oethValueFlows-native-staking

Contract calls

oethProcesses

Testing

Unit Tests

Unit tests are in contracts/test/strategies/nativeSSVStaking.js

yarn run test

Holesky fork tests

Holesky fork tests are in contracts/test/strategies/nativeSsvStaking.holesky.fork-test.js

yarn run node:hol
yarn run test:hol-fork

Mainnet fork tests

Mainnet fork tests are in contracts/test/strategies/nativeSsvStaking.fork-test.js

yarn run node
yarn run test:fork

Deployment

The deploy script for the Native Staking Strategy is contracts/deploy/mainnet/097_native_ssv_staking.js.

Operations

Hardhat tasks

@sparrowDom sparrowDom self-assigned this Apr 4, 2024
Copy link

github-actions bot commented Apr 4, 2024

Warnings
⚠️ 👀 This PR needs at least 2 reviewers

Generated by 🚫 dangerJS against 8748170

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

Attention: Patch coverage is 96.44444% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 62.99%. Comparing base (e6396db) to head (8748170).

Files Patch % Lines
contracts/contracts/oracle/OETHFixedOracle.sol 0.00% 4 Missing ⚠️
.../strategies/NativeStaking/ValidatorRegistrator.sol 95.65% 3 Missing ⚠️
...ategies/NativeStaking/NativeStakingSSVStrategy.sol 98.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2015      +/-   ##
==========================================
+ Coverage   60.54%   62.99%   +2.45%     
==========================================
  Files          59       65       +6     
  Lines        3021     3243     +222     
  Branches      590      839     +249     
==========================================
+ Hits         1829     2043     +214     
- Misses       1189     1197       +8     
  Partials        3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

EXIT_COMPLETE // validator has funds withdrawn to the EigenPod and is removed from the SSV
}

event RegistratorAddressChanged(address oldAddress, address newAddress);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this has been a pattern used for a while in Origin dollar, but do you need both the old and new addresses in change events? Can we just have the new address?


balance = activeDepositedValidators * 32 ether;
balance += beaconChainRewardWETH;
balance += FEE_ACCUMULATOR_ADDRESS.balance;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to include the MEV rewards in the FeeAccumulator if those are going to be harvested and sent to through the dripper.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks this is a nice catch. I see I haven't given it enough though as to how (W)ETH should be handled.
Lets continue discussion here so we have it all in 1 place: https://www.notion.so/originprotocol/How-should-accounting-function-handle-ETH-after-accounting-4ed3a9352c0f4fc8aa6bb866022a25cb

/// @dev Governor that can manually correct the accounting
address public accountingGovernor;
/// @dev Strategist that can pause the accounting
address public strategist;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best to read the strategist from the Vault rather than duplicating the maintenance in the strategy. This is what OETH AMO strategy does

    modifier onlyStrategist() {
        require(
            msg.sender == IVault(vaultAddress).strategistAddr(),
            "Caller is not the Strategist"
        );
        _;
    }

naddison36 and others added 10 commits April 22, 2024 17:35
* Update Natspec

* Generated docs for native eth strategy

* Prettier and linter
Fixed spelling of ValidatorAccountant events
Implemented depositSSV

* Updated Natspec
Moved MAX_STAKE on ValidatorAccountant to a constant

* Removed strategist from strategy as its already maintained in the Vault

* Fix compilation error

* Fix unit tests

* fix linter
* 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
* 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

Natspec updates

refactoring of native staking unit tests
* add basic steps to deploy OETH to holesky

* prettier

* minor change

* holesky deployment ifles
holesky deployment files

* add holesky deployment files

* minor fix

* minor fixes

* make the fork tests run on Holesky

* add some more tests

* testing SSV staking on Holesky

* refactor where deployment files are located

* more progress on deployment

* add deposit to validator deployment files

* remove log

* prettier

* lint

* move file

* SSV cluster info (#2036)

* add ability to fetch SSV cluster information

* prettier
* 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
@naddison36 naddison36 mentioned this pull request May 16, 2024
5 tasks
sparrowDom and others added 21 commits May 16, 2024 17:31
* 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>
* Deployed new NativeStakingSSVStrategy

* Generated latest Native Staking Strategy diagrams
* 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>
* 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>
…-scanner

Moved duplicate getClusterInfo in tasks to utils
… 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>
@naddison36 naddison36 mentioned this pull request May 30, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Works related to contracts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants