Skip to content

Commit

Permalink
fix processTransaction's gasLimit
Browse files Browse the repository at this point in the history
  • Loading branch information
jangko committed Apr 19, 2023
1 parent 12aea42 commit 918c130
Show file tree
Hide file tree
Showing 13 changed files with 201 additions and 19 deletions.
2 changes: 2 additions & 0 deletions nimbus/core/executor/executor_helpers.nim
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ proc makeReceipt*(vmState: BaseVMState; txType: TxType): Receipt =
else:
rec.isHash = true
rec.hash = vmState.stateDB.rootHash
# we set the status for the t8n output consistency
rec.status = vmState.status

rec.receiptType = txType
rec.cumulativeGasUsed = vmState.cumulativeGasUsed
Expand Down
7 changes: 5 additions & 2 deletions nimbus/core/executor/process_block.nim
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import
../../utils/utils,
../../vm_state,
../../vm_types,
../../utils/debug,
../clique,
../dao,
./calculate_reward,
Expand Down Expand Up @@ -45,10 +46,12 @@ proc processTransactions*(vmState: BaseVMState;
for txIndex, tx in transactions:
var sender: EthAddress
if not tx.getSender(sender):
return err("Could not get sender for tx with index " & $(txIndex) & ": " & $(tx))
let debugTx =tx.debug()
return err("Could not get sender for tx with index " & $(txIndex) & ": " & debugTx)
let rc = vmState.processTransaction(tx, sender, header)
if rc.isErr:
return err("Error processing tx with index " & $(txIndex) & ": " & $(tx))
let debugTx =tx.debug()
return err("Error processing tx with index " & $(txIndex) & ": " & debugTx)
vmState.receipts[txIndex] = vmState.makeReceipt(tx.txType)
ok()

Expand Down
24 changes: 19 additions & 5 deletions nimbus/core/executor/process_transaction.nim
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,17 @@ proc eip1559BaseFee(header: BlockHeader; fork: EVMFork): UInt256 =
if FkLondon <= fork:
result = header.baseFee

proc commitOrRollbackDependingOnGasUsed(vmState: BaseVMState, accTx: SavePoint, gasLimit: GasInt, gasBurned: GasInt, priorityFee: GasInt): Result[GasInt, void] {.raises: [Defect, RlpError].} =
proc commitOrRollbackDependingOnGasUsed(vmState: BaseVMState, accTx: SavePoint,
header: BlockHeader, tx: Transaction,
gasBurned: GasInt, priorityFee: GasInt): Result[GasInt, void] {.raises: [Defect, RlpError].} =
# 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
# header `gasUsed` and the `vmState.cumulativeGasUsed` at a later stage.
if gasLimit < vmState.cumulativeGasUsed + gasBurned:
if header.gasLimit < vmState.cumulativeGasUsed + gasBurned:
vmState.stateDB.rollback(accTx)
debug "invalid tx: block header gasLimit reached",
maxLimit = gasLimit,
maxLimit = header.gasLimit,
gasUsed = vmState.cumulativeGasUsed,
addition = gasBurned
return err()
Expand All @@ -51,6 +53,10 @@ proc commitOrRollbackDependingOnGasUsed(vmState: BaseVMState, accTx: SavePoint,
vmState.stateDB.commit(accTx)
vmState.stateDB.addBalance(vmState.coinbase(), gasBurned.u256 * priorityFee.u256)
vmState.cumulativeGasUsed += gasBurned

# Return remaining gas to the block gas counter so it is
# available for the next transaction.
vmState.gasPool += tx.gasLimit - gasBurned
return ok(gasBurned)

proc asyncProcessTransactionImpl(
Expand All @@ -66,7 +72,7 @@ proc asyncProcessTransactionImpl(

#trace "Sender", sender
#trace "txHash", rlpHash = ty.rlpHash

let
roDB = vmState.readOnlyStateDB
baseFee256 = header.eip1559BaseFee(fork)
Expand All @@ -81,6 +87,14 @@ proc asyncProcessTransactionImpl(
if tx.to.isSome:
await ifNecessaryGetCode(vmState, tx.to.get)

# buy gas, then the gas goes into gasMeter
if vmState.gasPool < tx.gasLimit:
debug "gas limit reached",
gasLimit = vmState.gasPool,
gasNeeded = tx.gasLimit
return
vmState.gasPool -= tx.gasLimit

# Actually, the eip-1559 reference does not mention an early exit.
#
# Even though database was not changed yet but, a `persist()` directive
Expand All @@ -94,7 +108,7 @@ proc asyncProcessTransactionImpl(
accTx = vmState.stateDB.beginSavepoint
gasBurned = tx.txCallEvm(sender, vmState, fork)

res = commitOrRollbackDependingOnGasUsed(vmState, accTx, header.gasLimit, gasBurned, priorityFee)
res = commitOrRollbackDependingOnGasUsed(vmState, accTx, header, tx, gasBurned, priorityFee)

if vmState.generateWitness:
vmState.stateDB.collectWitnessData()
Expand Down
13 changes: 13 additions & 0 deletions nimbus/core/tx_pool/tx_tasks/tx_packer.nim
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ proc runTxCommit(pst: TxPackerStateRef; item: TxItemRef; gasBurned: GasInt)
if vmState.receipts.len <= inx:
vmState.receipts.setLen(inx + receiptsExtensionSize)

# Return remaining gas to the block gas counter so it is
# available for the next transaction.
vmState.gasPool += item.tx.gasLimit - gasBurned

# gasUsed accounting
vmState.cumulativeGasUsed += gasBurned
vmState.receipts[inx] = vmState.makeReceipt(item.tx.txType)

Expand Down Expand Up @@ -170,6 +175,14 @@ proc vmExecGrabItem(pst: TxPackerStateRef; item: TxItemRef): Result[bool,void]
xp = pst.xp
vmState = xp.chain.vmState

# Verify we have enough gas in gasPool
if vmState.gasPool < item.tx.gasLimit:
# skip this transaction and
# continue with next account
# if we don't have enough gas
return ok(false)
vmState.gasPool -= item.tx.gasLimit

# Validate transaction relative to the current vmState
if not xp.classifyValidatePacked(vmState, item):
return ok(false) # continue with next account
Expand Down
3 changes: 2 additions & 1 deletion nimbus/evm/state.nim
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ proc init(
self.parent = parent
self.timestamp = timestamp
self.gasLimit = gasLimit
self.gasPool = gasLimit
self.fee = fee
self.prevRandao = prevRandao
self.blockDifficulty = difficulty
Expand Down Expand Up @@ -67,7 +68,7 @@ proc init(
ac = ac,
parent = parent,
timestamp = timestamp,
gasLimit = gasLimit,
gasLimit = gasLimit,
fee = fee,
prevRandao= prevRandao,
difficulty= difficulty,
Expand Down
1 change: 1 addition & 0 deletions nimbus/evm/types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type
BaseVMState* = ref object of RootObj
prevHeaders* : seq[BlockHeader]
com* : CommonRef
gasPool* : GasInt
parent* : BlockHeader
timestamp* : EthTime
gasLimit* : GasInt
Expand Down
7 changes: 4 additions & 3 deletions tests/test_rpc.nim
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ proc setupEnv(com: CommonRef, signer, ks2: EthAddress, ctx: EthContext): TestEnv
Return

let
vmHeader = BlockHeader(parentHash: parentHash)
vmHeader = BlockHeader(parentHash: parentHash, gasLimit: 5_000_000)
vmState = BaseVMState.new(
parent = BlockHeader(stateRoot: parent.stateRoot),
header = vmHeader,
Expand All @@ -75,7 +75,7 @@ proc setupEnv(com: CommonRef, signer, ks2: EthAddress, ctx: EthContext): TestEnv
)
unsignedTx2 = Transaction(
txType : TxLegacy,
nonce : 0,
nonce : 1,
gasPrice: 1_200,
gasLimit: 70_000,
value : 2.u256,
Expand All @@ -91,7 +91,8 @@ proc setupEnv(com: CommonRef, signer, ks2: EthAddress, ctx: EthContext): TestEnv
vmState.cumulativeGasUsed = 0
for txIndex, tx in txs:
let sender = tx.getSender()
discard vmState.processTransaction(tx, sender, vmHeader)
let rc = vmState.processTransaction(tx, sender, vmHeader)
doAssert(rc.isOk, "Invalid transaction")
vmState.receipts[txIndex] = makeReceipt(vmState, tx.txType)

let
Expand Down
17 changes: 9 additions & 8 deletions tests/test_txpool.nim
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ proc runTxLoader(noisy = true; capture = loadSpecs) =
check 0.GasPrice <= minGasPrice
check minGasPrice <= maxGasPrice


proc runTxPoolTests(noisy = true) =
let elapNoisy = false

Expand Down Expand Up @@ -688,7 +689,7 @@ proc runTxPackerTests(noisy = true) =

# verify incremental packing
check lastItem.info != newItem.info
check saveStats.packed <= newStats.packed
check saveStats.packed >= newStats.packed

noisy.say "***", "2st bLock size=", newTotal, " stats=", newStats.pp

Expand Down Expand Up @@ -752,7 +753,7 @@ proc runTxPackerTests(noisy = true) =
" max=", xq.maxGasLimit,
" slack=", xq.maxGasLimit - xq.gasCumulative

check smallerBlockSize < xq.gasCumulative
check smallerBlockSize <= xq.gasCumulative
check 0 < xq.profitability

# Well, this ratio should be above 100 but might be slightly less
Expand Down Expand Up @@ -780,7 +781,7 @@ proc runTxPackerTests(noisy = true) =
# 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.
xq.flags = xq.flags + {packItemsMaxGasLimit}
xq.flags = xq.flags #+ {packItemsMaxGasLimit}

# Invoke packer
let blk = xq.ethBlock
Expand Down Expand Up @@ -814,7 +815,7 @@ proc runTxPackerTests(noisy = true) =
bdy = BlockBody(transactions: blk.txs)
hdr = block:
var rc = blk.header
rc.gasLimit = blk.header.gasUsed
rc.gasLimit = blk.header.gasLimit
rc.testKeySign

# Make certain that some tx was set up so that its gasLimit overlaps
Expand All @@ -825,7 +826,7 @@ proc runTxPackerTests(noisy = true) =
setTraceLevel()

# Test low-level function for adding the new block to the database
xq.chain.maxMode = (packItemsMaxGasLimit in xq.flags)
#xq.chain.maxMode = (packItemsMaxGasLimit in xq.flags)
xq.chain.clearAccounts
check xq.chain.vmState.processBlock(poa, hdr, bdy).isOK

Expand Down Expand Up @@ -886,9 +887,9 @@ when isMainModule:
noisy.runTxPoolTests
noisy.runTxPackerTests

#runTxPoolCliqueTest()
#runTxPoolPosTest()
#noisy.runTxHeadDelta
runTxPoolCliqueTest()
runTxPoolPosTest()
noisy.runTxHeadDelta

#noisy.runTxLoader(dir = ".")
#noisy.runTxPoolTests
Expand Down
9 changes: 9 additions & 0 deletions tools/t8n/t8n_test.nim
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,15 @@ const
output: T8nOutput(alloc: false, result: false),
expExitCode: ErrorJson.int,
),
TestSpec(
name : "GasUsedHigherThanBlockGasLimitButNotWithRefundsSuicideLast_Frontier",
base : "testdata/00-516",
input : t8nInput(
"alloc.json", "txs.rlp", "env.json", "Frontier", "5000000000000000000",
),
output: T8nOutput(alloc: true, result: true),
expOut: "exp.json",
),
]

proc main() =
Expand Down
16 changes: 16 additions & 0 deletions tools/t8n/testdata/00-516/alloc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
"balance" : "0x4a723dc6b40b8a9a000000",
"code" : "0x",
"nonce" : "0x00",
"storage" : {
}
},
"0xaaaf5374fce5edbc8e2a8697c15331677e6ebf0b" : {
"balance" : "0x02540be400",
"code" : "0x73a94f5374fce5edbc8e2a8697c15331677e6ebf0bff",
"nonce" : "0x00",
"storage" : {
}
}
}
13 changes: 13 additions & 0 deletions tools/t8n/testdata/00-516/env.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"currentCoinbase" : "0x8888f1f195afa192cfee860698584c030f4c9db1",
"currentNumber" : "0x01",
"currentTimestamp" : "0x5db6fbd9",
"currentGasLimit" : "0x023dd4",
"previousHash" : "0xce1f26f798dd03c8782d63b3e42e79a64eaea5694ea686ac5d7ce3df5171d1ae",
"parentTimestamp" : "0x54c98c81",
"parentDifficulty" : "0x020000",
"parentUncleHash" : "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
"blockHashes" : {
"0" : "0xce1f26f798dd03c8782d63b3e42e79a64eaea5694ea686ac5d7ce3df5171d1ae"
}
}
Loading

0 comments on commit 918c130

Please sign in to comment.