trie: make rhs-proof align with last key in range proofs #28311
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
holiman
force-pushed
the
trie_prep_pt2
branch
from
43bb679
to
f641ef7
Compare
holiman
commented
Oct 11, 2023
| @@ -576,7 +487,7 @@ func TestBadRangeProof(t *testing.T) { | |||
| case 2: | |||
| // Gapped entry slice | |||
| index = mrand.Intn(end - start) | |||
| if (index == 0 && start < 100) || (index == end-start-1 && end <= 100) { | |||
| if (index == 0 && start < 100) || (index == end-start-1) { | |||
There was a problem hiding this comment.
@rjl493456442 what was the significance of end <= 100 there?
There was a problem hiding this comment.
honestly I don't remember why I put the 100 there.
There was a problem hiding this comment.
// Gapped entry slice
index = mrand.Intn(end - start)
if index == 0 || index == end-start-1 {
continue
}
should be fine to drop the 100.
rjl493456442
approved these changes
Oct 13, 2023
| @@ -520,6 +520,7 @@ func VerifyRangeProof(rootHash common.Hash, firstKey []byte, lastKey []byte, key | |||
| } | |||
| return false, nil | |||
| } | |||
| var lastKey = keys[len(keys)-1] | |||
There was a problem hiding this comment.
please move this line after the comment.
There was a problem hiding this comment.
Are you sure? The comment refers to the clause that follows it, it does not refer to the lastKey variable?
| @@ -576,7 +487,7 @@ func TestBadRangeProof(t *testing.T) { | |||
| case 2: | |||
| // Gapped entry slice | |||
| index = mrand.Intn(end - start) | |||
| if (index == 0 && start < 100) || (index == end-start-1 && end <= 100) { | |||
| if (index == 0 && start < 100) || (index == end-start-1) { | |||
There was a problem hiding this comment.
honestly I don't remember why I put the 100 there.
|
please fix the conflicting files |
holiman
force-pushed
the
trie_prep_pt2
branch
from
3924dec
to
11703d9
Compare
devopsbo3
pushed a commit
to HorizenOfficial/go-ethereum
that referenced
this pull request
Nov 10, 2023
) During snap-sync, we request ranges of values: either a range of accounts or a range of storage values. For any large trie, e.g. the main account trie or a large storage trie, we cannot fetch everything at once. Short version; we split it up and request in multiple stages. To do so, we use an origin field, to say "Give me all storage key/values where key > 0x20000000000000000". When the server fulfils this, the server provides the first key after origin, let's say 0x2e030000000000000 -- never providing the exact origin. However, the client-side needs to be able to verify that the 0x2e03.. indeed is the first one after 0x2000.., and therefore the attached proof concerns the origin, not the first key. So, short-short version: the left-hand side of the proof relates to the origin, and is free-standing from the first leaf. On the other hand, (pun intended), the right-hand side, there's no such 'gap' between "along what path does the proof walk" and the last provided leaf. The proof must prove the last element (unless there are no elements). Therefore, we can simplify the semantics for trie.VerifyRangeProof by removing an argument. This doesn't make much difference in practice, but makes it so that we can remove some tests. The reason I am raising this is that the upcoming stacktrie-based verifier does not support such fancy features as standalone right-hand borders.
devopsbo3
added a commit
to HorizenOfficial/go-ethereum
that referenced
this pull request
Nov 10, 2023
…ereum#28311)" This reverts commit 51d0547.
devopsbo3
added a commit
to HorizenOfficial/go-ethereum
that referenced
this pull request
Nov 10, 2023
…ereum#28311)" This reverts commit 51d0547.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During snap-sync, we request ranges of values: either a range of accounts or a range of storage values. For any large trie, e.g. the main account trie or a large storage trie, we cannot fetch everything at once.
Short version; we split it up and request in multiple stages. To do so, we use an
originfield, to say "Give me all storage key/values where key >0x20000000000000000". When the server fulfils this, the server provides the first key afterorigin, let's say0x2e030000000000000-- never providing the exact origin. However, the client-side needs to be able to verify that the0x2e03..indeed is the first one after0x2000.., and therefore the attached proof concerns theorigin, not the first key.So, short-short version: the left-hand side of the proof relates to the
origin, and is free-standing from the first leaf.On the other hand, (pun intended), the right-hand side, there's no such 'gap' between "along what path does the proof walk" and the last provided leaf. The proof must prove the last element (unless there are no elements).
Therefore, we can simplify the semantics for
trie.VerifyRangeProofby removing an argument. This doesn't make much difference in practice, but makes it so that we can remove some tests. The reason I am raising this is that the upcoming stacktrie-based verifier does not support such fancy features as standalone right-hand borders.