fix `/eth/v1/beacon/deposit_snapshot` for EIP-4881 (#6038)

Fix the `/eth/v1/beacon/deposit_snapshot` API to produce proper EIP-4881
compatible `DepositTreeSnapshot` responses. The endpoint used to expose
a Nimbus-specific database internal format.

Also fix trusted node sync to consume properly formatted EIP-4881 data
with `--with-deposit-snapshot`, and `--finalized-deposit-tree-snapshot`
beacon node launch option to use the EIP-4881 data. Further ensure that
`ncli_testnet` produces EIP-4881 formatted data for interoperability.

.gitmodules

@@ -230,3 +230,8 @@
 	url = https://github.com/eth-clients/holesky
 	ignore = untracked
 	branch = main
+[submodule "vendor/EIPs"]
+	path = vendor/EIPs
+	url = https://github.com/ethereum/EIPs
+	ignore = untracked
+	branch = master

AllTests-mainnet.md

@@ -433,6 +433,15 @@ OK: 253/253 Fail: 0/253 Skip: 0/253
 + Testing uints        inputs - valid                                                        OK
 ```
 OK: 10/12 Fail: 0/12 Skip: 2/12
+## EIP-4881
+```diff
++ deposit_cases                                                                              OK
++ empty_root                                                                                 OK
++ finalization                                                                               OK
++ invalid_snapshot                                                                           OK
++ snapshot_cases                                                                             OK
+```
+OK: 5/5 Fail: 0/5 Skip: 0/5
 ## EL Configuration
 ```diff
 + Empty config file                                                                          OK
@@ -999,4 +1008,4 @@ OK: 2/2 Fail: 0/2 Skip: 0/2
 OK: 9/9 Fail: 0/9 Skip: 0/9
 
 ---TOTAL---
-OK: 672/677 Fail: 0/677 Skip: 5/677
+OK: 677/682 Fail: 0/682 Skip: 5/682

beacon_chain/el/eth1_chain.nim

@@ -362,17 +362,18 @@ func clear*(chain: var Eth1Chain) =
   chain.headMerkleizer = chain.finalizedDepositsMerkleizer
   chain.hasConsensusViolation = false
 
-proc init*(T: type Eth1Chain,
+proc init*(
-           cfg: RuntimeConfig,
+    T: type Eth1Chain,
-           db: BeaconChainDB,
+    cfg: RuntimeConfig,
-           depositContractBlockNumber: uint64,
+    db: BeaconChainDB,
-           depositContractBlockHash: Eth2Digest): T =
+    depositContractBlockNumber: uint64,
+    depositContractBlockHash: Eth2Digest): T =
   let
     (finalizedBlockHash, depositContractState) =
       if db != nil:
-        let treeSnapshot = db.getDepositContractSnapshot()
+        let snapshot = db.getDepositContractSnapshot()
-        if treeSnapshot.isSome:
+        if snapshot.isSome:
-          (treeSnapshot.get.eth1Block, treeSnapshot.get.depositContractState)
+          (snapshot.get.eth1Block, snapshot.get.depositContractState)
         else:
           let oldSnapshot = db.getUpgradableDepositSnapshot()
           if oldSnapshot.isSome:

beacon_chain/networking/network_metadata.nim

@@ -83,7 +83,6 @@ type
     depositContractBlockHash*: Eth2Digest
 
     genesis*: GenesisMetadata
-    genesisDepositsSnapshot*: string
 
 func hasGenesis*(metadata: Eth2NetworkMetadata): bool =
   metadata.genesis.kind != NoGenesis
@@ -119,7 +118,6 @@ proc loadEth2NetworkMetadata*(
   try:
     let
       genesisPath = path & "/genesis.ssz"
-      genesisDepositsSnapshotPath = path & "/genesis_deposit_contract_snapshot.ssz"
       configPath = path & "/config.yaml"
       deployBlockPath = path & "/deploy_block.txt"
       depositContractBlockPath = path & "/deposit_contract_block.txt"
@@ -179,11 +177,6 @@ proc loadEth2NetworkMetadata*(
         readBootstrapNodes(bootstrapNodesPath) &
         readBootEnr(bootEnrPath))
 
-      genesisDepositsSnapshot = if fileExists(genesisDepositsSnapshotPath):
-        readFile(genesisDepositsSnapshotPath)
-      else:
-        ""
-
     ok Eth2NetworkMetadata(
       eth1Network: eth1Network,
       cfg: runtimeConfig,
@@ -200,8 +193,7 @@ proc loadEth2NetworkMetadata*(
         elif fileExists(genesisPath) and not isCompileTime:
           GenesisMetadata(kind: UserSuppliedFile, path: genesisPath)
         else:
-          GenesisMetadata(kind: NoGenesis),
+          GenesisMetadata(kind: NoGenesis))
-      genesisDepositsSnapshot: genesisDepositsSnapshot)
 
   except PresetIncompatibleError as err:
     err err.msg

beacon_chain/nimbus_beacon_node.nim

@@ -646,14 +646,18 @@ proc init*(T: type BeaconNode,
   if config.finalizedDepositTreeSnapshot.isSome:
     let
       depositTreeSnapshotPath = config.finalizedDepositTreeSnapshot.get.string
-      depositContractSnapshot = try:
+      snapshot =
-        SSZ.loadFile(depositTreeSnapshotPath, DepositContractSnapshot)
+        try:
-      except SszError as err:
+          SSZ.loadFile(depositTreeSnapshotPath, DepositTreeSnapshot)
-        fatal "Deposit tree snapshot loading failed",
+        except SszError as err:
-              err = formatMsg(err, depositTreeSnapshotPath)
+          fatal "Deposit tree snapshot loading failed",
-        quit 1
+                err = formatMsg(err, depositTreeSnapshotPath)
-      except CatchableError as err:
+          quit 1
-        fatal "Failed to read deposit tree snapshot file", err = err.msg
+        except CatchableError as err:
+          fatal "Failed to read deposit tree snapshot file", err = err.msg
+          quit 1
+      depositContractSnapshot = DepositContractSnapshot.init(snapshot).valueOr:
+        fatal "Invalid deposit tree snapshot file"
         quit 1
     db.putDepositContractSnapshot(depositContractSnapshot)
 

beacon_chain/rpc/rest_beacon_api.nim

@@ -141,13 +141,7 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) =
       return RestApiResponse.jsonError(Http404,
                                        NoFinalizedSnapshotAvailableError)
 
-    RestApiResponse.jsonResponse(
+    RestApiResponse.jsonResponse(snapshot.getTreeSnapshot())
-      RestDepositSnapshot(
-        finalized: snapshot.depositContractState.branch,
-        deposit_root: snapshot.getDepositRoot(),
-        deposit_count: snapshot.getDepositCountU64(),
-        execution_block_hash: snapshot.eth1Block,
-        execution_block_height: snapshot.blockHeight))
 
   # https://ethereum.github.io/beacon-APIs/#/Beacon/getGenesis
   router.api2(MethodGet, "/eth/v1/beacon/genesis") do () -> RestApiResponse:

beacon_chain/spec/datatypes/base.nim

@@ -442,6 +442,17 @@ type
     branch*: array[DEPOSIT_CONTRACT_TREE_DEPTH, Eth2Digest]
     deposit_count*: array[32, byte] # Uint256
 
+  # https://eips.ethereum.org/EIPS/eip-4881
+  FinalizedDepositTreeBranch* =
+    List[Eth2Digest, Limit DEPOSIT_CONTRACT_TREE_DEPTH]
+
+  DepositTreeSnapshot* = object
+    finalized*: FinalizedDepositTreeBranch
+    deposit_root*: Eth2Digest
+    deposit_count*: uint64
+    execution_block_hash*: Eth2Digest
+    execution_block_height*: uint64
+
   # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.7/specs/phase0/beacon-chain.md#validator
   ValidatorStatus* = object
     # This is a validator without the expensive, immutable, append-only parts

beacon_chain/spec/deposit_snapshots.nim

@@ -55,10 +55,97 @@ func getDepositRoot*(
 func isValid*(d: DepositContractSnapshot, wantedDepositRoot: Eth2Digest): bool =
   ## `isValid` requires the snapshot to be self-consistent and
   ## to point to a specific Ethereum block
-  return not (d.eth1Block.isZeroMemory or
+  not d.eth1Block.isZeroMemory and d.getDepositRoot() == wantedDepositRoot
-              d.blockHeight == 0 or
-              d.getDepositRoot() != wantedDepositRoot)
 
 func matches*(snapshot: DepositContractSnapshot, eth1_data: Eth1Data): bool =
   snapshot.getDepositCountU64() == eth1_data.deposit_count and
   snapshot.getDepositRoot() == eth1_data.deposit_root
+
+# https://eips.ethereum.org/EIPS/eip-4881
+func getExpandedBranch(
+    finalized: FinalizedDepositTreeBranch,
+    deposit_count: uint64
+): Opt[array[DEPOSIT_CONTRACT_TREE_DEPTH, Eth2Digest]] =
+  var
+    branch: array[DEPOSIT_CONTRACT_TREE_DEPTH, Eth2Digest]
+    idx = finalized.len
+  for i in 0 ..< DEPOSIT_CONTRACT_TREE_DEPTH:
+    if (deposit_count and (1'u64 shl i)) != 0:
+      dec idx
+      branch[i] = finalized[idx]
+  if idx != 0:
+    return Opt.none array[DEPOSIT_CONTRACT_TREE_DEPTH, Eth2Digest]
+  Opt.some branch
+
+func init(
+    T: type DepositsMerkleizer,
+    finalized: FinalizedDepositTreeBranch,
+    deposit_root: Eth2Digest,
+    deposit_count: uint64): Opt[DepositsMerkleizer] =
+  let branch = ? getExpandedBranch(finalized, deposit_count)
+  var res = Opt.some DepositsMerkleizer.init(branch, deposit_count)
+  if res.get().getDepositsRoot() != deposit_root:
+    res.reset()
+  res
+
+func init*(
+    T: type DepositsMerkleizer,
+    snapshot: DepositTreeSnapshot): Opt[DepositsMerkleizer] =
+  DepositsMerkleizer.init(
+    snapshot.finalized, snapshot.deposit_root, snapshot.deposit_count)
+
+func init*(
+    T: type DepositContractSnapshot,
+    snapshot: DepositTreeSnapshot): Opt[DepositContractSnapshot] =
+  var res = Opt.some DepositContractSnapshot(
+    eth1Block: snapshot.execution_block_hash,
+    depositContractState: DepositContractState(
+      branch: ? getExpandedBranch(snapshot.finalized, snapshot.deposit_count),
+      deposit_count: depositCountBytes(snapshot.deposit_count)),
+    blockHeight: snapshot.execution_block_height)
+  if not res.get.isValid(snapshot.deposit_root):
+    res.reset()
+  res
+
+func getFinalizedBranch(
+    branch: openArray[Eth2Digest],
+    deposit_count: uint64): FinalizedDepositTreeBranch =
+  doAssert branch.len == DEPOSIT_CONTRACT_TREE_DEPTH
+  var
+    finalized: FinalizedDepositTreeBranch
+    i = branch.high
+  while i > 0:
+    dec i
+    if (deposit_count and (1'u64 shl i)) != 0:
+      doAssert finalized.add branch[i.int]
+  finalized
+
+func getFinalizedBranch(
+    merkleizer: DepositsMerkleizer): FinalizedDepositTreeBranch =
+  let chunks = merkleizer.getCombinedChunks()
+  doAssert chunks.len == DEPOSIT_CONTRACT_TREE_DEPTH + 1
+  getFinalizedBranch(
+    chunks[0 ..< DEPOSIT_CONTRACT_TREE_DEPTH],
+    merkleizer.getChunkCount())
+
+func getTreeSnapshot*(
+    merkleizer: var DepositsMerkleizer,
+    execution_block_hash: Eth2Digest,
+    execution_block_height: uint64): DepositTreeSnapshot =
+  DepositTreeSnapshot(
+    finalized: merkleizer.getFinalizedBranch(),
+    deposit_root: merkleizer.getDepositsRoot(),
+    deposit_count: merkleizer.getChunkCount(),
+    execution_block_hash: execution_block_hash,
+    execution_block_height: execution_block_height)
+
+func getTreeSnapshot*(
+    snapshot: DepositContractSnapshot): DepositTreeSnapshot =
+  let deposit_count = snapshot.getDepositCountU64()
+  DepositTreeSnapshot(
+    finalized: getFinalizedBranch(
+      snapshot.depositContractState.branch, deposit_count),
+    deposit_root: snapshot.getDepositRoot(),
+    deposit_count: deposit_count,
+    execution_block_hash: snapshot.eth1Block,
+    execution_block_height: snapshot.blockHeight)

beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim

@@ -67,6 +67,7 @@ RestJson.useDefaultSerializationFor(
   DenebSignedBlockContents,
   Deposit,
   DepositData,
+  DepositTreeSnapshot,
   DistributedKeystoreInfo,
   EmptyBody,
   Eth1Data,
@@ -125,7 +126,6 @@ RestJson.useDefaultSerializationFor(
   RestCommitteeSubscription,
   RestContributionAndProof,
   RestDepositContract,
-  RestDepositSnapshot,
   RestEpochRandao,
   RestEpochSyncCommittee,
   RestExecutionPayload,

beacon_chain/spec/eth2_apis/rest_types.nim

@@ -16,7 +16,7 @@
 import
   std/[json, tables],
   stew/base10, web3/primitives, httputils,
-  ".."/forks,
+  ".."/[deposit_snapshots, forks],
   ".."/mev/deneb_mev
 
 from ".."/datatypes/capella import BeaconBlockBody
@@ -368,13 +368,6 @@ type
     chain_id*: string
     address*: string
 
-  RestDepositSnapshot* = object
-    finalized*: array[DEPOSIT_CONTRACT_TREE_DEPTH, Eth2Digest]
-    deposit_root*: Eth2Digest
-    deposit_count*: uint64
-    execution_block_hash*: Eth2Digest
-    execution_block_height*: uint64
-
   RestBlockInfo* = object
     slot*: Slot
     blck* {.serializedFieldName: "block".}: Eth2Digest
@@ -547,7 +540,7 @@ type
   GetBlockRootResponse* = DataOptimisticObject[RestRoot]
   GetDebugChainHeadsV2Response* = DataEnclosedObject[seq[RestChainHeadV2]]
   GetDepositContractResponse* = DataEnclosedObject[RestDepositContract]
-  GetDepositSnapshotResponse* = DataEnclosedObject[RestDepositSnapshot]
+  GetDepositSnapshotResponse* = DataEnclosedObject[DepositTreeSnapshot]
   GetEpochCommitteesResponse* = DataEnclosedObject[seq[RestBeaconStatesCommittees]]
   GetForkScheduleResponse* = DataEnclosedObject[seq[Fork]]
   GetGenesisResponse* = DataEnclosedObject[RestGenesis]

beacon_chain/trusted_node_sync.nim

@@ -33,18 +33,10 @@ proc fetchDepositSnapshot(
   except CatchableError as e:
     return err("The trusted node likely does not support the /eth/v1/beacon/deposit_snapshot end-point:" & e.msg)
 
-  let data = resp.data.data
+  let snapshot = DepositContractSnapshot.init(resp.data.data).valueOr:
-  let snapshot = DepositContractSnapshot(
-    eth1Block: data.execution_block_hash,
-    depositContractState: DepositContractState(
-      branch: data.finalized,
-      deposit_count: depositCountBytes(data.deposit_count)),
-    blockHeight: data.execution_block_height)
-
-  if not snapshot.isValid(data.deposit_root):
     return err "The obtained deposit snapshot contains self-contradictory data"
 
-  return ok snapshot
+  ok snapshot
 
 from ./spec/datatypes/deneb import asSigVerified, shortLog
 

ncli/ncli_testnet.nim

@@ -477,7 +477,7 @@ proc doCreateTestnet*(config: CliConfig,
       createDepositContractSnapshot(
         deposits,
         genesisExecutionPayloadHeader.block_hash,
-        genesisExecutionPayloadHeader.block_number))
+        genesisExecutionPayloadHeader.block_number).getTreeSnapshot())
 
     initialState[].genesis_validators_root
 

tests/test_deposit_snapshots.nim

@@ -9,10 +9,12 @@
 {.used.}
 
 import
-  std/[os, random, strutils, times],
+  std/[json, os, random, sequtils, strutils, times],
-  chronos, stew/results, unittest2, chronicles,
+  chronos, stew/[base10, results], chronicles, unittest2,
+  yaml,
   ../beacon_chain/beacon_chain_db,
-  ../beacon_chain/spec/deposit_snapshots
+  ../beacon_chain/spec/deposit_snapshots,
+  ./consensus_spec/os_ops
 
 from eth/db/kvstore import kvStore
 from nimcrypto import toDigest
@@ -171,10 +173,8 @@ suite "DepositContractSnapshot":
     # Use our hard-coded ds1 as a model.
     var model: OldDepositContractSnapshot
     check(decodeSSZ(ds1, model))
-    # Check blockHeight.
+    # Check initialization. blockHeight cannot be validated and may be 0.
-    var dcs = model.toDepositContractSnapshot(0)
+    var dcs = model.toDepositContractSnapshot(11052984)
-    check(not dcs.isValid(ds1Root))
-    dcs.blockHeight = 11052984
     check(dcs.isValid(ds1Root))
     # Check eth1Block.
     dcs.eth1Block = ZERO
@@ -194,3 +194,117 @@ suite "DepositContractSnapshot":
     dcs.depositContractState.deposit_count =
       model.depositContractState.deposit_count
     check(dcs.isValid(ds1Root))
+
+suite "EIP-4881":
+  type DepositTestCase = object
+    deposit_data: DepositData
+    deposit_data_root: Eth2Digest
+    eth1_data: Eth1Data
+    block_height: uint64
+    snapshot: DepositTreeSnapshot
+
+  proc loadTestCases(
+      path: string
+  ): seq[DepositTestCase] {.raises: [
+      IOError, KeyError, ValueError, YamlConstructionError, YamlParserError].} =
+    yaml.loadToJson(os_ops.readFile(path))[0].mapIt:
+      DepositTestCase(
+        deposit_data: DepositData(
+          pubkey: ValidatorPubKey.fromHex(
+            it["deposit_data"]["pubkey"].getStr()).expect("valid"),
+          withdrawal_credentials: Eth2Digest.fromHex(
+            it["deposit_data"]["withdrawal_credentials"].getStr()),
+          amount: Gwei(Base10.decode(uint64,
+            it["deposit_data"]["amount"].getStr()).expect("valid")),
+          signature: ValidatorSig.fromHex(
+            it["deposit_data"]["signature"].getStr()).expect("valid")),
+        deposit_data_root: Eth2Digest.fromHex(it["deposit_data_root"].getStr()),
+        eth1_data: Eth1Data(
+          deposit_root: Eth2Digest.fromHex(
+            it["eth1_data"]["deposit_root"].getStr()),
+          deposit_count: Base10.decode(uint64,
+            it["eth1_data"]["deposit_count"].getStr()).expect("valid"),
+          block_hash: Eth2Digest.fromHex(
+            it["eth1_data"]["block_hash"].getStr())),
+        block_height: uint64(it["block_height"].getInt()),
+        snapshot: DepositTreeSnapshot(
+          finalized: it["snapshot"]["finalized"].foldl((block:
+            check: a[].add Eth2Digest.fromHex(b.getStr())
+            a), newClone default(List[
+              Eth2Digest, Limit DEPOSIT_CONTRACT_TREE_DEPTH]))[],
+          deposit_root: Eth2Digest.fromHex(
+            it["snapshot"]["deposit_root"].getStr()),
+          deposit_count: uint64(
+            it["snapshot"]["deposit_count"].getInt()),
+          execution_block_hash: Eth2Digest.fromHex(
+            it["snapshot"]["execution_block_hash"].getStr()),
+          execution_block_height: uint64(
+            it["snapshot"]["execution_block_height"].getInt())))
+
+  const path = currentSourcePath.rsplit(DirSep, 1)[0]/
+    ".."/"vendor"/"EIPs"/"assets"/"eip-4881"/"test_cases.yaml"
+  let testCases = loadTestCases(path)
+  for testCase in testCases:
+    check testCase.deposit_data_root == hash_tree_root(testCase.deposit_data)
+
+  test "empty_root":
+    var empty = DepositsMerkleizer.init()
+    check empty.getDepositsRoot() == Eth2Digest.fromHex(
+      "0xd70a234731285c6804c2a4f56711ddb8c82c99740f207854891028af34e27e5e")
+
+  test "deposit_cases":
+    var tree = DepositsMerkleizer.init()
+    for testCase in testCases:
+      tree.addChunk testCase.deposit_data_root.data
+      var snapshot = DepositsMerkleizer.init(tree.toDepositContractState())
+      let expected = testCase.eth1_data.deposit_root
+      check:
+        snapshot.getDepositsRoot() == expected
+        tree.getDepositsRoot() == expected
+
+  test "finalization":
+    var tree = DepositsMerkleizer.init()
+    for testCase in testCases[0 ..< 128]:
+      tree.addChunk testCase.deposit_data_root.data
+    let originalRoot = tree.getDepositsRoot()
+    check originalRoot == testCases[127].eth1_data.deposit_root
+    var finalized = DepositsMerkleizer.init()
+    for testCase in testCases[0 .. 100]:
+      finalized.addChunk testCase.deposit_data_root.data
+    var snapshot = finalized.getTreeSnapshot(
+      testCases[100].eth1_data.block_hash, testCases[100].block_height)
+    check snapshot == testCases[100].snapshot
+    var copy = DepositsMerkleizer.init(snapshot).expect("just produced")
+    for testCase in testCases[101 ..< 128]:
+      copy.addChunk testCase.deposit_data_root.data
+    check tree.getDepositsRoot() == copy.getDepositsRoot()
+    for testCase in testCases[101 .. 105]:
+      finalized.addChunk testCase.deposit_data_root.data
+    snapshot = finalized.getTreeSnapshot(
+      testCases[105].eth1_data.block_hash, testCases[105].block_height)
+    copy = DepositsMerkleizer.init(snapshot).expect("just produced")
+    var fullTreeCopy = DepositsMerkleizer.init()
+    for testCase in testCases[0 .. 105]:
+      fullTreeCopy.addChunk testCase.deposit_data_root.data
+    let
+      depositRoots = testCases[106 ..< 128].mapIt(it.deposit_data_root)
+      proofs1 = copy.addChunksAndGenMerkleProofs(depositRoots)
+      proofs2 = fullTreeCopy.addChunksAndGenMerkleProofs(depositRoots)
+    check proofs1 == proofs2
+
+  test "snapshot_cases":
+    var tree = DepositsMerkleizer.init()
+    for testCase in testCases:
+      tree.addChunk testCase.deposit_data_root.data
+      let snapshot = tree.getTreeSnapshot(
+        testCase.eth1_data.block_hash, testCase.block_height)
+      check snapshot == testCase.snapshot
+
+  test "invalid_snapshot":
+    let invalidSnapshot = DepositTreeSnapshot(
+      finalized: default(FinalizedDepositTreeBranch),
+      deposit_root: ZERO_HASH,
+      deposit_count: 0,
+      execution_block_hash: ZERO_HASH,
+      execution_block_height: 0)
+    check DepositsMerkleizer.init(invalidSnapshot).isNone()

vendor/EIPs

@@ -0,0 +1 @@
+Subproject commit 73fbb29019c19887235c1da456cfbfd5b4835184