SSV Native staking

[sparrowDom]

trafficstars

Contracts

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

Diagrams

NativeStakingSSVStrategy

FeeAccumulator

Processes

Value Flows

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

Contract calls

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

Operations

Hardhat tasks

[github-actions[bot]]

	Warnings
:warning:	:eyes: This PR needs at least 2 reviewers

Generated by :no_entry_sign: dangerJS against af77be4bcb56a585e4053316edbebdcea053801f

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 75.32895% with 75 lines in your changes missing coverage. Please review.

Project coverage is 61.86%. Comparing base (a52ee8e) to head (af77be4).

Files	Patch %	Lines
...ts/contracts/strategies/LidoWithdrawalStrategy.sol	30.43%	48 Missing :warning:
.../strategies/NativeStaking/ValidatorRegistrator.sol	75.32%	19 Missing :warning:
contracts/contracts/oracle/OETHFixedOracle.sol	0.00%	4 Missing :warning:
contracts/contracts/harvest/BaseHarvester.sol	57.14%	3 Missing :warning:
...ategies/NativeStaking/NativeStakingSSVStrategy.sol	98.36%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2015      +/-   ##
==========================================
+ Coverage   60.54%   61.86%   +1.31%     
==========================================
  Files          59       66       +7     
  Lines        3021     3322     +301     
  Branches      779      649     -130     
==========================================
+ Hits         1829     2055     +226     
- Misses       1189     1264      +75     
  Partials        3        3              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[openzeppelin-code[bot]]

SSV Native staking

Generated at commit: 125cad8b3ee08afd65bc5ca95e891e33e46e892e

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	3 3 0 18 42 66
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

---

For more details view the full report in OpenZeppelin Code Inspector

[notion-workspace[bot]]

OETH Native ETH Strategy

[DanielVF]

Posting my mostly complete review

Code Review: Native Staking

Requirements

OETH should be able to deposit and withdraw ETH from SSV controlled native staking as a yield earning strategy. It is planned that this will be the primary yield for OETH and that OETH will become an LST.

This is a difficult strategy to build because validators operate cross chain, and the execution chain only has very poor information about the actual status of validators. What information can be gathered far to gas intensive to be effective.

Because Ethereum has horrible support for LSTs, there are essentially two approaches to building an LST. The first is YOLO trust of an address that updates the execution layer about what is going on the validator layer. The second is building a massive offchain oracle infrastructure and operating it.

We've chosen a third route:

1. If everything goes smoothly, the accounting operates off onchain data.
2. A limited-trust address interacts with P2P/SSV for validator provisioning. The amount of damage this address can do is limited to a maximum amount.
3. In the event of things getting out of sync enough to cause accounting errors, the strategist can make limited corrections at a certain rate over time.
4. In the event of things being very out of sync, governance can act to correct.

The overall concept is contract enforced damage mitigation.

We want to transition to fully onchain accounting once that is possible from future Ethereum updates.

Authentication

• [x] Never use tx.origin
• [x] Every external/public function is supposed to be externally accessible
• [x] Every external/public function has the correct authentication

Ethereum

_Unlike almost all of contracts, we do send and receive ETH from this contract. However, we only send ETH to two trusted addresss.

🟠 There are three methods to prevent reentrancy: only calling trusted contracts, reentrancy guards, and writing functions in order such that reentrancy is not an issue. Usually we try to hit all three. This code only does the first.

• [ ] Contract does not send or receive Ethereum.
• [ ] Contract has no payable methods.
• [x] Contract is not vulnerable to being sent self destruct ETH

Cryptographic code

The only crypto used is hashing the validator public keys in order to look them up in a mapping. I don't think that can be abused to get different public keys to map to the same hash.

Otherwise the contract passes through signature data, but does not use it itself.

Gas problems

• [x] Contracts with for loops must have either:
  • [ ] A way to remove items
  • [ ] Can be upgraded to get unstuck
  • [x] Size can only controlled by admins
• [x] Contracts with for loops must not allow end users to add unlimited items to a loop that is used by others or admins.

Black magic

• [x] Does not contain selfdestruct
• [x] Does not use delegatecall outside of proxying.
• [x] Address.isContract should be treated as if could return anything at any time, because that's reality.

Overflow

• [x] Code is solidity version >= 0.8.0
• [x] All for loops use uint256

Proxy

• [x] No storage variable initialized at definition when contract used as a proxy implementation.

Events

🟣 Deposit SSV does not emit directly, but that's okay. We could monitor the SSV contract for this if we wished.

Rounding

• [x] Contract rounds in the protocols favor
• [x] Contract does not have bugs from loosing rounding precision
• [x] Code correctly multiplies before division
• [ ] Contract does not have bugs from zero or near zero amounts

Dependencies

• [x] Review any new contract dependencies thoroughly (e.g. OpenZeppelin imports) when new dependencies are added or version of dependencies changes.
• [x] If OpenZeppelin ACL roles are use review & enumerate all of them.
• [x] Check OpenZeppelin security vulnerabilities and see if any apply to current PR considering the version of OpenZeppelin contract used.

External calls

• [x] Contract addresses passed in are validated
  • [x] 🔴 No validation in withdraw (made inline note, fixed).
• [x] No unsafe external calls
• [ ] Reentrancy guards on all state changing functions
  • [ ] Still doesn't protect against external contracts changing the state of the world if they are called.
• [x] No malicious behaviors
• [x] Low level call() must require success.
• [x] No slippage attacks (we need to validate expected tokens received)
• [x] Oracles, one of:
  • [x] It's complicated
  • [x] No oracles
  • [ ] Oracles can't be bent
  • [ ] If oracle can be bent, it won't hurt us.
