Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to roll back stateless mode implementation in a single PR #2209

Merged
merged 3 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions nimbus/config.nim
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ type
Default
Full ## Beware, experimental
#Snap ## Beware, experimental
Stateless ## Beware, experimental

NimbusConf* = object of RootObj
## Main Nimbus configuration object
Expand Down Expand Up @@ -171,7 +170,7 @@ type
"- default -- legacy sync mode\n" &
"- full -- full blockchain archive\n" &
# "- snap -- experimental snap mode (development only)\n" &
"- stateless -- experimental stateless mode (development only)"
""
defaultValue: SyncMode.Default
defaultValueDesc: $SyncMode.Default
abbr: "y"
Expand Down Expand Up @@ -484,11 +483,6 @@ type
defaultValueDesc: $defaultAdminListenAddressDesc
name: "metrics-address" }: IpAddress

statelessModeDataSourceUrl* {.
desc: "URL of the node to use as a data source for on-demand data fetching via the JSON-RPC API"
defaultValue: ""
name: "stateless-data-source-url" .}: string

trustedSetupFile* {.
desc: "Load EIP-4844 trusted setup file"
defaultValue: none(string)
Expand Down
20 changes: 11 additions & 9 deletions nimbus/core/chain/persist_blocks.nim
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ type

const
CleanUpEpoch = 30_000.u256
## Regular checks for history clean up (applies to single state DB)
## Regular checks for history clean up (applies to single state DB). This
## is mainly a debugging/testing feature so that the database can be held
## a bit smaller. It is not applicable to a full node.

# ------------------------------------------------------------------------------
# Private
Expand All @@ -57,7 +59,8 @@ proc getVmState(c: ChainRef, header: BlockHeader):
return err()
return ok(vmState)

proc purgeExpiredBlocks(db: CoreDbRef) {.inline, raises: [RlpError].} =

proc purgeOutOfJournalBlocks(db: CoreDbRef) {.inline, raises: [RlpError].} =
## Remove non-reachable blocks from KVT database
var blkNum = db.getOldestJournalBlockNumber()
if 0 < blkNum:
Expand Down Expand Up @@ -85,10 +88,9 @@ proc persistBlocksImpl(c: ChainRef; headers: openArray[BlockHeader];
let vmState = c.getVmState(headers[0]).valueOr:
return ValidationResult.Error

# Needed for figuring out whether KVT cleanup is due (see at the end)
let (fromBlock, toBlock) = (headers[0].blockNumber, headers[^1].blockNumber)

trace "Persisting blocks", fromBlock, toBlock

for i in 0 ..< headers.len:
let (header, body) = (headers[i], bodies[i])

Expand Down Expand Up @@ -197,11 +199,11 @@ proc persistBlocksImpl(c: ChainRef; headers: openArray[BlockHeader];
# `persistent()` together with the respective block number.
c.db.persistent(headers[0].blockNumber - 1)

# For a single state ledger, there is only a limited backlog. So clean up
# regularly (the `CleanUpEpoch` should not be too small as each lookup pulls
# a journal entry from disk.)
if (fromBlock mod CleanUpEpoch) <= (toBlock - fromBlock):
c.db.purgeExpiredBlocks()
if c.com.pruneHistory:
# There is a feature for test systems to regularly clean up older blocks
# from the database, not appicable to a full node set up.
if(fromBlock mod CleanUpEpoch) <= (toBlock - fromBlock):
c.db.purgeOutOfJournalBlocks()

ValidationResult.OK

Expand Down
15 changes: 9 additions & 6 deletions nimbus/core/executor/process_block.nim
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ import

# Factored this out of procBlkPreamble so that it can be used directly for
# stateless execution of specific transactions.
proc processTransactions*(vmState: BaseVMState;
header: BlockHeader;
transactions: seq[Transaction]): Result[void, string]
{.gcsafe, raises: [CatchableError].} =
proc processTransactions*(
vmState: BaseVMState;
header: BlockHeader;
transactions: seq[Transaction];
): Result[void, string]
{.gcsafe, raises: [CatchableError].} =
Copy link
Member

@arnetheduck arnetheduck May 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this strange code formatting coming from, and why is it part of this revert? This is not how we format proc anywhere else, we should generally move in the direction of the style guide / nep1 / nph.

vmState.receipts = newSeq[Receipt](transactions.len)
vmState.cumulativeGasUsed = 0

Expand Down Expand Up @@ -151,8 +153,9 @@ proc procBlkEpilogue(vmState: BaseVMState;
proc processBlock*(
vmState: BaseVMState; ## Parent environment of header/body block
header: BlockHeader; ## Header/body block to add to the blockchain
body: BlockBody): ValidationResult
{.gcsafe, raises: [CatchableError].} =
body: BlockBody;
): ValidationResult
{.gcsafe, raises: [CatchableError].} =
## Generalised function to processes `(header,body)` pair for any network,
## regardless of PoA or not.
##
Expand Down
73 changes: 25 additions & 48 deletions nimbus/core/executor/process_transaction.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Nimbus
# Copyright (c) 2018-2023 Status Research & Development GmbH
# Copyright (c) 2018-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)
Expand All @@ -12,18 +12,16 @@

import
std/strutils,
stew/results,
../../common/common,
../../db/ledger,
../../transaction/call_evm,
../../transaction/call_common,
../../transaction,
../../vm_state,
../../vm_types,
../../evm/async/operations,
../../constants,
../validate,
chronos,
stew/results
../validate

# ------------------------------------------------------------------------------
# Private functions
Expand All @@ -37,10 +35,13 @@ proc eip1559BaseFee(header: BlockHeader; fork: EVMFork): UInt256 =
result = header.baseFee

proc commitOrRollbackDependingOnGasUsed(
vmState: BaseVMState, accTx: LedgerSpRef,
header: BlockHeader, tx: Transaction,
gasBurned: GasInt, priorityFee: GasInt):
Result[GasInt, string] {.raises: [].} =
vmState: BaseVMState;
accTx: LedgerSpRef;
header: BlockHeader;
tx: Transaction;
gasBurned: GasInt;
priorityFee: GasInt;
): Result[GasInt, string] =
# Make sure that the tx does not exceed the maximum cumulative limit as
# set in the block header. Again, the eip-1559 reference does not mention
# an early stop. It would rather detect differing values for the block
Expand All @@ -63,14 +64,14 @@ proc commitOrRollbackDependingOnGasUsed(
vmState.gasPool += tx.gasLimit - gasBurned
return ok(gasBurned)

proc asyncProcessTransactionImpl(
proc processTransactionImpl(
vmState: BaseVMState; ## Parent accounts environment for transaction
tx: Transaction; ## Transaction to validate
sender: EthAddress; ## tx.getSender or tx.ecRecover
header: BlockHeader; ## Header for the block containing the current tx
fork: EVMFork): Future[Result[GasInt, string]]
# wildcard exception, wrapped below
{.async, gcsafe.} =
fork: EVMFork;
): Result[GasInt, string]
{.raises: [CatchableError].} =
## Modelled after `https://eips.ethereum.org/EIPS/eip-1559#specification`_
## which provides a backward compatible framwork for EIP1559.

Expand All @@ -85,14 +86,10 @@ proc asyncProcessTransactionImpl(
# Return failure unless explicitely set `ok()`
var res: Result[GasInt, string] = err("")

await ifNecessaryGetAccounts(vmState, @[sender, vmState.coinbase()])
if tx.to.isSome:
await ifNecessaryGetCode(vmState, tx.to.get)

# buy gas, then the gas goes into gasMeter
if vmState.gasPool < tx.gasLimit:
return err("gas limit reached. gasLimit=$1, gasNeeded=$2" % [
$vmState.gasPool, $tx.gasLimit])
return err("gas limit reached. gasLimit=" & $vmState.gasPool &
", gasNeeded=" & $tx.gasLimit)

vmState.gasPool -= tx.gasLimit

Expand Down Expand Up @@ -163,45 +160,25 @@ proc processBeaconBlockRoot*(vmState: BaseVMState, beaconRoot: Hash256):
statedb.persist(clearEmptyAccount = true, clearCache = false)
ok()

proc asyncProcessTransaction*(
vmState: BaseVMState; ## Parent accounts environment for transaction
tx: Transaction; ## Transaction to validate
sender: EthAddress; ## tx.getSender or tx.ecRecover
header: BlockHeader; ## Header for the block containing the current tx
fork: EVMFork): Future[Result[GasInt,string]]
{.async, gcsafe.} =
## Process the transaction, write the results to accounts db. The function
## returns the amount of gas burned if executed.
return await vmState.asyncProcessTransactionImpl(tx, sender, header, fork)

# FIXME-duplicatedForAsync
proc asyncProcessTransaction*(
vmState: BaseVMState; ## Parent accounts environment for transaction
tx: Transaction; ## Transaction to validate
sender: EthAddress; ## tx.getSender or tx.ecRecover
header: BlockHeader): Future[Result[GasInt,string]]
{.async, gcsafe.} =
## Variant of `asyncProcessTransaction()` with `*fork* derived
## from the `vmState` argument.
let fork = vmState.com.toEVMFork(header.forkDeterminationInfo)
return await vmState.asyncProcessTransaction(tx, sender, header, fork)

proc processTransaction*(
vmState: BaseVMState; ## Parent accounts environment for transaction
tx: Transaction; ## Transaction to validate
sender: EthAddress; ## tx.getSender or tx.ecRecover
header: BlockHeader; ## Header for the block containing the current tx
fork: EVMFork): Result[GasInt,string]
{.gcsafe, raises: [CatchableError].} =
return waitFor(vmState.asyncProcessTransaction(tx, sender, header, fork))
fork: EVMFork;
): Result[GasInt,string]
{.raises: [CatchableError].} =
vmState.processTransactionImpl(tx, sender, header, fork)

proc processTransaction*(
vmState: BaseVMState; ## Parent accounts environment for transaction
tx: Transaction; ## Transaction to validate
sender: EthAddress; ## tx.getSender or tx.ecRecover
header: BlockHeader): Result[GasInt,string]
{.gcsafe, raises: [CatchableError].} =
return waitFor(vmState.asyncProcessTransaction(tx, sender, header))
header: BlockHeader;
): Result[GasInt,string]
{.raises: [CatchableError].} =
let fork = vmState.com.toEVMFork(header.forkDeterminationInfo)
vmState.processTransaction(tx, sender, header, fork)

# ------------------------------------------------------------------------------
# End
Expand Down
13 changes: 0 additions & 13 deletions nimbus/db/core_db/core_apps_newapi.nim
Original file line number Diff line number Diff line change
Expand Up @@ -925,19 +925,6 @@ proc persistHeaderToDbWithoutSetHead*(
headerHash, action="put()", `error`=($$error)
return

# FIXME-Adam: This seems like a bad idea. I don't see a way to get the score
# in stateless mode, but it seems dangerous to just shove the header into
# the DB *without* also storing the score.
proc persistHeaderToDbWithoutSetHeadOrScore*(db: CoreDbRef; header: BlockHeader) =
db.addBlockNumberToHashLookup(header)
let
kvt = db.newKvt()
blockHash = header.blockHash
kvt.put(genericHashKey(blockHash).toOpenArray, rlp.encode(header)).isOkOr:
warn logTxt "persistHeaderToDbWithoutSetHeadOrScore()",
blockHash, action="put()", `error`=($$error)
return

proc persistUncles*(db: CoreDbRef, uncles: openArray[BlockHeader]): Hash256 =
## Persists the list of uncles to the database.
## Returns the uncles hash.
Expand Down
98 changes: 0 additions & 98 deletions nimbus/db/incomplete_db.nim

This file was deleted.

6 changes: 1 addition & 5 deletions nimbus/db/ledger/backend/accounts_ledger.nim
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,7 @@ proc ledgerMethods(lc: impl.AccountsLedgerRef): LedgerFns =
proc ledgerExtras(lc: impl.AccountsLedgerRef): LedgerExtras =
LedgerExtras(
getMptFn: proc(): CoreDbMptRef =
lc.rawTrie.CoreDxAccRef.getMpt.CoreDbMptRef,

rawRootHashFn: proc(): Hash256 =
lc.rawTrie.state())

lc.rawTrie.CoreDxAccRef.getMpt.CoreDbMptRef)

proc newAccountsLedgerRef(
db: CoreDbRef;
Expand Down
5 changes: 0 additions & 5 deletions nimbus/db/ledger/base.nim
Original file line number Diff line number Diff line change
Expand Up @@ -372,11 +372,6 @@ proc getMpt*(ldg: LedgerRef): CoreDbMptRef =
result = ldg.extras.getMptFn()
ldg.ifTrackApi: debug apiTxt, api, elapsed, result

proc rawRootHash*(ldg: LedgerRef): Hash256 =
ldg.beginTrackApi LdgRawRootHashFn
result = ldg.extras.rawRootHashFn()
ldg.ifTrackApi: debug apiTxt, api, elapsed, result

# ------------------------------------------------------------------------------
# Public virtual read-only methods
# ------------------------------------------------------------------------------
Expand Down
Loading
Loading