EIP-2681: Limit account nonce to 2^64-1 #934

gumb0 · 2021-08-24T13:10:08Z

https://eips.ethereum.org/EIPS/eip-2681
Geth change is in ethereum/go-ethereum#23853

EIP applies retroactively, therefore I put >=Frontier for CREATE and >=Constantinople for CREATE2.
(Also not sure what should be done with already generated legacy transaction tests, probably should be updated)

winsvega · 2021-08-29T00:46:40Z

working to revive ttTransaction Tests

winsvega · 2021-10-15T21:52:30Z

checked now with new transaction tests

winsvega · 2021-10-15T21:53:09Z

ah wait its the state tests

marioevz · 2021-10-15T23:09:28Z

src/GeneralStateTestsFiller/stCreate2/CREATE2_HighNonceFiller.yml

+       b94f5374fce5edbc8e2a8697c15331677e6ebf0b:
+          nonce: '0xffffffffffffffff'
+          storage:
+            '1': 1


The EIP states that the CREATE opcodes should "abort with an exceptional halt" when the nonce is about to overflow.
I would expect the storage remains unchanged, is this ok?

Good question, I interpret the EIP text as opcodes themselves end with an exceptional halt, but not the execution frame they're called from (similar to not having enough balance for CREATE's value transfer, call stack overflow, address already existing etc.)

marioevz · 2021-11-02T00:17:23Z

Test cases look good to me, but I think we should add a positive test case for both CREATE/CREATE2 where the nonce is (2**64) - 2, and the contract is still created.
Also, it seems like the current outcome of CREATE_HighNonce is that the nonce wraps around to zero, and STORAGE[0] is equal to the address as if the contract was actually created, although the EIP does not specify what the top of the stack should look like in this scenario.
I'm currently testing on ethereum/go-ethereum@551bd6e.

gumb0 · 2021-11-02T12:14:59Z

Also, it seems like the current outcome of CREATE_HighNonce is that the nonce wraps around to zero, and STORAGE[0] is equal to the address as if the contract was actually created,

Yes, current geth's behavior is wrapping around, I linked the possible patch in the description ewasm/go-ethereum@36580c6 which I used to generate.

although the EIP does not specify what the top of the stack should look like in this scenario.

EIP specifies exceptional halt, I think this implies 0 returned on stack, as in all other exceptional cases.

marioevz

Request for two extra test cases and two minor changes.

marioevz · 2021-11-02T21:30:51Z

src/GeneralStateTestsFiller/stCreateTest/CREATE_HighNonceFiller.yml

+       b94f5374fce5edbc8e2a8697c15331677e6ebf0b:
+          nonce: '0xffffffffffffffff'
+          storage:
+            '1': 1


Could you add a check here for:
storage:
'0': 0

and

04e9a8460199e670ffb592f93a2f74bdcb44b0bd:
'shouldnotexist': '1'

storage '0': 0 is implicitly checked I think (error if some storage present that is not in test expectations), but added.

yes.
but look after cases where you have
sstore(0, test()) and a pre storage is empty.

if smth goes wrong, the transaction reverted, storage remains unchanged and the expect section not showing any error, but the actual exection of test() was aborted or not performed properly

If I understand you correctly, that's why sstore(1, 1) is done there in the end
(adding storage '0': 0 wouldn't help this or change anything)

marioevz · 2021-11-02T21:33:39Z

src/GeneralStateTestsFiller/stCreateTest/CREATE_HighNonceFiller.yml

+          sstore(0, create(0, 0, 5))
+          sstore(1, 1)
+        }
+      nonce: '0xffffffffffffffff'


Could we add a positive test case CREATE_HighNonceMinus1, where nonce is '0xfffffffffffffffe' and contract '0xd061b08a84ebc70fe797f9bd62f4269ef8274a13' is actually created?

marioevz · 2021-11-02T22:18:08Z

src/GeneralStateTestsFiller/stCreate2/CREATE2_HighNonceFiller.yml

+          nonce: 1
+       b94f5374fce5edbc8e2a8697c15331677e6ebf0b:
+          nonce: '0xffffffffffffffff'
+          storage:


We should also check:

storage:
'0': 0

and

77dd5d2a2b742ca01ee2cfff306445e3741ef744:
'shouldnotexist': '1'

