Skip to content

Commit

Permalink
Make test op memory work again (#2236)
Browse files Browse the repository at this point in the history
* Remove crufty `pruneTrie` arguments

* Replaced legacy `distinct_trie` logic by new `ledger` functionality

why:
  The module `distinct_trie` is supported by `Aristo` in trivial cases.

* Activate `test_op_memory`
  • Loading branch information
mjfh committed May 28, 2024
1 parent 08e98eb commit 3a62250
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 32 deletions.
4 changes: 2 additions & 2 deletions nimbus/db/ledger/accounts_ledger.nim
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ template noRlpException(info: static[string]; code: untyped) =

# The AccountsLedgerRef is modeled after TrieDatabase for it's transaction style
proc init*(x: typedesc[AccountsLedgerRef], db: CoreDbRef,
root: KeccakHash, pruneTrie = true): AccountsLedgerRef =
root: KeccakHash): AccountsLedgerRef =
new result
result.ledger = AccountLedger.init(db, root, pruneTrie)
result.ledger = AccountLedger.init(db, root)
result.kvt = db.newKvt() # save manually in `persist()`
result.witnessCache = initTable[EthAddress, WitnessData]()
discard result.beginSavepoint
Expand Down
12 changes: 3 additions & 9 deletions nimbus/db/ledger/distinct_ledgers.nim
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,18 @@ proc init*(
T: type AccountLedger;
db: CoreDbRef;
root: Hash256;
pruneOk = true;
): T =
const
info = "AccountLedger.init(): "
let
ctx = db.ctx
trie = block:
col = block:
let rc = ctx.newColumn(CtAccounts, root)
if rc.isErr:
raiseAssert info & $$rc.error
rc.value
mpt = block:
let rc = ctx.getAcc(trie)
let rc = ctx.getAcc(col)
if rc.isErr:
raiseAssert info & $$rc.error
rc.value
Expand Down Expand Up @@ -170,14 +169,9 @@ proc init*(
al: AccountLedger;
account: CoreDbAccount;
reHashOk = true;
pruneOk = false;
): T =
## Storage trie constructor.
##
## Note that the argument `pruneOk` should be left `false` on the legacy
## `CoreDb` backend. Otherwise, pruning might kill some unwanted entries from
## storage tries ending up with an unstable database leading to crashes (see
## https://github.com/status-im/nimbus-eth1/issues/932.)
const
info = "StorageLedger/init(): "
let
Expand All @@ -191,7 +185,7 @@ proc init*(
ctx = db.ctx
trie = if stt.isNil: ctx.newColumn(account.address) else: stt
mpt = block:
let rc = ctx.getMpt(trie, pruneOk)
let rc = ctx.getMpt(trie)
if rc.isErr:
raiseAssert info & $$rc.error
rc.value
Expand Down
2 changes: 1 addition & 1 deletion tests/all_tests.nim
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ cliBuilder:
./test_op_arith,
./test_op_bit,
./test_op_env,
#./test_op_memory, -- fails
./test_op_memory,
./test_op_misc,
./test_op_custom,
#./test_state_db, -- does not compile
Expand Down
33 changes: 13 additions & 20 deletions tests/macro_assembler.nim
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@ import
stew/shims/macros

import
../nimbus/db/[ledger, distinct_tries],
../nimbus/db/ledger,
../nimbus/evm/types,
../nimbus/vm_internals,
../nimbus/transaction/[call_common, call_evm],
../nimbus/[vm_types, vm_state],
../nimbus/core/pow/difficulty

from ../nimbus/db/aristo
import EmptyBlob

# Ditto, for GasPrice.
import ../nimbus/transaction except GasPrice
import ../tools/common/helpers except LogLevel
Expand Down Expand Up @@ -63,15 +66,6 @@ type

const
idToOpcode = CacheTable"NimbusMacroAssembler"
var
coreDbType* = DefaultDbMemory
## This variable needs to be accessible for unit tests like
## `test_op_memory` which implicitely uses the `initStorageTrie()` call
## from the `distinct_tries` module. The `Aristo` API cannot handle that
## because it needs the account address for accessing the storage trie.
##
## This problem can be fixed here in the `verifyAsmResult()` function once
## there is the time to do it ...

static:
for n in Op:
Expand Down Expand Up @@ -278,11 +272,7 @@ const
proc initVMEnv*(network: string): BaseVMState =
let
conf = getChainConfig(network)
cdb = block:
# Need static binding
case coreDbType:
of AristoDbMemory: newCoreDbRef AristoDbMemory
else: raiseAssert "unsupported: " & $coreDbType
cdb = DefaultDbMemory.newCoreDbRef()
com = CommonRef.new(
cdb,
conf,
Expand Down Expand Up @@ -345,15 +335,18 @@ proc verifyAsmResult(vmState: BaseVMState, boa: Assembler, asmResult: CallResult

var stateDB = vmState.stateDB
stateDB.persist()
var
storageRoot = stateDB.getStorageRoot(codeAddress)
trie = initStorageTrie(com.db, storageRoot)

let
al = AccountLedger.init(com.db, EMPTY_ROOT_HASH)
acc = al.fetch(codeAddress).expect "Valid Account Handle"
sl = StorageLedger.init(al, acc)

for kv in boa.storage:
let key = kv[0].toHex()
let val = kv[1].toHex()
let keyBytes = (@(kv[0]))
let actual = trie.getSlotBytes(keyBytes).toHex()
let slot = UInt256.fromBytesBE kv[0]
let data = sl.fetch(slot).valueOr: EmptyBlob
let actual = data.toHex
let zerosLen = 64 - (actual.len)
let value = repeat('0', zerosLen) & actual
if val != value:
Expand Down

0 comments on commit 3a62250

Please sign in to comment.