Fix rpc.sendRawTransaction and txPool: reject invalid transaction earlier

nimbus/core/executor/process_block.nim

@@ -16,7 +16,6 @@ import
   ../../transaction,
   ../../vm_state,
   ../../vm_types,
-  ../../utils/debug,
   ../clique,
   ../dao,
   ./calculate_reward,
@@ -38,12 +37,10 @@ proc processTransactions*(vmState: BaseVMState;
   for txIndex, tx in transactions:
     var sender: EthAddress
     if not tx.getSender(sender):
-      let debugTx =tx.debug()
+      return err("Could not get sender for tx with index " & $(txIndex))
-      return err("Could not get sender for tx with index " & $(txIndex) & ": " & debugTx)
     let rc = vmState.processTransaction(tx, sender, header)
     if rc.isErr:
-      let debugTx =tx.debug()
+      return err("Error processing tx with index " & $(txIndex) & ":" & rc.error)
-      return err("Error processing tx with index " & $(txIndex) & ":\n" & debugTx & "\n" & rc.error)
     vmState.receipts[txIndex] = vmState.makeReceipt(tx.txType)
   ok()
 

nimbus/core/tx_pool.nim

@@ -910,6 +910,11 @@ proc addLocal*(xp: TxPoolRef;
   xp.add(tx, "local tx")
   ok()
 
+proc inPoolAndOk*(xp: TxPoolRef; txHash: Hash256): bool =
+  let res = xp.getItem(txHash)
+  if res.isErr: return false
+  res.get().reject == txInfoOk
+
 # ------------------------------------------------------------------------------
 # End
 # ------------------------------------------------------------------------------

nimbus/core/tx_pool/tx_chain.nim

@@ -276,6 +276,11 @@ proc baseFee*(dh: TxChainRef): GasPrice =
   else:
     0.GasPrice
 
+proc excessBlobGas*(dh: TxChainRef): uint64 =
+  ## Getter, baseFee for the next bock header. This value is auto-generated
+  ## when a new insertion point is set via `head=`.
+  dh.txEnv.vmState.parent.excessBlobGas.get(0'u64)
+
 proc nextFork*(dh: TxChainRef): EVMFork =
   ## Getter, fork of next block
   dh.com.toEVMFork(dh.txEnv.vmState.forkDeterminationInfoForVMState)

nimbus/core/tx_pool/tx_tasks/tx_classify.nim

@@ -17,6 +17,7 @@ import
   ../../../vm_state,
   ../../../vm_types,
   ../../validate,
+  ../../eip4844,
   ../tx_chain,
   ../tx_desc,
   ../tx_item,
@@ -36,30 +37,7 @@ logScope:
 # ------------------------------------------------------------------------------
 
 proc checkTxBasic(xp: TxPoolRef; item: TxItemRef): bool =
-  ## Inspired by `p2p/validate.validateTransaction()`
+  validateTxBasic(item.tx, xp.chain.nextFork).isOk
-  if item.tx.txType == TxEip2930 and xp.chain.nextFork < FkBerlin:
-    debug "invalid tx: Eip2930 Tx type detected before Berlin"
-    return false
-
-  if item.tx.txType == TxEip1559 and xp.chain.nextFork < FkLondon:
-    debug "invalid tx: Eip1559 Tx type detected before London"
-    return false
-
-  if item.tx.gasLimit < item.tx.intrinsicGas(xp.chain.nextFork):
-    debug "invalid tx: not enough gas to perform calculation",
-      available = item.tx.gasLimit,
-      require = item.tx.intrinsicGas(xp.chain.nextFork)
-    return false
-
-  if item.tx.txType == TxEip1559:
-    # The total must be the larger of the two
-    if item.tx.maxFee < item.tx.maxPriorityFee:
-      debug "invalid tx: maxFee is smaller than maPriorityFee",
-        maxFee = item.tx.maxFee,
-        maxPriorityFee = item.tx.maxPriorityFee
-      return false
-
-  true
 
 proc checkTxNonce(xp: TxPoolRef; item: TxItemRef): bool
     {.gcsafe,raises: [CatchableError].} =
@@ -117,19 +95,30 @@ proc txGasCovered(xp: TxPoolRef; item: TxItemRef): bool =
 
 proc txFeesCovered(xp: TxPoolRef; item: TxItemRef): bool =
   ## Ensure that the user was willing to at least pay the base fee
-  if item.tx.txType == TxEip1559:
+  ## And to at least pay the current data gasprice
+  if item.tx.txType >= TxEip1559:
     if item.tx.maxFee.GasPriceEx < xp.chain.baseFee:
       debug "invalid tx: maxFee is smaller than baseFee",
         maxFee = item.tx.maxFee,
         baseFee = xp.chain.baseFee
       return false
+
+  if item.tx.txType >= TxEip4844:
+    let
+      excessBlobGas = xp.chain.excessBlobGas
+      blobGasPrice = getBlobGasPrice(excessBlobGas)
+    if item.tx.maxFeePerBlobGas.uint64 < blobGasPrice:
+      debug "invalid tx: maxFeePerBlobGas smaller than blobGasPrice",
+        maxFeePerBlobGas=item.tx.maxFeePerBlobGas,
+        blobGasPrice=blobGasPrice
+      return false
   true
 
 proc txCostInBudget(xp: TxPoolRef; item: TxItemRef): bool =
   ## Check whether the worst case expense is covered by the price budget,
   let
     balance = xp.chain.getBalance(item.sender)
-    gasCost = item.tx.gasLimit.u256 * item.tx.gasPrice.u256
+    gasCost = item.tx.gasCost
   if balance < gasCost:
     debug "invalid tx: not enough cash for gas",
       available = balance,
@@ -148,7 +137,7 @@ proc txCostInBudget(xp: TxPoolRef; item: TxItemRef): bool =
 proc txPreLondonAcceptableGasPrice(xp: TxPoolRef; item: TxItemRef): bool =
   ## For legacy transactions check whether minimum gas price and tip are
   ## high enough. These checks are optional.
-  if item.tx.txType != TxEip1559:
+  if item.tx.txType < TxEip1559:
 
     if stageItemsPlMinPrice in xp.pFlags:
       if item.tx.gasPrice.GasPriceEx < xp.pMinPlGasPrice:
@@ -162,7 +151,7 @@ proc txPreLondonAcceptableGasPrice(xp: TxPoolRef; item: TxItemRef): bool =
 
 proc txPostLondonAcceptableTipAndFees(xp: TxPoolRef; item: TxItemRef): bool =
   ## Helper for `classifyTxPacked()`
-  if item.tx.txType == TxEip1559:
+  if item.tx.txType >= TxEip1559:
 
     if stageItems1559MinTip in xp.pFlags:
       if item.tx.effectiveGasTip(xp.chain.baseFee) < xp.pMinTipPrice:

nimbus/core/validate.nim

@@ -232,7 +232,7 @@ proc validateUncles(com: CommonRef; header: BlockHeader;
 # Public function, extracted from executor
 # ------------------------------------------------------------------------------
 
-func gasCost(tx: Transaction): UInt256 =
+func gasCost*(tx: Transaction): UInt256 =
   if tx.txType >= TxEip4844:
     tx.gasLimit.u256 * tx.maxFee.u256 + tx.getTotalBlobGas.u256 * tx.maxFeePerBlobGas.u256
   elif tx.txType >= TxEip1559:
@@ -240,19 +240,10 @@ func gasCost(tx: Transaction): UInt256 =
   else:
     tx.gasLimit.u256 * tx.gasPrice.u256
 
-proc validateTransaction*(
+proc validateTxBasic*(
-    roDB:     ReadOnlyStateDB; ## Parent accounts environment for transaction
     tx:       Transaction;     ## tx to validate
-    sender:   EthAddress;      ## tx.getSender or tx.ecRecover
-    maxLimit: GasInt;          ## gasLimit from block header
-    baseFee:  UInt256;         ## baseFee from block header
-    excessBlobGas: uint64;    ## excessBlobGas from parent block header
     fork:     EVMFork): Result[void, string] =
 
-  let
-    balance = roDB.getBalance(sender)
-    nonce = roDB.getNonce(sender)
-
   if tx.txType == TxEip2930 and fork < FkBerlin:
     return err("invalid tx: Eip2930 Tx type detected before Berlin")
 
@@ -265,6 +256,68 @@ proc validateTransaction*(
   if fork >= FkShanghai and tx.contractCreation and tx.payload.len > EIP3860_MAX_INITCODE_SIZE:
     return err("invalid tx: initcode size exceeds maximum")
 
+  try:
+    # The total must be the larger of the two
+    if tx.maxFee < tx.maxPriorityFee:
+      return err("invalid tx: maxFee is smaller than maPriorityFee. maxFee=$1, maxPriorityFee=$2" % [
+        $tx.maxFee, $tx.maxPriorityFee])
+
+    if tx.gasLimit < tx.intrinsicGas(fork):
+      return err("invalid tx: not enough gas to perform calculation. avail=$1, require=$2" % [
+        $tx.gasLimit, $tx.intrinsicGas(fork)])
+
+    if fork >= FkCancun:
+      if tx.payload.len > MAX_CALLDATA_SIZE:
+        return err("invalid tx: payload len exceeds MAX_CALLDATA_SIZE. len=" &
+          $tx.payload.len)
+
+      if tx.accessList.len > MAX_ACCESS_LIST_SIZE:
+        return err("invalid tx: access list len exceeds MAX_ACCESS_LIST_SIZE. len=" &
+          $tx.accessList.len)
+
+      for i, acl in tx.accessList:
+        if acl.storageKeys.len > MAX_ACCESS_LIST_STORAGE_KEYS:
+          return err("invalid tx: access list storage keys len exceeds MAX_ACCESS_LIST_STORAGE_KEYS. " &
+            "index=$1, len=$2" % [$i, $acl.storageKeys.len])
+
+    if tx.txType >= TxEip4844:
+      if tx.to.isNone:
+        return err("invalid tx: destination must be not empty")
+
+      if tx.versionedHashes.len == 0:
+        return err("invalid tx: there must be at least one blob")
+
+      if tx.versionedHashes.len > MaxAllowedBlob.int:
+        return err("invalid tx: versioned hashes len exceeds MaxAllowedBlob=" & $MaxAllowedBlob &
+          ". get=" & $tx.versionedHashes.len)
+
+      for i, bv in tx.versionedHashes:
+        if bv.data[0] != BLOB_COMMITMENT_VERSION_KZG:
+          return err("invalid tx: one of blobVersionedHash has invalid version. " &
+            "get=$1, expect=$2" % [$bv.data[0].int, $BLOB_COMMITMENT_VERSION_KZG.int])
+
+  except CatchableError as ex:
+    return err(ex.msg)
+
+  ok()
+
+proc validateTransaction*(
+    roDB:     ReadOnlyStateDB; ## Parent accounts environment for transaction
+    tx:       Transaction;     ## tx to validate
+    sender:   EthAddress;      ## tx.getSender or tx.ecRecover
+    maxLimit: GasInt;          ## gasLimit from block header
+    baseFee:  UInt256;         ## baseFee from block header
+    excessBlobGas: uint64;    ## excessBlobGas from parent block header
+    fork:     EVMFork): Result[void, string] =
+
+  let res = validateTxBasic(tx, fork)
+  if res.isErr:
+    return res
+
+  let
+    balance = roDB.getBalance(sender)
+    nonce = roDB.getNonce(sender)
+
   # Note that the following check bears some plausibility but is _not_
   # covered by the eip-1559 reference (sort of) pseudo code, for details
   # see `https://eips.ethereum.org/EIPS/eip-1559#specification`_
@@ -290,11 +343,6 @@ proc validateTransaction*(
       return err("invalid tx: maxFee is smaller than baseFee. maxFee=$1, baseFee=$2" % [
         $tx.maxFee, $baseFee])
 
-    # The total must be the larger of the two
-    if tx.maxFee < tx.maxPriorityFee:
-      return err("invalid tx: maxFee is smaller than maPriorityFee. maxFee=$1, maxPriorityFee=$2" % [
-        $tx.maxFee, $tx.maxPriorityFee])
-
     # the signer must be able to fully afford the transaction
     let gasCost = tx.gasCost()
 
@@ -306,10 +354,6 @@ proc validateTransaction*(
       return err("invalid tx: not enough cash to send. avail=$1, availMinusGas=$2, require=$3" % [
         $balance, $(balance-gasCost), $tx.value])
 
-    if tx.gasLimit < tx.intrinsicGas(fork):
-      return err("invalid tx: not enough gas to perform calculation. avail=$1, require=$2" % [
-        $tx.gasLimit, $tx.intrinsicGas(fork)])
-
     if tx.nonce != nonce:
       return err("invalid tx: account nonce mismatch. txNonce=$1, accNonce=$2" % [
         $tx.nonce, $nonce])
@@ -327,37 +371,7 @@ proc validateTransaction*(
       return err("invalid tx: sender is not an EOA. sender=$1, codeHash=$2" % [
         sender.toHex, codeHash.data.toHex])
 
-
+    if tx.txType >= TxEip4844:
-    if fork >= FkCancun:
-      if tx.payload.len > MAX_CALLDATA_SIZE:
-        return err("invalid tx: payload len exceeds MAX_CALLDATA_SIZE. len=" &
-          $tx.payload.len)
-
-      if tx.accessList.len > MAX_ACCESS_LIST_SIZE:
-        return err("invalid tx: access list len exceeds MAX_ACCESS_LIST_SIZE. len=" &
-          $tx.accessList.len)
-
-      for i, acl in tx.accessList:
-        if acl.storageKeys.len > MAX_ACCESS_LIST_STORAGE_KEYS:
-          return err("invalid tx: access list storage keys len exceeds MAX_ACCESS_LIST_STORAGE_KEYS. " &
-            "index=$1, len=$2" % [$i, $acl.storageKeys.len])
-
-    if tx.txType == TxEip4844:
-      if tx.to.isNone:
-        return err("invalid tx: destination must be not empty")
-
-      if tx.versionedHashes.len == 0:
-        return err("invalid tx: there must be at least one blob")
-
-      if tx.versionedHashes.len > MaxAllowedBlob.int:
-        return err("invalid tx: versioned hashes len exceeds MaxAllowedBlob=" & $MaxAllowedBlob &
-          ". get=" & $tx.versionedHashes.len)
-
-      for i, bv in tx.versionedHashes:
-        if bv.data[0] != BLOB_COMMITMENT_VERSION_KZG:
-          return err("invalid tx: one of blobVersionedHash has invalid version. " &
-            "get=$1, expect=$2" % [$bv.data[0].int, $BLOB_COMMITMENT_VERSION_KZG.int])
-
       # ensure that the user was willing to at least pay the current data gasprice
       let blobGasPrice = getBlobGasPrice(excessBlobGas)
       if tx.maxFeePerBlobGas.uint64 < blobGasPrice:

nimbus/graphql/ethapi.nim

@@ -1399,11 +1399,6 @@ const queryProcs = {
   "chainID": queryChainId
 }
 
-proc inPoolAndOk(ctx: GraphqlContextRef, txHash: Hash256): bool =
-  let res = ctx.txPool.getItem(txHash)
-  if res.isErr: return false
-  res.get().reject == txInfoOk
-
 proc sendRawTransaction(ud: RootRef, params: Args, parent: Node): RespResult {.apiPragma.} =
   # if tx validation failed, the result will be null
   let ctx = GraphqlContextRef(ud)
@@ -1414,7 +1409,7 @@ proc sendRawTransaction(ud: RootRef, params: Args, parent: Node): RespResult {.a
 
     ctx.txPool.add(tx)
 
-    if ctx.inPoolAndOk(txHash):
+    if ctx.txPool.inPoolAndOk(txHash):
       return resp(txHash)
     else:
       return err("transaction rejected by txpool")

nimbus/rpc/p2p.nim

@@ -269,11 +269,14 @@ proc setupEthRpc*(
     ## Returns the transaction hash, or the zero hash if the transaction is not yet available.
     ## Note: Use eth_getTransactionReceipt to get the contract address, after the transaction was mined, when you created a contract.
     let
-      txBytes = hexToSeqByte(data.string)
+      txBytes  = hexToSeqByte(data.string)
       signedTx = decodeTx(txBytes)
+      txHash   = rlpHash(signedTx)
 
     txPool.add(signedTx)
-    result = rlpHash(signedTx).ethHashStr
+    if not txPool.inPoolAndOk(txHash):
+      raise newException(ValueError, "transaction rejected by txpool")
+    result = txHash.ethHashStr
 
   server.rpc("eth_call") do(call: EthCall, quantityTag: string) -> HexDataStr:
     ## Executes a new message call immediately without creating a transaction on the block chain.

nimbus/rpc/rpc_utils.nim

@@ -190,7 +190,7 @@ proc populateTransactionObject*(tx: Transaction, header: BlockHeader, txIndex: i
 
   if tx.txType >= TxEIP4844:
     result.maxFeePerBlobGas = some(encodeQuantity(tx.maxFeePerBlobGas.uint64))
-    #result.versionedHashes = some(tx.versionedHashes)
+    result.versionedHashes = some(tx.versionedHashes)
 
 proc populateBlockObject*(header: BlockHeader, chain: CoreDbRef, fullTx: bool, isUncle = false): BlockObject
     {.gcsafe, raises: [CatchableError].} =

nimbus/sync/handlers/eth.nim

@@ -87,11 +87,6 @@ proc inPool(ctx: EthWireRef, txHash: Hash256): bool =
   let res = ctx.txPool.getItem(txHash)
   res.isOk
 
-proc inPoolAndOk(ctx: EthWireRef, txHash: Hash256): bool =
-  let res = ctx.txPool.getItem(txHash)
-  if res.isErr: return false
-  res.get().reject == txInfoOk
-
 proc successorHeader(db: CoreDbRef,
                      h: BlockHeader,
                      output: var BlockHeader,
@@ -288,7 +283,7 @@ proc fetchTransactions(ctx: EthWireRef, reqHashes: seq[Hash256], peer: Peer): Fu
 
   var newTxHashes = newSeqOfCap[Hash256](reqHashes.len)
   for txHash in reqHashes:
-    if ctx.inPoolAndOk(txHash):
+    if ctx.txPool.inPoolAndOk(txHash):
       newTxHashes.add txHash
 
   let peers = ctx.getPeers(peer)
@@ -485,7 +480,7 @@ method handleAnnouncedTxs*(ctx: EthWireRef, peer: Peer, txs: openArray[Transacti
   for i, txHash in txHashes:
     # Nodes must not automatically broadcast blob transactions to
     # their peers. per EIP-4844 spec
-    if ctx.inPoolAndOk(txHash) and txs[i].txType != TxEip4844:
+    if ctx.txPool.inPoolAndOk(txHash) and txs[i].txType != TxEip4844:
       newTxHashes.add txHash
       validTxs.add txs[i]
 

premix/parser.nim

@@ -91,6 +91,14 @@ proc fromJson*(n: JsonNode, name: string, x: var TxType) =
   else:
     x = hexToInt(node.getStr(), int).TxType
 
+proc fromJson*(n: JsonNode, name: string, x: var seq[Hash256]) =
+  let node = n[name]
+  var h: Hash256
+  x = newSeqOfCap[Hash256](node.len)
+  for v in node:
+    hexToByteArray(v.getStr(), h.data)
+    x.add h
+
 proc parseBlockHeader*(n: JsonNode): BlockHeader =
   n.fromJson "parentHash", result.parentHash
   n.fromJson "sha3Uncles", result.ommersHash
@@ -153,6 +161,13 @@ proc parseTransaction*(n: JsonNode): Transaction =
     if accessList.len > 0:
       for acn in accessList:
         tx.accessList.add parseAccessPair(acn)
+
+  if tx.txType >= TxEip4844:
+    n.fromJson "maxFeePerBlobGas", tx.maxFeePerBlobGas
+
+  if n.hasKey("versionedHashes") and n["versionedHashes"].kind != JNull:
+    n.fromJson "versionedHashes", tx.versionedHashes
+
   tx
 
 proc parseWithdrawal*(n: JsonNode): Withdrawal =