• [x] Do not call balanceOf for external contracts to determine what they will do when they use internal accounting

Tests

not checked yet

• [ ] Each publicly callable method has a test
• [ ] Each logical branch has a test
• [ ] Each require() has a test
• [ ] Edge conditions are tested
• [ ] If tests interact with AMM make sure enough edge cases (pool tilts) are tested. Ideally with fuzzing.

Deploy

• [x] Deployer permissions are removed after deploy

Strategy Specific

Strategy checks

• [x] Check balance cannot be manipulated up AND down by an attacker
• [x] No read only reentrancy on downstream protocols during checkBalance
• [x] All reward tokens are collected
• [x] The harvester can sell all reward tokens
• [x] No funds are left in the contract that should not be as a result of depositing or withdrawing
• [x] All funds can be recovered from the strategy by some combination of depositAll, withdraw, or withdrawAll()
  • [x] Way more complicated, but we can get funds out.
• [x] WithdrawAll() can always withdraw an amount equal to or larger than checkBalances report, even in spite of attacker manipulation.
• [x] WithdrawAll() cannot be MEV'd
• [ ] Strategist cannot steal funds
  • [x] Limited amounts though

Accounting Invariants

• [x] depWeth should go up whenever WETH is added via the vault
• [x] depWeth should go down whenever WETH is staked
• [x] depWeth should go down whenever WETH is manually withdrawn to the vault
• [ ] depWETH should never be greater than the standing WETH balance (not checked yet)
• [x] depWETH should never be less than the standing WETH balance unless someone donates
• [x] ETH only arrives from off chain + mev rewards (outside donations)
• [x] Sitting ETH is only ever sent to WETH
• [x] All funds sent upwards to the vault and harvester are done in WETH
• [x] ActVal * 32 should always be less than the sum of WETH sent to the contract
  • [x] not counting manual adjustments
• [x] ETH balance at rest should always be greater than or equal to consensus rewards
  • [x] manual adjustments can set this below, but it will revert at the end of the manual set when it calls do accounting
• [x] After a successful doAccounting, eth balance should equal consensus rewards
  • [x] unless accounting fails and the contract is paused

What happens if:

• We exit all validators? - should be fairly sane. No more rewards than we would normally get.
• Lots of validators get slashed? - accounting will undercount how many validators have withdrawn. We must pause() the strategy accounting when this happens on the beacon chain, before the slashed validators eventually get withdrawn.
• All validators go down for a week - accounting thinks we have an extra validator, and would need manual adjustment.
• We miss a rewards beat - we blow the fuse and stop updating.
• gas prices spike for a week - no real effect
• Validator max sizes go up, the number of global active validators goes down by 20x, resulting in a 20x withdraw cycle speedup - okay if we call do accounting 20x more often. Otherwise we need to migrate to larger stakes and/or switch to using the better accounting which is probably gas possible now.

Logic

Are there bugs in the logic?

Still not specifically reviewed yet

• [ ] Correct usage of global & local variables. -> they might differentiate only by an underscore that can be overlooked (e.g. address vs _address).

Deployment Considerations

• [ ] SSV strategy proxy must be deployed by the relayer account
• [ ] Before non testing amounts of funds are in, we must have monitoring for slashings and missed validations

Internal State

See above in "Accounting Invariants" section

Attack

This code is inherently dangerous. We focus on damage mitigation rather than the usual perfection.

First, there is no way for a smart contract to ensure that a staking deposit will end up with correct withdrawal address in the beacon chain. We are essentially shipping out gold bars via packing labels created on an EOA.

To mitigate this risk, we limit how many deposits can be made before requiring a 5 of 8 multisig to verify that the previous deposits were correctly made. This caps our max losses at about two million per strategy contract.

We could reduce the deposit risk by a magnitude and remove all humans from the loop by doing the following:

• Deposit 1 ETH into a validator, increment, mark validator as pending
• When this validator is active, post back to the contract with a proof that the validator withdraw address is recorded correctly. This deposit 31 ETH and marks the validator as registered.

By keeping a limited number of deposits in the pending state, we reduce our risk. 1 ETH instead of 32 ETH to test out a validator deposit is also a fairly big win.

Secondly, it would take approximately 50% of all yield to be spent in gas to be mostly sure about what each validator is doin on the beacon chain. Instead we take a guess based on the amount of funds we see coming in. This can be wrong and need manual correction. We also throttle the amount of manual correction to avoid malicious actions.

Outside the scope of this contract, failures or vulnerabilities with SSV or P2P could result in mass slashings of our validators.

Withdrawing funds is also ultimately outside the contract's control. We can ask the SSV contract to emit an event which hopefully is picked up. There are other off-chain ways to kick off withdraws.

SSV rewards are also outside our contracts control, since they go only to the EOA that deployed the contracts.

ETH rewards are not distinguished from withdraws, both go to the same place, via the same mechanism. We use heuristics to discriminate between them.

Our strategy contracts have four jobs:

1. Deposit money
2. Withdraw money
3. Collect yield + rewards
4. Know the balance

Four out of four of these cannot be guaranteed by the code.

Monitoring is critical here.

Other code:

We might be in trouble if the SSV deposit made callbacks to somewhere that an attacker could hook into. A quick glance at the SSV code shows that not only is it dynamic, but extremely dynamic in its dynamicness.

🟠 Let's add nonReentrant to stakeETH, doAccounting and manuallyFixAccounting so something weird happening with SSV can't mess anything up on our side.

Flavor

The strategy is broken into three different files in an inheritance hierarchy. In general I'm not a fan of this, as it makes it more difficult to see the big picture, but the cutpoints here are very good, and each has clear ownership of difference concerns, and small interaction with other layers. Works here.

Otherwise, the flavor is now good.