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.

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

Add a new input for modlen = 999188. #405

Merged
merged 1 commit into from
Jan 30, 2018
Merged

Add a new input for modlen = 999188. #405

merged 1 commit into from
Jan 30, 2018

Conversation

pirapira
Copy link
Member

@pirapira pirapira merged commit 9b1f07c into develop Jan 30, 2018
@pirapira pirapira deleted the modlen-999188 branch January 30, 2018 09:46
@veox
Copy link

veox commented Sep 1, 2018

I'm trying to debug why this test fails in py-evm (PR ethereum/py-evm#1224).

A few questions (also explored in more detail there):

  • Is the rationale for its existence provided anywhere?
  • What are the expected storage values after executing the transaction? (EDIT: Seems "empty"?..)
  • What is the expected gas consumption?

Could clarifications on these be given, please?

@winsvega
Copy link
Collaborator

winsvega commented Sep 2, 2018

You could see the expected storage and gas in result blockchain test.

Rationaly in this discussion:

ethereum/py-evm#1224

@veox
Copy link

veox commented Sep 2, 2018

(Split in sections for clarity.)

Storage values

You could see the expected storage and gas in result blockchain test.

The src for these tests specify "empty" storage:

For the merge commit in this PR - the one being tested in py-evm (9b1f07c58a70d1b17c4489c49eb9bebf4a27d290):

"result" : {
"1000000000000000000000000000000000000000" : {
"storage" : {
}

For current latest develop (commit 55946c752613a5392c6b347d0a74956ffb361bd7):

"result" : {
"1000000000000000000000000000000000000000" : {
"storage" : {
}

The resulting (generated) test files don't have a section for this at all; the information, I assume, is hashed into the block hash, but that's it.

PR merge commit:

{
"hash" : "0x9afe4785e71ef1d32638c3cc7c632e2557005d62fe735cae5aaad6f302e4cea3",
"indexes" : {
"data" : 4,
"gas" : 0,
"value" : 0
},
"logs" : "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347"

Latest develop:

{
"hash" : "0x9afe4785e71ef1d32638c3cc7c632e2557005d62fe735cae5aaad6f302e4cea3",
"indexes" : {
"data" : 4,
"gas" : 0,
"value" : 0
},
"logs" : "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347"

Gas usage

You could see the expected storage and gas in result blockchain test.

All I can find - in both the "source" and "generated" variants - is gas index (transaction position in block), gas limit (block/transaction), and gas price, but not the amount of gas the transaction is to consume.

So, uh... no, I can't see it, or it's not there. :)

Could I get an exact link?..

Rationale

Rationaly in this discussion:

ethereum/py-evm#1224

That's the same link that I posted - to the PR where I'm battling this test case. :)

@veox
Copy link

veox commented Sep 2, 2018

Should I open a separate issue for this, so it gets greater exposure?

@winsvega
Copy link
Collaborator

winsvega commented Sep 2, 2018

 "result" : { 
     "1000000000000000000000000000000000000000" : { 
         "storage" : { 
} 

means the storage is expected to be empty.
if there was no storage in expect section - means storage is not checked.

to get the actual result storage look at postState section of a filled blockchain test

you could see the gas usage at balance of coinbase account as it is the on getting reward for transaction execution. in a blockchain test that is generated from state test if substract mining reward you will get the gas used by transaction * gas price.
gas price usually = 1

gasUsed is usually checked among clients when the test is failing for some reason. it is checked with debug in the step by step transaction execution. it happens rarely.

what is your issue with that test? seems like you guessed right the memory allocation check of this test.
unfortunatelly Yoichi could not help you at the moment.

@veox
Copy link

veox commented Sep 3, 2018

means the storage is expected to be empty.
if there was no storage in expect section - means storage is not checked.

That is mighty strange. The storage for that contract is already filled in the "pre" section of src, with a placeholder value 0xffffffff:

"pre" : {
"1000000000000000000000000000000000000000" : {
"balance" : "",
"code" : "{ (CALLDATACOPY 0 0 (CALLDATASIZE)) [[1]] (CALLCODE (GAS) 5 0 0 (CALLDATASIZE) 1000 32) [[2]](MLOAD 1000) [[3]](RETURNDATASIZE) }",
"nonce" : "0",
"storage" : {
"0x03" : "0xffffffff"
}
},

So, for the storage to become empty, it must be cleared; but that would mean 0 being written to it, which - according to the contract - means the return value from the precompile was 0 bytes long. EDIT: Or, possibly, the precompile call never happened?..

The storage value at location 0x02 will always be 0, no matter if precompile execution was successful or not. See ethereum/py-evm#1224 (comment) as to why.

The storage value at location 0x01 is whatever was left on top of the stack when CALLCODE to the precompile finishes.

what is your issue with that test? seems like you guessed right the memory allocation check of this test.

In short: it seems that this test case describes that the call to the precompile fails (at least in the implementation that filled the test JSON), but py-evm performs it just fine (albeit with hefty gas use).

Hence my previous request for clarification on rationale: is it to test for success or failure?


to get the actual result storage look at postState section of a filled blockchain test

There is no postState section in the filled JSON file. :(

There is a post section, but it's barren, specifying only block hash and logs, and indexes for data from transaction section - to be used in the transaction that's to generate said hash and logs.

There is no balance specified for the executing account other than the one before execution.

Therefore, I can't perform the suggested calculation.

Hence my previous request for clarification on gas use.

@winsvega
Copy link
Collaborator

winsvega commented Sep 3, 2018

That is mighty strange. The storage for that contract is already filled in the "pre" section of src, with a placeholder value 0xffffffff:

you dont have to worry about expect section actually.

There is no postState section in the filled JSON file. :(

there is: https://github.com/ethereum/tests/blob/develop/BlockchainTests/GeneralStateTests/stReturnDataTest/modexp_modsize0_returndatasize_d0g0v0.json#L69

The storage value at location 0x02 will always be 0, no matter if precompile execution was successful or not.

I think this place checks that the data was put in return data buffer. not in memory.
RETURNDATASIZE is checked for sure.

@veox
Copy link

veox commented Sep 3, 2018

OK, looks like I was looking at the wrong file then. X_X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants