refactor: decouple structs used in the ICSVerifier

lidofinance · Feb 7, 2024 · 8dc7a63 · 8dc7a63
1 parent 892c326
commit 8dc7a63
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 84 deletions.
diff --git a/src/CSVerifier.sol b/src/CSVerifier.sol
@@ -67,22 +67,22 @@ contract CSVerifier is ICSVerifier {
 
     function processWithdrawalProof(
         ProvableBeaconBlockHeader calldata beaconBlock,
-        WithdrawalProofContext calldata ctx,
+        WithdrawalWitness calldata witness,
         uint256 nodeOperatorId,
         uint256 keyIndex
     ) external {
         {
             bytes32 trustedHeaderRoot = _getParentBlockRoot(
                 beaconBlock.rootsTimestamp
             );
-            bytes32 headerRoot = beaconBlock.blockHeader.hashTreeRoot();
+            bytes32 headerRoot = beaconBlock.header.hashTreeRoot();
             if (trustedHeaderRoot != headerRoot) {
                 revert InvalidBlockHeader();
             }
         }
 
         ForkVersion fork = forkSelector.findFork(
-            Slot.wrap(beaconBlock.blockHeader.slot)
+            Slot.wrap(beaconBlock.header.slot)
         );
 
         bytes memory pubkey = module.getNodeOperatorSigningKeys(
@@ -92,8 +92,8 @@ contract CSVerifier is ICSVerifier {
         );
 
         Withdrawal memory withdrawal = _processWithdrawalProof(
-            ctx,
-            beaconBlock.blockHeader.stateRoot,
+            witness,
+            beaconBlock.header.stateRoot,
             fork,
             pubkey
         );
@@ -106,47 +106,42 @@ contract CSVerifier is ICSVerifier {
     }
 
     function processHistoricalWithdrawalProof(
-        ProvableHistoricalBlockHeader calldata beaconBlock,
-        WithdrawalProofContext calldata ctx,
+        ProvableBeaconBlockHeader calldata beaconBlock,
+        HistoricalHeaderWitness calldata oldBlock,
+        WithdrawalWitness calldata witness,
         uint256 nodeOperatorId,
         uint256 keyIndex
     ) external {
         {
             bytes32 trustedHeaderRoot = _getParentBlockRoot(
-                beaconBlock.anchorBlock.rootsTimestamp
+                beaconBlock.rootsTimestamp
             );
-            bytes32 headerRoot = beaconBlock
-                .anchorBlock
-                .blockHeader
-                .hashTreeRoot();
+            bytes32 headerRoot = beaconBlock.header.hashTreeRoot();
             if (trustedHeaderRoot != headerRoot) {
                 revert InvalidBlockHeader();
             }
         }
 
         {
             // Check the validity of the historical block root against the anchor block header (accessible from EIP-4788).
-            bytes32 anchorStateRoot = beaconBlock
-                .anchorBlock
-                .blockHeader
-                .stateRoot;
+            bytes32 anchorStateRoot = beaconBlock.header.stateRoot;
             ForkVersion anchorFork = forkSelector.findFork(
-                Slot.wrap(beaconBlock.anchorBlock.blockHeader.slot)
+                Slot.wrap(beaconBlock.header.slot)
             );
             // solhint-disable-next-line func-named-parameters
             _verifyBlockRootProof(
                 anchorFork,
                 anchorStateRoot,
-                beaconBlock.historicalBlock.hashTreeRoot(),
-                beaconBlock.blockRootGIndex,
-                beaconBlock.blockRootProof
+                oldBlock.header.hashTreeRoot(),
+                oldBlock.rootGIndex,
+                oldBlock.proof
             );
         }
 
         // Fork may get a new value depends on the historical state root.
-        bytes32 stateRoot = beaconBlock.historicalBlock.stateRoot;
+        bytes32 stateRoot = oldBlock.header.stateRoot;
         ForkVersion fork = forkSelector.findFork(
-            Slot.wrap(beaconBlock.historicalBlock.slot)
+            Slot.wrap(oldBlock.header.slot)
         );
 
         bytes memory pubkey = module.getNodeOperatorSigningKeys(
@@ -156,7 +151,7 @@ contract CSVerifier is ICSVerifier {
         );
 
         Withdrawal memory withdrawal = _processWithdrawalProof(
-            ctx,
+            witness,
             stateRoot,
             fork,
             pubkey
@@ -211,50 +206,50 @@ contract CSVerifier is ICSVerifier {
 
     // @dev `stateRoot` is already validated.
     function _processWithdrawalProof(
-        WithdrawalProofContext calldata ctx,
+        WithdrawalWitness calldata witness,
         bytes32 stateRoot,
         ForkVersion fork,
         bytes memory pubkey
     ) internal view returns (Withdrawal memory withdrawal) {
-        address withdrawalAddress = _wcToAddress(ctx.withdrawalCredentials);
+        address withdrawalAddress = _wcToAddress(witness.withdrawalCredentials);
         if (withdrawalAddress != locator.withdrawalVault()) {
             revert InvalidWithdrawalAddress();
         }
 
-        if (_getEpoch() < ctx.withdrawableEpoch) {
+        if (_getEpoch() < witness.withdrawableEpoch) {
             revert ValidatorNotWithdrawn();
         }
 
         Validator memory validator = Validator({
             pubkey: pubkey,
-            withdrawalCredentials: ctx.withdrawalCredentials,
-            effectiveBalance: ctx.effectiveBalance, // TODO: Should we accept zero effective balance only?
-            slashed: ctx.slashed,
-            activationEligibilityEpoch: ctx.activationEligibilityEpoch,
-            activationEpoch: ctx.activationEpoch,
-            exitEpoch: ctx.exitEpoch,
-            withdrawableEpoch: ctx.withdrawableEpoch
+            withdrawalCredentials: witness.withdrawalCredentials,
+            effectiveBalance: witness.effectiveBalance, // TODO: Should we accept zero effective balance only?
+            slashed: witness.slashed,
+            activationEligibilityEpoch: witness.activationEligibilityEpoch,
+            activationEpoch: witness.activationEpoch,
+            exitEpoch: witness.exitEpoch,
+            withdrawableEpoch: witness.withdrawableEpoch
         });
 
         SSZ.verifyProof(
-            ctx.validatorProof,
+            witness.validatorProof,
             stateRoot,
             validator.hashTreeRoot(),
-            _getValidatorGI(fork, ctx.validatorIndex)
+            _getValidatorGI(fork, witness.validatorIndex)
         );
 
         withdrawal = Withdrawal({
-            index: ctx.withdrawalIndex,
-            validatorIndex: ctx.validatorIndex,
+            index: witness.withdrawalIndex,
+            validatorIndex: witness.validatorIndex,
             withdrawalAddress: withdrawalAddress,
-            amount: ctx.amount
+            amount: witness.amount
         });
 
         SSZ.verifyProof(
-            ctx.withdrawalProof,
+            witness.withdrawalProof,
             stateRoot,
             withdrawal.hashTreeRoot(),
-            _getWithdrawalGI(fork, ctx.withdrawalOffset)
+            _getWithdrawalGI(fork, witness.withdrawalOffset)
         );
     }
 

diff --git a/src/interfaces/ICSVerifier.sol b/src/interfaces/ICSVerifier.sol
@@ -7,7 +7,12 @@ import { BeaconBlockHeader } from "../lib/Types.sol";
 import { GIndex } from "../lib/GIndex.sol";
 
 interface ICSVerifier {
-    struct WithdrawalProofContext {
+    struct ProvableBeaconBlockHeader {
+        BeaconBlockHeader header; // Header of a block which root is a root at rootsTimestamp.
+        uint64 rootsTimestamp; // To be passed to the EIP-4788 block roots contract.
+    }
+
+    struct WithdrawalWitness {
         // ── Withdrawal fields ─────────────────────────────────────────────────
         uint8 withdrawalOffset; // In the withdrawals list.
         uint64 withdrawalIndex; // Network-wise.
@@ -22,33 +27,31 @@ interface ICSVerifier {
         uint64 exitEpoch;
         uint64 withdrawableEpoch;
         // ── Proofs ────────────────────────────────────────────────────────────
-        // All proofs are relative to the block header state root.
         bytes32[] withdrawalProof;
         bytes32[] validatorProof;
     }
 
-    struct ProvableBeaconBlockHeader {
-        BeaconBlockHeader blockHeader; // Header of a block which root is a root at rootsTimestamp.
-        uint64 rootsTimestamp; // To be passed to the EIP-4788 block roots contract.
-    }
-
-    struct ProvableHistoricalBlockHeader {
-        ProvableBeaconBlockHeader anchorBlock;
-        GIndex blockRootGIndex;
-        bytes32[] blockRootProof;
-        BeaconBlockHeader historicalBlock;
+    // A witness for a block header which root is accessible via `historical_summaries` field.
+    struct HistoricalHeaderWitness {
+        BeaconBlockHeader header;
+        GIndex rootGIndex;
+        bytes32[] proof;
     }
 
+    /// @notice `witness` is a withdrawal witness against the `beaconBlock`'s state root.
     function processWithdrawalProof(
         ProvableBeaconBlockHeader calldata beaconBlock,
-        WithdrawalProofContext calldata ctx,
+        WithdrawalWitness calldata witness,
         uint256 nodeOperatorId,
         uint256 keyIndex
     ) external;
 
+    /// @notice `oldHeader` is a beacon block header witness against the `beaconBlock`'s state root.
+    /// @notice `witness` is a withdrawal witness against the `oldHeader`'s state root.
     function processHistoricalWithdrawalProof(
-        ProvableHistoricalBlockHeader calldata beaconBlock,
-        WithdrawalProofContext calldata ctx,
+        ProvableBeaconBlockHeader calldata beaconBlock,
+        HistoricalHeaderWitness calldata oldBlock,
+        WithdrawalWitness calldata witness,
         uint256 nodeOperatorId,
         uint256 keyIndex
     ) external;

diff --git a/test/CSVerifier.t.sol b/test/CSVerifier.t.sol
@@ -23,7 +23,7 @@ contract CSVerifierTest is Test {
         bytes _pubkey;
         address _withdrawalAddress;
         ICSVerifier.ProvableBeaconBlockHeader beaconBlock;
-        ICSVerifier.WithdrawalProofContext ctx;
+        ICSVerifier.WithdrawalWitness witness;
     }
 
     CSVerifier public verifier;
@@ -95,8 +95,13 @@ contract CSVerifierTest is Test {
             ""
         );
 
-        vm.warp(fixture.beaconBlock.blockHeader.slot * 12);
+        vm.warp(fixture.beaconBlock.header.slot * 12);
 
-        verifier.processWithdrawalProof(fixture.beaconBlock, fixture.ctx, 0, 0);
+        verifier.processWithdrawalProof(
+            fixture.beaconBlock,
+            fixture.witness,
+            0,
+            0
+        );
     }
 }
diff --git a/test/CSVerifierHistorical.t.sol b/test/CSVerifierHistorical.t.sol
@@ -22,8 +22,9 @@ contract CSVerifierHistoricalTest is Test {
         bytes32 _blockRoot;
         bytes _pubkey;
         address _withdrawalAddress;
-        ICSVerifier.ProvableHistoricalBlockHeader beaconBlock;
-        ICSVerifier.WithdrawalProofContext ctx;
+        ICSVerifier.ProvableBeaconBlockHeader beaconBlock;
+        ICSVerifier.HistoricalHeaderWitness oldBlock;
+        ICSVerifier.WithdrawalWitness witness;
     }
 
     CSVerifier public verifier;
@@ -69,7 +70,7 @@ contract CSVerifierHistoricalTest is Test {
     function test_processWithdrawalProof() public {
         vm.mockCall(
             verifier.BEACON_ROOTS(),
-            abi.encode(fixture.beaconBlock.anchorBlock.rootsTimestamp),
+            abi.encode(fixture.beaconBlock.rootsTimestamp),
             abi.encode(fixture._blockRoot)
         );
 
@@ -95,11 +96,13 @@ contract CSVerifierHistoricalTest is Test {
             ""
         );
 
-        vm.warp(fixture.beaconBlock.historicalBlock.slot * 12);
+        vm.warp(fixture.oldBlock.header.slot * 12);
 
+        // solhint-disable-next-line func-named-parameters
         verifier.processHistoricalWithdrawalProof(
             fixture.beaconBlock,
-            fixture.ctx,
+            fixture.oldBlock,
+            fixture.witness,
             0,
             0
         );

diff --git a/test/fixtures/CSVerifier/historicalWithdrawal.json b/test/fixtures/CSVerifier/historicalWithdrawal.json
@@ -3,18 +3,25 @@
   "_pubkey": "0x805ca7091bde2f34b93d17ea0c95b9e3f349768abf452c064b81bdbd43d8aaabb59ba9c0b8786a35a4fe1163583788d1",
   "_withdrawalAddress": "0x0000000000000000000000003202b5b0602274197ccb22b8c02cec9539990c7c",
   "beaconBlock": {
-    "anchorBlock": {
-      "blockHeader": {
-        "00__slot": 7471926,
-        "01__proposerIndex": 634821,
-        "02__parentRoot": "0x4aba763e7f625caa4592007d8f18b553ef39d191bf9d52289dbb91ea3cf914d5",
-        "03__stateRoot": "0xa960cc6db4089149664d7b845da32572a5d5e39d973c4a41f1e2c4668877fabc",
-        "04__bodyRoot": "0x10a1761482c4d4b4c3af4cb93884f6c2058f47428ba66389801a96bd6fbb224a"
-      },
-      "rootsTimestamp": 1706545128
+    "blockHeader": {
+      "00__slot": 7471926,
+      "01__proposerIndex": 634821,
+      "02__parentRoot": "0x4aba763e7f625caa4592007d8f18b553ef39d191bf9d52289dbb91ea3cf914d5",
+      "03__stateRoot": "0xa960cc6db4089149664d7b845da32572a5d5e39d973c4a41f1e2c4668877fabc",
+      "04__bodyRoot": "0x10a1761482c4d4b4c3af4cb93884f6c2058f47428ba66389801a96bd6fbb224a"
     },
-    "blockRootGIndex": "0x000000000000000000000000000000000000000000000000001d800022ccc60d",
-    "blockRootProof": [
+    "rootsTimestamp": 1706545128
+  },
+  "oldBlock": {
+    "00__header": {
+      "00__slot": 6335686,
+      "01__proposerIndex": 448343,
+      "02__parentRoot": "0xdcb51158e03ac1e1e8ce3030ff54f04ad976ebcffce3eed50646e355bf4326b7",
+      "03__stateRoot": "0xecf6a4ec29520fc514f11e6ff0293308f2a691523d9a4fb11dd323011885ebf8",
+      "04__bodyRoot": "0x667bd898e759ed03f37b72bc5bfe65b9144d2b7538196422e5b55cdff204499f"
+    },
+    "01__rootGIndex": "0x000000000000000000000000000000000000000000000000001d800022ccc60d",
+    "02__proof": [
       "0xe91cdf41a18b44827288f1fc40cc35d134be79852d19ec5cf04280f5a4baf188",
       "0x53c2a057b9557708cd0ecce6e96df808fe7d0a3d3c00e829923734d45d507a5c",
       "0x6abd5bd537919fde81226112f1ceac9372c1e9c36189be0a3653c60bbbfa9e91",
@@ -59,16 +66,9 @@
       "0xdb56114e00fdd4c1f85c892bf35ac9a89289aaecb1ebd0a96cde606a748b5d71",
       "0x7dd0465d42d6ce81c2024ff4e14a9fd02fb66f927c32e2bfa599659edb6bd44d",
       "0x8f72e0711a34cf0414209efb21b328c09096173582f1cb54c8dcca35d44e050e"
-    ],
-    "historicalBlock": {
-      "00__slot": 6335686,
-      "01__proposerIndex": 448343,
-      "02__parentRoot": "0xdcb51158e03ac1e1e8ce3030ff54f04ad976ebcffce3eed50646e355bf4326b7",
-      "03__stateRoot": "0xecf6a4ec29520fc514f11e6ff0293308f2a691523d9a4fb11dd323011885ebf8",
-      "04__bodyRoot": "0x667bd898e759ed03f37b72bc5bfe65b9144d2b7538196422e5b55cdff204499f"
-    }
+    ]
   },
-  "ctx": {
+  "witness": {
     "00__withdrawalOffset": 0,
     "01__withdrawalIndex": 14286084,
     "02__validatorIndex": 494828,

diff --git a/test/fixtures/CSVerifier/withdrawal.json b/test/fixtures/CSVerifier/withdrawal.json
@@ -12,7 +12,7 @@
     },
     "rootsTimestamp": 1706545128
   },
-  "ctx": {
+  "witness": {
     "00__withdrawalOffset": 1,
     "01__withdrawalIndex": 28346378,
     "02__validatorIndex": 673610,