diff --git a/GeneralStateTests.md b/GeneralStateTests.md index 42c2f2af3..ebe372b46 100644 --- a/GeneralStateTests.md +++ b/GeneralStateTests.md @@ -58,9 +58,9 @@ OK: 0/46 Fail: 0/46 Skip: 46/46 OK: 0/2 Fail: 2/2 Skip: 0/2 ## stBadOpcode ```diff -- badOpcodes.json Fail ++ badOpcodes.json OK ``` -OK: 0/1 Fail: 1/1 Skip: 0/1 +OK: 1/1 Fail: 0/1 Skip: 0/1 ## stBugs ```diff + evmBytecode.json OK @@ -348,10 +348,10 @@ OK: 0/58 Fail: 58/58 Skip: 0/58 - callcodecallcodecallcode_111_OOGMAfter_1.json Fail - callcodecallcodecallcode_111_OOGMAfter_2.json Fail - callcodecallcodecallcode_111_OOGMAfter_3.json Fail -- contractCreationMakeCallThatAskMoreGasThenTransactionProvided.jsonFail ++ contractCreationMakeCallThatAskMoreGasThenTransactionProvided.jsonOK - createInitFail_OOGduringInit.json Fail ``` -OK: 3/30 Fail: 25/30 Skip: 2/30 +OK: 4/30 Fail: 24/30 Skip: 2/30 ## stCodeCopyTest ```diff ExtCodeCopyTests.json Skip @@ -360,10 +360,10 @@ OK: 0/1 Fail: 0/1 Skip: 1/1 ## stCodeSizeLimit ```diff - codesizeInit.json Fail -- codesizeOOGInvalidSize.json Fail -- codesizeValid.json Fail ++ codesizeOOGInvalidSize.json OK ++ codesizeValid.json OK ``` -OK: 0/3 Fail: 3/3 Skip: 0/3 +OK: 2/3 Fail: 1/3 Skip: 0/3 ## stCreateTest ```diff - CREATE_AcreateB_BSuicide_BStore.json Fail @@ -505,12 +505,12 @@ OK: 1/1 Fail: 0/1 Skip: 0/1 ## stHomesteadSpecific ```diff - contractCreationOOGdontLeaveEmptyContract.json Fail -- contractCreationOOGdontLeaveEmptyContractViaTransaction.json Fail ++ contractCreationOOGdontLeaveEmptyContractViaTransaction.json OK - createContractViaContract.json Fail - createContractViaContractOOGInitCode.json Fail - createContractViaTransactionCost53000.json Fail ``` -OK: 0/5 Fail: 5/5 Skip: 0/5 +OK: 1/5 Fail: 4/5 Skip: 0/5 ## stInitCodeTest ```diff - CallContractToCreateContractAndCallItOOG.json Fail @@ -526,13 +526,13 @@ OK: 0/5 Fail: 5/5 Skip: 0/5 - OutOfGasPrefundedContractCreation.json Fail - ReturnTest.json Fail - ReturnTest2.json Fail -- StackUnderFlowContractCreation.json Fail ++ StackUnderFlowContractCreation.json OK - TransactionCreateAutoSuicideContract.json Fail -- TransactionCreateRandomInitCode.json Fail -- TransactionCreateStopInInitcode.json Fail -- TransactionCreateSuicideInInitcode.json Fail ++ TransactionCreateRandomInitCode.json OK ++ TransactionCreateStopInInitcode.json OK ++ TransactionCreateSuicideInInitcode.json OK ``` -OK: 1/18 Fail: 17/18 Skip: 0/18 +OK: 5/18 Fail: 13/18 Skip: 0/18 ## stLogTests ```diff - log0_emptyMem.json Fail @@ -1599,7 +1599,7 @@ OK: 28/37 Fail: 6/37 Skip: 3/37 - RevertDepthCreateAddressCollision.json Fail - RevertDepthCreateOOG.json Fail + RevertInCallCode.json OK -- RevertInCreateInInit.json Fail ++ RevertInCreateInInit.json OK + RevertInDelegateCall.json OK + RevertInStaticCall.json OK + RevertOnEmptyStack.json OK @@ -1632,7 +1632,7 @@ OK: 28/37 Fail: 6/37 Skip: 3/37 - TouchToEmptyAccountRevert2.json Fail - TouchToEmptyAccountRevert3.json Fail ``` -OK: 14/43 Fail: 24/43 Skip: 5/43 +OK: 15/43 Fail: 23/43 Skip: 5/43 ## stShift ```diff sar00.json Skip @@ -1699,11 +1699,11 @@ OK: 38/40 Fail: 0/40 Skip: 2/40 OK: 8/16 Fail: 8/16 Skip: 0/16 ## stSpecialTest ```diff -- FailedCreateRevertsDeletion.json Fail ++ FailedCreateRevertsDeletion.json OK - JUMPDEST_Attack.json Fail - JUMPDEST_AttackwithJump.json Fail OverflowGasMakeMoney.json Skip -- StackDepthLimitSEC.json Fail ++ StackDepthLimitSEC.json OK block504980.json Skip - deploymentError.json Fail failed_tx_xcf416c53.json Skip @@ -1713,18 +1713,18 @@ OK: 8/16 Fail: 8/16 Skip: 0/16 txCost-sec73.json Skip - tx_e1c174e2.json Fail ``` -OK: 1/13 Fail: 6/13 Skip: 6/13 +OK: 3/13 Fail: 4/13 Skip: 6/13 ## stStackTests ```diff shallowStack.json Skip stackOverflow.json Skip stackOverflowDUP.json Skip stackOverflowM1.json Skip -- stackOverflowM1DUP.json Fail ++ stackOverflowM1DUP.json OK stackOverflowM1PUSH.json Skip stackOverflowPUSH.json Skip ``` -OK: 0/7 Fail: 1/7 Skip: 6/7 +OK: 1/7 Fail: 0/7 Skip: 6/7 ## stStaticCall ```diff static_ABAcalls0.json Skip @@ -2091,7 +2091,7 @@ OK: 26/67 Fail: 39/67 Skip: 2/67 - CreateMessageReverted.json Fail - CreateMessageSuccess.json Fail + CreateTransactionReverted.json OK -- CreateTransactionSuccess.json Fail ++ CreateTransactionSuccess.json OK + EmptyTransaction.json OK - EmptyTransaction2.json Fail - EmptyTransaction3.json Fail @@ -2131,7 +2131,7 @@ OK: 26/67 Fail: 39/67 Skip: 2/67 + UserTransactionZeroCost.json OK + UserTransactionZeroCostWithData.json OK ``` -OK: 26/44 Fail: 18/44 Skip: 0/44 +OK: 27/44 Fail: 17/44 Skip: 0/44 ## stTransitionTest ```diff - createNameRegistratorPerTxsAfter.json Fail diff --git a/nimbus/p2p/chain.nim b/nimbus/p2p/chain.nim index a55ba1495..98fd61a0b 100644 --- a/nimbus/p2p/chain.nim +++ b/nimbus/p2p/chain.nim @@ -1,5 +1,5 @@ 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, + ../vm/[computation, interpreter_dispatch, message], ../constants, stint, nimcrypto, ../vm_state_transactions, eth_trie/memdb, eth_trie, rlp, sugar @@ -63,64 +63,16 @@ proc processTransaction(db: var AccountStateDB, t: Transaction, sender: EthAddre if transactionFailed: return - var gasUsed = t.payload.intrinsicGas.GasInt + var gasUsed = t.payload.intrinsicGas.GasInt # += 32000 appears in Homestead when contract create - 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: - # TODO: split out into separate function?; interleaving the cases obfuscates - let vmState = newBaseVMState(head, chainDB) - - # TODO: setupComputation refactoring - let contractAddress = generateAddress(sender, t.accountNonce) - let msg = newMessage(t.gasLimit - gasUsed, t.gasPrice, t.to, sender, t.value, @[], t.payload, - options = newMessageOptions(origin = sender, - createAddress = contractAddress)) - var c = newBaseComputation(vmState, head.blockNumber, msg) - - if execComputation(c, vmState): - 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 - - var codeCost = 200 * c.output.len - - # This apparently is not supposed to actually consume the gas, just be able to, - # for purposes of accounting. Py-EVM apparently does consume the gas, but it is - # not matching observed blockchain balances if consumeGas is called. - if gasRemaining >= codeCost.u256: - db.setCode(contractAddress, c.output.toRange) - else: - # XXX: Homestead behaves differently; reverts state on gas failure - # https://github.com/ethereum/py-evm/blob/master/eth/vm/forks/homestead/computation.py - codeCost = 0 - db.setCode(contractAddress, ByteRange()) - db.addBalance(sender, (t.gasLimit.u256 - gasUsed2 - codeCost.u256)*t.gasPrice.u256) - return (gasUsed2 + codeCost.u256) * 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 + # TODO: re-derive sender in callee for cleaner interface, perhaps + var vmState = newBaseVMState(head, chainDB) + return applyCreateTransaction(db, t, head, vmState, sender) else: let code = db.getCode(t.to) diff --git a/nimbus/vm_state_transactions.nim b/nimbus/vm_state_transactions.nim index 58ddaf37b..1392624be 100644 --- a/nimbus/vm_state_transactions.nim +++ b/nimbus/vm_state_transactions.nim @@ -10,7 +10,7 @@ import eth_common, ./constants, ./errors, ./vm/computation, ./transaction, ./vm_types, ./vm_state, ./block_types, ./db/[db_chain, state_db], ./utils/header, - ./vm/interpreter + ./vm/interpreter, ./utils/addresses func intrinsicGas*(data: openarray[byte]): GasInt = result = 21_000 @@ -60,6 +60,58 @@ proc execComputation*(computation: var BaseComputation, vmState: BaseVMState): b except ValueError: result = false +proc applyCreateTransaction*(db: var AccountStateDB, t: Transaction, head: BlockHeader, vmState: var BaseVMState, sender: EthAddress, useHomestead: bool = false): UInt256 = + doAssert t.isContractCreation + # TODO: clean up params + echo "Contract creation" + + let gasUsed = t.payload.intrinsicGas.GasInt + (if useHomestead: 32000 else: 0) + + # TODO: setupComputation refactoring + let contractAddress = generateAddress(sender, t.accountNonce) + let msg = newMessage(t.gasLimit - gasUsed, t.gasPrice, t.to, sender, t.value, @[], t.payload, + options = newMessageOptions(origin = sender, + createAddress = contractAddress)) + var c = newBaseComputation(vmState, head.blockNumber, msg) + + if execComputation(c, vmState): + 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 + + var codeCost = 200 * c.output.len + + # This apparently is not supposed to actually consume the gas, just be able to, + # for purposes of accounting. Py-EVM apparently does consume the gas, but it is + # not matching observed blockchain balances if consumeGas is called. + if gasRemaining >= codeCost.u256: + db.setCode(contractAddress, c.output.toRange) + else: + # XXX: Homestead behaves differently; reverts state on gas failure + # https://github.com/ethereum/py-evm/blob/master/eth/vm/forks/homestead/computation.py + codeCost = 0 + db.setCode(contractAddress, ByteRange()) + db.addBalance(sender, (t.gasLimit.u256 - gasUsed2 - codeCost.u256)*t.gasPrice.u256) + return (gasUsed2 + codeCost.u256) * 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 + # especially when it's cross-function, it's ugly/fragile + db.addBalance(sender, t.value) + echo "isError: ", c.isError + return t.gasLimit.u256 * t.gasPrice.u256 + 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 diff --git a/tests/test_generalstate_failing.nim b/tests/test_generalstate_failing.nim index 4cb058906..2568dabe1 100644 --- a/tests/test_generalstate_failing.nim +++ b/tests/test_generalstate_failing.nim @@ -15,7 +15,6 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool = let allowedFailingGeneralStateTests = @[ "ContractCreationSpam.json", "CrashingTransaction.json", - "badOpcodes.json", "call_OOG_additionalGasCosts1.json", "callcall_00.json", "callcode_checkPC.json", @@ -116,11 +115,8 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool = "callcodecallcodecallcode_111_OOGMAfter_1.json", "callcodecallcodecallcode_111_OOGMAfter_2.json", "callcodecallcodecallcode_111_OOGMAfter_3.json", - "contractCreationMakeCallThatAskMoreGasThenTransactionProvided.json", "createInitFail_OOGduringInit.json", "codesizeInit.json", - "codesizeOOGInvalidSize.json", - "codesizeValid.json", "CREATE_AcreateB_BSuicide_BStore.json", "CREATE_ContractSSTOREDuringInit.json", "CREATE_ContractSuicideDuringInit.json", @@ -191,7 +187,6 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool = "RawDelegateCallGas.json", "RawDelegateCallGasMemory.json", "contractCreationOOGdontLeaveEmptyContract.json", - "contractCreationOOGdontLeaveEmptyContractViaTransaction.json", "createContractViaContract.json", "createContractViaContractOOGInitCode.json", "createContractViaTransactionCost53000.json", @@ -207,11 +202,7 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool = "OutOfGasPrefundedContractCreation.json", "ReturnTest.json", "ReturnTest2.json", - "StackUnderFlowContractCreation.json", "TransactionCreateAutoSuicideContract.json", - "TransactionCreateRandomInitCode.json", - "TransactionCreateStopInInitcode.json", - "TransactionCreateSuicideInInitcode.json", "log0_emptyMem.json", "log0_logMemStartTooHigh.json", "log0_logMemsizeTooHigh.json", @@ -417,7 +408,6 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool = "NashatyrevSuicideRevert.json", "RevertDepthCreateAddressCollision.json", "RevertDepthCreateOOG.json", - "RevertInCreateInInit.json", "RevertOpcodeCalls.json", "RevertOpcodeCreate.json", "RevertOpcodeDirectCall.json", @@ -445,13 +435,10 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool = "TestContractInteraction.json", "TestContractSuicide.json", "TestCryptographicFunctions.json", - "FailedCreateRevertsDeletion.json", "JUMPDEST_Attack.json", "JUMPDEST_AttackwithJump.json", - "StackDepthLimitSEC.json", "deploymentError.json", "tx_e1c174e2.json", - "stackOverflowM1DUP.json", "ABAcalls0.json", "ABAcalls1.json", "ABAcalls2.json", @@ -493,7 +480,6 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool = "testRandomTest.json", "CreateMessageReverted.json", "CreateMessageSuccess.json", - "CreateTransactionSuccess.json", "EmptyTransaction2.json", "EmptyTransaction3.json", "InternalCallHittingGasLimitSuccess.json", diff --git a/tests/test_generalstate_json.nim b/tests/test_generalstate_json.nim index 997c74670..f4835216c 100644 --- a/tests/test_generalstate_json.nim +++ b/tests/test_generalstate_json.nim @@ -13,7 +13,7 @@ import ./test_helpers, ../nimbus/[constants, errors], ../nimbus/[vm_state, vm_types, vm_state_transactions], - ../nimbus/utils/header, + ../nimbus/utils/[header, addresses], ../nimbus/vm/interpreter, ../nimbus/db/[db_chain, state_db] @@ -47,10 +47,25 @@ proc testFixtureIndexes(header: BlockHeader, pre: JsonNode, transaction: Transac let gas_cost = transaction.gasLimit.u256 * transaction.gasPrice.u256 vmState.mutateStateDB: db.setNonce(sender, db.getNonce(sender) + 1) - db.addBalance(transaction.to, transaction.value) db.subBalance(sender, transaction.value + gas_cost) + if transaction.isContractCreation and transaction.payload.len > 0: + vmState.mutateStateDB: + # TODO: move into applyCreateTransaction + # fixtures/GeneralStateTests/stTransactionTest/TransactionSendingToZero.json + # fixtures/GeneralStateTests/stTransactionTest/TransactionSendingToEmpty.json + #db.addBalance(generateAddress(sender, transaction.accountNonce), transaction.value) + + let createGasUsed = applyCreateTransaction(db, transaction, header, vmState, sender, true) + db.addBalance(header.coinbase, createGasUsed) + return var computation = setupComputation(header, vmState, transaction, sender) + + vmState.mutateStateDB: + # contract creation transaction.to == 0, so ensure happens after + db.addBalance(transaction.to, transaction.value) + + # What remains is call and/or value transfer if execComputation(computation, vmState): let gasRemaining = computation.gasMeter.gasRemaining.u256 @@ -62,6 +77,8 @@ proc testFixtureIndexes(header: BlockHeader, pre: JsonNode, transaction: Transac vmState.mutateStateDB: for deletedAccount in computation.getAccountsForDeletion: db.deleteAccount deletedAccount + # TODO if the balance/etc calls were gated on gAFD or similar, + # that would simplify/combine codepaths 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 b62cfeabd..33841fd4c 100644 --- a/tests/test_helpers.nim +++ b/tests/test_helpers.nim @@ -50,6 +50,8 @@ func failIn32Bits(folder, name: string): bool = "sha3_dejavu.json", "HighGasLimit.json", "OverflowGasRequire2.json", + "RevertInCreateInInit.json", + "FailedCreateRevertsDeletion.json", # TODO: obvious theme; check returndatasize/returndatacopy "call_ecrec_success_empty_then_returndatasize.json", @@ -225,11 +227,15 @@ proc getFixtureTransaction*(j: JsonNode, dataIndex, gasIndex, valueIndex: int): result.gasPrice = j["gasPrice"].getStr.parseHexInt result.gasLimit = j["gasLimit"][gasIndex].getStr.parseHexInt - # Another distinct case "" as special hex string, but at least here, - # it has some semantic meaning in Ethereum -- contract creation. The - # hex parsing routine tripping over this is at least the third. + # TODO: there are a couple fixtures which appear to distinguish between + # empty and 0 transaction.to. let rawTo = j["to"].getStr - result.to = (if rawTo == "": "0x" else: rawTo).parseAddress + if rawTo == "": + result.to = "0x".parseAddress + result.isContractCreation = true + else: + result.to = rawTo.parseAddress + result.isContractCreation = false result.value = fromHex(UInt256, j["value"][valueIndex].getStr) result.payload = j["data"][dataIndex].getStr.safeHexToSeqByte