Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Oct 21 2017 21:24
    @jpitts banned @Musk55
David
@dvdplm
So we’re still digging but it seems like at least some of our failures stem from changes to ”storage” in the test files between constantinople and now.

This should (?) be considered an empty storage slot:

                "storage" : {
                    "0x00" : "0x00"
                }

The files in LegacyTests/ do not have storage entries like the above.

Martin Holst Swende
@holiman
@voith that's not the same as parity, or, well, only partally. THe reason RevertPrecompiledTouch_d0g0v0_Byzantium is due to how the situation around revert/oog on empty accounts that also happens to be precompiles.
David
@dvdplm
Should we update to ethereum/tests#640 or rather wait for it to be merged to master?
Martin Holst Swende
@holiman

I'm a bit confused by the current layout of tests.... @winsvega For blockchain tests, there are both

  • BlockchainTests/GeneralStateTests/..., and
  • BlockchainTests/ValidBlocks/bcStateTests/...

Is one, or both of those 'derived' from GeneralStateTests? The reason I'm asking is that geth can run GeneralStateTests 'natively', and wants to skip the ones that have been converted into Blockchain tests

Martin Holst Swende
@holiman
Also, we're skipping randomStatetest94.json - it uses 4.6 TGas, and easily cause OOM. Maybe that one should be moved to stTimeConsuming or something ?
Martin Holst Swende
@holiman
Other than that, tests LGTM now
Danno Ferrin
@shemnon
randon94 is also on Besu’s watch list, my PR for the update has it blacklisted. 4.6TGas is not a reasonable “all the time” test.
Noel Maersk
@veox
randomStatetest94.json takes 22 seconds to run in py-evm (for Istanbul only), and indeed causes a +3 GiB memory use peak on my workstation. Not as time-consuming as some other tests, though.
Holger Drewes
@holgerd77
Just published the Ethereum tests release v7.0.0-beta.1 which can serve as a first stable basis for testing against the Istanbul tests integrated into different test types.
(this is a beta, so not everything is ready, nevertheless)
David
@dvdplm
Nice!
Voith Mascarenhas
@voith
:tada:
Gregory Markou
@GregTheGreek

x-post from /allcoredevs

hey all - while doing interop tests on ETC, we made a sanity test tool that spams a network with pretty much every opcode (i think we exclude jump and a few others). Essentially, if you spin up multiple implementations (geth, parity, besu, etc...) you can run our suite and check if the blockhashes have diverted. It could still use a bit of work, the process for testing the blockhashes is manual right now (during out test we would check the peer counts manually in a script)

repo: https://github.com/ChainSafe/Anemone

If you find it useful and want a feature just make an issue happy to help make it more fluid. Hopefully it helps with any local dev (we had it hooked up to a ci) when testing experimental changes :)

Martin Holst Swende
@holiman
That's pretty nice.. I haven't looked into the code really, but in case it just randomly executes opcodes, it would be good to also ensure that the percentage of calls that go OOG is low, otherwise potential consensus errors will just be hidden behind all-gas-eating OOG executions @GregTheGreek
Gregory Markou
@GregTheGreek

@holiman thanks! I don't think we'll ever run into (can double check) because we actually deploy a unique contract per opcode (eg: https://github.com/ChainSafe/Anemone/blob/master/solidity/contracts/delegatecall.sol)

We also have an OOG test in there (https://github.com/ChainSafe/Anemone/blob/master/solidity/contracts/CallOOG.sol)

Martin Holst Swende
@holiman
Ah, I see. Well, I think there's a different problem. See e..g. https://github.com/ChainSafe/Anemone/blob/master/solidity/contracts/booleanOperators.sol#L13 , which just does assembly { pop(and(2,3)) }. Even if all clients produce different results for and(2,3), you won't ever get a consensus error, because it won't influence the end result. You just execute it, and throw away the result. If you want to trigger a consensus vuln, you need to do something with it. Either store result via SSTORE, or change the execution flow if the result isn't what you expected
I do similar ops in our fuzzing framework(s), but what I do there is inspect the stack after every op, and thus can catch it
Gregory Markou
@GregTheGreek

ahhh interesting. thats a good point . variable just gets discarded immediately, doesn't update the state.

Good catch!

Martin Holst Swende
@holiman
You could e.g. xor all results into a "sponge" memory variable, and then store that
Gregory Markou
@GregTheGreek
That's an elegant solution... i like it
James Hancock
@MadeofTin
:) all this progress (: FeelsGoodMan
Gregory Markou
@GregTheGreek
I think we'll need to change the execution order though, to be absolutely certain everythin works as expected: sstore, mload etc... then start doing chained contracts. I like it, thanks @holiman I'll keep you all update when we make some changes
:)
Martin Holst Swende
@holiman
@shemnon did you have a testcase for that net-sstore thing where it hit 2300? cc @winsvega
Danno Ferrin
@shemnon
No, I haven’t had time yet.
Danno Ferrin
@shemnon
PR for EIP2200 - ethereum/tests#649
Andrei Maiboroda
@gumb0
Hey @shemnon you've added this file, does Besu require this string? https://github.com/ethereum/tests/blob/a860325af5d84eb5d5b01f218eaa36e5181dafb6/Retesteth/default/genesis/Istanbul.json#L78
I think geth ignores it, but in aleth it's called differently (blake2_compression), so if Besu doesn't need it, I'd rename it there in the repo
Danno Ferrin
@shemnon
Besu doesn’t need it. As long as there is some appropriatly descriptive name I’m not terribly attached to which one it is.
Does Geth care about that value?
And Parity/Nethermind?
Martin Holst Swende
@holiman

Does Geth care about that value?

Nope

Noel Maersk
@veox
In the same file, shouldn't 0x03 have name ripemd160?.. It has sha256, same as 0x02 (copy-paste error?..).
Danno Ferrin
@shemnon
Not just that file, looks like all the genesis files in that dir have the exact same issue.
Andrei Maiboroda
@gumb0
oh right, good catch, i didn't get to running ripemd tests with retesteth yet, or I would spot it
I'll submit a PR
aleth should just get rid of this requirement to list precompiles in the chain config...
Tomasz Kajetan Stańczak
@tkstanczak
hi
can we remove this test:
  Test(randomStatetest94_Istanbul)
or at least decrease the gas limit there?
image.png
Martin Holst Swende
@holiman
Tomasz Kajetan Stańczak
@tkstanczak
we have, but why would we keep a test which is blacklisted in BeSu, Geth and Nethermind? if it does a relevant thing then it should just test with lower gas (unless it specifically tests the max gas limit)
Danno Ferrin
@shemnon
There is a category for long runnning tests. I feel the correct action is to move this test there.
Besu’s issue is that it runs fine as the only test in the harness, but once it’s run in production queues where we run the tests in parallel memory becomes a problem.
Danno Ferrin
@shemnon
stTimeConsuming is only a state test category, perhaps we should make a bcTimeConsuming and wire that logic into retesteth
Tomasz Kajetan Stańczak
@tkstanczak
this causes some trouble on windows in some projects
can we consider shortening the test names / nesting
BlockchainTestsFiller/GeneralStateTests/stMemExpandingEIP150Calls/CallAskMoreGasOnDepth2ThenTransactionHasWithMemExpandingCalls_d0g0v0Filler.json