From d4e95fae947ae2cf86797ad603ff45a01f398386 Mon Sep 17 00:00:00 2001 From: tersec Date: Thu, 27 Sep 2018 19:09:26 +0000 Subject: [PATCH] refactor aspects of transaction processing out of GeneralStateTests handler; get p2p/chain runnning through block 48680; combine/deduplicate two different but equivalent methods of caulculating transaction intrinsic gas cost; fix BaseTransaction references to just Transaction references in hitherto unreferenced stubbed out source so it builds; clean up some imports from refactoring (#161) --- nimbus/p2p/chain.nim | 59 ++++++++++++++++++++++--------- nimbus/vm_state_transactions.nim | 60 +++++++++++++++++++++++++++++--- tests/test_generalstate_json.nim | 47 +++---------------------- tests/test_helpers.nim | 16 +-------- 4 files changed, 103 insertions(+), 79 deletions(-) diff --git a/nimbus/p2p/chain.nim b/nimbus/p2p/chain.nim index 2d6cc1008..b7d285083 100644 --- a/nimbus/p2p/chain.nim +++ b/nimbus/p2p/chain.nim @@ -1,6 +1,8 @@ import ../db/[db_chain, state_db], eth_common, chronicles, ../vm_state, ../vm_types, ../transaction, ranges, ../vm/[computation, interpreter_dispatch, message], ../constants, stint, nimcrypto, ../utils/addresses, - eth_trie/memdb, eth_trie, rlp + ../vm_state_transactions, + eth_trie/memdb, eth_trie, rlp, + sugar type @@ -31,13 +33,6 @@ method getSuccessorHeader*(c: Chain, h: BlockHeader, output: var BlockHeader): b method getBlockBody*(c: Chain, blockHash: KeccakHash): BlockBodyRef = result = nil -proc dataGas(data: openarray[byte]): GasInt = - for i in data: - if i == 0: - result += 4 - else: - result += 68 - proc processTransaction(db: var AccountStateDB, t: Transaction, sender: EthAddress, head: BlockHeader, chainDB: BaseChainDB): UInt256 = ## Process the transaction, write the results to db. ## Returns amount of ETH to be rewarded to miner @@ -47,6 +42,9 @@ proc processTransaction(db: var AccountStateDB, t: Transaction, sender: EthAddre db.setNonce(sender, db.getNonce(sender) + 1) var transactionFailed = false + #t.dump + + # TODO: combine/refactor re validate let upfrontGasCost = t.gasLimit.u256 * t.gasPrice.u256 let upfrontCost = upfrontGasCost + t.value var balance = db.getBalance(sender) @@ -65,26 +63,53 @@ proc processTransaction(db: var AccountStateDB, t: Transaction, sender: EthAddre if transactionFailed: return - var gasUsed = 21000.GasInt - if t.payload.len != 0: - gasUsed += dataGas(t.payload) + var gasUsed = t.payload.intrinsicGas.GasInt + #echo "gasUsed is ", gasUsed if t.isContractCreation: # gasUsed += 32000 # This appears in Homestead. echo "Contract creation" + # TODO: check the 200x constant up front if gasUsed > t.gasLimit: echo "Transaction failed. Out of gas." transactionFailed = true else: if t.isContractCreation: - db.setCode(generateAddress(sender, t.accountNonce), t.payload.toRange) - let msg = newMessage(t.gasLimit, t.gasPrice, t.to, sender, t.value, @[], t.payload) + # TODO: split out into separate function?; interleaving the cases obfuscates let vmState = newBaseVMState(head, chainDB) + + # TODO: setupComputation + let msg = newMessage(t.gasLimit - gasUsed, t.gasPrice, t.to, sender, t.value, @[], t.payload) var c = newBaseComputation(vmState, head.blockNumber, msg) - c.executeOpcodes() - echo "isError: ", c.isError - # TODO: Handle failure, and gas consumption + + if execComputation(c, vmState): + let contractAddress = generateAddress(sender, t.accountNonce) + db.setCode(contractAddress, c.output.toRange) + db.addBalance(contractAddress, t.value) + + # XXX: copy/pasted from GST fixture + # TODO: more merging/refactoring/etc + # also a couple lines can collapse because variable used once + # once verified in GST fixture + let + gasRemaining = c.gasMeter.gasRemaining.u256 + gasRefunded = c.gasMeter.gasRefunded.u256 + gasUsed2 = t.gasLimit.u256 - gasRemaining + gasRefund = min(gasRefunded, gasUsed2 div 2) + gasRefundAmount = (gasRefund + gasRemaining) * t.gasPrice.u256 + #echo "gasRemaining is ", gasRemaining, " and gasRefunded = ", gasRefunded, " and gasUsed2 = ", gasUsed2, " and gasRefund = ", gasRefund, " and gasRefundAmount = ", gasRefundAmount + let codeCost = (200 * c.output.len).u256 + db.addBalance(sender, (t.gasLimit.u256 - gasUsed2 - codeCost)*t.gasPrice.u256) + echo c.output.len, " bytes: ", c.output + return (gasUsed2 + codeCost) * t.gasPrice.u256 + + 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 + db.addBalance(sender, t.value) + echo "isError: ", c.isError + return upfrontGasCost else: let code = db.getCode(t.to) @@ -150,6 +175,7 @@ method persistBlocks*(c: Chain, headers: openarray[BlockHeader], bodies: openarr assert(false, "Could not get sender") var mainReward = blockReward + gasReward + #echo "mainReward = ", mainReward , " with blockReward = ", blockReward, " and gasReward = ", gasReward if headers[i].ommersHash != EMPTY_UNCLE_HASH: let h = c.db.persistUncles(bodies[i].uncles) @@ -160,6 +186,7 @@ method persistBlocks*(c: Chain, headers: openarray[BlockHeader], bodies: openarr uncleReward = uncleReward * blockReward uncleReward = uncleReward div 8.u256 stateDb.addBalance(bodies[i].uncles[u].coinbase, uncleReward) + #echo "adding via unclehash to mainReward: ", (blockReward div 32.u256) mainReward += blockReward div 32.u256 # Reward beneficiary diff --git a/nimbus/vm_state_transactions.nim b/nimbus/vm_state_transactions.nim index 81bf79162..58ddaf37b 100644 --- a/nimbus/vm_state_transactions.nim +++ b/nimbus/vm_state_transactions.nim @@ -6,11 +6,61 @@ # at your option. This file may not be copied, modified, or distributed except according to those terms. import - strformat, tables, + ranges/typedranges, sequtils, strformat, tables, + eth_common, ./constants, ./errors, ./vm/computation, - ./transaction, ./vm_types, ./vm_state, ./block_types, ./db/db_chain, ./utils/header + ./transaction, ./vm_types, ./vm_state, ./block_types, ./db/[db_chain, state_db], ./utils/header, + ./vm/interpreter -method executeTransaction(vmState: var BaseVMState, transaction: BaseTransaction): (BaseComputation, BlockHeader) {.base.}= +func intrinsicGas*(data: openarray[byte]): GasInt = + result = 21_000 + for i in data: + if i == 0: + result += 4 + else: + result += 68 + +proc validateTransaction*(vmState: BaseVMState, transaction: Transaction, sender: EthAddress): bool = + # XXX: https://github.com/status-im/nimbus/issues/35#issuecomment-391726518 + # XXX: lots of avoidable u256 construction + var readOnlyDB = vmState.readOnlyStateDB + let limitAndValue = transaction.gasLimit.u256 + transaction.value + let gas_cost = transaction.gasLimit.u256 * transaction.gasPrice.u256 + + transaction.gasLimit >= transaction.payload.intrinsicGas and + transaction.gasPrice <= (1 shl 34) and + limitAndValue <= readOnlyDB.getBalance(sender) and + transaction.accountNonce == readOnlyDB.getNonce(sender) and + readOnlyDB.getBalance(sender) >= gas_cost + +proc setupComputation*(header: BlockHeader, vmState: var BaseVMState, transaction: Transaction, sender: EthAddress) : BaseComputation = + let message = newMessage( + gas = transaction.gasLimit - transaction.payload.intrinsicGas, + gasPrice = transaction.gasPrice, + to = transaction.to, + sender = sender, + value = transaction.value, + data = transaction.payload, + code = vmState.readOnlyStateDB.getCode(transaction.to).toSeq, + options = newMessageOptions(origin = sender, + createAddress = transaction.to)) + + result = newBaseComputation(vmState, header.blockNumber, message) + result.precompiles = initTable[string, Opcode]() + doAssert result.isOriginComputation + +proc execComputation*(computation: var BaseComputation, vmState: BaseVMState): bool = + try: + computation.executeOpcodes() + #[vmState.mutateStateDB: + for deletedAccount in computation.getAccountsForDeletion: + db.deleteAccount deletedAccount]# + + result = not computation.isError + except ValueError: + result = false + +method executeTransaction(vmState: var BaseVMState, transaction: Transaction): (BaseComputation, BlockHeader) {.base.}= # Execute the transaction in the vm # TODO: introduced here: https://github.com/ethereum/py-evm/commit/21c57f2d56ab91bb62723c3f9ebe291d0b132dde # Refactored/Removed here: https://github.com/ethereum/py-evm/commit/cc991bf @@ -18,7 +68,7 @@ method executeTransaction(vmState: var BaseVMState, transaction: BaseTransaction raise newException(ValueError, "Must be implemented by subclasses") -method addTransaction*(vmState: var BaseVMState, transaction: BaseTransaction, computation: BaseComputation, b: Block): (Block, Table[string, string]) = +method addTransaction*(vmState: var BaseVMState, transaction: Transaction, computation: BaseComputation, b: Block): (Block, Table[string, string]) = # Add a transaction to the given block and # return `trieData` to store the transaction data in chaindb in VM layer # Update the bloomFilter, transaction trie and receipt trie roots, bloom_filter, @@ -49,7 +99,7 @@ method addTransaction*(vmState: var BaseVMState, transaction: BaseTransaction, c method applyTransaction*( vmState: var BaseVMState, - transaction: BaseTransaction, + transaction: Transaction, b: Block, isStateless: bool): (BaseComputation, Block, Table[string, string]) = # Apply transaction to the given block diff --git a/tests/test_generalstate_json.nim b/tests/test_generalstate_json.nim index 2e969937e..997c74670 100644 --- a/tests/test_generalstate_json.nim +++ b/tests/test_generalstate_json.nim @@ -6,13 +6,13 @@ # at your option. This file may not be copied, modified, or distributed except according to those terms. import - unittest, strformat, strutils, sequtils, tables, json, ospaths, times, + unittest, strformat, strutils, tables, json, ospaths, times, byteutils, ranges/typedranges, nimcrypto/[keccak, hash], rlp, eth_trie/[types, memdb], eth_common, eth_keys, ./test_helpers, ../nimbus/[constants, errors], - ../nimbus/[vm_state, vm_types], + ../nimbus/[vm_state, vm_types, vm_state_transactions], ../nimbus/utils/header, ../nimbus/vm/interpreter, ../nimbus/db/[db_chain, state_db] @@ -23,47 +23,6 @@ suite "generalstate json tests": jsonTest("GeneralStateTests", testFixture) -proc validateTransaction(vmState: BaseVMState, transaction: Transaction, sender: EthAddress): bool = - # XXX: https://github.com/status-im/nimbus/issues/35#issuecomment-391726518 - # XXX: lots of avoidable u256 construction - var readOnlyDB = vmState.readOnlyStateDB - let limitAndValue = transaction.gasLimit.u256 + transaction.value - let gas_cost = transaction.gasLimit.u256 * transaction.gasPrice.u256 - - transaction.gasLimit >= transaction.getFixtureIntrinsicGas and - transaction.gasPrice <= (1 shl 34) and - limitAndValue <= readOnlyDB.getBalance(sender) and - transaction.accountNonce == readOnlyDB.getNonce(sender) and - readOnlyDB.getBalance(sender) >= gas_cost - -proc setupComputation(header: BlockHeader, vmState: var BaseVMState, transaction: Transaction, sender: EthAddress) : BaseComputation = - let message = newMessage( - gas = transaction.gasLimit - transaction.getFixtureIntrinsicGas, - gasPrice = transaction.gasPrice, - to = transaction.to, - sender = sender, - value = transaction.value, - data = transaction.payload, - code = vmState.readOnlyStateDB.getCode(transaction.to).toSeq, - options = newMessageOptions(origin = sender, - createAddress = transaction.to)) - - # doAssert not message.isCreate - result = newBaseComputation(vmState, header.blockNumber, message) - result.precompiles = initTable[string, Opcode]() - doAssert result.isOriginComputation - -proc execComputation(computation: var BaseComputation, vmState: var BaseVMState): bool = - try: - computation.executeOpcodes() - vmState.mutateStateDB: - for deletedAccount in computation.getAccountsForDeletion: - db.deleteAccount deletedAccount - - result = not computation.isError - except ValueError: - result = false - proc testFixtureIndexes(header: BlockHeader, pre: JsonNode, transaction: Transaction, sender: EthAddress, expectedHash: string) = var vmState = newBaseVMState(header, newBaseChainDB(newMemoryDb())) vmState.mutateStateDB: @@ -101,6 +60,8 @@ proc testFixtureIndexes(header: BlockHeader, pre: JsonNode, transaction: Transac gasRefundAmount = (gasRefund + gasRemaining) * transaction.gasPrice.u256 vmState.mutateStateDB: + for deletedAccount in computation.getAccountsForDeletion: + db.deleteAccount deletedAccount if header.coinbase notin computation.getAccountsForDeletion: db.subBalance(header.coinbase, gasRefundAmount) db.addBalance(header.coinbase, gas_cost) diff --git a/tests/test_helpers.nim b/tests/test_helpers.nim index 34681f162..b62cfeabd 100644 --- a/tests/test_helpers.nim +++ b/tests/test_helpers.nim @@ -6,7 +6,7 @@ # at your option. This file may not be copied, modified, or distributed except according to those terms. import - os, macros, json, sequtils, strformat, strutils, parseutils, ospaths, tables, + os, macros, json, strformat, strutils, parseutils, ospaths, tables, byteutils, eth_common, eth_keys, ranges/typedranges, ../nimbus/[vm_state, constants], ../nimbus/db/[db_chain, state_db], @@ -247,17 +247,3 @@ proc getFixtureTransactionSender*(j: JsonNode): EthAddress = else: # XXX: appropriate failure mode; probably raise something discard - -proc getFixtureIntrinsicGas*(transaction: Transaction) : auto = - # Py-EVM has _get_homestead_intrinsic_gas and _get_frontier_intrinsic_gas - # Using former. - - # TODO: refactor and pull from nimbus/vm/interpreter/gas_costs.nim - let - gasTransaction = 21_000 - gasTXDataZero = 4 - gasTXDataNonZero = 68 - numZeroBytes = transaction.payload.count(0) - gasCosts = forkToSchedule(FkHomestead) - - result = gasTransaction + gasTXDataZero * numZeroBytes + gasTXDataNonZero * (transaction.payload.len - numZeroBytes)