-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add support for SMT ExclusionProofs
#153
Changes from all commits
6489ea7
56d948c
f66850b
e0d375a
68015c7
28ec1e8
3f55b9b
faf351b
7964fb0
96009fb
b980cc0
bccef22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,11 +52,35 @@ func VerifyMembership(spec *ProofSpec, root CommitmentRoot, proof *CommitmentPro | |
func VerifyNonMembership(spec *ProofSpec, root CommitmentRoot, proof *CommitmentProof, key []byte) bool { | ||
// decompress it before running code (no-op if not compressed) | ||
proof = Decompress(proof) | ||
np := getNonExistProofForKey(spec, proof, key) | ||
if np == nil { | ||
var err error | ||
switch proof.Proof.(type) { | ||
case *CommitmentProof_Nonexist: | ||
np := getNonExistProofForKey(spec, proof, key) | ||
if np == nil { | ||
return false | ||
} | ||
err = np.Verify(spec, root, key) | ||
case *CommitmentProof_Exclusion: | ||
ex := getExclusionProofForKey(proof, key) | ||
if ex == nil { | ||
return false | ||
} | ||
err = ex.Verify(spec, root, key) | ||
case *CommitmentProof_Batch: | ||
np := getNonExistProofForKey(spec, proof, key) | ||
ex := getExclusionProofForKey(proof, key) | ||
if np == nil && ex == nil { | ||
return false | ||
} | ||
if np != nil { | ||
err = np.Verify(spec, root, key) | ||
} else { | ||
err = ex.Verify(spec, root, key) | ||
} | ||
default: | ||
return false | ||
} | ||
err := np.Verify(spec, root, key) | ||
|
||
return err == nil | ||
} | ||
|
||
|
@@ -103,6 +127,13 @@ func CombineProofs(proofs []*CommitmentProof) (*CommitmentProof, error) { | |
}, | ||
} | ||
entries = append(entries, entry) | ||
} else if excl := proof.GetExclusion(); excl != nil { | ||
entry := &BatchEntry{ | ||
Proof: &BatchEntry_Exclusion{ | ||
Exclusion: excl, | ||
}, | ||
} | ||
entries = append(entries, entry) | ||
} else if non := proof.GetNonexist(); non != nil { | ||
entry := &BatchEntry{ | ||
Proof: &BatchEntry_Nonexist{ | ||
|
@@ -148,6 +179,23 @@ func getExistProofForKey(proof *CommitmentProof, key []byte) *ExistenceProof { | |
return nil | ||
} | ||
|
||
func getExclusionProofForKey(proof *CommitmentProof, key []byte) *ExclusionProof { | ||
switch p := proof.Proof.(type) { | ||
case *CommitmentProof_Exclusion: | ||
ep := p.Exclusion | ||
if bytes.Equal(ep.Key, key) { | ||
return ep | ||
} | ||
case *CommitmentProof_Batch: | ||
for _, sub := range p.Batch.Entries { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this the intended behaviour: for a batch of exclusion proofs we just return the first valid one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are checking the batch for the exclusion proof with the correct key. If there are multiple then they are the same proof and we need only return 1. When we validate a batch in entirety we go through all proofs and execute them individually. This is just that we were given a batch entry and are looking to prove just the one with the matching key. |
||
if ep := sub.GetExclusion(); ep != nil && bytes.Equal(ep.Key, key) { | ||
return ep | ||
} | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func getNonExistProofForKey(spec *ProofSpec, proof *CommitmentProof, key []byte) *NonExistenceProof { | ||
switch p := proof.Proof.(type) { | ||
case *CommitmentProof_Nonexist: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,7 +43,7 @@ var TendermintSpec = &ProofSpec{ | |
}, | ||
} | ||
|
||
// SmtSpec constrains the format for SMT proofs (as implemented by github.com/celestiaorg/smt) | ||
// SmtSpec constrains the format for SMT proofs (as implemented by github.com/pokt-network/smt) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙌 |
||
var SmtSpec = &ProofSpec{ | ||
LeafSpec: &LeafOp{ | ||
Hash: HashOp_SHA256, | ||
|
@@ -86,13 +86,18 @@ func (p *CommitmentProof) Calculate() (CommitmentRoot, error) { | |
return v.Exist.Calculate() | ||
case *CommitmentProof_Nonexist: | ||
return v.Nonexist.Calculate() | ||
case *CommitmentProof_Exclusion: | ||
return v.Exclusion.Calculate() | ||
case *CommitmentProof_Batch: | ||
if len(v.Batch.GetEntries()) == 0 || v.Batch.GetEntries()[0] == nil { | ||
return nil, errors.New("batch proof has empty entry") | ||
} | ||
if e := v.Batch.GetEntries()[0].GetExist(); e != nil { | ||
return e.Calculate() | ||
} | ||
if x := v.Batch.GetEntries()[0].GetExclusion(); x != nil { | ||
return x.Calculate() | ||
} | ||
if n := v.Batch.GetEntries()[0].GetNonexist(); n != nil { | ||
return n.Calculate() | ||
} | ||
|
@@ -163,6 +168,106 @@ func (p *ExistenceProof) calculate(spec *ProofSpec) (CommitmentRoot, error) { | |
return res, nil | ||
} | ||
|
||
h5law marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func (p *ExclusionProof) Calculate() (CommitmentRoot, error) { | ||
if p.GetLeaf() == nil { | ||
return nil, errors.New("exclusion proof needs defined LeafOp") | ||
} | ||
|
||
// As the actual_path and actual_value_hash are already hashed both the | ||
// prehash_key and prehash_value fields should be set to HashOp_NO_HASH | ||
// this is because they have been populated from the unrelated leaf found | ||
// in place of the key we were looking for, hashing them again would | ||
// result in an incorrect leaf node hash and invalidate the proof. | ||
if p.Leaf.PrehashKey != HashOp_NO_HASH { | ||
return nil, errors.New("exclusion proof must have leaf with PrehashKey == NO_HASH") | ||
} | ||
if p.Leaf.PrehashValue != HashOp_NO_HASH { | ||
return nil, errors.New("exclusion proof must have leaf with PrehashValue == NO_HASH") | ||
} | ||
Comment on lines
+181
to
+186
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this a requirement? Consider adding a comment or updating the error message with the reasoning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I left a comment explaining this in the proto but will add again here |
||
|
||
// leaf step takes the key and value as input | ||
res, err := p.Leaf.Apply(p.ActualPath, p.ActualValueHash) | ||
if err != nil { | ||
return nil, fmt.Errorf("leaf, %w", err) | ||
} | ||
// check if the actual value was a placeholder and replace with empty hash | ||
// assumes the placeholder value is [32]byte | ||
if bytes.Equal(p.ActualValueHash, SmtSpec.InnerSpec.EmptyChild) { | ||
res = SmtSpec.InnerSpec.EmptyChild | ||
} | ||
|
||
// the rest just take the output of the last step (reducing it) | ||
for _, step := range p.Path { | ||
res, err = step.Apply(res) | ||
if err != nil { | ||
return nil, fmt.Errorf("inner, %w", err) | ||
} | ||
} | ||
return res, nil | ||
} | ||
|
||
// Verify the ExclusionProof by checking the existence of either: | ||
// 1. An unrelated leaf where the leaf we are looking for should be | ||
// 2. A placeholder leaf where the leaf we are looking for is | ||
// | ||
// Ref: https://github.com/pokt-network/pocket/blob/main/ibc/docs/ics23.md#proof-verification | ||
func (p *ExclusionProof) Verify(spec *ProofSpec, root CommitmentRoot, key []byte) error { | ||
if err := p.CheckAgainstSpec(spec); err != nil { | ||
return err | ||
} | ||
if !bytes.Equal(keyForComparison(spec, key), keyForComparison(spec, p.Key)) { | ||
return errors.New("provided key doesn't match proof key") | ||
} | ||
calc, err := p.Calculate() | ||
if err != nil { | ||
return fmt.Errorf("error calculating root, %w", err) | ||
} | ||
if !bytes.Equal(root, calc) { | ||
return errors.New("calculcated root doesn't match provided root") | ||
} | ||
return nil | ||
} | ||
|
||
// CheckAgainstSpec will verify the leaf and all path steps are in the format defined in spec | ||
func (p *ExclusionProof) CheckAgainstSpec(spec *ProofSpec) error { | ||
if p.GetLeaf() == nil { | ||
return errors.New("exclusion Proof needs defined LeafOp") | ||
} | ||
|
||
// Custom leaf spec validation | ||
if p.Leaf.Hash != spec.LeafSpec.Hash { | ||
return fmt.Errorf("unexpected HashOp: %d", p.Leaf.Hash) | ||
} | ||
if p.Leaf.PrehashKey != HashOp_NO_HASH { | ||
return errors.New("exclusion proof must have leaf with PrehashKey == NO_HASH") | ||
} | ||
if p.Leaf.PrehashValue != HashOp_NO_HASH { | ||
return errors.New("exclusion proof must have leaf with PrehashValue == NO_HASH") | ||
} | ||
if p.Leaf.Length != spec.LeafSpec.Length { | ||
return fmt.Errorf("unexpected LengthOp: %d", p.Leaf.Length) | ||
} | ||
if !bytes.HasPrefix(p.Leaf.Prefix, spec.LeafSpec.Prefix) { | ||
return fmt.Errorf("leaf Prefix doesn't start with %X", spec.LeafSpec.Prefix) | ||
} | ||
if spec.MinDepth > 0 && len(p.Path) < int(spec.MinDepth) { | ||
return fmt.Errorf("innerOps depth too short: %d", len(p.Path)) | ||
} | ||
if spec.MaxDepth > 0 && len(p.Path) > int(spec.MaxDepth) { | ||
return fmt.Errorf("innerOps depth too long: %d", len(p.Path)) | ||
} | ||
|
||
layerNum := 1 | ||
|
||
for _, inner := range p.Path { | ||
if err := inner.CheckAgainstSpec(spec, layerNum); err != nil { | ||
return fmt.Errorf("inner, %w", err) | ||
} | ||
layerNum += 1 | ||
} | ||
return nil | ||
} | ||
|
||
// Calculate determines the root hash that matches the given nonexistence rpoog. | ||
// You must validate the result is what you have in a header. | ||
// Returns error if the calculations cannot be performed. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change was made as the current
make proto-check-breaking
doesn't work it would say something about needing 2 contianers? I will revert this if someone can say how to make it work as intended.