go-ethereum

go-ethereum copied to clipboard

Changes Included

Adds isStateTest flag into the t8n's environment, in order to execute the transition without block-level operations, such as:

• ApplyDAOHardFork
• The mining reward
• Withdrawals
• ParentBeaconBlockRoot contract operation at the end of the block

This should be taken into account for Prague to also inhibit operations of the deposit requests, withdrawal requests and consolidation requests.

Alloc output difference on the same test for the Cancun fork with and without the flag: isStateTest: false

"0x000f3df6d732807ef1319fb7b8bb8522d0beac02": {
        "code": "0x3373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500",
        "storage": {
            "0x00000000000000000000000000000000000000000000000000000000000003e8": "0x00000000000000000000000000000000000000000000000000000000000003e8"
        },
        "balance": "0x0",
        "nonce": "0x1"
    },

isStateTest: true

"0x000f3df6d732807ef1319fb7b8bb8522d0beac02": {
        "code": "0x3373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500",
        "balance": "0x0",
        "nonce": "0x1"
    },

This doesn't feel right to me. If we were to support such a function, it should be a CLI flag rather than part of the env. However, I don't think we should support even that. By adding more code paths to t8n, we create further intricacies to think about when writing tests. Doesn't this mean also that test consumers need to be able to run tests without the system calls?

This doesn't feel right to me. If we were to support such a function, it should be a CLI flag rather than part of the env.

This required for t8n tools that run in server mode, because they start once and the parameters cannot be modified afterwards, and sending this value through the env means that each test execution can be in either state mode or block mode.

However, I don't think we should support even that. By adding more code paths to t8n, we create further intricacies to think about when writing tests.

Not so much when it comes to writing tests because the automatic filling takes care of everything, but do you mean when updating t8n?

Doesn't this mean also that test consumers need to be able to run tests without the system calls?

Yes but only state test, and any client could choose to skip running state test since we fill them as blockchain tests also.

This doesn't feel right to me. If we were to support such a function, it should be a CLI flag rather than part of the env.

This required for t8n tools that run in server mode,

t8n doesn't have a server-mode. The tool which invokes t8n can just pass it as a command line parameter. Seems like I am misunderstanding something

This doesn't feel right to me. If we were to support such a function, it should be a CLI flag rather than part of the env.

This required for t8n tools that run in server mode,

t8n doesn't have a server-mode. The tool which invokes t8n can just pass it as a command line parameter. Seems like I am misunderstanding something

Yes, it's not a problem to specify it as a parameter for some t8n tools and as an env field for others. I can make the change to this PR so it takes a parameter.

Well, implementation details aside, I don't think the semantics are right. IsStateTest is a wonky meta-information. It conveys "run this as a state-test", but it's not clear what that means, IMO.

I don't think we should add specific evm-logic depending on what type of test we execute. It makes the testing too synthetic. I would rather we figure it out, and if needed add other flags that are more directed at state processing logic, such as "do not apply withdrawals". But I am not sure anything is needed (see below).

---

The mining reward

This is already handled, since years ago:

$ go run ./cmd/evm t8n -h | grep reward
          --state.reward value                (default: 0)                      
                Mining reward. Set to -1 to disable

By supplying --state.reward=-1, there is no reward at all (which is different from 0, which doesn't add any funds but does touch the coinbase account).

ApplyDAOHardFork

I don't think this is an issue, is it?

Withdrawals

If there are no withdrawals (and a statetest doesn't have them?), then isn't this moot already?

ParentBeaconBlockRoot contract operation at the end of the block

Does this matter? If the statetest doesn't contain any code at this address, nothing will happen.

Well, implementation details aside, I don't think the semantics are right. IsStateTest is a wonky meta-information. It conveys "run this as a state-test", but it's not clear what that means, IMO.

I don't think we should add specific evm-logic depending on what type of test we execute. It makes the testing too synthetic. I would rather we figure it out, and if needed add other flags that are more directed at state processing logic, such as "do not apply withdrawals". But I am not sure anything is needed (see below).

The mining reward

This is already handled, since years ago:

$ go run ./cmd/evm t8n -h | grep reward
          --state.reward value                (default: 0)                      
                Mining reward. Set to -1 to disable

By supplying --state.reward=-1, there is no reward at all (which is different from 0, which doesn't add any funds but does touch the coinbase account).

ApplyDAOHardFork

I don't think this is an issue, is it?

Withdrawals

If there are no withdrawals (and a statetest doesn't have them?), then isn't this moot already?

ParentBeaconBlockRoot contract operation at the end of the block

Does this matter? If the statetest doesn't contain any code at this address, nothing will happen.

I agree with all points above, thanks for the response, closing this for now.