From 8193a4ae6a7d918dc12964f1b1fcea70ccd05d35 Mon Sep 17 00:00:00 2001 From: andri lim Date: Wed, 27 Feb 2019 10:30:03 +0700 Subject: [PATCH] refactor gas used in transaction --- nimbus/p2p/executor.nim | 47 ++++++++++++++------------------ nimbus/tracer.nim | 10 +++---- nimbus/vm_state_transactions.nim | 18 ++++++------ tests/test_generalstate_json.nim | 2 +- 4 files changed, 33 insertions(+), 44 deletions(-) diff --git a/nimbus/p2p/executor.nim b/nimbus/p2p/executor.nim index dda4877fb..78ad1516e 100644 --- a/nimbus/p2p/executor.nim +++ b/nimbus/p2p/executor.nim @@ -6,7 +6,7 @@ import options, ../vm/[computation, interpreter_dispatch, message], ../vm/interpreter/vm_forks -proc contractCall(t: Transaction, vmState: BaseVMState, sender: EthAddress, forkOverride=none(Fork)): UInt256 = +proc contractCall(tx: Transaction, vmState: BaseVMState, sender: EthAddress, forkOverride=none(Fork)): GasInt = # TODO: this function body was copied from GST with it's comments and TODOs. # Right now it's main purpose is to produce VM tracing when syncing block with # contract call. At later stage, this proc together with applyCreateTransaction @@ -15,33 +15,30 @@ proc contractCall(t: Transaction, vmState: BaseVMState, sender: EthAddress, fork # TODO: replace with cachingDb or similar approach; necessary # when calls/subcalls/etc come in, too. var db = vmState.accountDb - let storageRoot = db.getStorageRoot(t.to) + let storageRoot = db.getStorageRoot(tx.to) - var computation = setupComputation(vmState, t, sender, forkOverride) + var computation = setupComputation(vmState, tx, sender, forkOverride) # contract creation transaction.to == 0, so ensure happens after - db.addBalance(t.to, t.value) - - let header = vmState.blockHeader - let gasCost = t.gasLimit.u256 * t.gasPrice.u256 + db.addBalance(tx.to, tx.value) if execComputation(computation): let - gasRemaining = computation.gasMeter.gasRemaining.u256 - gasRefunded = computation.getGasRefund().u256 - gasUsed = t.gasLimit.u256 - gasRemaining + gasRemaining = computation.gasMeter.gasRemaining + gasRefunded = computation.getGasRefund() + gasUsed = tx.gasLimit - gasRemaining gasRefund = min(gasRefunded, gasUsed div 2) - gasRefundAmount = (gasRefund + gasRemaining) * t.gasPrice.u256 + gasRefundAmount = (gasRefund + gasRemaining).u256 * tx.gasPrice.u256 db.addBalance(sender, gasRefundAmount) - return (t.gasLimit.u256 - gasRemaining - gasRefund) * t.gasPrice.u256 + return (tx.gasLimit - gasRemaining - gasRefund) else: - db.subBalance(t.to, t.value) - db.addBalance(sender, t.value) - db.setStorageRoot(t.to, storageRoot) + db.subBalance(tx.to, tx.value) + db.addBalance(sender, tx.value) + db.setStorageRoot(tx.to, storageRoot) if computation.tracingEnabled: computation.traceError() vmState.clearLogs() - return t.gasLimit.u256 * t.gasPrice.u256 + return tx.gasLimit # this proc should not be here, we need to refactor # processTransaction @@ -50,7 +47,7 @@ proc isPrecompiles*(address: EthAddress): bool {.inline.} = if address[i] != 0: return result = address[19] >= 1.byte and address[19] <= 8.byte -proc processTransaction*(t: Transaction, sender: EthAddress, vmState: BaseVMState): UInt256 = +proc processTransaction*(t: Transaction, sender: EthAddress, vmState: BaseVMState): GasInt = ## Process the transaction, write the results to db. ## Returns amount of ETH to be rewarded to miner trace "Sender", sender @@ -68,10 +65,10 @@ proc processTransaction*(t: Transaction, sender: EthAddress, vmState: BaseVMStat var balance = db.getBalance(sender) if balance < upfrontCost: if balance <= upfrontGasCost: - result = balance + result = (balance div t.gasPrice.u256).truncate(GasInt) balance = 0.u256 else: - result = upfrontGasCost + result = t.gasLimit balance -= upfrontGasCost transactionFailed = true else: @@ -81,7 +78,7 @@ proc processTransaction*(t: Transaction, sender: EthAddress, vmState: BaseVMStat if transactionFailed: return - var gasUsed = t.payload.intrinsicGas.GasInt # += 32000 appears in Homestead when contract create + var gasUsed = t.intrinsicGas # += 32000 appears in Homestead when contract create if gasUsed > t.gasLimit: debug "Transaction failed. Out of gas." @@ -114,7 +111,7 @@ proc processTransaction*(t: Transaction, sender: EthAddress, vmState: BaseVMStat refund += t.value db.addBalance(sender, refund) - return gasUsed.u256 * t.gasPrice.u256 + return gasUsed type # TODO: these types need to be removed @@ -167,15 +164,11 @@ proc processBlock*(chainDB: BaseChainDB, head, header: BlockHeader, body: BlockB for txIndex, tx in body.transactions: var sender: EthAddress if tx.getSender(sender): - let txFee = processTransaction(tx, sender, vmState) - - # perhaps this can be altered somehow - # or processTransaction return only gasUsed - # a `div` here is ugly and possibly div by zero - let gasUsed = (txFee div tx.gasPrice.u256).truncate(GasInt) + let gasUsed = processTransaction(tx, sender, vmState) cumulativeGasUsed += gasUsed # miner fee + let txFee = gasUsed.u256 * tx.gasPrice.u256 stateDb.addBalance(header.coinbase, txFee) else: debug "Could not get sender", txIndex, tx diff --git a/nimbus/tracer.nim b/nimbus/tracer.nim index ea7869fe1..4378cea14 100644 --- a/nimbus/tracer.nim +++ b/nimbus/tracer.nim @@ -102,11 +102,11 @@ proc traceTransaction*(db: BaseChainDB, header: BlockHeader, stateDiff["beforeRoot"] = %($stateDb.rootHash) beforeRoot = stateDb.rootHash - let txFee = processTransaction(tx, sender, vmState) + let gasUsed = processTransaction(tx, sender, vmState) + let txFee = gasUsed.u256 * tx.gasPrice.u256 stateDb.addBalance(header.coinbase, txFee) if idx == txIndex: - gasUsed = (txFee div tx.gasPrice.u256).truncate(GasInt) after.captureAccount(stateDb, sender, senderName) after.captureAccount(stateDb, recipient, recipientName) after.captureAccount(stateDb, header.coinbase, minerName) @@ -204,10 +204,8 @@ proc traceBlock*(db: BaseChainDB, header: BlockHeader, body: BlockBody, tracerFl var gasUsed = GasInt(0) for tx in body.transactions: - let - sender = tx.getSender - txFee = processTransaction(tx, sender, vmState) - gasUsed = gasUsed + (txFee div tx.gasPrice.u256).truncate(GasInt) + let sender = tx.getSender + gasUsed = gasUsed + processTransaction(tx, sender, vmState) result = vmState.getTracingResult() result["gas"] = %gasUsed diff --git a/nimbus/vm_state_transactions.nim b/nimbus/vm_state_transactions.nim index 2c869b6e9..47ece871c 100644 --- a/nimbus/vm_state_transactions.nim +++ b/nimbus/vm_state_transactions.nim @@ -80,7 +80,7 @@ proc execComputation*(computation: var BaseComputation): bool = else: snapshot.revert() -proc applyCreateTransaction*(tx: Transaction, vmState: BaseVMState, sender: EthAddress, forkOverride=none(Fork)): UInt256 = +proc applyCreateTransaction*(tx: Transaction, vmState: BaseVMState, sender: EthAddress, forkOverride=none(Fork)): GasInt = doAssert tx.isContractCreation # TODO: clean up params trace "Contract creation" @@ -96,12 +96,10 @@ proc applyCreateTransaction*(tx: Transaction, vmState: BaseVMState, sender: EthA # also a couple lines can collapse because variable used once # once verified in GST fixture let - gasRemaining = c.gasMeter.gasRemaining.u256 - gasRefunded = c.getGasRefund().u256 - gasUsed = tx.gasLimit.u256 - gasRemaining + gasRemaining = c.gasMeter.gasRemaining + gasRefunded = c.getGasRefund() + gasUsed = tx.gasLimit - gasRemaining gasRefund = min(gasRefunded, gasUsed div 2) - gasRefundAmount = (gasRefund + gasRemaining) * tx.gasPrice.u256 - #echo "gasRemaining is ", gasRemaining, " and gasRefunded = ", gasRefunded, " and gasUsed = ", gasUsed, " and gasRefund = ", gasRefund, " and gasRefundAmount = ", gasRefundAmount var codeCost = 200 * c.output.len @@ -112,7 +110,7 @@ proc applyCreateTransaction*(tx: Transaction, vmState: BaseVMState, sender: EthA if not c.isSuicided(contractAddress): # make changes only if it not selfdestructed db.addBalance(contractAddress, tx.value) - if gasRemaining >= codeCost.u256: + if gasRemaining >= codeCost: db.setCode(contractAddress, c.output.toRange) else: # XXX: Homestead behaves differently; reverts state on gas failure @@ -120,8 +118,8 @@ proc applyCreateTransaction*(tx: Transaction, vmState: BaseVMState, sender: EthA codeCost = 0 db.setCode(contractAddress, ByteRange()) - db.addBalance(sender, (tx.gasLimit.u256 - gasUsed - codeCost.u256 + gasRefund) * tx.gasPrice.u256) - return (gasUsed + codeCost.u256 - gasRefund) * tx.gasPrice.u256 + db.addBalance(sender, (tx.gasLimit - gasUsed - codeCost + gasRefund).u256 * tx.gasPrice.u256) + return (gasUsed + codeCost - gasRefund) else: # FIXME: don't do this revert, but rather only subBalance correctly # the if transactionfailed at end is what is supposed to pick it up @@ -132,7 +130,7 @@ proc applyCreateTransaction*(tx: Transaction, vmState: BaseVMState, sender: EthA if c.tracingEnabled: c.traceError() vmState.clearLogs() - return tx.gasLimit.u256 * tx.gasPrice.u256 + return tx.gasLimit #[ method executeTransaction(vmState: BaseVMState, transaction: Transaction): (BaseComputation, BlockHeader) {.base.}= diff --git a/tests/test_generalstate_json.nim b/tests/test_generalstate_json.nim index 0d4758f0a..2f1a0257d 100644 --- a/tests/test_generalstate_json.nim +++ b/tests/test_generalstate_json.nim @@ -60,7 +60,7 @@ proc testFixtureIndexes(prevStateRoot: Hash256, header: BlockHeader, pre: JsonNo #db.addBalance(generateAddress(sender, transaction.accountNonce), transaction.value) let createGasUsed = applyCreateTransaction(transaction, vmState, sender, some(fork)) - db.addBalance(header.coinbase, createGasUsed) + db.addBalance(header.coinbase, createGasUsed.u256 * transaction.gasPrice.u256) return var computation = setupComputation(vmState, transaction, sender, some(fork))