More witness fixes (#2009)

* Update experimental rpc test to do further validation on proof responses.

* Enable zero value storage slots in the witness cache data so that proofs will be returned when a storage slot is updated to zero. Refactor and simplify implementation of getProofs endpoint.

* Improve test validation.

* Minor fixes and added test to track the changes introduced in every block using a local state.

* Refactor and cleanup test.

* Comments added to test and account cache fixes applied to account ledger.

* Return updated storage slots even when storage is empty and add test to build.

* Fix copyright and remove incorrect depth check during witness building in writeShortRlp assertion.

nimbus/db/ledger/accounts_cache.nim

@@ -1,5 +1,5 @@
 # Nimbus
-# Copyright (c) 2023 Status Research & Development GmbH
+# Copyright (c) 2023-2024 Status Research & Development GmbH
 # Licensed under either of
 #  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or
 #    http://www.apache.org/licenses/LICENSE-2.0)
@@ -664,11 +664,6 @@ func update(wd: var WitnessData, acc: RefAccount) =
       wd.storageKeys.incl k
 
   for k, v in acc.overlayStorage:
-    if v.isZero and k notin wd.storageKeys:
-      continue
-    if v.isZero and k in wd.storageKeys:
-      wd.storageKeys.excl k
-      continue
     wd.storageKeys.incl k
 
 func witnessData(acc: RefAccount): WitnessData =

nimbus/db/ledger/accounts_ledger.nim

@@ -707,11 +707,6 @@ proc update(wd: var WitnessData, acc: AccountRef) =
       wd.storageKeys.incl k
 
   for k, v in acc.overlayStorage:
-    if v.isZero and k notin wd.storageKeys:
-      continue
-    if v.isZero and k in wd.storageKeys:
-      wd.storageKeys.excl k
-      continue
     wd.storageKeys.incl k
 
 proc witnessData(acc: AccountRef): WitnessData =

nimbus/rpc/experimental.nim

@@ -16,14 +16,13 @@ import
   ../[transaction, vm_state, constants, vm_types],
   ../db/state_db,
   rpc_types, rpc_utils,
-  ../core/tx_pool,
   ../common/common,
   ../utils/utils,
   ../beacon/web3_eth_conv,
   ./filters,
   ../core/executor/process_block,
   ../db/ledger,
-  ../../stateless/[witness_verification, witness_types],
+  ../../stateless/[witness_verification, witness_types, multi_keys],
   ./p2p
 
 type
@@ -33,7 +32,7 @@ type
 proc getBlockWitness*(
     com: CommonRef,
     blockHeader: BlockHeader,
-    statePostExecution: bool): (KeccakHash, BlockWitness, WitnessFlags)
+    statePostExecution: bool): (MultikeysRef, BlockWitness)
     {.raises: [RlpError, BlockNotFound, ValueError, CatchableError].} =
 
   let
