Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

20 new working GeneralStateTests #148

Merged
merged 1 commit into from
Sep 18, 2018
Merged

20 new working GeneralStateTests #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 23 additions & 23 deletions GeneralStateTests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
34 changes: 10 additions & 24 deletions nimbus/vm/interpreter/opcodes_impl.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid the call to repeat here? It performs a memory allocations that's not really necessary.

The previous code is more convoluted for sure, but it actually seems more economical to me when it comes to branching. There is a potential hidden branch in every call to min and max and you would still call mem.write even when repeat returns a zero-len sequence.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think min/max are optimised to not need branches in most compilers, being replaced with cmp and conditional mov, and it does make for easier to read code. However I think there's a lot to be said for explicit code when performance matters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zah yes, I'll look into a better approach than repeat. I share your concern here. In this version I went for readability, but to be fair, this code's seems plausibly more performance-sensitive than most in Nimbus. It's worth optimizing.

@coffeepots yes, that min and max are often branchless was part of my thought process. The issue I had with the explicit code was that it created multiple subcases that I found harder to verify were all correct, created some duplication, and obscured the degree to which the different nominally disjoint code flows were similar in practice. I'm not strongly opinionated about this though.


op address, inline = true:
## 0x30, Get address of currently executing account.
Expand Down
5 changes: 3 additions & 2 deletions nimbus/vm/interpreter/utils/macros_procs_opcodes.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
9 changes: 9 additions & 0 deletions nimbus/vm/interpreter/utils/utils_numeric.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 0 additions & 2 deletions nimbus/vm/memory.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading