Skip to content
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

Closed
wants to merge 12 commits into from
Closed
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
DOCKER := $(shell which docker)
HTTPS_GIT := https://github.com/cosmos/cosmos-sdk.git
HTTPS_GIT := https://github.com/cosmos/ics23.git
Copy link
Author

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.


##### GO commands #####

Expand Down
50 changes: 50 additions & 0 deletions go/compress.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ func compressEntry(entry *BatchEntry, lookup *[]*InnerOp, registry map[string]in
}
}

if exclusion := entry.GetExclusion(); exclusion != nil {
return &CompressedBatchEntry{
Proof: &CompressedBatchEntry_Exclusion{
Exclusion: compressExclusion(exclusion, lookup, registry),
},
}
}

non := entry.GetNonexist()
return &CompressedBatchEntry{
Proof: &CompressedBatchEntry_Nonexist{
Expand Down Expand Up @@ -88,6 +96,23 @@ func compressExist(exist *ExistenceProof, lookup *[]*InnerOp, registry map[strin
return res
}

func compressExclusion(exclusion *ExclusionProof, lookup *[]*InnerOp, registry map[string]int32) *CompressedExclusionProof {
if exclusion == nil {
return nil
}
res := &CompressedExclusionProof{
Key: exclusion.Key,
ActualPath: exclusion.ActualPath,
ActualValueHash: exclusion.ActualValueHash,
Leaf: exclusion.Leaf,
Path: make([]int32, len(exclusion.Path)),
}
for i, step := range exclusion.Path {
res.Path[i] = compressStep(step, lookup, registry)
}
return res
}

func compressStep(step *InnerOp, lookup *[]*InnerOp, registry map[string]int32) int32 {
bz, err := step.Marshal()
if err != nil {
Expand Down Expand Up @@ -131,6 +156,14 @@ func decompressEntry(entry *CompressedBatchEntry, lookup []*InnerOp) *BatchEntry
}
}

if exclusion := entry.GetExclusion(); exclusion != nil {
return &BatchEntry{
Proof: &BatchEntry_Exclusion{
Exclusion: decompressExclusion(exclusion, lookup),
},
}
}

non := entry.GetNonexist()
return &BatchEntry{
Proof: &BatchEntry_Nonexist{
Expand Down Expand Up @@ -158,3 +191,20 @@ func decompressExist(exist *CompressedExistenceProof, lookup []*InnerOp) *Existe
}
return res
}

func decompressExclusion(exclusion *CompressedExclusionProof, lookup []*InnerOp) *ExclusionProof {
if exclusion == nil {
return nil
}
res := &ExclusionProof{
Key: exclusion.Key,
ActualPath: exclusion.ActualPath,
ActualValueHash: exclusion.ActualValueHash,
Leaf: exclusion.Leaf,
Path: make([]*InnerOp, len(exclusion.Path)),
}
for i, step := range exclusion.Path {
res.Path[i] = lookup[step]
}
return res
}
54 changes: 51 additions & 3 deletions go/ics23.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {

Choose a reason for hiding this comment

The 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?

Copy link
Author

Choose a reason for hiding this comment

The 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:
Expand Down
96 changes: 95 additions & 1 deletion go/proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

var SmtSpec = &ProofSpec{
LeafSpec: &LeafOp{
Hash: HashOp_SHA256,
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -163,6 +168,95 @@ 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")
}
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

Choose a reason for hiding this comment

The 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

Copy link
Author

Choose a reason for hiding this comment

The 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, make([]byte, 32)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability NIT: Move make([]byte, 32) into a constant in this package or a local var and rename to smtPlaceholder

res = make([]byte, 32)
}

// 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
}

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.
Expand Down
Loading