create proofs with post state #384
Conversation
|
This is missing specific tests, btw |
gballet
force-pushed
the
proofs-with-post-state
branch
3 times, most recently
from
1eebc7f
to
f392bd3
Compare
gballet
force-pushed
the
proofs-with-post-state
branch
4 times, most recently
from
6764a0c
to
d220d72
Compare
| // if a post-state tree is present, merge its proof elements with | ||
| // those of the pre-state tree, so that they can be proved together. | ||
| if postroot != nil { | ||
| pe_post, _, _, err := GetCommitmentsForMultiproof(postroot, keys, resolver) |
There was a problem hiding this comment.
One question: here keys is the same set of keys used for pre-state and post-state proving.
But if the block execution read 1000 keys but only wrote into 1 key; shouldn't keys in this line only have 1 key instead of the 1000 ones? If that isn't the case, that would probably mean that proof_(c/f/y/z)s will have duplicate cuadruples.
As in, in the post-state VKT, there will be keys in keys that have the same value as in the pre-state VKT (e.g: 999 in this example). So, that means that there will be many repeated openings.
Maybe I'm a bit confused.
There was a problem hiding this comment.
Well the opening themselves will not be integrated to the proof. But you are correct that they will be computed. This is reusing a function that does too much, and at the end values that are just read will find themselves in the proof. This is not incorrect but it's much more than necessary.
There was a problem hiding this comment.
Not going to fix it in this PR, created #393 to track it.
proof_ipa.go
Outdated
| tr := common.NewTranscript("vt") | ||
| mpArg := ipa.CreateMultiProof(tr, cfg.conf, proof_cis, proof_fs, proof_zs) |
There was a problem hiding this comment.
I think at some point we should hide this transcript creation from clients.
It's a bit odd, and I don't think there's a use case in which you want to call CreateMultiProof with an existing non-pristine transcript.
If makes sense to you, I can add it to some TODO list I'm keeping.
There was a problem hiding this comment.
Sure, add it to the never-ending Todo list 😜
proof_ipa.go
Outdated
| // PostStateTreeFromProof uses the pre-state trie and the list of updated values | ||
| // to produce the stateless post-state trie. | ||
| func PostStateTreeFromProof(preroot VerkleNode, statediff StateDiff) (VerkleNode, error) { |
There was a problem hiding this comment.
Nit: I'd rename this to POstStateTreeFromStateDiff since it doesn't require a proof or similar.
| if len(data.keys) != len(data.values) { | ||
| t.Fatalf("incompatible number of keys and values: %d != %d", len(data.keys), len(data.values)) | ||
| } |
There was a problem hiding this comment.
Nit: dunno if you want to add the same check to compare len(data.updatekeys) != len(data.updatevalues).
| if bytes.Equal(k[:31], info[string(p)].stem) { | ||
| values[k[31]] = proof.Values[i] | ||
| values[k[31]] = proof.PreValues[i] | ||
| } |
There was a problem hiding this comment.
I still feel this if is a bit strange. We're reconstructing the values to be assigned in the leaf when we'll soon create the LeafNode.
But see what we do in CreatePath(...):
- We use that
valueslice as expected. - We do some extra assignments from
stemInfo.
Is the latter needed? Or a noop? Or maybe I'm missing something.
There was a problem hiding this comment.
yeah I can't remember why that is, and removing it seems to work. I won't remove it in this PR but I'm making a note of it in #395
gballet
force-pushed
the
proofs-with-post-state
branch
from
b1cb716
to
5a9b0c7
Compare
gballet
force-pushed
the
proofs-with-post-state
branch
from
5a9b0c7
to
c4d7e93
Compare
Changes the signature of
MakeVerkleMultiProofto accept an extra root: if that root isn'tnil, it means that the post state of the tree is also added to the proof.