From c775c906a200cd41de7be170d06511ffbabb37e4 Mon Sep 17 00:00:00 2001 From: andri lim Date: Fri, 5 Jul 2024 17:15:48 +0700 Subject: [PATCH] Fix LedgerRef storage iterator and add test (#2458) --- nimbus/db/aristo/aristo_nearby.nim | 4 +- nimbus/db/core_db/base_iterators.nim | 2 +- nimbus/db/ledger/accounts_ledger.nim | 30 +++++----- tests/test_ledger.nim | 83 +++++++++++++++++++++++++--- 4 files changed, 92 insertions(+), 27 deletions(-) diff --git a/nimbus/db/aristo/aristo_nearby.nim b/nimbus/db/aristo/aristo_nearby.nim index 83e46f66e..bb578d6b1 100644 --- a/nimbus/db/aristo/aristo_nearby.nim +++ b/nimbus/db/aristo/aristo_nearby.nim @@ -474,14 +474,14 @@ iterator rightPairsStorage*( db: AristoDbRef; # Database layer accPath: Hash256; # Account the storage data belong to start = low(PathID); # Before or at first value - ): (PathID,Blob) = + ): (PathID,UInt256) = ## Variant of `rightPairs()` for a storage tree block body: let stoID = db.fetchStorageID(accPath).valueOr: break body if stoID.isValid: for (lty,pyl) in db.rightPairs LeafTie(root: stoID, path: start): - yield (lty.path, pyl.rawBlob) + yield (lty.path, pyl.stoData) # ---------------- diff --git a/nimbus/db/core_db/base_iterators.nim b/nimbus/db/core_db/base_iterators.nim index ea1a520af..19f6aadbc 100644 --- a/nimbus/db/core_db/base_iterators.nim +++ b/nimbus/db/core_db/base_iterators.nim @@ -75,7 +75,7 @@ iterator pairs*(mpt: CoreDbMptRef): (Blob, Blob) = raiseAssert: "Unsupported database type: " & $mpt.dbType mpt.ifTrackNewApi: debug newApiTxt, api, elapsed -iterator slotPairs*(acc: CoreDbAccRef; accPath: Hash256): (Blob, Blob) = +iterator slotPairs*(acc: CoreDbAccRef; accPath: Hash256): (Blob, UInt256) = ## Trie traversal, only supported for `CoreDbMptRef` ## acc.setTrackNewApi AccSlotPairsIt diff --git a/nimbus/db/ledger/accounts_ledger.nim b/nimbus/db/ledger/accounts_ledger.nim index dde87f17e..08510cd05 100644 --- a/nimbus/db/ledger/accounts_ledger.nim +++ b/nimbus/db/ledger/accounts_ledger.nim @@ -13,14 +13,15 @@ import std/[tables, hashes, sets, typetraits], chronicles, - eth/[common, rlp], + eth/common, results, stew/keyed_queue, ../../stateless/multi_keys, "../.."/[constants, utils/utils], ../access_list as ac_access_list, ../../evm/code_bytes, - ".."/[core_db, storage_types, transient_storage] + ".."/[core_db, storage_types, transient_storage], + ../aristo/aristo_blobify export code_bytes @@ -142,12 +143,6 @@ proc resetCoreDbAccount(ac: AccountsLedgerRef, acc: AccountRef) = acc.statement.balance = emptyEthAccount.balance acc.statement.codeHash = emptyEthAccount.codeHash -template noRlpException(info: static[string]; code: untyped) = - try: - code - except RlpError as e: - raiseAssert info & ", name=\"" & $e.name & "\", msg=\"" & e.msg & "\"" - # The AccountsLedgerRef is modeled after TrieDatabase for it's transaction style proc init*(x: typedesc[AccountsLedgerRef], db: CoreDbRef, root: KeccakHash): AccountsLedgerRef = @@ -384,7 +379,7 @@ proc persistStorage(acc: AccountRef, ac: AccountsLedgerRef) = discard let key = slot.toBytesBE.keccakHash.data.slotHashToSlotKey - rc = ac.kvt.put(key.toOpenArray, rlp.encode(slot)) + rc = ac.kvt.put(key.toOpenArray, blobify(slot).data) if rc.isErr: warn logTxt "persistStorage()", slot, error=($$rc.error) @@ -727,13 +722,16 @@ iterator storage*( ): (UInt256, UInt256) = # beware that if the account not persisted, # the storage root will not be updated - noRlpException "storage()": - for (slotHash, value) in ac.ledger.slotPairs eAddr.toAccountKey: - let rc = ac.kvt.get(slotHashToSlotKey(slotHash).toOpenArray) - if rc.isErr: - warn logTxt "storage()", slotHash, error=($$rc.error) - else: - yield (rlp.decode(rc.value, UInt256), rlp.decode(value, UInt256)) + for (slotHash, value) in ac.ledger.slotPairs eAddr.toAccountKey: + let rc = ac.kvt.get(slotHashToSlotKey(slotHash).toOpenArray) + if rc.isErr: + warn logTxt "storage()", slotHash, error=($$rc.error) + continue + let r = deblobify(rc.value, UInt256) + if r.isErr: + warn logTxt "storage.deblobify", slotHash, msg=r.error + continue + yield (r.value, value) iterator cachedStorage*(ac: AccountsLedgerRef, address: EthAddress): (UInt256, UInt256) = let acc = ac.getAccount(address, false) diff --git a/tests/test_ledger.nim b/tests/test_ledger.nim index 62fbc4f34..a54247519 100644 --- a/tests/test_ledger.nim +++ b/tests/test_ledger.nim @@ -28,7 +28,9 @@ import const genesisFile = "tests/customgenesis/cancun123.json" hexPrivKey = "af1a9be9f1a54421cac82943820a0fe0f601bb5f4f6d0bccc81c613f0ce6ae22" - senderAddr = hexToByteArray[20]("73cf19657412508833f618a15e8251306b3e6ee5") + +# The above privKey will generate this address +# senderAddr = hexToByteArray[20]("73cf19657412508833f618a15e8251306b3e6ee5") type TestEnv = object @@ -391,7 +393,7 @@ proc runLedgerTransactionTests(noisy = true) = let ledger = env.com.getLedger(head) env.runTrial4(ledger, n, rollback = true) -proc runLedgerBesicOperationsTests() = +proc runLedgerBasicOperationsTests() = suite "Ledger basic operations tests": setup: const emptyAcc {.used.} = newAccount() @@ -454,7 +456,7 @@ proc runLedgerBesicOperationsTests() = check x.originalStorage.len == 3 check y.originalStorage.len == 3 - test "accounts cache": + test "Ledger various operations": var ac = LedgerRef.init(memDB, EMPTY_ROOT_HASH) var addr1 = initAddr(1) @@ -494,8 +496,7 @@ proc runLedgerBesicOperationsTests() = db.setStorage(addr1, 1.u256, 10.u256) check rootHash == db.rootHash - # accounts cache readonly operations - # use previous hash + # Ledger readonly operations using previous hash var ac2 = LedgerRef.init(memDB, rootHash) var addr2 = initAddr(2) @@ -515,7 +516,7 @@ proc runLedgerBesicOperationsTests() = # state trie at all check ac2.rootHash == rootHash - test "accounts cache code retrieval after persist called": + test "Ledger code retrieval after persist called": var ac = LedgerRef.init(memDB, EMPTY_ROOT_HASH) var addr2 = initAddr(2) ac.setCode(addr2, code) @@ -656,7 +657,7 @@ proc runLedgerBesicOperationsTests() = check ac.vts(0xcc, 7, 88) == false check ac.vts(0xdd, 2, 66) == false - test "accounts cache contractCollision": + test "ledger contractCollision": # use previous hash var ac = LedgerRef.init(memDB, EMPTY_ROOT_HASH) let addr2 = initAddr(2) @@ -678,13 +679,79 @@ proc runLedgerBesicOperationsTests() = ac.setNonce(addr4, 1) check ac.contractCollision(addr4) == true + test "Ledger storage iterator": + var ac = LedgerRef.init(memDB, EMPTY_ROOT_HASH) + let addr2 = initAddr(2) + ac.setStorage(addr2, 1.u256, 2.u256) + ac.setStorage(addr2, 2.u256, 3.u256) + + var keys: seq[UInt256] + var vals: seq[UInt256] + for k, v in ac.cachedStorage(addr2): + keys.add k + vals.add v + + # before persist, there are storages in cache + check keys.len == 2 + check vals.len == 2 + + check 1.u256 in keys + check 2.u256 in keys + + # before persist, the values are all original values + check vals == @[0.u256, 0.u256] + + keys.reset + vals.reset + + for k, v in ac.storage(addr2): + keys.add k + vals.add k + + # before persist, there are no storages in db + check keys.len == 0 + check vals.len == 0 + + ac.persist() + for k, v in ac.cachedStorage(addr2): + keys.add k + vals.add v + + # after persist, there are storages in cache + check keys.len == 2 + check vals.len == 2 + + check 1.u256 in keys + check 2.u256 in keys + + # after persist, the values are what we put into + check 2.u256 in vals + check 3.u256 in vals + + keys.reset + vals.reset + + for k, v in ac.storage(addr2): + keys.add k + vals.add v + + # after persist, there are storages in db + check keys.len == 2 + check vals.len == 2 + + check 1.u256 in keys + check 2.u256 in keys + + check 2.u256 in vals + check 3.u256 in vals + # ------------------------------------------------------------------------------ # Main function(s) # ------------------------------------------------------------------------------ proc ledgerMain*(noisy = defined(debug)) = noisy.runLedgerTransactionTests - runLedgerBesicOperationsTests() + runLedgerBasicOperationsTests() when isMainModule: var noisy = defined(debug)