mirror of
https://github.com/status-im/nimbus-eth1.git
synced 2025-01-11 21:04:11 +00:00
Jordan/fix txs sort order (#1025)
* Fix database sort order for local txs why: For convenience, packed txs were stored in the block sorted by rank->nonce. Using local accounts, the greedy grabber uses the sort order (local,non-local)->rank->nonce which leads to a wrong calculation of the txRoot. * Housekeeping details: Replaced a couple of local eip1559TxNormalization() functions by a single public
This commit is contained in:
Jordan Hrycaj
GitHub
parent
60032bf106
commit
84ff179cd9
@ -676,7 +676,7 @@ proc ethBlock*(xp: TxPoolRef): EthBlock
|
||||
## order to build the block.
|
||||
xp.packerVmExec # updates vmState
|
||||
result.header = xp.chain.getHeader # uses updated vmState
|
||||
for (_,nonceList) in xp.txDB.decAccount(txItemPacked):
|
||||
for (_,nonceList) in xp.txDB.packingOrderAccounts(txItemPacked):
|
||||
result.txs.add toSeq(nonceList.incNonce).mapIt(it.tx)
|
||||
|
||||
proc gasCumulative*(xp: TxPoolRef): GasInt
|
||||
@ -864,6 +864,10 @@ proc resLocal*(xp: TxPoolRef; account: EthAddress) =
|
||||
## considered for packing after the locally tagged accounts.
|
||||
xp.txDB.resLocal(account)
|
||||
|
||||
proc flushLocals*(xp: TxPoolRef) =
|
||||
## Untag all *local* addresses on the system.
|
||||
xp.txDB.flushLocals
|
||||
|
||||
proc accountRanks*(xp: TxPoolRef): TxTabsLocality =
|
||||
## Returns two lists, one for local and the other for non-local accounts.
|
||||
## Any of these lists is sorted by the highest rank first. This sorting
|
||||
|
||||
@ -347,6 +347,10 @@ proc resLocal*(xp: TxTabsRef; sender: EthAddress) =
|
||||
## Untag *local* `sender` address argument.
|
||||
xp.byLocal.del(sender)
|
||||
|
||||
proc flushLocals*(xp: TxTabsRef) =
|
||||
## Untag all *local* addresses on the system.
|
||||
xp.byLocal.clear
|
||||
|
||||
# ------------------------------------------------------------------------------
|
||||
# Public iterators, `TxRank` > `(EthAddress,TxStatusNonceRef)`
|
||||
# ------------------------------------------------------------------------------
|
||||
@ -394,6 +398,21 @@ iterator decAccount*(xp: TxTabsRef; bucket: TxItemStatus;
|
||||
# Get next ranked address list (top down index walk)
|
||||
rcRank = xp.byRank.lt(rank) # potenially modified database
|
||||
|
||||
iterator packingOrderAccounts*(xp: TxTabsRef; bucket: TxItemStatus):
|
||||
(EthAddress,TxStatusNonceRef)
|
||||
{.gcsafe,raises: [Defect,KeyError].} =
|
||||
## Loop over accounts from a particular bucket ordered by
|
||||
## + local ranks, higest one first
|
||||
## + remote ranks, higest one first
|
||||
## For the `txItemStaged` bucket, this iterator defines the packing order
|
||||
## for transactions (important when calculationg the *txRoot*.)
|
||||
for (account,nonceList) in xp.decAccount(bucket):
|
||||
if xp.isLocal(account):
|
||||
yield (account,nonceList)
|
||||
for (account,nonceList) in xp.decAccount(bucket):
|
||||
if not xp.isLocal(account):
|
||||
yield (account,nonceList)
|
||||
|
||||
# ------------------------------------------------------------------------------
|
||||
# Public iterators, `TxRank` > `(EthAddress,TxSenderNonceRef)`
|
||||
# ------------------------------------------------------------------------------
|
||||
|
||||
@ -86,12 +86,6 @@ proc checkTxNonce(xp: TxPoolRef; item: TxItemRef): bool
|
||||
|
||||
true
|
||||
|
||||
proc eip1559TxNormalization(tx: Transaction): Transaction =
|
||||
result = tx
|
||||
if tx.txType < TxEip1559:
|
||||
result.maxPriorityFee = tx.gasPrice
|
||||
result.maxFee = tx.gasPrice
|
||||
|
||||
# ------------------------------------------------------------------------------
|
||||
# Private function: active tx classifier check helpers
|
||||
# ------------------------------------------------------------------------------
|
||||
@ -179,6 +173,22 @@ proc txPostLondonAcceptableTipAndFees(xp: TxPoolRef; item: TxItemRef): bool =
|
||||
return false
|
||||
true
|
||||
|
||||
# ------------------------------------------------------------------------------
|
||||
# Public helper function, needed here and in the packer
|
||||
# ------------------------------------------------------------------------------
|
||||
|
||||
func eip1559TxNormalization*(tx: Transaction;
|
||||
baseFee: GasPrice; fork: Fork): Transaction =
|
||||
## This function adjusts a legacy transaction to EIP-1559 standard. This
|
||||
## is needed particularly when using the `validateTransaction()` utility
|
||||
## with legacy transactions.
|
||||
result = tx
|
||||
if tx.txType < TxEip1559:
|
||||
result.maxPriorityFee = tx.gasPrice
|
||||
result.maxFee = tx.gasPrice
|
||||
if FkLondon <= fork:
|
||||
result.gasPrice = min(tx.maxPriorityFee + baseFee.int64, tx.maxFee)
|
||||
|
||||
# ------------------------------------------------------------------------------
|
||||
# Public functionss
|
||||
# ------------------------------------------------------------------------------
|
||||
@ -238,10 +248,7 @@ proc classifyValidatePacked*(xp: TxPoolRef;
|
||||
xp.chain.limits.maxLimit
|
||||
else:
|
||||
xp.chain.limits.trgLimit
|
||||
|
||||
var tx = item.tx.eip1559TxNormalization
|
||||
if FkLondon <= fork:
|
||||
tx.gasPrice = min(tx.maxPriorityFee + baseFee.truncate(int64), tx.maxFee)
|
||||
tx = item.tx.eip1559TxNormalization(xp.chain.baseFee, fork)
|
||||
|
||||
roDB.validateTransaction(tx, item.sender, gasLimit, baseFee, fork)
|
||||
|
||||
|
||||
@ -66,12 +66,6 @@ template safeExecutor(info: string; code: untyped) =
|
||||
let e = getCurrentException()
|
||||
raise newException(TxPackerError, info & "(): " & $e.name & " -- " & e.msg)
|
||||
|
||||
proc eip1559TxNormalization(tx: Transaction): Transaction =
|
||||
result = tx
|
||||
if tx.txType < TxEip1559:
|
||||
result.maxPriorityFee = tx.gasPrice
|
||||
result.maxFee = tx.gasPrice
|
||||
|
||||
proc persist(pst: TxPackerStateRef)
|
||||
{.gcsafe,raises: [Defect,RlpError].} =
|
||||
## Smart wrapper
|
||||
@ -87,13 +81,10 @@ proc runTx(pst: TxPackerStateRef; item: TxItemRef): GasInt
|
||||
{.gcsafe,raises: [Defect,CatchableError].} =
|
||||
## Execute item transaction and update `vmState` book keeping. Returns the
|
||||
## `gasUsed` after executing the transaction.
|
||||
var
|
||||
tx = item.tx.eip1559TxNormalization
|
||||
let
|
||||
fork = pst.xp.chain.nextFork
|
||||
baseFee = pst.xp.chain.head.baseFee
|
||||
if FkLondon <= fork:
|
||||
tx.gasPrice = min(tx.maxPriorityFee + baseFee.truncate(int64), tx.maxFee)
|
||||
baseFee = pst.xp.chain.head.baseFee.truncate(uint64).GasPrice
|
||||
tx = item.tx.eip1559TxNormalization(baseFee, fork)
|
||||
|
||||
safeExecutor "tx_packer.runTx":
|
||||
# Execute transaction, may return a wildcard `Exception`
|
||||
@ -252,18 +243,6 @@ proc vmExecCommit(pst: TxPackerStateRef)
|
||||
xp.chain.profit = balanceDelta()
|
||||
xp.chain.reward = balanceDelta()
|
||||
|
||||
iterator rankedAccounts(xp: TxPoolRef): TxStatusNonceRef
|
||||
{.gcsafe,raises: [Defect,KeyError].} =
|
||||
## Loop over staged accounts ordered by
|
||||
## + local ranks, higest one first
|
||||
## + remote ranks, higest one first
|
||||
for (account,nonceList) in xp.txDB.decAccount(txItemStaged):
|
||||
if xp.txDB.isLocal(account):
|
||||
yield nonceList
|
||||
for (account,nonceList) in xp.txDB.decAccount(txItemStaged):
|
||||
if not xp.txDB.isLocal(account):
|
||||
yield nonceList
|
||||
|
||||
# ------------------------------------------------------------------------------
|
||||
# Public functions
|
||||
# ------------------------------------------------------------------------------
|
||||
@ -277,7 +256,7 @@ proc packerVmExec*(xp: TxPoolRef) {.gcsafe,raises: [Defect,CatchableError].} =
|
||||
var pst = xp.vmExecInit
|
||||
|
||||
block loop:
|
||||
for nonceList in pst.xp.rankedAccounts:
|
||||
for (_,nonceList) in pst.xp.txDB.packingOrderAccounts(txItemStaged):
|
||||
|
||||
block account:
|
||||
for item in nonceList.incNonce:
|
||||
|
||||
@ -60,6 +60,9 @@ const
|
||||
# parameter should be above 100%.
|
||||
decreasingBlockProfitRatioPC = 92
|
||||
|
||||
# Make some percentage of the accounts local accouns.
|
||||
accountExtractPC = 10
|
||||
|
||||
# test block chain
|
||||
networkId = GoerliNet # MainNet
|
||||
|
||||
@ -69,15 +72,18 @@ var
|
||||
|
||||
prng = prngSeed.initRand
|
||||
|
||||
# to be set up in runTxLoader()
|
||||
# To be set up in runTxLoader()
|
||||
statCount: array[TxItemStatus,int] # per status bucket
|
||||
|
||||
txList: seq[TxItemRef]
|
||||
effGasTips: seq[GasPriceEx]
|
||||
|
||||
# running block chain
|
||||
# Running block chain
|
||||
bcDB: BaseChainDB
|
||||
|
||||
# Accounts to be considered local
|
||||
localAccounts: seq[EthAddress]
|
||||
|
||||
# ------------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ------------------------------------------------------------------------------
|
||||
@ -177,6 +183,13 @@ proc runTxLoader(noisy = true; capture = loadSpecs) =
|
||||
loadTxs = capture.numTxs,
|
||||
noisy = veryNoisy)
|
||||
|
||||
# Extract some of the least profitable accounts and hold them so
|
||||
# they could be made local at a later stage
|
||||
let
|
||||
accr = xp.accountRanks
|
||||
nExtract = (accr.remote.len * accountExtractPC + 50) div 100
|
||||
localAccounts = accr.remote[accr.remote.len - nExtract .. ^1]
|
||||
|
||||
# Make sure that sample extraction from file was ok
|
||||
check capture.minBlockTxs <= nTxs
|
||||
check capture.numTxs == xp.nItems.total
|
||||
@ -742,7 +755,9 @@ proc runTxPackerTests(noisy = true) =
|
||||
|
||||
block:
|
||||
var
|
||||
xq = bcDB.toTxPool(txList, ntBaseFee, noisy = noisy)
|
||||
xq = bcDB.toTxPool(txList, ntBaseFee,
|
||||
local = localAccounts,
|
||||
noisy = noisy)
|
||||
let
|
||||
(nMinTxs, nTrgTxs) = (15, 15)
|
||||
(nMinAccounts, nTrgAccounts) = (1, 8)
|
||||
@ -814,22 +829,13 @@ proc runTxPackerTests(noisy = true) =
|
||||
" increase=", xq.gasCumulative - smallerBlockSize,
|
||||
" trg/max=", blockProfitRatio, "%"
|
||||
|
||||
let
|
||||
accountExtractPC = 10
|
||||
acc = xq.accountRanks
|
||||
nExtract = (acc.remote.len * accountExtractPC + 50) div 100
|
||||
accExtracted = acc.remote[acc.remote.len - nExtract .. ^1]
|
||||
|
||||
noisy.say "***", "accounts",
|
||||
" local=", acc.local.pp,
|
||||
" remote=", acc.remote.pp,
|
||||
" remote.len=", acc.remote.len,
|
||||
" nExtract=", nExtract,
|
||||
" accExtracted=", accExtracted.pp
|
||||
|
||||
# if true: return
|
||||
test "Store generated block in block chain database":
|
||||
|
||||
noisy.say "***", "locality",
|
||||
" locals=", xq.accountRanks.local.len,
|
||||
" remotes=", xq.accountRanks.remote.len
|
||||
|
||||
# Force maximal block size. Accidentally, the latest tx should have
|
||||
# a `gasLimit` exceeding the available space on the block `gasLimit`
|
||||
# which will be checked below.
|
||||
@ -875,11 +881,15 @@ proc runTxPackerTests(noisy = true) =
|
||||
# much less than permitted so this block will be accepted.
|
||||
check 0 < overlap
|
||||
|
||||
#setTraceLevel()
|
||||
|
||||
# Test low-level function for adding the new block to the database
|
||||
xq.chain.maxMode = (packItemsMaxGasLimit in xq.flags)
|
||||
xq.chain.clearAccounts
|
||||
check xq.chain.vmState.processBlock(poa, hdr, bdy).isOK
|
||||
|
||||
setErrorLevel()
|
||||
|
||||
# Re-allocate using VM environment from `persistBlocks()`
|
||||
check BaseVMState.new(hdr, bcDB).processBlock(poa, hdr, bdy).isOK
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user