From 2f1f5d1722fd717a7187485c39113bcd39c5b47a Mon Sep 17 00:00:00 2001 From: Dustin Brody Date: Mon, 17 Sep 2018 17:35:41 -0700 Subject: [PATCH] 20 new working GeneralStateTests --- GeneralStateTests.md | 46 +++++++++---------- nimbus/vm/interpreter/opcodes_impl.nim | 34 ++++---------- .../utils/macros_procs_opcodes.nim | 5 +- nimbus/vm/interpreter/utils/utils_numeric.nim | 9 ++++ nimbus/vm/memory.nim | 2 - tests/test_generalstate_json.nim | 30 ++++++++---- 6 files changed, 67 insertions(+), 59 deletions(-) diff --git a/GeneralStateTests.md b/GeneralStateTests.md index 654b90ab86..fc93f2e7c2 100644 --- a/GeneralStateTests.md +++ b/GeneralStateTests.md @@ -974,7 +974,7 @@ OK: 0/16 Fail: 0/16 Skip: 16/16 + randomStatetest112.json OK + randomStatetest114.json OK + randomStatetest115.json OK -- randomStatetest116.json Fail ++ randomStatetest116.json OK + randomStatetest117.json OK + randomStatetest118.json OK + randomStatetest119.json OK @@ -1026,7 +1026,7 @@ OK: 0/16 Fail: 0/16 Skip: 16/16 + randomStatetest167.json OK + randomStatetest169.json OK + randomStatetest17.json OK -- randomStatetest170.json Fail ++ randomStatetest170.json OK + randomStatetest171.json OK + randomStatetest172.json OK + randomStatetest173.json OK @@ -1053,7 +1053,7 @@ OK: 0/16 Fail: 0/16 Skip: 16/16 + randomStatetest196.json OK + randomStatetest197.json OK + randomStatetest198.json OK -- randomStatetest199.json Fail ++ randomStatetest199.json OK + randomStatetest2.json OK + randomStatetest20.json OK + randomStatetest200.json OK @@ -1062,7 +1062,7 @@ OK: 0/16 Fail: 0/16 Skip: 16/16 + randomStatetest204.json OK - randomStatetest205.json Fail + randomStatetest206.json OK -- randomStatetest207.json Fail ++ randomStatetest207.json OK + randomStatetest208.json OK + randomStatetest209.json OK + randomStatetest210.json OK @@ -1089,15 +1089,15 @@ OK: 0/16 Fail: 0/16 Skip: 16/16 + randomStatetest232.json OK + randomStatetest233.json OK + randomStatetest236.json OK -- randomStatetest237.json Fail ++ randomStatetest237.json OK + randomStatetest238.json OK + randomStatetest24.json OK + randomStatetest241.json OK + randomStatetest242.json OK + randomStatetest243.json OK -- randomStatetest244.json Fail ++ randomStatetest244.json OK + randomStatetest245.json OK -- randomStatetest246.json Fail ++ randomStatetest246.json OK + randomStatetest247.json OK - randomStatetest248.json Fail + randomStatetest249.json OK @@ -1108,7 +1108,7 @@ OK: 0/16 Fail: 0/16 Skip: 16/16 + randomStatetest254.json OK + randomStatetest257.json OK + randomStatetest259.json OK -- randomStatetest26.json Fail ++ randomStatetest26.json OK + randomStatetest260.json OK + randomStatetest261.json OK + randomStatetest263.json OK @@ -1148,11 +1148,11 @@ OK: 0/16 Fail: 0/16 Skip: 16/16 + randomStatetest298.json OK + randomStatetest299.json OK + randomStatetest3.json OK -- randomStatetest30.json Fail ++ randomStatetest30.json OK + randomStatetest300.json OK + randomStatetest301.json OK + randomStatetest302.json OK -- randomStatetest303.json Fail ++ randomStatetest303.json OK + randomStatetest304.json OK + randomStatetest305.json OK - randomStatetest306.json Fail @@ -1226,7 +1226,7 @@ OK: 0/16 Fail: 0/16 Skip: 16/16 + randomStatetest377.json OK + randomStatetest378.json OK + randomStatetest379.json OK -- randomStatetest38.json Fail ++ randomStatetest38.json OK + randomStatetest380.json OK + randomStatetest381.json OK + randomStatetest382.json OK @@ -1284,7 +1284,7 @@ OK: 0/16 Fail: 0/16 Skip: 16/16 + randomStatetest97.json OK + randomStatetest98.json OK ``` -OK: 294/327 Fail: 29/327 Skip: 4/327 +OK: 305/327 Fail: 18/327 Skip: 4/327 ## stRandom2 ```diff + 201503110226PYTHON_DUP6.json OK @@ -1295,7 +1295,7 @@ OK: 294/327 Fail: 29/327 Skip: 4/327 + randomStatetest387.json OK + randomStatetest388.json OK + randomStatetest389.json OK -- randomStatetest391.json Fail ++ randomStatetest391.json OK randomStatetest393.json Skip + randomStatetest395.json OK + randomStatetest396.json OK @@ -1361,7 +1361,7 @@ OK: 294/327 Fail: 29/327 Skip: 4/327 + randomStatetest465.json OK + randomStatetest466.json OK - randomStatetest467.json Fail -- randomStatetest468.json Fail ++ randomStatetest468.json OK + randomStatetest469.json OK + randomStatetest470.json OK + randomStatetest471.json OK @@ -1373,7 +1373,7 @@ OK: 294/327 Fail: 29/327 Skip: 4/327 + randomStatetest477.json OK + randomStatetest478.json OK + randomStatetest480.json OK -- randomStatetest481.json Fail ++ randomStatetest481.json OK + randomStatetest482.json OK + randomStatetest483.json OK + randomStatetest484.json OK @@ -1397,7 +1397,7 @@ OK: 294/327 Fail: 29/327 Skip: 4/327 + randomStatetest505.json OK + randomStatetest506.json OK + randomStatetest507.json OK -- randomStatetest508.json Fail ++ randomStatetest508.json OK + randomStatetest509.json OK + randomStatetest510.json OK + randomStatetest511.json OK @@ -1451,12 +1451,12 @@ OK: 294/327 Fail: 29/327 Skip: 4/327 + randomStatetest567.json OK + randomStatetest569.json OK + randomStatetest571.json OK -- randomStatetest572.json Fail ++ randomStatetest572.json OK + randomStatetest573.json OK + randomStatetest574.json OK + randomStatetest575.json OK + randomStatetest576.json OK -- randomStatetest577.json Fail ++ randomStatetest577.json OK + randomStatetest578.json OK - randomStatetest579.json Fail + randomStatetest580.json OK @@ -1496,7 +1496,7 @@ OK: 294/327 Fail: 29/327 Skip: 4/327 + randomStatetest625.json OK randomStatetest626.json Skip - randomStatetest627.json Fail -- randomStatetest628.json Fail ++ randomStatetest628.json OK + randomStatetest629.json OK + randomStatetest630.json OK - randomStatetest632.json Fail @@ -1515,7 +1515,7 @@ OK: 294/327 Fail: 29/327 Skip: 4/327 - randomStatetest646.json Fail randomStatetest647.json Skip ``` -OK: 201/227 Fail: 22/227 Skip: 4/227 +OK: 208/227 Fail: 15/227 Skip: 4/227 ## stRecursiveCreate ```diff - recursiveCreate.json Fail @@ -2119,19 +2119,19 @@ OK: 23/67 Fail: 42/67 Skip: 2/67 + TransactionDataCosts652.json OK + TransactionFromCoinbaseHittingBlockGasLimit.json OK - TransactionFromCoinbaseHittingBlockGasLimit1.json Fail -- TransactionFromCoinbaseNotEnoughFounds.json Fail ++ TransactionFromCoinbaseNotEnoughFounds.json OK + TransactionNonceCheck.json OK + TransactionNonceCheck2.json OK - TransactionSendingToEmpty.json Fail + TransactionSendingToZero.json OK + TransactionToAddressh160minusOne.json OK + TransactionToItself.json OK -- TransactionToItselfNotEnoughFounds.json Fail ++ TransactionToItselfNotEnoughFounds.json OK + UserTransactionGasLimitIsTooLowWhenZeroCost.json OK + UserTransactionZeroCost.json OK + UserTransactionZeroCostWithData.json OK ``` -OK: 20/44 Fail: 24/44 Skip: 0/44 +OK: 22/44 Fail: 22/44 Skip: 0/44 ## stTransitionTest ```diff - createNameRegistratorPerTxsAfter.json Fail diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index 2fb641f61c..c8da6144a0 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -6,7 +6,7 @@ # at your option. This file may not be copied, modified, or distributed except according to those terms. import - strformat, times, ranges, + strformat, times, ranges, sequtils, chronicles, stint, nimcrypto, ranges/typedranges, eth_common, ./utils/[macros_procs_opcodes, utils_numeric], ./gas_meter, ./gas_costs, ./opcode_values, ./vm_forks, @@ -193,7 +193,7 @@ op sha3, inline = true, startPos, length: ## 0x20, Compute Keccak-256 hash. let (pos, len) = (startPos.toInt, length.toInt) - if pos < 0 or len < 0: + if pos < 0 or len < 0 or pos > int32.high: raise newException(OutOfBoundsRead, "Out of bounds memory access") computation.gasMeter.consumeGas( @@ -212,34 +212,20 @@ op sha3, inline = true, startPos, length: # ########################################## # 30s: Environmental Information -# TODO - simplify: https://github.com/status-im/nimbus/issues/67 proc writePaddedResult(mem: var Memory, data: openarray[byte], memPos, dataPos, len: Natural, paddingValue = 0.byte) = + let prevLen = mem.len mem.extend(memPos, len) - let dataEndPosition = dataPos.int64 + len - 1 - if dataEndPosition < data.len: - mem.write(memPos, data[dataPos .. dataEndPosition]) - else: - var presentElements = data.len - dataPos - if presentElements > 0: - mem.write(memPos, data.toOpenArray(dataPos, data.len - 1)) - else: - presentElements = 0 - - # Note, we don't need to write padding bytes - # mem.extend already pads with zero properly - -func cleanMemRef(x: UInt256): int {.inline.} = - ## Sanitize memory addresses, catch negative or impossibly big offsets - # See https://github.com/status-im/nimbus/pull/97 for more info - # For rational on shr, see https://github.com/status-im/nimbus/pull/101 - const upperBound = (high(int32) shr 2).u256 - if x > upperBound: - return high(int32) shr 2 - return x.toInt + let sourceBytes = data[min(dataPos, data.len) .. min(data.len - 1, dataEndPosition)] + + mem.write(memPos, sourceBytes) + + # Don't duplicate zero-padding of mem.extend + let paddingOffset = memPos + sourceBytes.len + mem.write(paddingOffset, repeat(paddingValue, max(prevLen - paddingOffset, 0))) op address, inline = true: ## 0x30, Get address of currently executing account. diff --git a/nimbus/vm/interpreter/utils/macros_procs_opcodes.nim b/nimbus/vm/interpreter/utils/macros_procs_opcodes.nim index a079f0dc90..9f576d83f9 100644 --- a/nimbus/vm/interpreter/utils/macros_procs_opcodes.nim +++ b/nimbus/vm/interpreter/utils/macros_procs_opcodes.nim @@ -12,7 +12,8 @@ import macros, strformat, stint, ../../computation, ../../stack, ../../code_stream, ../../../constants, ../../../vm_types, ../../memory, - ../../../errors, ../../message, ../../interpreter/[gas_meter, opcode_values] + ../../../errors, ../../message, ../../interpreter/[gas_meter, opcode_values], + ../../interpreter/utils/utils_numeric proc pop(tree: var NimNode): NimNode = ## Returns the last value of a NimNode and remove it @@ -102,7 +103,7 @@ macro genSwap*(): untyped = proc logImpl(c: var BaseComputation, opcode: Op, topicCount: int) = assert(topicCount in 0 .. 4) let (memStartPosition, size) = c.stack.popInt(2) - let (memPos, len) = (memStartPosition.toInt, size.toInt) + let (memPos, len) = (memStartPosition.cleanMemRef, size.cleanMemRef) if memPos < 0 or len < 0: raise newException(OutOfBoundsRead, "Out of bounds memory access") diff --git a/nimbus/vm/interpreter/utils/utils_numeric.nim b/nimbus/vm/interpreter/utils/utils_numeric.nim index 485e816622..2dffcf77d4 100644 --- a/nimbus/vm/interpreter/utils/utils_numeric.nim +++ b/nimbus/vm/interpreter/utils/utils_numeric.nim @@ -41,3 +41,12 @@ proc extractSign*(v: var UInt256, sign: var bool) = proc setSign*(v: var UInt256, sign: bool) {.inline.} = if sign: flipSign(v) + +func cleanMemRef*(x: UInt256): int {.inline.} = + ## Sanitize memory addresses, catch negative or impossibly big offsets + # See https://github.com/status-im/nimbus/pull/97 for more info + # For rationale on shr, see https://github.com/status-im/nimbus/pull/101 + const upperBound = (high(int32) shr 2).u256 + if x > upperBound: + return high(int32) shr 2 + return x.toInt diff --git a/nimbus/vm/memory.nim b/nimbus/vm/memory.nim index 0418258e9a..d40d18a1ae 100644 --- a/nimbus/vm/memory.nim +++ b/nimbus/vm/memory.nim @@ -49,8 +49,6 @@ proc write*(memory: var Memory, startPos: Natural, value: openarray[byte]) = return #echo size #echo startPos - #validateGte(startPos, 0) - #validateGte(size, 0) validateLte(startPos + size, memory.len) let index = memory.len if memory.len < startPos + size: diff --git a/tests/test_generalstate_json.nim b/tests/test_generalstate_json.nim index af8ab76521..6254804b0d 100644 --- a/tests/test_generalstate_json.nim +++ b/tests/test_generalstate_json.nim @@ -29,7 +29,7 @@ proc stringFromBytes(x: ByteRange): string = result[i] = char(x[i]) proc testFixture(fixtures: JsonNode, testStatusIMPL: var TestStatus) = - # XXX: this is becoming a mess. refactor. + # XXX: this is a terrible mess. refactor. var fixture: JsonNode for label, child in fixtures: fixture = child @@ -60,9 +60,13 @@ proc testFixture(fixtures: JsonNode, testStatusIMPL: var TestStatus) = # XXX: https://github.com/status-im/nimbus/issues/35#issuecomment-391726518 # TODO: put yellow paper ref here from that link justifying the limit (1 shl 34 is stand-in) + # XXX: clean up lots of avoidable u256 construction var readOnlyDB = vmState.readOnlyStateDB + let limitAndValue = transaction.gasLimit.u256 + transaction.value if transaction.gasLimit < transaction.getFixtureIntrinsicGas or transaction.gasPrice > (1 shl 34) or + limitAndValue > readOnlyDB.getBalance(sender) or + #limitAndValue > header.gasLimit.u256 or transaction.accountNonce != readOnlyDB.getNonce(sender) or readOnlyDB.getBalance(sender) < gas_cost: vmState.mutateStateDb: @@ -87,7 +91,6 @@ proc testFixture(fixtures: JsonNode, testStatusIMPL: var TestStatus) = # TODO: combine some of these # Also, in general, map out/etc the whole vmState.mutateStateDB flow set db.setBalance(sender, db.getBalance(sender) - gas_cost) - db.increaseBalance(currentCoinbase, gas_cost) db.setNonce(sender, db.getNonce(sender) + 1) db.increaseBalance(transaction.to, transaction.value) db.setBalance(sender, db.getBalance(sender) - transaction.value) @@ -108,7 +111,6 @@ proc testFixture(fixtures: JsonNode, testStatusIMPL: var TestStatus) = # doAssert not message.isCreate var computation = newBaseComputation(vmState, header.blockNumber, message) - # XXX: https://github.com/status-im/nimbus/issues/122 computation.precompiles = initTable[string, Opcode]() doAssert computation.isOriginComputation @@ -120,9 +122,6 @@ proc testFixture(fixtures: JsonNode, testStatusIMPL: var TestStatus) = let deletedAccounts = computation.getAccountsForDeletion computation.gasMeter.refundGas(24_000 * deletedAccounts.len) - vmState.mutateStateDB: - for deletedAccount in deletedAccounts: - db.deleteAccount deletedAccount let gasRemaining = computation.gasMeter.gasRemaining.u256 @@ -131,24 +130,39 @@ proc testFixture(fixtures: JsonNode, testStatusIMPL: var TestStatus) = gasRefund = min(gasRefunded, gasUsed div 2) gasRefundAmount = (gasRefund + gasRemaining) * transaction.gasPrice.u256 + # TODO: investigate if these mutate blocks can be combined + vmState.mutateStateDB: + for deletedAccount in deletedAccounts: + db.deleteAccount deletedAccount + if not computation.isError: vmState.mutateStateDB: - db.setBalance(currentCoinbase, db.getBalance(currentCoinbase) - gasRefundAmount) + if currentCoinbase notin deletedAccounts: + db.setBalance(currentCoinbase, db.getBalance(currentCoinbase) - gasRefundAmount) + db.increaseBalance(currentCoinbase, gas_cost) db.increaseBalance(sender, gasRefundAmount) # TODO: only here does one commit, with some nuance/caveat else: + # XXX: both error paths are intentionally indentical, for merging, with refactoring # TODO: replace with transactional commit/revert state (foo.revert or implicit) vmState.mutateStateDB: + # XXX: the coinbase has to be committed; the rest are basically reverts db.setBalance(transaction.to, db.getBalance(transaction.to) - transaction.value) db.increaseBalance(sender, transaction.value) db.setStorageRoot(transaction.to, storageRoot) + db.increaseBalance(currentCoinbase, gas_cost) except ValueError: # TODO: replace with transactional commit/revert state (foo.revert or implicit) vmState.mutateStateDB: + # XXX: the coinbase has to be committed; the rest are basically reverts db.setBalance(transaction.to, db.getBalance(transaction.to) - transaction.value) db.increaseBalance(sender, transaction.value) db.setStorageRoot(transaction.to, storageRoot) - echo "Computation error" + db.increaseBalance(currentCoinbase, gas_cost) + + #echo vmState.readOnlyStateDB.dumpAccount("b94f5374fce5edbc8e2a8697c15331677e6ebf0b") + #echo vmState.readOnlyStateDB.dumpAccount("a94f5374fce5edbc8e2a8697c15331677e6ebf0b") + #echo vmState.readOnlyStateDB.dumpAccount("c94f5374fce5edbc8e2a8697c15331677e6ebf0b") # TODO: do this right doAssert "0x" & `$`(vmState.readOnlyStateDB.rootHash).toLowerAscii == fixture["post"]["Homestead"][0]["hash"].getStr