Add EIP7736 support #457

weiihann · 2024-10-11T10:01:58Z

Primary changes:

Add new StateEpoch type
Modify Get, Insert and Delete to include StateEpoch. If trying to access expired leaf node, returns error.
Additional lastEpoch field in LeafNode, included in serialization and node commitment calculation
Add new ExpiryLeafNode struct

gballet

Thank you for this PR. This is a good start, although it requires a lot of polishing, and probably a couple spec discussions as well. I left a few comments for you to consider, I can have another pass when you've had a look.

gballet · 2024-10-14T09:20:48Z

benchs/main.go

it would be nice to have another benchmark that checks how much time it takes to expire the data. For example, a benchmark that inserts a few "old" leaves in the tree, then a lot more "newer" leaves, and then see how much it takes to delete all leaves.

We are not looking into bulk-expiring the tree, I think it would be insane, but to estimate what it would take to expire a few leaves per block, in case we detect that they are still present in the tree in case they are expired. Think of a "garbage collect on access" type of approach.

conversion.go

expired_leaf.go

state_epoch.go

proof_ipa.go

weiihann · 2024-10-24T13:55:51Z

Here's a general explanation of how expiry works with stateless proofs:

There are 4 scenarios for a given stem:

Both prestate and poststate not expired

Works as normal

Both prestate and poststate expired

Works as normal, there shouldn't be any state diffs because they are expired

Prestate not expired, poststate expired

Works as normal, this indicates that there are no state updates from prestate to poststate, so state diffs do not exist

Prestate expired, poststate not expired

This indicates that a resurrection has occurred. How to verify:
- We added a new extension status type called extStatusExpired to indicate if a given stem has expired.
- If an extension status of a stem is extStatusExpired and any of the postvalue in the stem given in the proof is not absent, then we know that there's a resurrection.

gballet

Left more change requests

conversion.go

tree_test.go

gballet · 2024-10-29T15:21:34Z

proof_ipa.go

@@ -94,6 +94,7 @@ type SuffixStateDiffs []SuffixStateDiff
 type StemStateDiff struct {
 	Stem        [StemSize]byte   `json:"stem"`
 	SuffixDiffs SuffixStateDiffs `json:"suffixDiffs"`
+	Resurrected bool             `json:"resurrected"`


This is going to be taking more space. I would do a expiredStem field like we have poaStems. But no need to take action on this right now, as long as it works it's fine. This is a later-stage optimization.

encoding.go

tree.go

revert unnecessary changes remove unnecessary changes

…ip7736/master # Conflicts: # encoding.go # tree.go

gballet

This PR must be updated to refresh the period upon a read
Also, I don't think that the tests are actually checking that an expiration has been proven. but maybe I misundertsood.
It also seems to me that inserting a sibling to an expired node isn't tested.

epoch.go

gballet · 2024-12-10T19:27:17Z

epoch.go

+	"encoding/binary"
+)
+
+type StatePeriod uint16


I think I already asked that, but why uint16? On some archs with strong alignment constaints, that could cause a lot of slowdowns, and, however small that slowdown is, I don't know why we would pay that cost?

note that if this is because we don't want to take too much space with the leaf encoding, we can always store it as a uint16 (or even a uint8, that's 128 years before we need to upgrade!)

As per our convo, this is a premature optimization and this comment should be discarded. We can look into it in the future.

conversion.go

tree.go

proof_test.go

proof_ipa.go

expired_tree_test.go

This reverts commit b670596.

edits

weiihann · 2025-01-05T07:02:26Z

Change Log (5/1/2025):

Add Revive() to VerkleNode interface
Proof now includes the periods of each stem in pre-state tree. How stateless proof works with expiry:
- Given a pre-state tree with expired stem, we first perform Revive()
- This particular Revive() operation should not refresh the period of the leaf node to prevent changing commitment
- We now have a pre-state tree with expired stems resolved
- Generate proof. For each stem, the period is included in the proof
- In the verification process, the pre-state tree can be reconstructed from the proof, and the period of each stem is resolved correctly
Minor code quality improvements and bug fixes

gballet

It's getting into shape! I'm not sure that i understand the point of skipUpdate: why shouldn't the counter/commitment be updated upon revival? We can think of a use case where someone resurects an account for someone else, and doesn't directly write/read from that account.

gballet · 2025-01-07T16:58:29Z

tree.go

+		return errMissingNodeInStateless
+	case Empty:
+		// TODO(weiihann): double confirm this
+		return errors.New("cannot revive an empty node")


