rename `DepositTreeSnapshot` -> `DepositContractSnapshot` (#6036)

EIP-4881 was never correctly implemented, the `DepositTreeSnapshot`
structure has nothing to do with its actual definition. Reflect that
by renaming the type to a Nimbus-specific `DepositContractSnapshot`,
so that an actual EIP-4881 implementation can use the correct names.

- https://eips.ethereum.org/EIPS/eip-4881#specification

Notably, `DepositTreeSnapshot` contains a compressed sequence in
`finalized`, only containing the minimally required intermediate roots.

That also explains the incorrect REST response reported in #5508.

The non-canonical representation was introduced in #4303 and is also
persisted in the database. We'll have to maintain it for a while.

AllTests-mainnet.md

@@ -141,7 +141,7 @@ OK: 4/4 Fail: 0/4 Skip: 0/4
 + Missing Authorization header [Beacon Node] [Preset: mainnet]                               OK
 ```
 OK: 5/5 Fail: 0/5 Skip: 0/5
-## DepositTreeSnapshot
+## DepositContractSnapshot
 ```diff
 + Migration                                                                                  OK
 + SSZ                                                                                        OK

beacon_chain/beacon_chain_db.nim

@@ -179,7 +179,7 @@ type
     kOldDepositContractSnapshot
       ## Deprecated:
       ## This was the merkleizer checkpoint produced by processing the
-      ## finalized deposits (similar to kDepositTreeSnapshot, but before
+      ## finalized deposits (similar to kDepositContractSnapshot, but before
       ## the EIP-4881 support was introduced). Currently, we read from
       ## it during upgrades and we keep writing data to it as a measure
       ## allowing the users to downgrade to a previous version of Nimbus.
@@ -194,7 +194,12 @@ type
     kHashToStateDiff # Obsolete
     kHashToStateOnlyMutableValidators
     kBackfillBlock # Obsolete, was in `unstable` for a while, but never released
-    kDepositTreeSnapshot # EIP-4881-compatible deposit contract state snapshot
+    kDepositContractSnapshot
+      ## Deposit contract state snapshot derived from EIP-4881 data.
+      ## This key also stores intermediate hashes that are no longer used
+      ## for future deposits, beyond the `finalized` branch from EIP-4881.
+      ## Those extra hashes may be set to ZERO_HASH when importing from a
+      ## compressed EIP-4881 `DepositTreeSnapshot`.
 
   BeaconBlockSummary* = object
     ## Cache of beacon block summaries - during startup when we construct the
@@ -909,10 +914,10 @@ proc putTailBlock*(db: BeaconChainDB, key: Eth2Digest) =
 proc putGenesisBlock*(db: BeaconChainDB, key: Eth2Digest) =
   db.keyValues.putRaw(subkey(kGenesisBlock), key)
 
-proc putDepositTreeSnapshot*(db: BeaconChainDB,
+proc putDepositContractSnapshot*(
-                             snapshot: DepositTreeSnapshot) =
+    db: BeaconChainDB, snapshot: DepositContractSnapshot) =
   db.withManyWrites:
-    db.keyValues.putSnappySSZ(subkey(kDepositTreeSnapshot),
+    db.keyValues.putSnappySSZ(subkey(kDepositContractSnapshot),
                               snapshot)
     # TODO: We currently store this redundant old snapshot in order
     #       to allow the users to rollback to a previous version
@@ -921,12 +926,13 @@ proc putDepositTreeSnapshot*(db: BeaconChainDB,
     db.keyValues.putSnappySSZ(subkey(kOldDepositContractSnapshot),
                               snapshot.toOldDepositContractSnapshot)
 
-proc hasDepositTreeSnapshot*(db: BeaconChainDB): bool =
+proc hasDepositContractSnapshot*(db: BeaconChainDB): bool =
-  expectDb(subkey(kDepositTreeSnapshot) in db.keyValues)
+  expectDb(subkey(kDepositContractSnapshot) in db.keyValues)
 
-proc getDepositTreeSnapshot*(db: BeaconChainDB): Opt[DepositTreeSnapshot] =
+proc getDepositContractSnapshot*(db: BeaconChainDB): Opt[DepositContractSnapshot] =
-  result.ok(default DepositTreeSnapshot)
+  result.ok(default DepositContractSnapshot)
-  let r = db.keyValues.getSnappySSZ(subkey(kDepositTreeSnapshot), result.get)
+  let r = db.keyValues.getSnappySSZ(
+    subkey(kDepositContractSnapshot), result.get)
   if r != GetResult.found: result.err()
 
 proc getUpgradableDepositSnapshot*(db: BeaconChainDB): Option[OldDepositContractSnapshot] =

beacon_chain/el/eth1_chain.nim

@@ -166,7 +166,7 @@ proc pruneOldBlocks(chain: var Eth1Chain, depositIndex: uint64) =
 
   if chain.finalizedDepositsMerkleizer.getChunkCount > initialChunks:
     chain.finalizedBlockHash = lastBlock.hash
-    chain.db.putDepositTreeSnapshot DepositTreeSnapshot(
+    chain.db.putDepositContractSnapshot DepositContractSnapshot(
       eth1Block: lastBlock.hash,
       depositContractState: chain.finalizedDepositsMerkleizer.toDepositContractState,
       blockHeight: lastBlock.number)
@@ -370,7 +370,7 @@ proc init*(T: type Eth1Chain,
   let
     (finalizedBlockHash, depositContractState) =
       if db != nil:
-        let treeSnapshot = db.getDepositTreeSnapshot()
+        let treeSnapshot = db.getDepositContractSnapshot()
         if treeSnapshot.isSome:
           (treeSnapshot.get.eth1Block, treeSnapshot.get.depositContractState)
         else:
@@ -378,7 +378,7 @@ proc init*(T: type Eth1Chain,
           if oldSnapshot.isSome:
             (oldSnapshot.get.eth1Block, oldSnapshot.get.depositContractState)
           else:
-            db.putDepositTreeSnapshot DepositTreeSnapshot(
+            db.putDepositContractSnapshot DepositContractSnapshot(
               eth1Block: depositContractBlockHash,
               blockHeight: depositContractBlockNumber)
             (depositContractBlockHash, default(DepositContractState))

beacon_chain/nimbus_beacon_node.nim

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

beacon_chain/rpc/rest_beacon_api.nim

@@ -133,7 +133,7 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) =
   # https://github.com/ethereum/EIPs/blob/master/EIPS/eip-4881.md
   router.api2(MethodGet, "/eth/v1/beacon/deposit_snapshot") do (
     ) -> RestApiResponse:
-    let snapshot = node.db.getDepositTreeSnapshot().valueOr:
+    let snapshot = node.db.getDepositContractSnapshot().valueOr:
       # This can happen in a very short window after the client is started,
       # but the snapshot record still haven't been upgraded in the database.
       # Returning 404 should be easy to handle for the clients - they just need

beacon_chain/spec/deposit_snapshots.nim

@@ -21,42 +21,44 @@ type
     eth1Block*: Eth2Digest
     depositContractState*: DepositContractState
 
-  DepositTreeSnapshot* = object
+  DepositContractSnapshot* = object
-    ## https://eips.ethereum.org/EIPS/eip-4881
     eth1Block*: Eth2Digest
     depositContractState*: DepositContractState
     blockHeight*: uint64
 
-func toDepositTreeSnapshot*(d: OldDepositContractSnapshot,
+func toDepositContractSnapshot*(
-                            blockHeight: uint64): DepositTreeSnapshot =
+    d: OldDepositContractSnapshot,
-  DepositTreeSnapshot(
+    blockHeight: uint64): DepositContractSnapshot =
+  DepositContractSnapshot(
     eth1Block: d.eth1Block,
     depositContractState: d.depositContractState,
     blockHeight: blockHeight)
 
-func toOldDepositContractSnapshot*(d: DepositTreeSnapshot): OldDepositContractSnapshot =
+func toOldDepositContractSnapshot*(
-  OldDepositContractSnapshot(eth1Block: d.eth1Block,
+    d: DepositContractSnapshot): OldDepositContractSnapshot =
-                             depositContractState: d.depositContractState)
+  OldDepositContractSnapshot(
+    eth1Block: d.eth1Block,
+    depositContractState: d.depositContractState)
 
-template getDepositCountU64*(d: OldDepositContractSnapshot |
+template getDepositCountU64*(
-                                DepositTreeSnapshot): uint64 =
+    d: OldDepositContractSnapshot | DepositContractSnapshot): uint64 =
   depositCountU64(d.depositContractState.deposit_count)
 
-func getDepositRoot*(d: OldDepositContractSnapshot |
+func getDepositRoot*(
-                        DepositTreeSnapshot): Eth2Digest =
+    d: OldDepositContractSnapshot | DepositContractSnapshot): Eth2Digest =
   var merk = DepositsMerkleizer.init(d.depositContractState)
   let hash = merk.getFinalHash()
   # TODO: mixInLength should accept unsigned int instead of int as
   # this right now cuts in half the theoretical number of deposits.
   return mixInLength(hash, int(merk.getChunkCount()))
 
-func isValid*(d: DepositTreeSnapshot, wantedDepositRoot: Eth2Digest): bool =
+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
               d.blockHeight == 0 or
               d.getDepositRoot() != wantedDepositRoot)
 
-func matches*(snapshot: DepositTreeSnapshot, eth1_data: Eth1Data): bool =
+func matches*(snapshot: DepositContractSnapshot, eth1_data: Eth1Data): bool =
   snapshot.getDepositCountU64() == eth1_data.deposit_count and
   snapshot.getDepositRoot() == eth1_data.deposit_root

beacon_chain/trusted_node_sync.nim

@@ -24,8 +24,9 @@ const
   largeRequestsTimeout = 60.seconds # Downloading large items such as states.
   smallRequestsTimeout = 30.seconds # Downloading smaller items such as blocks and deposit snapshots.
 
-proc fetchDepositSnapshot(client: RestClientRef):
+proc fetchDepositSnapshot(
-                          Future[Result[DepositTreeSnapshot, string]] {.async.} =
+    client: RestClientRef
+): Future[Result[DepositContractSnapshot, string]] {.async.} =
   let resp = try:
     awaitWithTimeout(client.getDepositSnapshot(), smallRequestsTimeout):
       return err "Fetching /eth/v1/beacon/deposit_snapshot timed out"
@@ -33,7 +34,7 @@ proc fetchDepositSnapshot(client: RestClientRef):
     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 = DepositTreeSnapshot(
+  let snapshot = DepositContractSnapshot(
     eth1Block: data.execution_block_hash,
     depositContractState: DepositContractState(
       branch: data.finalized,
@@ -393,7 +394,7 @@ proc doTrustedNodeSync*(
           info "Writing deposit contracts snapshot",
                depositRoot = depositSnapshot.get.getDepositRoot(),
                depositCount = depositSnapshot.get.getDepositCountU64
-          db.putDepositTreeSnapshot(depositSnapshot.get)
+          db.putDepositContractSnapshot(depositSnapshot.get)
         else:
           warn "The downloaded deposit snapshot does not agree with the downloaded state"
       else:

ncli/ncli_testnet.nim

@@ -347,15 +347,16 @@ func `as`(blk: BlockObject, T: type deneb.ExecutionPayloadHeader): T =
     blob_gas_used: uint64 blk.blobGasUsed.getOrDefault(),
     excess_blob_gas: uint64 blk.excessBlobGas.getOrDefault())
 
-func createDepositTreeSnapshot(deposits: seq[DepositData],
+func createDepositContractSnapshot(
-                               blockHash: Eth2Digest,
+    deposits: seq[DepositData],
-                               blockHeight: uint64): DepositTreeSnapshot =
+    blockHash: Eth2Digest,
+    blockHeight: uint64): DepositContractSnapshot =
   var merkleizer = DepositsMerkleizer.init()
   for i, deposit in deposits:
     let htr = hash_tree_root(deposit)
     merkleizer.addChunk(htr.data)
 
-  DepositTreeSnapshot(
+  DepositContractSnapshot(
     eth1Block: blockHash,
     depositContractState: merkleizer.toDepositContractState,
     blockHeight: blockHeight)
@@ -473,7 +474,7 @@ proc doCreateTestnet*(config: CliConfig,
 
     SSZ.saveFile(
       config.outputDepositTreeSnapshot.string,
-      createDepositTreeSnapshot(
+      createDepositContractSnapshot(
         deposits,
         genesisExecutionPayloadHeader.block_hash,
         genesisExecutionPayloadHeader.block_number))

research/block_sim.nim

@@ -160,7 +160,7 @@ cli do(slots = SLOTS_PER_EPOCH * 7,
   defer: db.close()
 
   ChainDAGRef.preInit(db, genesisState[])
-  db.putDepositTreeSnapshot(depositTreeSnapshot)
+  db.putDepositContractSnapshot(depositTreeSnapshot)
 
   let rng = HmacDrbgContext.new()
   var

research/simutils.nim

@@ -16,7 +16,7 @@ from std/stats import RunningStat, mean, push, standardDeviationS
 from std/strformat import `&`
 from std/times import cpuTime
 from ../beacon_chain/filepath import secureCreatePath
-from ../beacon_chain/spec/deposit_snapshots import DepositTreeSnapshot
+from ../beacon_chain/spec/deposit_snapshots import DepositContractSnapshot
 
 template withTimer*(stats: var RunningStat, body: untyped) =
   # TODO unify timing somehow
@@ -63,8 +63,9 @@ func getSimulationConfig*(): RuntimeConfig {.compileTime.} =
   cfg.DENEB_FORK_EPOCH = 2.Epoch
   cfg
 
-proc loadGenesis*(validators: Natural, validate: bool):
+proc loadGenesis*(
-                 (ref ForkedHashedBeaconState, DepositTreeSnapshot) =
+    validators: Natural,
+    validate: bool): (ref ForkedHashedBeaconState, DepositContractSnapshot) =
   const genesisDir = "test_sim"
   if (let res = secureCreatePath(genesisDir); res.isErr):
     fatal "Could not create directory",
@@ -110,7 +111,7 @@ proc loadGenesis*(validators: Natural, validate: bool):
 
       let contractSnapshot =
         try:
-          SSZ.loadFile(contractSnapshotFn, DepositTreeSnapshot)
+          SSZ.loadFile(contractSnapshotFn, DepositContractSnapshot)
         except IOError as exc:
           fatal "Deposit contract snapshot failed to load",
             fileName = contractSnapshotFn, exc = exc.msg
@@ -133,7 +134,7 @@ proc loadGenesis*(validators: Natural, validate: bool):
     var merkleizer = init DepositsMerkleizer
     for d in deposits:
       merkleizer.addChunk hash_tree_root(d).data
-    let contractSnapshot = DepositTreeSnapshot(
+    let contractSnapshot = DepositContractSnapshot(
       depositContractState: merkleizer.toDepositContractState)
 
     let res = (ref ForkedHashedBeaconState)(

scripts/test_merge_node.nim

@@ -55,7 +55,7 @@ proc run() {.async.} =
   let
     elManager = newClone ELManager.init(
       defaultRuntimeConfig, db = nil, nil, @[paramStr(1)],
-      none(DepositTreeSnapshot), none(Eth1Network), false,
+      none(DepositContractSnapshot), none(Eth1Network), false,
       some readJwtSecret(paramStr(2)).get)
 
   try:

tests/test_deposit_snapshots.nim

@@ -25,15 +25,16 @@ template databaseRoot: string = getTempDir().joinPath(ROOT)
 template key1: array[1, byte] = [byte(kOldDepositContractSnapshot)]
 
 type
-  DepositSnapshotUpgradeProc = proc(old: OldDepositContractSnapshot): DepositTreeSnapshot
+  DepositSnapshotUpgradeProc = proc(
-                                   {.gcsafe, raises: [].}
+      old: OldDepositContractSnapshot
+  ): DepositContractSnapshot {.gcsafe, raises: [].}
 
 proc ifNecessaryMigrateDCS(db: BeaconChainDB,
                            upgradeProc: DepositSnapshotUpgradeProc) =
-  if not db.hasDepositTreeSnapshot():
+  if not db.hasDepositContractSnapshot():
     let oldSnapshot = db.getUpgradableDepositSnapshot()
     if oldSnapshot.isSome:
-      db.putDepositTreeSnapshot upgradeProc(oldSnapshot.get)
+      db.putDepositContractSnapshot upgradeProc(oldSnapshot.get)
 
 # Hexlified copy of
 # eth2-networks/shared/mainnet/genesis_deposit_contract_snapshot.ssz
@@ -84,7 +85,8 @@ proc fixture1() =
   kv.put(key1, compressed).expect("")
   db.close()
 
-proc inspectDCS(snapshot: OldDepositContractSnapshot | DepositTreeSnapshot) =
+proc inspectDCS(
+    snapshot: OldDepositContractSnapshot | DepositContractSnapshot) =
   ## Inspects a DCS and checks if all of its data corresponds to
   ## what's encoded in ds1.
   const zero = toDigest("0000000000000000000000000000000000000000000000000000000000000000")
@@ -118,11 +120,11 @@ proc inspectDCS(snapshot: OldDepositContractSnapshot | DepositTreeSnapshot) =
   # Check deposit root.
   check(snapshot.getDepositRoot == root)
 
-proc inspectDCS(snapshot: DepositTreeSnapshot, wantedBlockHeight: uint64) =
+proc inspectDCS(snapshot: DepositContractSnapshot, wantedBlockHeight: uint64) =
   inspectDCS(snapshot)
   check(snapshot.blockHeight == wantedBlockHeight)
 
-suite "DepositTreeSnapshot":
+suite "DepositContractSnapshot":
   setup:
     randomize()
 
@@ -139,21 +141,22 @@ suite "DepositTreeSnapshot":
     # Start with a fresh database.
     removeDir(databaseRoot)
     createDir(databaseRoot)
-    # Make sure there's no DepositTreeSnapshot yet.
+    # Make sure there's no DepositContractSnapshot yet.
     let db = BeaconChainDB.new(databaseRoot, inMemory=false)
-    check(db.getDepositTreeSnapshot().isErr())
+    check(db.getDepositContractSnapshot().isErr())
     # Setup fixture.
     fixture1()
-    # Make sure there's still no DepositTreeSnapshot as
+    # Make sure there's still no DepositContractSnapshot as
-    # BeaconChainDB::getDepositTreeSnapshot() checks only for DCSv2.
+    # BeaconChainDB::getDepositContractSnapshot() checks only for DCSv2.
-    check(db.getDepositTreeSnapshot().isErr())
+    check(db.getDepositContractSnapshot().isErr())
     # Migrate DB.
-    db.ifNecessaryMigrateDCS do (d: OldDepositContractSnapshot) -> DepositTreeSnapshot:
+    db.ifNecessaryMigrateDCS do (
-      d.toDepositTreeSnapshot(11052984)
+        d: OldDepositContractSnapshot) -> DepositContractSnapshot:
+      d.toDepositContractSnapshot(11052984)
     # Make sure now there actually is a snapshot.
-    check(db.getDepositTreeSnapshot().isOk())
+    check(db.getDepositContractSnapshot().isOk())
     # Inspect content.
-    let snapshot = db.getDepositTreeSnapshot().expect("")
+    let snapshot = db.getDepositContractSnapshot().expect("")
     inspectDCS(snapshot, 11052984)
 
   test "depositCount":
@@ -169,7 +172,7 @@ suite "DepositTreeSnapshot":
     var model: OldDepositContractSnapshot
     check(decodeSSZ(ds1, model))
     # Check blockHeight.
-    var dcs = model.toDepositTreeSnapshot(0)
+    var dcs = model.toDepositContractSnapshot(0)
     check(not dcs.isValid(ds1Root))
     dcs.blockHeight = 11052984
     check(dcs.isValid(ds1Root))
@@ -188,5 +191,6 @@ suite "DepositTreeSnapshot":
     for i in 0..len(dcs.depositContractState.deposit_count)-1:
       dcs.depositContractState.deposit_count[i] = 0
     check(not dcs.isValid(ds1Root))
-    dcs.depositContractState.deposit_count = model.depositContractState.deposit_count
+    dcs.depositContractState.deposit_count =
+      model.depositContractState.deposit_count
     check(dcs.isValid(ds1Root))