Attempt to fix bugs discovered by kurtosis

status-im · Dec 25, 2024 · 9681ce7 · 9681ce7
1 parent 0f8cfb0
commit 9681ce7
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 15 deletions.
diff --git a/nimbus/beacon/api_handler/api_forkchoice.nim b/nimbus/beacon/api_handler/api_forkchoice.nim
@@ -10,12 +10,13 @@
 import
   std/[typetraits],
   results,
-  ../beacon_engine,
   eth/common/[headers, hashes, times],
   web3/execution_types,
-  ./api_utils,
   chronicles,
-  ../web3_eth_conv
+  ../../core/tx_pool,
+  ../beacon_engine,
+  ../web3_eth_conv,
+  ./api_utils
 
 {.push gcsafe, raises:[CatchableError].}
 
@@ -200,6 +201,7 @@ proc forkchoiceUpdated*(ben: BeaconEngineRef,
       gasUsed = bundle.payload.gasUsed,
       blobGasUsed = bundle.payload.blobGasUsed.get(Quantity(0)),
       id = id.toHex,
+      txPoolLen = ben.txPool.len,
       attrs = attrs
 
     return validFCU(Opt.some(id), blockHash)

diff --git a/nimbus/beacon/api_handler/api_newpayload.nim b/nimbus/beacon/api_handler/api_newpayload.nim
@@ -9,13 +9,14 @@
 
 import
   results,
-  ../web3_eth_conv,
+  chronicles,
   eth/common/hashes,
-  ../beacon_engine,
   web3/[execution_types, primitives],
+  ../../core/tx_pool,
+  ../web3_eth_conv,
+  ../beacon_engine,
   ../payload_conv,
-  ./api_utils,
-  chronicles
+  ./api_utils
 
 {.push gcsafe, raises:[CatchableError].}
 
@@ -228,6 +229,8 @@ proc newPayload*(ben: BeaconEngineRef,
     let blockHash = latestValidHash(db, parent, ttd)
     return invalidStatus(blockHash, vres.error())
 
+  ben.txPool.removeNewBlockTxs(blk, Opt.some(blockHash))
+
   info "New payload received and validated",
     number = header.number,
     hash = blockHash.short,

diff --git a/nimbus/beacon/beacon_engine.nim b/nimbus/beacon/beacon_engine.nim
@@ -126,6 +126,9 @@ func com*(ben: BeaconEngineRef): CommonRef =
 func chain*(ben: BeaconEngineRef): ForkedChainRef =
   ben.txPool.chain
 
+func txPool*(ben: BeaconEngineRef): TxPoolRef =
+  ben.txPool
+
 func get*(ben: BeaconEngineRef, hash: Hash32,
           header: var Header): bool =
   ben.queue.get(hash, header)

diff --git a/nimbus/core/chain/forked_chain.nim b/nimbus/core/chain/forked_chain.nim
@@ -799,3 +799,18 @@ proc isCanonicalAncestor*(c: ForkedChainRef,
   let canonHash = c.db.getBlockHash(blockNumber).valueOr:
     return false
   canonHash == blockHash
+
+iterator txHashInRange*(c: ForkedChainRef, fromHash: Hash32, toHash: Hash32): Hash32 =
+  ## exclude base from iteration, new block produced by txpool
+  ## should not reach base
+  var prevHash = fromHash
+  while prevHash != c.baseHash:
+    c.blocks.withValue(prevHash, item) do:
+      if toHash == prevHash:
+        break
+      for tx in item.blk.transactions:
+        let txHash = rlpHash(tx)
+        yield txHash
+      prevHash = item.blk.header.parentHash
+    do:
+      break
diff --git a/nimbus/core/tx_pool.nim b/nimbus/core/tx_pool.nim
@@ -18,7 +18,7 @@ import
   ./tx_pool/tx_packer,
   ./chain/forked_chain,
   ./casper
-  
+
 from eth/common/eth_types_rlp import rlpHash
 
 # ------------------------------------------------------------------------------
@@ -77,9 +77,19 @@ export
 # removeTx(xp: TxPoolRef, id: Hash32)
 # removeExpiredTxs(xp: TxPoolRef, lifeTime: Duration)
 
-proc removeNewBlockTxs*(xp: TxPoolRef, blk: Block) =
-  for tx in blk.transactions:
-    let txHash = rlpHash(tx)
+proc removeNewBlockTxs*(xp: TxPoolRef, blk: Block, optHash = Opt.none(Hash32)) =
+  # Remove only the latest block transactions
+  if blk.header.parentHash == xp.parentHash:
+    for tx in blk.transactions:
+      let txHash = rlpHash(tx)
+      xp.removeTx(txHash)
+    return
+
+  let fromHash = if optHash.isSome: optHash.get
+                 else: blk.header.blockHash
+
+  # Also remove transactions from older blocks
+  for txHash in xp.chain.txHashInRange(fromHash, xp.parentHash):
     xp.removeTx(txHash)
 
 type AssembledBlock* = object

diff --git a/nimbus/core/tx_pool/tx_desc.nim b/nimbus/core/tx_pool/tx_desc.nim
@@ -110,11 +110,11 @@ proc insertToSenderTab(xp: TxPoolRef; item: TxItemRef): Result[void, TxError] =
     sn.insertOrReplace(item)
     xp.senderTab.append(sn)
     return ok()
-
-  if sn.len >= MAX_TXS_PER_ACCOUNT:
-    return err(txErrorSenderMaxTxs)
-
+
   let current = xp.getCurrentFromSenderTab(item).valueOr:
+    if sn.len >= MAX_TXS_PER_ACCOUNT:
+      return err(txErrorSenderMaxTxs)
+
     # no equal sender/nonce,
     # insert into txpool
     sn.insertOrReplace(item)
@@ -168,6 +168,9 @@ proc getBalance*(xp: TxPoolRef; account: Address): UInt256 =
 proc getNonce*(xp: TxPoolRef; account: Address): AccountNonce =
   xp.vmState.ledger.getNonce(account)
 
+func parentHash*(xp: TxPoolRef): Hash32 =
+  xp.vmState.blockCtx.parentHash
+
 template chain*(xp: TxPoolRef): ForkedChainRef =
   xp.chain