core/vm: implement EIP-2681: Limit account nonce to 2^64-1

[gumb0]

This retroactively implements requirements or EIP-2681 for the account nonce upper limit.
Consensus tests (covering only CREATE/CREATE2 case): ethereum/tests#934

Opening PR as requested in ethereum/tests#934 (comment)

[holiman]

What about the case where the EOA goes above the same limit, shouldn't the tx be rejected in that case? Do we already have that in place?

[gumb0]

What about the case where the EOA goes above the same limit, shouldn't the tx be rejected in that case? Do we already have that in place?

Yes it should, tx_pool doesn't have such check, but nonces are already uint64 there. Digging deeper to find where it's decoded...

[gumb0]

Here I found another place where the check is missing when external transaction updates the sender's nonce.

go-ethereum/core/state_transition.go

Line 321 in 03bc8b7

st.state.SetNonce(msg.From(), st.state.GetNonce(sender.Address())+1)

EIP is not very clear for this case, (it only mentions tx nonce being above 64 bit, which is already rejected by geth as invalid tx encoding) I think it would make sense for a transaction to be valid and fail.

cc @axic

[gumb0]

And creation transaction BTW goes through the same code path as CREATE/CREATE2 and with this PR should already result in failed execution

go-ethereum/core/state_transition.go

Line 318 in 03bc8b7

ret, _, st.gas, vmerr = st.evm.Create(sender, st.data, st.gas, st.value)

[holiman]

Txpool is unrelated - the important part is the consensus engine. Check state-transition.go

[axic]

EIP is not very clear for this case, (it only mentions tx nonce being above 64 bit, which is already rejected by geth as invalid tx encoding) I think it would make sense for a transaction to be valid and fail.

The mempool is not part of the EIP as clients are free to decide how they select transactions.

[gumb0]

EIP is not very clear for this case, (it only mentions tx nonce being above 64 bit, which is already rejected by geth as invalid tx encoding) I think it would make sense for a transaction to be valid and fail.

The mempool is not part of the EIP as clients are free to decide how they select transactions.

"is not very clear" did not refer to the mempool, but to external transaction updating sender's nonce when included.

[axic]

EIP is not very clear for this case, (it only mentions tx nonce being above 64 bit, which is already rejected by geth as invalid tx encoding) I think it would make sense for a transaction to be valid and fail.

The mempool is not part of the EIP as clients are free to decide how they select transactions.

"is not very clear" did not refer to the mempool, but to external transaction updating sender's nonce when included.

Such transactions should not be accepted, i.e. they are invalid.

[gumb0]

"is not very clear" did not refer to the mempool, but to external transaction updating sender's nonce when included.

Such transactions should not be accepted, i.e. they are invalid.

This would make transactions with nonce == 0xffffffffffffffff invalid (and not only where the nonce exceeds 2^64-1 as EIP states)

[axic]

This would make transactions with nonce == 0xffffffffffffffff invalid (and not only where the nonce exceeds 2^64-1 as EIP states)

I mean those which exceed are invalid.

[holiman]

This would make transactions with nonce == 0xffffffffffffffff invalid (and not only where the nonce exceeds 2^64-1 as EIP states)

I'd agree with that: by themselves they are valid, but since nonce increase is a consequence of including them, they lead to breaking the rule. Hence they must be rejected and cannot be included in a block. (not in the txpool though, this is a consensus rule)

[axic]

Hmm, that's an interesting point, that the increase is part of the inclusion, so transaction nonce 2^64-1 can't be included. We should perhaps clarify this in the EIP.

[holiman]

Triage discussion: we should fix both nonce-out-of-bounds cases in the same PR

[gumb0]

I think a change to evm transaction tool is also needed.

[holiman]

Hmpf...

ERROR: Permission to ewasm/go-ethereum.git denied to holiman.

Please cherry-pick from https://github.com/holiman/go-ethereum/commits/deletelater

[holiman]

LGTM, if you also add the testcase from my branch