ethereum/tests

Metropolis Testing Guide - https://gist.github.com/Souptacular/fd197b1fac7c6d2660b0bef27a33ed40

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

can we remove this test:

  Test(randomStatetest94_Istanbul)

or at least decrease the gas limit there?

Martin Holst Swende

@holiman

@tkstanczak see https://gitter.im/ethereum/tests?at=5d8c6d25b38cc849bf1402fe and https://gitter.im/ethereum/tests?at=5d8cc32c47de0a719d13938a -- can't you blacklist it ?

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

Where communities thrive

People

Repo info

Activity