responded to rest of Jeff Commons comments

docs/EigenPods.md

@@ -3,7 +3,7 @@
 
 ## Overview
 
-This document explains *EigenPods*, the mechanism by which EigenLayer facilitates the restaking of native beacon chain ether.  The EigenPods subprotocol allows entities that own validators that are apart of Ethereum's consensus to repoint their withdrawal credentials (explained later) to contracts in the EigenPods subprotocol that have certain mechanisms to ensure safe restaking.
+This document explains *EigenPods*, the mechanism by which EigenLayer facilitates the restaking of native beacon chain ether.  The EigenPods subprotocol allows entities that own validators that are a part of Ethereum's consensus to repoint their withdrawal credentials (explained later) to contracts in the EigenPods subprotocol.
 
 It is important to contrast this with the restaking of liquid staking derivatives (LSDs) on EigenLayer. EigenLayer will integrate with liquid staking protocols "above the hood", meaning that withdrawal credentials will be pointed to EigenLayer at the smart contract layer rather than the consensus layer. This is because liquid staking protocols need their contracts to be in possession of the withdrawal credentials in order to not have platform risk on EigenLayer. As always, this means that value of liquid staking derivatives carries a discount due to additional smart contract risk.
 
@@ -34,15 +34,15 @@ The following sections are all related to managing Consensus Layer (CL) and Exec
 When EigenPod contracts are initially deployed, the "restaking" functionality is turned off - the withdrawal credential proof has not been initiated yet.  In this "non-restaking" mode, the contract may be used by its owner freely to withdraw validator balances from the beacon chain via the `withdrawBeforeRestaking` function. This function routes the withdrawn balance directly to the `DelayedWithdrawalRouter` contract.  Once the EigenPod's owner verifies that their withdrawal credentials are pointed to the EigenPod via `verifyWithdrawalCredentialsAndBalance`, the `hasRestaked` flag will be set to true and any withdrawals must now be proven for via the `verifyAndProcessWithdrawal` function.  
 
 ### Merkle Proof of Correctly Pointed Withdrawal Credentials
-After staking an Ethereum validator with its withdrawal credentials pointed to their EigenPod, a staker must show that the new validator exists and has its withdrawal credentials pointed to the EigenPod, by proving it against a beacon state root with a call to `verifyWithdrawalCredentialsAndBalance`. The EigenPod will verify the proof (along with checking for replays and other conditions) and, if the ETH validator's effective balance is proven to be greater than or equal to `REQUIRED_BALANCE_GWEI`, then the EigenPod will pass the validator's effective balance value through its own hysteresis calculation (see [here](#hysteresis)), which effectively underestimates the effective balance of the validator by 1 ETH.  Then a call is made to the EigenPodManager to forward a call to the StrategyManager, crediting the staker with those shares of the virtual beacon chain ETH strategy.
+After staking an Ethereum validator with its withdrawal credentials pointed to their EigenPod, a staker must show that the new validator exists and has its withdrawal credentials pointed to the EigenPod, by proving it against a beacon state root with a call to `verifyWithdrawalCredentialsAndBalance`. The EigenPod will verify the proof (along with checking for replays and other conditions) and, if the ETH validator's effective balance is proven to be greater than or equal to `MAX_VALIDATOR_BALANCE_GWEI`, then the EigenPod will pass the validator's effective balance value through its own hysteresis calculation (see [here](#hysteresis)), which effectively underestimates the effective balance of the validator by 1 ETH.  Then a call is made to the EigenPodManager to forward a call to the StrategyManager, crediting the staker with those shares of the virtual beacon chain ETH strategy.
 
 ### Effective Restaked Balance - Hysteresis {#hysteresis}
-To convey to EigenLayer that an EigenPod has validator's restaked on it, anyone can submit a proof against a beacon chain state root the proves that a validator has their withdrawal credentials pointed to the pod.  The proof is verified and the EigenPod calls the EigenPodMananger that calls the StrategyManager which records the validators proven balance run through the hysteresis function worth of ETH in the "beaconChainETH" strategy.  Each EigenPod keeps track of all of the validators by the hash of their public key. For each validator, their validator index and current balance in EigenLayer is kept track of.
+To convey to EigenLayer that an EigenPod has validator(s) restaked on it, anyone can submit a proof against a beacon chain state root the proves that a validator has their withdrawal credentials pointed to the pod.  The proof is verified and the EigenPod calls the EigenPodMananger that calls the StrategyManager which records the validators proven balance run through the hysteresis function worth of ETH in the "beaconChainETH" strategy.  Each EigenPod keeps track of all of the validators by the hash of their public key. For each validator, their validator index and current balance in EigenLayer is kept track of.
 
 ### Proofs of Validator Balance Updates
 EigenLayer pessimistically assumes the validator has less ETH that they actually have restaked in order for the protocol to have an accurate view of the validator's restaked assets even in the case of an uncorrelated slashing event, for which the penalty is >=1 ETH.
 In the case that a validator's balance drops close to or below what is noted in EigenLayer, AVSs need to be notified of that ASAP, in order to get an accurate view of their security. 
-In the case that a validator's balance, when run through the hysteresis function, is lower or higher than what is restaked on EigenLayer, anyone is allowed to permissionlessly prove that the balance of a certain validator. If the proof is valid, the StrategyManager decrements the pod owners beacon chain ETH shares by however much is staked on EigenLayer and adds the new proven stake, i.e., the strategyManager's view of the staker's shares is an accurate representation of the consensys layer as long as timely balance update proofs are submitted.  
+In the case that a validator's balance, when run through the hysteresis function, is lower or higher than what is restaked on EigenLayer, anyone is allowed to permissionlessly prove the new balance of the validator, triggering an update in EigenLayer. If the proof is valid, the StrategyManager decrements the pod owners beacon chain ETH shares by however much is staked on EigenLayer and adds the new proven stake, i.e., the strategyManager's view of the staker's shares is an accurate representation of the consensus layer as long as timely balance update proofs are submitted.  
 
 ### Proofs of Full/Partial Withdrawals
 
@@ -54,13 +54,9 @@ We also must prove the `executionPayload.blockNumber > mostRecentWithdrawalBlock
 
 In this second case, in order to withdraw their balance from the EigenPod, stakers must provide a valid proof of their full withdrawal against a beacon chain state root. Full withdrawals are differentiated from partial withdrawals by checking against the validator in question's 'withdrawable epoch'; if the validator's withdrawable epoch is less than or equal to the slot's epoch, then the validator has fully withdrawn because a full withdrawal is only processable at or after the withdrawable epoch has passed. Once the full withdrawal is successfully verified, there are 2 cases, each handled slightly differently:
 
-1. If the withdrawn amount is greater than `REQUIRED_BALANCE_GWEI`, then `REQUIRED_BALANCE_WEI` is held for processing through EigenLayer's normal withdrawal path, while the excess amount above `REQUIRED_BALANCE_GWEI` is marked as instantly withdrawable.
+1. If the withdrawn amount is greater than `MAX_VALIDATOR_BALANCE_GWEI_GWEI`, then `MAX_VALIDATOR_BALANCE_GWEI` is held for processing through EigenLayer's normal withdrawal path, while the excess amount above `MAX_VALIDATOR_BALANCE_GWEI` is marked as instantly withdrawable.
 
-2. If the withdrawn amount is greater than `REQUIRED_BALANCE_GWEI` and the validator *was* previously proven to be "overcommitted", then `REQUIRED_BALANCE_WEI` is held for processing through EigenLayer's normal withdrawal path, while the excess amount above `REQUIRED_BALANCE_GWEI` is marked as instantly withdrawable, identical to (1) above. Additionally, the podOwner's beaconChainShares in EigenLayer are increased by `REQUIRED_BALANCE_WEI` to counter-balance the decrease that occurred during the [overcommittment fraudproof process](#fraud-proofs-for-overcommitted-balances).
-
-3. If the amount withdrawn is less than `REQUIRED_BALANCE_GWEI` and the validator was *not* previously proven to be "overcommitted", then the full withdrawal amount is held for processing through EigenLayer's normal withdrawal path, and any excess 'beaconChainETH' shares in EigenLayer are immediately removed, somewhat similar to the process outlined in [fraud proofs for overcommitted balances]. 
-
-4. If the amount withdrawn is less than `REQUIRED_BALANCE_GWEI` and the validator *was* previously proven to be "overcommitted", then the full withdrawal amount is held for processing through EigenLayer's normal withdrawal path, and the podOwner is credited with enough beaconChainETH shares in EigenLayer to complete the normal withdrawal process; this last step is necessary since the validator's virtual beaconChainETH shares were previously removed from EigenLayer as part of the overcommittment fraudproof process.
+2. If the withdrawn amount is less than `MAX_VALIDATOR_BALANCE_GWEI`, then the amount being withdrawn is held for processing through EigenLayer's normal withdrawal path.
 
 ### The EigenPod Invariant
 The core complexity of the EigenPods system is to ensure that EigenLayer continuously has an accurate picture of the state of the beacon chain balances repointed to it.  In other words, the invariant that governs this system is:

src/contracts/core/StrategyManagerStorage.sol

@@ -61,6 +61,8 @@ abstract contract StrategyManagerStorage is IStrategyManager {
     /// @notice Mapping: strategy => whether or not stakers are allowed to deposit into it
     mapping(IStrategy => bool) public strategyIsWhitelistedForDeposit;
 
+    uint256 internal _deprecatedStorage;
+
     IStrategy public constant beaconChainETHStrategy = IStrategy(0xbeaC0eeEeeeeEEeEeEEEEeeEEeEeeeEeeEEBEaC0);
 
     constructor(IDelegationManager _delegation, IEigenPodManager _eigenPodManager, ISlasher _slasher) {

src/contracts/interfaces/IEigenPod.sol

@@ -90,20 +90,20 @@ interface IEigenPod {
     function validatorPubkeyHashToInfo(bytes32 validatorPubkeyHash) external view returns (ValidatorInfo memory);
 
 
-    ///@notice mapping that tracks proven partial withdrawals
+    ///@notice mapping that tracks proven withdrawals
     function provenWithdrawal(bytes32 validatorPubkeyHash, uint64 slot) external view returns (bool);
 
-    /// @notice this is a mapping of validator indices to a Validator struct containing pertinent info about the validator
+    /// @notice This returns the status of a given validator
     function validatorStatus(bytes32 pubkeyHash) external view returns(VALIDATOR_STATUS);
 
 
     /**
-     * @notice This function verifies that the withdrawal credentials of the podOwner are pointed to
-     * this contract. It also verifies the current (not effective) balance  of the validator.  It verifies the provided proof of the ETH validator against the beacon chain state
+     * @notice This function verifies that the withdrawal credentials of validator(s) owned by the podOwner are pointed to
+     * this contract. It also verifies the effective balance  of the validator.  It verifies the provided proof of the ETH validator against the beacon chain state
      * root, marks the validator as 'active' in EigenLayer, and credits the restaked ETH in Eigenlayer.
      * @param oracleBlockNumber is the Beacon Chain blockNumber whose state root the `proof` will be proven against.
-     * @param validatorIndices is the list of indices of the validator being proven, refer to consensus specs 
-     * @param proofs is an array of proofs, where each proof proves the ETH validator's balance and withdrawal credentials against a beacon chain state root
+     * @param validatorIndices is the list of indices of the validators being proven, refer to consensus specs 
+     * @param proofs is an array of proofs, where each proof proves each ETH validator's balance and withdrawal credentials against a beacon chain state root
      * @param validatorFields are the fields of the "Validator Container", refer to consensus specs 
      * for details: https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#validator
      */
@@ -144,6 +144,7 @@ interface IEigenPod {
      * @param validatorFields are the fields of the validator being proven
      * @param beaconChainETHStrategyIndex is the index of the beaconChainETHStrategy for the pod owner for the callback to 
      *        the EigenPodManager to the StrategyManager in case it must be removed from the podOwner's list of strategies
+     * @param oracleTimestamp is the timestamp of the oracle slot that the withdrawal is being proven against
      */
     function verifyAndProcessWithdrawals(
         BeaconChainProofs.WithdrawalProofs[] calldata withdrawalProofs, 
@@ -165,6 +166,6 @@ interface IEigenPod {
     function decrementWithdrawableRestakedExecutionLayerGwei(uint256 amountWei) external;
 
     /// @notice called by the eigenPodManager to increment the withdrawableRestakedExecutionLayerGwei 
-    /// in the pod, to reflect a completetion of a queued withdrawal
+    /// in the pod, to reflect a completion of a queued withdrawal as shares
     function incrementWithdrawableRestakedExecutionLayerGwei(uint256 amountWei) external;
 }

src/contracts/interfaces/IEigenPodManager.sol

@@ -37,8 +37,8 @@ interface IEigenPodManager is IPausable {
     function restakeBeaconChainETH(address podOwner, uint256 amount) external;
 
     /**
-     * @notice Removes beacon chain ETH from EigenLayer on behalf of the owner of an EigenPod, when the
-     * @param podOwner is the pod owner to be slashed
+     * @notice Records an update in beacon chain strategy shares in the strategy manager
+     * @param podOwner is the pod owner whose shares are to be updated,
      * @param beaconChainETHStrategyIndex is the index of the beaconChainETHStrategy in case it must be removed,
      * @param sharesDelta is the change in podOwner's beaconChainETHStrategy shares
      * @dev Callable only by the podOwner's EigenPod contract.
@@ -61,8 +61,10 @@ interface IEigenPodManager is IPausable {
      */
     function updateBeaconChainOracle(IBeaconChainOracle newBeaconChainOracle) external;
 
+    /// @notice decrements the proven amount of withdrawable ETH to reflect decrementation of shares
     function decrementWithdrawableRestakedExecutionLayerGwei(address podOwner, uint256 amountWei) external;
 
+    /// @notice increments the proven amount of withdrawable ETH to reflect incrementation of shares when a podOwner completes a withdrawal as shares
     function incrementWithdrawableRestakedExecutionLayerGwei(address podOwner, uint256 amountWei) external;
 
     /// @notice Returns the address of the `podOwner`'s EigenPod if it has been deployed.

src/contracts/interfaces/IStrategyManager.sol

@@ -59,11 +59,11 @@ interface IStrategyManager {
     function depositBeaconChainETH(address staker, uint256 amount) external;
 
     /**
-     * @notice Records an overcommitment event on behalf of a staker. The staker's beaconChainETH shares are decremented by `amount`.
-     * @param podOwner is the pod owner to be slashed
+     * @notice Records an update in beacon chain strategy shares in the strategy manager
+     * @param podOwner is the pod owner whose shares are to be updated,
      * @param beaconChainETHStrategyIndex is the index of the beaconChainETHStrategy in case it must be removed,
      * @param sharesDelta is the change in podOwner's beaconChainETHStrategy shares
-     * @dev Only callable by EigenPodManager.
+     * @dev Callable only by the podOwner's EigenPod contract.
      */
     function recordBeaconChainETHBalanceUpdate(address podOwner, uint256 beaconChainETHStrategyIndex, int256 sharesDelta)
         external;

src/contracts/libraries/BeaconChainProofs.sol

@@ -103,7 +103,7 @@ library BeaconChainProofs {
     bytes8 internal constant UINT64_MASK = 0xffffffffffffffff;
 
 
-
+    /// @notice This struct contains the merkle proofs and leaves needed to verify a partial/full withdrawal
     struct WithdrawalProofs {
         bytes32 beaconStateRoot;
         bytes latestBlockHeaderProof;
@@ -121,6 +121,7 @@ library BeaconChainProofs {
         bytes32 executionPayloadRoot;
     }
 
+    /// @notice This struct contains the merkle proofs and leaves needed to verify a balance update
     struct BalanceUpdateProofs {
         bytes32 beaconStateRoot;
         bytes latestBlockHeaderProof;
@@ -131,7 +132,7 @@ library BeaconChainProofs {
         bytes32 slotRoot;
     }
 
-    //struct ValidatorFieldsProof {
+    // @notice This struct contains the merkle proofs and leaves needed to verify a validator's withdrawal credential
     struct WithdrawalCredentialProofs {
         bytes32 beaconStateRoot;
         bytes latestBlockHeaderProof;
@@ -162,10 +163,10 @@ library BeaconChainProofs {
      * @param validatorFields the claimed fields of the validator
      */
     function verifyValidatorFields(
-        uint40 validatorIndex,
         bytes32 beaconStateRoot,
-        bytes calldata proof, 
-        bytes32[] calldata validatorFields
+        bytes32[] calldata validatorFields,
+        bytes calldata proof,
+        uint40 validatorIndex
     ) internal view {
 
         require(validatorFields.length == 2**VALIDATOR_FIELD_TREE_HEIGHT, "BeaconChainProofs.verifyValidatorFields: Validator fields has incorrect length");
@@ -191,10 +192,10 @@ library BeaconChainProofs {
      * @param balanceRoot is the serialized balance used to prove the balance of the validator (refer to `getBalanceFromBalanceRoot` above for detailed explanation)
      */
     function verifyValidatorBalance(
-        uint40 validatorIndex,
         bytes32 beaconStateRoot,
+        bytes32 balanceRoot,
         bytes calldata proof,
-        bytes32 balanceRoot
+        uint40 validatorIndex
     ) internal view {
         require(proof.length == 32 * ((BALANCE_TREE_HEIGHT + 1) + BEACON_STATE_FIELD_TREE_HEIGHT), "BeaconChainProofs.verifyValidatorBalance: Proof has incorrect length");
 
@@ -208,6 +209,13 @@ library BeaconChainProofs {
         require(Merkle.verifyInclusionSha256(proof, beaconStateRoot, balanceRoot, balanceIndex), "BeaconChainProofs.verifyValidatorBalance: Invalid merkle proof");
     }
 
+    /**
+     * @notice This function verifies the slot against the state root. the slot is 
+     * a tracked in the beacon state.
+     * @param beaconStateRoot is the beacon chain state root to be proven against.
+     * @param proof is the provided merkle proof
+     * @param slotRoot is hashtree root of the slot in the beacon state
+     */
     function verifySlotRoot(
         bytes32 beaconStateRoot,
         bytes32 slotRoot,
@@ -243,8 +251,8 @@ library BeaconChainProofs {
      */
     function verifyWithdrawalProofs(
         bytes32 beaconStateRoot,
-        WithdrawalProofs calldata proofs,
-        bytes32[] calldata withdrawalFields
+        bytes32[] calldata withdrawalFields,
+        WithdrawalProofs calldata proofs
     ) internal view {
         require(withdrawalFields.length == 2**WITHDRAWAL_FIELD_TREE_HEIGHT, "BeaconChainProofs.verifyWithdrawalProofs: withdrawalFields has incorrect length");