marioevz · 2021-11-02T22:18:58Z

src/GeneralStateTestsFiller/stCreate2/CREATE2_HighNonceFiller.yml

+          sstore(0, create2(0, 0, 5, 0))
+          sstore(1, 1)
+        }
+      nonce: '0xffffffffffffffff'


Could we also add here a positive test case CREATE2_HighNonceMinus1, where nonce is '0xfffffffffffffffe' and contract '0x77dd5d2a2b742ca01ee2cfff306445e3741ef744' is actually created?

marioevz

Looks good to me, thanks for the changes.
I think this PR still requires ewasm/go-ethereum@eip-2681 to be merged onto ethereum/go-ethereum though.

holiman · 2021-11-03T18:37:50Z

Geth change is in https://github.com/ewasm/go-ethereum/tree/eip-2681

@gumb0 there's no open PR about it, is there? If not, would you mind opening one?

gumb0 · 2021-11-11T11:09:33Z

I have also updated transaction tests for a new rule that nonce == 0xffffffffffffffff is forbidden (see update to EIP ethereum/EIPs#4437) The geth PR linked in description includes a change to t9n tool for this.

gumb0 · 2021-11-11T11:10:03Z

src/TransactionTestsFiller/ttNonce/TransactionWithHighNonce64Minus1Filler.json

@@ -3,6 +3,7 @@
    {
        "expectException" : 
        {
+            ">=Frontier": "NonceMax"


This new error also has to be added to retesteth config.

The corresponding geth string is

"NonceMax" : "nonce exceeds 2^64-1",

@winsvega should it be called TR_NonceMax ?

(Some transaction tests use errors with TR_ prefix, and others without, so not sure)

Added ethereum/retesteth#156 for this.

marioevz · 2021-11-11T18:45:28Z

src/TransactionTestsFiller/ttNonce/TransactionWithHighNonce64Minus2Filler.json

@@ -0,0 +1,20 @@
+{
+    "TransactionWithHighNonce64Minus1" :


Should be TransactionWithHighNonce64Minus2

marioevz

Looks good to me 👍

gumb0 · 2021-11-16T16:38:19Z

Rebased.

winsvega

ok. looks good.

how about a case where a contract with high nonce calls contract via delegate
and that subcall creates a contract. but since the context of execution moved. the actual contract creator changes.

holiman · 2021-11-16T19:13:31Z

how about a case where a contract with high nonce calls contract via delegate
and that subcall creates a contract. but since the context of execution moved. the actual contract creator changes.

DELEGATECALL doesn't move the context of execution, it just borrows some code and continues where it was. New scope, same address.

winsvega · 2021-11-16T19:54:03Z

I mean when A having nonce 0, delegatecall B which has maxnonce, and B creates contract what happens?
should work, no? A will be the creator, not B.

holiman · 2021-11-16T21:17:51Z

Yes, should work, will work

gumb0 · 2021-11-17T12:47:04Z

Rebased, removed change to TransactionWithHighNonce64Minus1 merged in #988.

winsvega · 2021-11-19T09:58:20Z

what about transaction coming from acc

gumb0 marked this pull request as draft August 24, 2021 13:12

gumb0 force-pushed the eip-2681 branch 3 times, most recently from 10932c7 to e17ac4f Compare August 24, 2021 14:03

winsvega closed this Oct 15, 2021

winsvega reopened this Oct 15, 2021

marioevz reviewed Oct 15, 2021

View reviewed changes

winsvega mentioned this pull request Nov 1, 2021

make sure there are transaction tests for nonce #978

Closed

8 tasks

winsvega closed this Nov 2, 2021

winsvega reopened this Nov 2, 2021

marioevz requested changes Nov 2, 2021

View reviewed changes

gumb0 force-pushed the eip-2681 branch from e17ac4f to 682165e Compare November 3, 2021 14:45

gumb0 marked this pull request as ready for review November 3, 2021 17:43

marioevz reviewed Nov 3, 2021

View reviewed changes

gumb0 mentioned this pull request Nov 3, 2021

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

Merged

winsvega marked this pull request as draft November 3, 2021 21:58

gumb0 commented Nov 11, 2021

View reviewed changes

gumb0 marked this pull request as ready for review November 11, 2021 15:22

gumb0 requested review from winsvega and marioevz November 11, 2021 15:22

marioevz reviewed Nov 11, 2021

View reviewed changes

gumb0 force-pushed the eip-2681 branch from 2f1a372 to 4f7623b Compare November 11, 2021 19:15

marioevz approved these changes Nov 11, 2021

View reviewed changes

marioevz mentioned this pull request Nov 16, 2021

Add NonceMax Error to t8ntool ethereum/retesteth#156

Merged

gumb0 force-pushed the eip-2681 branch from 4f7623b to 6da532e Compare November 16, 2021 16:38

winsvega approved these changes Nov 16, 2021

View reviewed changes

marioevz mentioned this pull request Nov 16, 2021

Add Expected Error to Existing HighNonce Test #988

Merged

gumb0 added 3 commits November 17, 2021 13:44

EIP-2681: Limit account nonce to 2^64-1

98f250c

EIP-2861: Tests CREATE/CREATE2 from account with max nonce - 1

c746ab6

EIP-2681: Transactions with nonce == 2^64-1 are invalid

5516881

gumb0 force-pushed the eip-2681 branch from 6da532e to 5516881 Compare November 17, 2021 12:45

winsvega merged commit cfe840a into ethereum:develop Nov 19, 2021

gumb0 deleted the eip-2681 branch February 7, 2022 10:52

gumb0 mentioned this pull request Feb 7, 2022

EIP-2681: Fix CREATE/CREATE2 max nonce error reporting rule ethereum/EIPs#4784

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EIP-2681: Limit account nonce to 2^64-1 #934

EIP-2681: Limit account nonce to 2^64-1 #934

gumb0 commented Aug 24, 2021 •

edited

Loading

winsvega commented Aug 29, 2021

winsvega commented Oct 15, 2021

winsvega commented Oct 15, 2021

marioevz Oct 15, 2021

gumb0 Oct 18, 2021

marioevz commented Nov 2, 2021

gumb0 commented Nov 2, 2021

marioevz left a comment

marioevz Nov 2, 2021

gumb0 Nov 3, 2021

winsvega Nov 3, 2021

gumb0 Nov 4, 2021

marioevz Nov 2, 2021

gumb0 Nov 3, 2021

marioevz Nov 2, 2021

marioevz Nov 2, 2021

gumb0 Nov 3, 2021

marioevz left a comment

holiman commented Nov 3, 2021

gumb0 commented Nov 11, 2021 •

edited

Loading

gumb0 Nov 11, 2021 •

edited

Loading

gumb0 Nov 11, 2021 •

edited

Loading

marioevz Nov 16, 2021

marioevz Nov 11, 2021

marioevz left a comment

gumb0 commented Nov 16, 2021

winsvega left a comment

holiman commented Nov 16, 2021

winsvega commented Nov 16, 2021

holiman commented Nov 16, 2021

gumb0 commented Nov 17, 2021

winsvega commented Nov 19, 2021

EIP-2681: Limit account nonce to 2^64-1 #934

EIP-2681: Limit account nonce to 2^64-1 #934

Conversation

gumb0 commented Aug 24, 2021 • edited Loading

winsvega commented Aug 29, 2021

winsvega commented Oct 15, 2021

winsvega commented Oct 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marioevz commented Nov 2, 2021

gumb0 commented Nov 2, 2021

marioevz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marioevz left a comment

Choose a reason for hiding this comment

holiman commented Nov 3, 2021

gumb0 commented Nov 11, 2021 • edited Loading

gumb0 Nov 11, 2021 • edited Loading

Choose a reason for hiding this comment

gumb0 Nov 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marioevz left a comment

Choose a reason for hiding this comment

gumb0 commented Nov 16, 2021

winsvega left a comment

Choose a reason for hiding this comment

holiman commented Nov 16, 2021

winsvega commented Nov 16, 2021

holiman commented Nov 16, 2021

gumb0 commented Nov 17, 2021

winsvega commented Nov 19, 2021

gumb0 commented Aug 24, 2021 •

edited

Loading

gumb0 commented Nov 11, 2021 •

edited

Loading

gumb0 Nov 11, 2021 •

edited

Loading

gumb0 Nov 11, 2021 •

edited

Loading