Witness Generation Bug Fixes (#1981)

* Return slots when verifying witness regardless of code length. * Prevent AccountCache WitnessData codeTouched from being reset. * Default to using {wfNoFlag} for witness flags in tests to allow running with data from before EIP170. * Add additional json files to experimental JSON RPC test. * Use HTTP RPC server in tests. * Add test to check that block witness contains bytecode.
status-im · Jan 24, 2024 · 54644fa · 54644fa
1 parent dbc1ae8
commit 54644fa
Show file tree

Hide file tree

Showing 16 changed files with 111 additions and 41 deletions.
diff --git a/fluffy/network/state/experimental/state_proof_types.nim b/fluffy/network/state/experimental/state_proof_types.nim
@@ -1,5 +1,5 @@
 # Nimbus
-# Copyright (c) 2023 Status Research & Development GmbH
+# Copyright (c) 2023-2024 Status Research & Development GmbH
 # Licensed and distributed under either of
 # * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT).
 # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0).

diff --git a/fluffy/network/state/experimental/state_proof_verification.nim b/fluffy/network/state/experimental/state_proof_verification.nim
@@ -1,5 +1,5 @@
 # Nimbus
-# Copyright (c) 2023 Status Research & Development GmbH
+# Copyright (c) 2023-2024 Status Research & Development GmbH
 # Licensed and distributed under either of
 # * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT).
 # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0).

diff --git a/nimbus/db/ledger/accounts_cache.nim b/nimbus/db/ledger/accounts_cache.nim
@@ -654,7 +654,9 @@ proc getStorageRoot*(ac: AccountsCache, address: EthAddress): Hash256 =
  else: acc.account.storageRoot
 
 func update(wd: var WitnessData, acc: RefAccount) =
- wd.codeTouched = CodeChanged in acc.flags
+ # once the code is touched make sure it doesn't get reset back to false in another update
+ if not wd.codeTouched:
+ wd.codeTouched = CodeChanged in acc.flags or CodeLoaded in acc.flags
 
  if not acc.originalStorage.isNil:
  for k, v in acc.originalStorage:

diff --git a/nimbus/db/ledger/accounts_ledger.nim b/nimbus/db/ledger/accounts_ledger.nim
@@ -657,7 +657,9 @@ proc getStorageRoot*(ac: AccountsLedgerRef, address: EthAddress): Hash256 =
  else: acc.account.storageVid.hash().valueOr: EMPTY_ROOT_HASH
 
 func update(wd: var WitnessData, acc: RefAccount) =
- wd.codeTouched = CodeChanged in acc.flags
+ # once the code is touched make sure it doesn't get reset back to false in another update
+ if not wd.codeTouched:
+ wd.codeTouched = CodeChanged in acc.flags or CodeLoaded in acc.flags
 
  if not acc.originalStorage.isNil:
  for k, v in acc.originalStorage:

diff --git a/nimbus/nimbus.nim b/nimbus/nimbus.nim
@@ -322,7 +322,7 @@ proc localServices(nimbus: NimbusNode, conf: NimbusConf,
  if RpcFlag.Debug in wsFlags:
  setupDebugRpc(com, nimbus.wsRpcServer)
  if RpcFlag.Exp in wsFlags:
- setupExpRpc(com, nimbus.rpcServer)
+ setupExpRpc(com, nimbus.wsRpcServer)
 
  nimbus.wsRpcServer.start()
 

diff --git a/nimbus/rpc.nim b/nimbus/rpc.nim
@@ -1,5 +1,5 @@
 # Nimbus
-# Copyright (c) 2022-2023 Status Research & Development GmbH
+# Copyright (c) 2022-2024 Status Research & Development GmbH
 # Licensed under either of
 # * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE))
 # * MIT license ([LICENSE-MIT](LICENSE-MIT))

diff --git a/nimbus/rpc/experimental.nim b/nimbus/rpc/experimental.nim
@@ -34,12 +34,15 @@ proc getBlockWitness*(
  com: CommonRef,
  blockHeader: BlockHeader,
  statePostExecution: bool): (KeccakHash, BlockWitness, WitnessFlags)
- {.raises: [CatchableError].} =
+ {.raises: [RlpError, BlockNotFound, ValueError, CatchableError].} =
 
  let
  chainDB = com.db
  blockHash = chainDB.getBlockHash(blockHeader.blockNumber)
  blockBody = chainDB.getBlockBody(blockHash)
+ # Initializing the VM will throw a Defect if the state doesn't exist.
+ # 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

diff --git a/stateless/test_block_witness.nim b/stateless/test_block_witness.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:https://www.apache.org/licenses/LICENSE-2.0)
@@ -24,7 +24,7 @@ type
 
 proc testGetBranch(tester: Tester, rootHash: KeccakHash, testStatusIMPL: var TestStatus) =
  var trie = initAccountsTrie(tester.memdb, rootHash)
- let flags = {wfEIP170}
+ let flags = {wfNoFlag}
 
  try:
  var wb = initWitnessBuilder(tester.memdb, rootHash, flags)

diff --git a/stateless/test_fuzz.nim b/stateless/test_fuzz.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:https://www.apache.org/licenses/LICENSE-2.0)
@@ -19,7 +19,7 @@ import
 test:
  var db = newCoreDbRef(LegacyDbMemory)
  try:
- var tb = initTreeBuilder(payload, db, {wfEIP170})
+ var tb = initTreeBuilder(payload, db, {wfNoFlag})
  let root = tb.buildTree()
  except ParsingError, ContractCodeError:
  debugEcho "Error detected ", getCurrentExceptionMsg()
diff --git a/stateless/test_witness_json.nim b/stateless/test_witness_json.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:https://www.apache.org/licenses/LICENSE-2.0)
@@ -132,7 +132,7 @@ proc runTest(filePath, fileName: string) =
  let t = parseTester(filePath, testStatusIMPL)
  var db = newCoreDbRef(LegacyDbMemory)
  try:
- var tb = initTreeBuilder(t.output, db, {wfEIP170})
+ var tb = initTreeBuilder(t.output, db, {wfNoFlag})
  let root = tb.buildTree()
  if t.error:
  check root != t.rootHash
@@ -149,7 +149,7 @@ proc writeFuzzData(filePath, fileName: string) =
 
  # this block below check the parsed json
  var db = newCoreDbRef(LegacyDbMemory)
- var tb = initTreeBuilder(t.output, db, {wfEIP170})
+ var tb = initTreeBuilder(t.output, db, {wfNoFlag})
  discard tb.buildTree()
 
  writeFile(filename, t.output)

diff --git a/stateless/test_witness_keys.nim b/stateless/test_witness_keys.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:https://www.apache.org/licenses/LICENSE-2.0)
@@ -101,14 +101,14 @@ proc runTest(numPairs: int, testStatusIMPL: var TestStatus,
  var mkeys = newMultiKeys(addrs)
  let rootHash = trie.rootHash
 
- var wb = initWitnessBuilder(memDB, rootHash, {wfEIP170})
+ var wb = initWitnessBuilder(memDB, rootHash, {wfNoFlag})
  var witness = wb.buildWitness(mkeys)
  var db = newCoreDbRef(LegacyDbMemory)
  when defined(useInputStream):
  var input = memoryInput(witness)
- var tb = initTreeBuilder(input, db, {wfEIP170})
+ var tb = initTreeBuilder(input, db, {wfNoFlag})
  else:
- var tb = initTreeBuilder(witness, db, {wfEIP170})
+ var tb = initTreeBuilder(witness, db, {wfNoFlag})
  let root = tb.buildTree()
  check root.data == rootHash.data
 
@@ -150,7 +150,7 @@ proc initMultiKeys(keys: openArray[string], storageMode: bool = false): Multikey
 proc parseInvalidInput(payload: openArray[byte]): bool =
  var db = newCoreDbRef(LegacyDbMemory)
  try:
- var tb = initTreeBuilder(payload, db, {wfEIP170})
+ var tb = initTreeBuilder(payload, db, {wfNoFlag})
  discard tb.buildTree()
  except ParsingError, ContractCodeError:
  result = true
@@ -285,10 +285,10 @@ proc witnessKeysMain*() =
  var mkeys = newMultiKeys(addrs)
  let rootHash = trie.rootHash
 
- var wb = initWitnessBuilder(memDB, rootHash, {wfEIP170})
+ var wb = initWitnessBuilder(memDB, rootHash, {wfNoFlag})
  var witness = wb.buildWitness(mkeys)
  var db = newCoreDbRef(LegacyDbMemory)
- var tb = initTreeBuilder(witness, db, {wfEIP170})
+ var tb = initTreeBuilder(witness, db, {wfNoFlag})
  let root = tb.buildTree()
  check root.data == rootHash.data
 

diff --git a/stateless/test_witness_verification.nim b/stateless/test_witness_verification.nim
@@ -55,7 +55,7 @@ proc buildWitness(
  accountsCache = AccountsCache.init(coreDb, emptyRlpHash, true)
  (rootHash, multiKeys) = setupStateDB(genAccounts, accountsCache)
 
- var wb = initWitnessBuilder(coreDb, rootHash, {wfEIP170})
+ var wb = initWitnessBuilder(coreDb, rootHash, {wfNoFlag})
  (rootHash, wb.buildWitness(multiKeys))
 
 proc checkWitnessDataMatchesAccounts(
@@ -80,7 +80,7 @@ proc witnessVerificationMain*() =
  let
  accounts = getGenesisAlloc("tests" / "customgenesis" / file)
  (stateRoot, witness) = buildWitness(accounts)
- verifyResult = verifyWitness(stateRoot, witness, {wfEIP170})
+ verifyResult = verifyWitness(stateRoot, witness, {wfNoFlag})
 
  check verifyResult.isOk()
  checkWitnessDataMatchesAccounts(accounts, verifyResult.get())
@@ -92,7 +92,7 @@ proc witnessVerificationMain*() =
  let
  accounts = getGenesisAlloc("tests" / "customgenesis" / file)
  (_, witness) = buildWitness(accounts)
- verifyResult = verifyWitness(badStateRoot, witness, {wfEIP170})
+ verifyResult = verifyWitness(badStateRoot, witness, {wfNoFlag})
 
  check verifyResult.isErr()
  check verifyResult.error() == "witness stateRoot doesn't match trustedStateRoot"

diff --git a/stateless/tree_from_witness.nim b/stateless/tree_from_witness.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:https://www.apache.org/licenses/LICENSE-2.0)

diff --git a/stateless/witness_verification.nim b/stateless/witness_verification.nim
@@ -38,12 +38,11 @@ proc buildAccountsTableFromKeys(
  else: @[]
  var storage = initTable[UInt256, UInt256]()
 
- if code.len() > 0:
- for slot in key.slots:
- let slotKey = fromBytesBE(UInt256, slot)
- let (slotValue, slotExists) = db.getStorage(key.address, slotKey)
- if slotExists:
- storage[slotKey] = slotValue
+ for slot in key.slots:
+ let slotKey = fromBytesBE(UInt256, slot)
+ let (slotValue, slotExists) = db.getStorage(key.address, slotKey)
+ if slotExists:
+ storage[slotKey] = slotValue
 
  accounts[key.address] = AccountData(
  account: account,

diff --git a/tests/test_configuration.nim b/tests/test_configuration.nim
@@ -1,5 +1,5 @@
 # Nimbus
-# Copyright (c) 2019-2023 Status Research & Development GmbH
+# Copyright (c) 2019-2024 Status Research & Development GmbH
 # Licensed under either of
 # * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or
 # http:https://www.apache.org/licenses/LICENSE-2.0)

diff --git a/tests/test_rpc_experimental_json.nim b/tests/test_rpc_experimental_json.nim
@@ -60,7 +60,7 @@ proc checkAndValidateWitnessAgainstProofs(
  witness: seq[byte],
  proofs: seq[ProofResponse]) =
 
- let verifyWitnessResult = verifyWitness(expectedStateRoot, witness, {wfEIP170})
+ let verifyWitnessResult = verifyWitness(expectedStateRoot, witness, {wfNoFlag})
  check verifyWitnessResult.isOk()
  let witnessData = verifyWitnessResult.value()
 
@@ -100,6 +100,8 @@ 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",
@@ -108,16 +110,56 @@ proc rpcExperimentalJsonMain*() =
  "block46402.json",
  "block47205.json",
  "block47216.json",
- "block48712.json"
- ]
+ "block48712.json",
+ "block48915.json",
+ "block49018.json",
+ "block49439.json",
+ "block49891.json",
+ "block50111.json",
+ "block78458.json",
+ "block81383.json",
+ "block81666.json",
+ "block85858.json",
+ "block146675.json",
+ "block116524.json",
+ "block196647.json",
+ "block226147.json",
+ "block226522.json",
+ "block231501.json",
+ "block243826.json",
+ "block248032.json",
+ "block299804.json",
+ "block420301.json",
+ "block512335.json",
+ "block652148.json",
+ "block668910.json",
+ "block1017395.json",
+ "block1149150.json",
+ "block1155095.json",
+ "block1317742.json",
+ "block1352922.json",
+ "block1368834.json",
+ "block1417555.json",
+ "block1431916.json",
+ "block1487668.json",
+ "block1920000.json",
+ "block1927662.json",
+ "block2463413.json",
+ "block2675000.json",
+ "block2675002.json",
+ "block4370000.json"
+ ]
+
+ let
+ RPC_HOST = "127.0.0.1"
+ RPC_PORT = 0 # let the OS choose a port
 
- let RPC_PORT = 0 # let the OS choose a port
  var
- rpcServer = newRpcSocketServer(["127.0.0.1:" & $RPC_PORT])
- client = newRpcSocketClient()
+ rpcServer = newRpcHttpServerWithParams(initTAddress(RPC_HOST, RPC_PORT))
+ client = newRpcHttpClient()
 
  rpcServer.start()
- waitFor client.connect(rpcServer.localAddress[0])
+ waitFor client.connect(RPC_HOST, rpcServer.localAddress[0].port, secure = false)
 
 
  test "exp_getWitnessByBlockNumber and exp_getProofsByBlockNumber - latest block pre-execution state":
@@ -192,9 +234,31 @@ proc rpcExperimentalJsonMain*() =
  expect JsonRpcError:
  discard await client.exp_getProofsByBlockNumber(blockNum, true)
 
+ test "Contract storage updated - bytecode should exist in witness":
+ for file in importFiles:
+ let
+ (com, parentStateRoot, stateRoot, blockNumber) = importBlockDataFromFile(file)
+ blockNum = blockId(blockNumber.truncate(uint64))
+
+ setupExpRpc(com, rpcServer)
+
+ let
+ witness = await client.exp_getWitnessByBlockNumber(blockNum, false)
+ proofs = await client.exp_getProofsByBlockNumber(blockNum, true)
+ verifyWitnessResult = verifyWitness(parentStateRoot, witness, {wfNoFlag})
+
+ check verifyWitnessResult.isOk()
+ let witnessData = verifyWitnessResult.value()
+
+ for proof in proofs:
+ let address = ethAddr(proof.address)
+ # if the storage was updated on an existing contract
+ if proof.storageProof.len() > 0 and witnessData.contains(address):
+ check witnessData[address].code.len() > 0
+
 
- rpcServer.stop()
- rpcServer.close()
+ waitFor rpcServer.stop()
+ waitFor rpcServer.closeWait()
 
 when isMainModule:
  rpcExperimentalJsonMain()