status-im
/
nimbus-eth1
mirror of https://github.com/status-im/nimbus-eth1.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Refactor contract creation into transaction code from P2P code; get 14 new GSTs working (#163) 

* checkpoint where refactoring and calling from GST runner doesn't break anything

* 14 new GSTs pass by refactoring contract creation out of p2p/chain and using from GST test runner

* 2 of 14 new tests fail in 32-bit builds

* switch from CREATE_CONTRACT_ADDRESS to isContractCreation

* switch another CREATE_CONTRACT_ADDRESS to isContactCreation
This commit is contained in:
tersec 2018-09-29 15:36:42 +00:00 committed by GitHub
parent 3f5fc9a034
commit 4f04332205
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 110 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
GeneralStateTests.md
View File

@ -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

58
nimbus/p2p/chain.nim
View File

@ -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)

54
nimbus/vm_state_transactions.nim
View File

@ -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

14
tests/test_generalstate_failing.nim
View File

@ -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",

21
tests/test_generalstate_json.nim
View File

@ -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)

14
tests/test_helpers.nim
View File

@ -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