@@ -44,7 +43,6 @@ proc getBlockWitness*(
     # Once we enable pruning we will need to check if the block state has been pruned
     # before trying to initialize the VM as we do here.
     vmState = BaseVMState.new(blockHeader, com)
-    flags = if vmState.fork >= FKSpurious: {wfEIP170} else: {}
 
   vmState.generateWitness = true # Enable saving witness data
   vmState.com.hardForkTransition(blockHeader)
@@ -59,34 +57,27 @@ proc getBlockWitness*(
   let mkeys = vmState.stateDB.makeMultiKeys()
 
   if statePostExecution:
-    result = (vmState.stateDB.rootHash, vmState.buildWitness(mkeys), flags)
+    result = (mkeys, vmState.buildWitness(mkeys))
   else:
-    # Reset state to what it was before executing the block of transactions
+    # Use the initial state from prior to executing the block of transactions
     let initialState = BaseVMState.new(blockHeader, com)
-    result = (initialState.stateDB.rootHash, initialState.buildWitness(mkeys), flags)
+    result = (mkeys, initialState.buildWitness(mkeys))
 
   dbTx.rollback()
 
-
 proc getBlockProofs*(
     accDB: ReadOnlyStateDB,
-    witnessRoot: KeccakHash,
+    mkeys: MultikeysRef): seq[ProofResponse] {.raises: [RlpError].} =
-    witness: BlockWitness,
-    flags: WitnessFlags): seq[ProofResponse] {.raises: [RlpError].} =
 
-  if witness.len() == 0:
+  var blockProofs = newSeq[ProofResponse]()
-    return @[]
 
-  let verifyWitnessResult = verifyWitness(witnessRoot, witness, flags)
+  for keyData in mkeys.keys:
-  doAssert verifyWitnessResult.isOk()
+    let address = keyData.address
+    var slots = newSeq[UInt256]()
 
-  var blockProofs = newSeqOfCap[ProofResponse](verifyWitnessResult.value().len())
+    if not keyData.storageKeys.isNil and accDB.accountExists(address):
-
+      for slotData in keyData.storageKeys.keys:
-  for address, account in verifyWitnessResult.value():
+        slots.add(fromBytesBE(UInt256, slotData.storageSlot))
-    var slots = newSeqOfCap[UInt256](account.storage.len())
-
-    for slotKey, _ in account.storage:
-      slots.add(slotKey)
 
     blockProofs.add(getProof(accDB, address, slots))
 
@@ -111,7 +102,7 @@ proc setupExpRpc*(com: CommonRef, server: RpcServer) =
 
     let
       blockHeader = chainDB.headerFromTag(quantityTag)
-      (_, witness, _) = getBlockWitness(com, blockHeader, statePostExecution)
+      (_, witness) = getBlockWitness(com, blockHeader, statePostExecution)
 
     return witness
 
@@ -124,11 +115,11 @@ proc setupExpRpc*(com: CommonRef, server: RpcServer) =
 
     let
       blockHeader = chainDB.headerFromTag(quantityTag)
-      (witnessRoot, witness, flags) = getBlockWitness(com, blockHeader, statePostExecution)
+      (mkeys, _) = getBlockWitness(com, blockHeader, statePostExecution)
 
     let accDB = if statePostExecution:
       getStateDB(blockHeader)
     else:
       getStateDB(chainDB.getBlockHeader(blockHeader.parentHash))
 
-    return getBlockProofs(accDB, witnessRoot, witness, flags)
+    return getBlockProofs(accDB, mkeys)

stateless/witness_from_tree.nim

@@ -1,5 +1,5 @@
 # Nimbus
-# Copyright (c) 2020-2023 Status Research & Development GmbH
+# Copyright (c) 2020-2024 Status Research & Development GmbH
 # Licensed under either of
 #  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or
 #    http://www.apache.org/licenses/LICENSE-2.0)
@@ -166,7 +166,7 @@ proc writeHashNode(wb: var WitnessBuilder, node: openArray[byte], depth: int, st
 
 proc writeShortRlp(wb: var WitnessBuilder, node: openArray[byte], depth: int, storageMode: bool)
     {.gcsafe, raises: [IOError].} =
-  doAssert(node.len < 32 and depth >= 9 and storageMode)
+  doAssert(node.len < 32 and storageMode)
   wb.writeByte(HashNodeType)
   wb.writeByte(ShortRlpPrefix)
   wb.writeByte(node.len)

tests/test_blockchain_json.nim

@@ -205,10 +205,7 @@ proc testGetBlockWitness(chain: ChainRef, parentHeader, currentHeader: BlockHead
   if currentStateRoot != currentHeader.stateRoot:
     raise newException(ValidationError, "Expected currentStateRoot == currentHeader.stateRoot")
 
-  # run getBlockWitness and check that the witnessRoot matches the parent stateRoot
+  let (mkeys, witness) = getBlockWitness(chain.com, currentHeader, false)
-  let (witnessRoot, witness, flags) = getBlockWitness(chain.com, currentHeader, false)
-  if witnessRoot != parentHeader.stateRoot:
-    raise newException(ValidationError, "Expected witnessRoot == parentHeader.stateRoot")
 
   # check that the vmstate hasn't changed after call to getBlockWitness
   if chain.vmstate.stateDB.rootHash != currentHeader.stateRoot:
@@ -218,14 +215,14 @@ proc testGetBlockWitness(chain: ChainRef, parentHeader, currentHeader: BlockHead
   if witness.len() > 0:
     let fgs = if chain.vmState.fork >= FKSpurious: {wfEIP170} else: {}
     var tb = initTreeBuilder(witness, chain.com.db, fgs)
-    let treeRoot = tb.buildTree()
+    let witnessRoot = tb.buildTree()
-    if treeRoot != witnessRoot:
+    if witnessRoot != parentHeader.stateRoot:
-      raise newException(ValidationError, "Expected treeRoot == witnessRoot")
+      raise newException(ValidationError, "Expected witnessRoot == parentHeader.stateRoot")
 
-  # use the witness to build the block proofs
+  # use the MultikeysRef to build the block proofs
   let
     ac = newAccountStateDB(chain.com.db, currentHeader.stateRoot, chain.com.pruneTrie)
-    blockProofs = getBlockProofs(state_db.ReadOnlyStateDB(ac), witnessRoot, witness, flags)
+    blockProofs = getBlockProofs(state_db.ReadOnlyStateDB(ac), mkeys)
   if witness.len() == 0 and blockProofs.len() != 0:
     raise newException(ValidationError, "Expected blockProofs.len() == 0")
 

tests/test_rpc_experimental_json.nim

@@ -13,6 +13,7 @@ import
   ../nimbus/core/chain,
   ../nimbus/common/common,
   ../nimbus/rpc,
+  ../../nimbus/db/[ledger, core_db],
   ../stateless/[witness_verification, witness_types],
   ./rpc/experimental_rpc_client
 
@@ -56,11 +57,16 @@ proc importBlockData(node: JsonNode): (CommonRef, Hash256, Hash256, UInt256) {.
   return (com, parent.stateRoot, header.stateRoot, blockNumber)
 
 proc checkAndValidateWitnessAgainstProofs(
+    db: CoreDbRef,
+    parentStateRoot: KeccakHash,
     expectedStateRoot: KeccakHash,
     witness: seq[byte],
     proofs: seq[ProofResponse]) =
 
-  let verifyWitnessResult = verifyWitness(expectedStateRoot, witness, {wfNoFlag})
+  let
+    stateDB = AccountsCache.init(db, parentStateRoot, false)
+    verifyWitnessResult = verifyWitness(expectedStateRoot, witness, {wfNoFlag})
+
   check verifyWitnessResult.isOk()
   let witnessData = verifyWitnessResult.value()
 
@@ -68,25 +74,60 @@ proc checkAndValidateWitnessAgainstProofs(
     witness.len() > 0
     proofs.len() > 0
     witnessData.len() > 0
-    witnessData.len() == proofs.len()
 
   for proof in proofs:
     let
       address = proof.address.ethAddr()
+      balance = proof.balance
+      nonce = proof.nonce.uint64
+      codeHash = proof.codeHash.toHash256()
+      storageHash = proof.storageHash.toHash256()
       slotProofs = proof.storageProof
+
+    if witnessData.contains(address):
+      let
         storageData = witnessData[address].storage
+        code = witnessData[address].code
 
       check:
-      witnessData.contains(address)
+        witnessData[address].account.balance == balance
-      witnessData[address].account.balance == proof.balance
+        witnessData[address].account.nonce == nonce
-      witnessData[address].account.nonce == proof.nonce.uint64
+        witnessData[address].account.codeHash == codeHash
-      witnessData[address].account.codeHash == proof.codeHash.toHash256()
-      storageData.len() == slotProofs.len()
 
       for slotProof in slotProofs:
-      check:
+        if storageData.contains(slotProof.key):
-        storageData.contains(slotProof.key)
+          check storageData[slotProof.key] == slotProof.value
-        storageData[slotProof.key] == slotProof.value
+
+      if code.len() > 0:
+        stateDB.setCode(address, code)
+
+    stateDB.setBalance(address, balance)
+    stateDB.setNonce(address, nonce)
+
+    for slotProof in slotProofs:
+      stateDB.setStorage(address, slotProof.key, slotProof.value)
+
+    # the account doesn't exist due to a self destruct
+    if codeHash == ZERO_HASH256 and storageHash == ZERO_HASH256:
+      stateDB.deleteAccount(address)
+
+    stateDB.persist()
+
+    check stateDB.getBalance(address) == balance
+    check stateDB.getNonce(address) == nonce
+
+    if codeHash == ZERO_HASH256 or codeHash == EMPTY_SHA3:
+      check stateDB.getCode(address).len() == 0
+      check stateDB.getCodeHash(address) == EMPTY_SHA3
+    else:
+      check stateDB.getCodeHash(address) == codeHash
+
+    if storageHash == ZERO_HASH256 or storageHash == EMPTY_ROOT_HASH:
+      check stateDB.getStorageRoot(address) == EMPTY_ROOT_HASH
+    else:
+      check stateDB.getStorageRoot(address) == storageHash
+
+  check stateDB.rootHash == expectedStateRoot
 
 proc importBlockDataFromFile(file: string): (CommonRef, Hash256, Hash256, UInt256) {. raises: [].} =
   try:
@@ -100,8 +141,6 @@ proc rpcExperimentalJsonMain*() =
 
   suite "rpc experimental json tests":
 
-    # The commented out json files below are failing due to hitting the RPC client and
-    # server defaultMaxRequestLength. Currently the limit is set to around 128kb.
     let importFiles = [
       "block97.json",
       "block98.json",
@@ -174,11 +213,11 @@ proc rpcExperimentalJsonMain*() =
           witness = await client.exp_getWitnessByBlockNumber("latest", false)
           proofs = await client.exp_getProofsByBlockNumber("latest", false)
 
-        checkAndValidateWitnessAgainstProofs(parentStateRoot, witness, proofs)
+        checkAndValidateWitnessAgainstProofs(com.db, parentStateRoot, parentStateRoot, witness, proofs)
 
     test "exp_getWitnessByBlockNumber and exp_getProofsByBlockNumber - latest block post-execution state":
       for file in importFiles:
-        let (com, _, stateRoot, _) = importBlockDataFromFile(file)
+        let (com, parentStateRoot, stateRoot, _) = importBlockDataFromFile(file)
 
         setupExpRpc(com, rpcServer)
 
@@ -186,7 +225,7 @@ proc rpcExperimentalJsonMain*() =
           witness = await client.exp_getWitnessByBlockNumber("latest", true)
           proofs = await client.exp_getProofsByBlockNumber("latest", true)
 
-        checkAndValidateWitnessAgainstProofs(stateRoot, witness, proofs)
+        checkAndValidateWitnessAgainstProofs(com.db, parentStateRoot, stateRoot, witness, proofs)
 
     test "exp_getWitnessByBlockNumber and exp_getProofsByBlockNumber - block by number pre-execution state":
       for file in importFiles:
@@ -200,12 +239,12 @@ proc rpcExperimentalJsonMain*() =
           witness = await client.exp_getWitnessByBlockNumber(blockNum, false)
           proofs = await client.exp_getProofsByBlockNumber(blockNum, false)
 
-        checkAndValidateWitnessAgainstProofs(parentStateRoot, witness, proofs)
+        checkAndValidateWitnessAgainstProofs(com.db, parentStateRoot, parentStateRoot, witness, proofs)
 
     test "exp_getWitnessByBlockNumber and exp_getProofsByBlockNumber - block by number post-execution state":
       for file in importFiles:
         let
-          (com, _, stateRoot, blockNumber) = importBlockDataFromFile(file)
+          (com, parentStateRoot, stateRoot, blockNumber) = importBlockDataFromFile(file)
           blockNum = blockId(blockNumber.truncate(uint64))
 
         setupExpRpc(com, rpcServer)
@@ -214,7 +253,7 @@ proc rpcExperimentalJsonMain*() =
           witness = await client.exp_getWitnessByBlockNumber(blockNum, true)
           proofs = await client.exp_getProofsByBlockNumber(blockNum, true)
 
-        checkAndValidateWitnessAgainstProofs(stateRoot, witness, proofs)
+        checkAndValidateWitnessAgainstProofs(com.db, parentStateRoot, stateRoot, witness, proofs)
 
     test "exp_getWitnessByBlockNumber and exp_getProofsByBlockNumber - block by number that doesn't exist":
       for file in importFiles:
@@ -254,11 +293,10 @@ proc rpcExperimentalJsonMain*() =
 
         for proof in proofs:
           let address = ethAddr(proof.address)
-          # if the storage was updated on an existing contract
+          # if the storage was read or updated on an existing contract
           if proof.storageProof.len() > 0 and witnessData.contains(address):
             check witnessData[address].code.len() > 0
 
-
     waitFor rpcServer.stop()
     waitFor rpcServer.closeWait()
 

tests/test_rpc_getproofs_track_state_changes.nim

@@ -0,0 +1,174 @@
+# Nimbus
+# Copyright (c) 2024 Status Research & Development GmbH
+# Licensed under either of
+#  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0)
+#  * MIT license ([LICENSE-MIT](LICENSE-MIT) or http://opensource.org/licenses/MIT)
+# at your option. This file may not be copied, modified, or distributed except according to those terms.
+
+# This test is intended to be run manually against a running instance of Nimbus-Eth1
+# so it should not be added to the test runner or to the CI test suite.
+#
+# It uses the exp_getProofsByBlockNumber endpoint to get the list of state updates
+# for each block, it then applies these updates against a local test state and
+# then checks the state root against the expected state root which is pulled from
+# the block header. The local test state is persisted to disk so that the data can
+# be re-used between separate test runs. The default database directory is the
+# current directory but this can be changed by setting the DATABASE_PATH const below.
+#
+# To run the test:
+# 1. Sync Nimbus up to or past the block number/s that you wish to test against.
+#    You can use the premix persist tool to do this if Nimbus is not able to sync.
+# 2. Start Nimbus with the http RPC-API enabled with the 'eth' and 'exp' namespaces
+#    turned on using this command: build/nimbus --rpc --rpc-api=eth,exp
+# 3. Start the test.
+
+import
+  std/tables,
+  unittest2,
+  web3/eth_api,
+  json_rpc/rpcclient,
+  stew/byteutils,
+  ../nimbus/core/chain,
+  ../nimbus/common/common,
+  ../nimbus/rpc,
+  ../../nimbus/db/[core_db, core_db/persistent, state_db/base],
+  ../stateless/[witness_verification, witness_types],
+  ./rpc/experimental_rpc_client
+
+const
+  RPC_HOST = "127.0.0.1"
+  RPC_PORT = Port(8545)
+  DATABASE_PATH = "."
+  START_BLOCK = 330_000
+  END_BLOCK = 1_000_000
+
+type
+  Hash256 = eth_types.Hash256
+
+template ethAddr*(x: Address): EthAddress =
+  EthAddress x
+
+template toHash256(hash: untyped): Hash256 =
+  fromHex(Hash256, hash.toHex())
+
+proc updateStateUsingProofsAndCheckStateRoot(
+    stateDB: AccountStateDB,
+    expectedStateRoot: Hash256,
+    witness: seq[byte],
+    proofs: seq[ProofResponse]) =
+
+  let verifyWitnessResult = verifyWitness(expectedStateRoot, witness, {wfNoFlag})
+  check verifyWitnessResult.isOk()
+  let witnessData = verifyWitnessResult.value()
+
+  check:
+    witness.len() > 0
+    proofs.len() > 0
+    witnessData.len() > 0
+
+  for proof in proofs:
+    let
+      address = proof.address.ethAddr()
+      balance = proof.balance
+      nonce = proof.nonce.uint64
+      codeHash = proof.codeHash.toHash256()
+      storageHash = proof.storageHash.toHash256()
+      slotProofs = proof.storageProof
+
+    if witnessData.contains(address):
+
+      let
+        storageData = witnessData[address].storage
+        code = witnessData[address].code
+
+      check:
+        witnessData[address].account.balance == balance
+        witnessData[address].account.nonce == nonce
+        witnessData[address].account.codeHash == codeHash
+
+      for slotProof in slotProofs:
+        if storageData.contains(slotProof.key):
+          check storageData[slotProof.key] == slotProof.value
+
+      if code.len() > 0:
+        stateDB.setCode(address, code)
+
+    if (balance == 0 and nonce == 0 and codeHash == ZERO_HASH256 and storageHash == ZERO_HASH256):
+      # Account doesn't exist:
+      # The account was deleted due to a self destruct and the data no longer exists in the state.
+      # The RPC API correctly returns zeroed values in this scenario which is the same behavior
+      # implemented by geth.
+      stateDB.setCode(address, @[])
+      stateDB.clearStorage(address)
+      stateDB.deleteAccount(address)
+    elif (balance == 0 and nonce == 0 and codeHash == EMPTY_SHA3 and storageHash == EMPTY_ROOT_HASH):
+      # Account exists but is empty:
+      # The account was deleted due to a self destruct or the storage was cleared/set to zero
+      # and the bytecode is empty.
+      # The RPC API correctly returns codeHash == EMPTY_SHA3 and storageHash == EMPTY_ROOT_HASH
+      # in this scenario which is the same behavior implemented by geth.
+      stateDB.setCode(address, @[])
+      stateDB.clearStorage(address)
+      stateDB.setBalance(address, 0.u256)
+      stateDB.setNonce(address, 0)
+    else:
+      # Account exists and is not empty:
+      stateDB.setBalance(address, balance)
+      stateDB.setNonce(address, nonce)
+      for slotProof in slotProofs:
+        stateDB.setStorage(address, slotProof.key, slotProof.value)
+
+    check stateDB.getBalance(address) == balance
+    check stateDB.getNonce(address) == nonce
+
+    if codeHash == ZERO_HASH256 or codeHash == EMPTY_SHA3:
+      check stateDB.getCode(address).len() == 0
+      check stateDB.getCodeHash(address) == EMPTY_SHA3
+    else:
+      check stateDB.getCodeHash(address) == codeHash
+
+    if storageHash == ZERO_HASH256 or storageHash == EMPTY_ROOT_HASH:
+      check stateDB.getStorageRoot(address) == EMPTY_ROOT_HASH
+    else:
+      check stateDB.getStorageRoot(address) == storageHash
+
+  check stateDB.rootHash == expectedStateRoot
+
+proc rpcGetProofsTrackStateChangesMain*() =
+
+  suite "rpc getProofs track state changes tests":
+
+    let client = newRpcHttpClient()
+    waitFor client.connect(RPC_HOST, RPC_PORT, secure = false)
+
+    test "Test tracking the changes introduced in every block":
+
+      let com = CommonRef.new(newCoreDbRef(LegacyDbPersistent, DATABASE_PATH), false)
+      com.initializeEmptyDb()
+      com.db.compensateLegacySetup()
+
+      let
+        blockHeader = waitFor client.eth_getBlockByNumber(blockId(START_BLOCK), false)
+        stateDB = newAccountStateDB(com.db, blockHeader.stateRoot.toHash256(), false)
+
+      for i in START_BLOCK..END_BLOCK:
+        let
+          blockNum = blockId(i.uint64)
+          blockHeader: BlockObject = waitFor client.eth_getBlockByNumber(blockNum, false)
+          witness = waitFor client.exp_getWitnessByBlockNumber(blockNum, true)
+          proofs = waitFor client.exp_getProofsByBlockNumber(blockNum, true)
+
+        updateStateUsingProofsAndCheckStateRoot(
+            stateDB,
+            blockHeader.stateRoot.toHash256(),
+            witness,
+            proofs)
+
+        if i mod 1000 == 0:
+          echo "Block number: ", i
+          echo "Expected block stateRoot: ", blockHeader.stateRoot
+          echo "Actual block stateRoot: ", stateDB.rootHash
+          doAssert blockHeader.stateRoot.toHash256() == stateDB.rootHash
+
+when isMainModule:
+  rpcGetProofsTrackStateChangesMain()

tests/test_tools_build.nim

@@ -1,5 +1,5 @@
 # nimbus
-# Copyright (c) 2018-2023 Status Research & Development GmbH
+# Copyright (c) 2018-2024 Status Research & Development GmbH
 # Licensed and distributed under either of
 #   * MIT license: [LICENSE-MIT](LICENSE-MIT) or http://opensource.org/licenses/MIT
 #   * Apache License, Version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0)
@@ -28,4 +28,5 @@ import
   ../tools/t8n/t8n,
   ../tools/t8n/t8n_test,
   ../tools/evmstate/evmstate,
-  ../tools/evmstate/evmstate_test
+  ../tools/evmstate/evmstate_test,
+  ./test_rpc_getproofs_track_state_changes