General State Tests Proposal
winsvega opened this issue · 16 comments
Specification
General State Test:
{
"testname" : {
"env" : {
"currentCoinbase" : "address",
"currentDifficulty" : "0x020000", //minimum difficulty for mining on blockchain
"currentGasLimit" : "u64", //not larger then maxGasLimit = 0x7fffffffffffffff
"currentNumber" : "0x01", //Irrelevant to hardfork parameters!
"currentTimestamp" : "1000", //for blockchain version
"previousHash" : "h256"
},
"post" : {
"EIP150" : [
{
"hash" : "3e6dacc1575c6a8c76422255eca03529bbf4c0dda75dfc110b22d6dc4152396f",
"indexes" : { "data" : 0, "gas" : 0, "value" : 0 }
},
{
"hash" : "99a450d8ce5b987a71346d8a0a1203711f770745c7ef326912e46761f14cd764",
"indexes" : { "data" : 0, "gas" : 0, "value" : 1 }
},
...
],
"EIP158" : [
{
"hash" : "3e6dacc1575c6a8c76422255eca03529bbf4c0dda75dfc110b22d6dc4152396f",
"indexes" : { "data" : 0, "gas" : 0, "value" : 0 }
},
{
"hash" : "99a450d8ce5b987a71346d8a0a1203711f770745c7ef326912e46761f14cd764",
"indexes" : { "data" : 0, "gas" : 0, "value" : 1 }
},
...
],
"Frontier" : [
...
],
"Homestead" : [
...
]
},
"pre" : {
//same as for StateTests
},
"transaction" : {
"data" : [ "" ],
"gasLimit" : [ "285000", "100000", "6000" ],
"gasPrice" : "0x01",
"nonce" : "0x00",
"secretKey" : "45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8",
"to" : "095e7baea6a6c7c4c2dfeb977efac326af552d87",
"value" : [ "10", "0" ]
}
}
}
Indexes section describes which values from given array to set for transaction before it's execution on a pre state. Transaction now has data, value, and gasLimit as arrays.
post section now has array of implemented forks. For each fork it has another array of execution results on that fork rules with post state root hash and transaction parameters.
all value fields are in hex and prefixed with '0x'
address, secretKey and hash fields are not prefixed with '0x'
Rationale
Due to the recent network changes it would be good to make state tests more general so we don't need to copy them each time a new hard fork occur. The new tests would have a single filler for a single test.
One test per file. So instead of previous stInitCodeTest.json file with lost of tests it would be stInitCodeTest folder with files describing every individual test from stInitCodeTest.json.
The new format will have results of a transaction execution on every implemented hardfork and in addition it would also allow to describe multiple transaction parameters such as array of data, value and gasLimit. Thus transaction would be executed with different parameters on a single pre state. This allows to test range of test cases where we need to check x, x+1, x-1 values results.
It would be great if the test contained the chain configuration, containing the fork settings to be used for the test:
{
"configuration": {
"homesteadBlock": "0xaabb",
"eip150": "0xaabb"
}
}I would like to add all things that are hex should start with 0x
right. all fields that are hex start with 0x - that is already implemented
@obscuren
we do not need the configuration.
transaction is being executed on all configurations.
no need for blocknumber to be used as configuration for the fork
all fields that are hex start with 0x - that is already implemented
https://github.com/ethereum/tests/blob/develop/VMTests/RandomTests/randomTest.json#L14-L15
VM tests has not been updated. especially random VM tests.
addresses do not start with 0x prefix
so you have
"a94....456" :
and
"to" : "a94...456"
I guess next step would be to clean up the VMTests. Perhaps it would be merged into general tests as well.
Can you explain what are hash and indexes in the results?
I would also suggest renaming secretKey to privateKey.
Indexes section describes which values from given array to set for transaction before it's execution on a pre state. Transaction now has data, value, and gasLimit as arrays.
post section now has array of implemented forks. For each fork it has another array of execution results on that fork rules with post state root hash and transaction parameters.
I think that hash and indexes can be merged into a single object, e.g.:
{
"state-root" : "3e6dacc1575c6a8c76422255eca03529bbf4c0dda75dfc110b22d6dc4152396f",
"data" : 0, "gas" : 0, "value" : 0
}
I've already wrote the parser :)
Remember that everyone will have to create similar parser after you, so simpler is better.
Do we need to care that much about making any new pre-Spurious Dragon tests? IMO if a client incorrectly implements or even omits some pre-Spurious feature, as long as it syncs the blockchain and it has fully correct behavior post-Spurious then we should consider it fine. I'd even say that the only tests we should still be keeping in the tests directory are (i) tests for the current active protocol version, and (ii) tests for the next one. With this in mind, is changing test formats still that valuable?
OTOH I do really like the idea of putting data fields into arrays like that to create many de-facto tests out of one.
ok first. this upgrade is not about making new tests for concrete fork.
it is about using all tests cases in all forks. so once defined - test scenario would be easily run on all forks (possibly more in the future) and thus we do not have to copy all of the test files again. it would be easier for us to check previous scenarios on a new fork.
the old State Tests could be removed once this upgrade is fully implemented
second. we do need tests for previous forks for two reasons.
a) in case we broke smth in the client that affects only specific fork rule. we could check that with tests.
b) in case someone build the new client he could use this tests to check their fork implementation.
here are the tests
(Frontier + Homestead checked scenarios)
https://github.com/ethereum/tests/tree/develop/GeneralStateTests
We finally got around to implementing this in go-ethereum and there are a few issues with this format:
- The indexes indirection is kind of weird. All we really want here is a way to say "run these transactions on the same pre state with different fork rules".
- Transactions need to be signed by the client that's testing. This is annoying because it's unclear which signature scheme to use, i.e. I can sign all txs with Homestead rules.
To fix these, I propose that the format should be changed as follows:
"transaction" should be removed. Instead, the post state should contain the RLP encoding of the (signed) transaction that is to be applied. Example:
{
"testname" : {
"env" : { ... },
"pre": { ... },
"transactions" : {
"EIP150" : [
{
"tx" : "0xrlp",
"postStateRoot" : "99a450d8ce5b987a71346d8a0a1203711f770745c7ef326912e46761f14cd764",
"logs": [ ... ]
}
]
}
}
}
we have to agree on a max logs length. one test could have up to 20 transactions in general.
There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.
This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.