If you go through a tree and the node that you are given indicates that the commitment is non-zero, you should definitely error. But if the commitment is 0, there's a case to be made that the tx should not fail... although, if it's simpler to handle, then let's just error.

gballet · 2025-01-07T16:59:53Z

tree.go

+
+		return nil
+	case *LeafNode:
+		return child.Revive(stem, values, oldPeriod, curPeriod, skipUpdate, resolver)


hmmmm, since no invididual leaf can be expired, I think this should be an error, or at least ignored.

I get it now, the field might not have been pruned by the collector. Add a comment here, so that I don't ask myself that question each time, please.

gballet · 2025-01-07T17:05:50Z

tree.go


 	// Serialize encodes the node to RLP.
 	Serialize() ([]byte, error)

 	// Copy a node and its children
 	Copy() VerkleNode

+	Revive(Stem, [][]byte, StatePeriod, StatePeriod, bool, NodeResolverFn) error


could you please document what the boolean does?

gballet requested review from gballet and jsign October 14, 2024 07:17

gballet reviewed Oct 14, 2024

View reviewed changes

gballet reviewed Oct 29, 2024

View reviewed changes

weiihann force-pushed the eip7736/master branch 2 times, most recently from 546eedd to 8a00f80 Compare October 30, 2024 03:36

weiihann added 16 commits October 30, 2024 14:48

feat: add last epoch support

39fdd6c

calculate leaf commitment for insert/delete

32ce419

Add expiry access rules

25ba7b9

Add ExpiredLeafNode

ec74e56

add StateEpoch to Get

3f23ce0

change epoch to timestamps, add encoding test

de94975

add basic expired leaf test

d536897

add basic operation tests

342e683

support stateless proof

5419a7d

revert unnecessary changes remove unnecessary changes

change access timestamp to state epoch

d037673

implement toDot for expired leaf

4f15b6c

make epoch size internal

3d0659a

fix bug in leaf commitment

d59b620

minor formatting improvements

cc43672

make state epoch uint16

861826a

remove epoch from GetProofItems

e558f36

weiihann force-pushed the eip7736/master branch from 8a00f80 to e558f36 Compare October 30, 2024 06:56

weiihann added 3 commits November 19, 2024 09:52

Merge branch 'master' of https://github.com/ethereum/go-verkle into e…

ae9ac46

…ip7736/master # Conflicts: # encoding.go # tree.go

change epoch to period

9e5c6fa

refactor isExpired

f631e65

gballet reviewed Dec 12, 2024

View reviewed changes

weiihann added 2 commits December 18, 2024 19:18

update expiry check

ed32767

use period consts

d68c4ae

weiihann added 13 commits December 20, 2024 12:25

refresh period upon a read

84e62fb

compare periods at DeleteAtStem

2bb9b2e

add Revive() to VerkleNode interface

1a73d65

add Revive()

64151cb

add lastPeriod to ExpiredLeafNode

b670596

Revert "add lastPeriod to ExpiredLeafNode"

36036fb

This reverts commit b670596.

fix period encoding bug

1ce49aa

add skipVerify in Revive()

c42a5c2

remove skipVerify

971c099

add lastPeriod to ExpiredLeafNode

4cee9df

add isExpired

fe3e25c

fix stateless proof

dd87e7f

Remove expiry and add period in stateless proof

2950a74

edits

weiihann force-pushed the eip7736/master branch from 93b1fb4 to 2950a74 Compare January 5, 2025 06:51

gballet reviewed Jan 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add EIP7736 support #457

Add EIP7736 support #457

weiihann commented Oct 11, 2024

gballet left a comment

gballet Oct 14, 2024

weiihann commented Oct 24, 2024

gballet left a comment

gballet Oct 29, 2024

gballet left a comment

gballet Dec 10, 2024

gballet Dec 10, 2024

weiihann Dec 18, 2024

weiihann commented Jan 5, 2025 •

edited

Loading

gballet left a comment

gballet Jan 7, 2025

gballet Jan 7, 2025

gballet Jan 7, 2025

gballet Jan 7, 2025

Add EIP7736 support #457

Are you sure you want to change the base?

Add EIP7736 support #457

Conversation

weiihann commented Oct 11, 2024

gballet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiihann commented Oct 24, 2024

gballet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gballet 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

weiihann commented Jan 5, 2025 • edited Loading

gballet 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

weiihann commented Jan 5, 2025 •

edited

Loading