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.

Sign up for GitHub

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

WIP: Fix ICS23-Proofs #6178

Closed
wants to merge 9 commits into from
Closed

WIP: Fix ICS23-Proofs #6178

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
73462b2
integrate @ethanfrey work on confio/ics23 into 23-commitment
AdityaSripal May 8, 2020
59d5fd1
start potential chain-proof implementation
AdityaSripal May 18, 2020
567fc51
Update x/ibc/07-tendermint/types/client_state.go
AdityaSripal May 19, 2020
2da7b0d
Update x/ibc/07-tendermint/types/msgs.go
AdityaSripal May 19, 2020
4c38f3e
add GetProofSpecs method to localhost clientstate
AdityaSripal May 19, 2020
dba44d8
Merge branch 'aditya/ics23-proofs' of github.com:cosmos/cosmos-sdk in…
AdityaSripal May 19, 2020
d26a2d8
move specs out of proofs
AdityaSripal May 20, 2020
f605880
implement batch proofs
AdityaSripal May 20, 2020
6c66834
replace MultiStoreProofOp with SimpleValueOp
AdityaSripal May 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ require (
github.com/99designs/keyring v1.1.5
github.com/bgentry/speakeasy v0.1.0
github.com/btcsuite/btcd v0.20.1-beta
github.com/confio/ics23/go v0.0.0-20200325200809-9f53dd0c4212
github.com/cosmos/go-bip39 v0.0.0-20180819234021-555e2067c45d
github.com/cosmos/ledger-cosmos-go v0.11.1
github.com/gibson042/canonicaljson-go v1.0.3
Expand Down
3 changes: 3 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk
github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8=
github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI=
github.com/confio/ics23 v0.6.0 h1:bQsi55t2+xjW6EWDl83IBF1VWurplbUu+OT6pukeiEo=
github.com/confio/ics23/go v0.0.0-20200325200809-9f53dd0c4212 h1:MgS8JP5m7fPl7kumRm+YyAe5le3JlwQ4n5T/JXvr36s=
github.com/confio/ics23/go v0.0.0-20200325200809-9f53dd0c4212/go.mod h1:W1I3XC8d9N8OTu/ct5VJ84ylcOunZwMXsWkd27nvVts=
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
Expand Down
25 changes: 16 additions & 9 deletions x/ibc/23-commitment/types/merkle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import (

"github.com/tendermint/tendermint/crypto/merkle"

"github.com/cosmos/cosmos-sdk/store/rootmulti"
ics23 "github.com/confio/ics23/go"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/exported"
host "github.com/cosmos/cosmos-sdk/x/ibc/24-host"
)
Expand Down Expand Up @@ -145,7 +146,8 @@ var _ exported.Proof = MerkleProof{}
// verifiable in conjunction with a known commitment root. Proofs should be
// succinct.
type MerkleProof struct {
Proof *merkle.Proof `json:"proof" yaml:"proof"`
Proof *ics23.CommitmentProof `json:"proof" yaml:"proof"`
Spec *ics23.ProofSpec `json:"spec" yaml:"spec"`
}

// GetCommitmentType implements ProofI
Expand All @@ -156,26 +158,31 @@ func (MerkleProof) GetCommitmentType() exported.Type {
// VerifyMembership verifies the membership pf a merkle proof against the given root, path, and value.
func (proof MerkleProof) VerifyMembership(root exported.Root, path exported.Path, value []byte) error {
if proof.IsEmpty() || root == nil || root.IsEmpty() || path == nil || path.IsEmpty() || len(value) == 0 {
return errors.New("empty params or proof")
return sdkerrors.Wrap(ErrInvalidProof, "empty params or proof")
}

runtime := rootmulti.DefaultProofRuntime()
return runtime.VerifyValue(proof.Proof, root.GetHash(), path.String(), value)
if !ics23.VerifyMembership(proof.Spec, root.GetHash(), proof.Proof, []byte(path.String()), value) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Is the key in the confio VerifyMembership function the path? Or is it the last element of the path?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume it's the full path

return sdkerrors.Wrapf(ErrInvalidProof, "invalid proof for path: %s", path.String())
}
return nil
}

// VerifyNonMembership verifies the absence of a merkle proof against the given root and path.
func (proof MerkleProof) VerifyNonMembership(root exported.Root, path exported.Path) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently VerifyNonMembership will verify absence at the smallest subtree, and then prove inclusion of all subtree upto the root.

Thus in the multistore example, we can prove absence of a value in the ibc store, but we can't use this to prove that storeABC does not exist in multistore

That seems sufficient for all ibc usecases.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine, let's just make sure we panic if we ever try to check a proof of absence of storeABC

if proof.IsEmpty() || root == nil || root.IsEmpty() || path == nil || path.IsEmpty() {
return errors.New("empty params or proof")
return sdkerrors.Wrap(ErrInvalidProof, "empty params or proof")
}

runtime := rootmulti.DefaultProofRuntime()
return runtime.VerifyAbsence(proof.Proof, root.GetHash(), path.String())
if !ics23.VerifyNonMembership(proof.Spec, root.GetHash(), proof.Proof, []byte(path.String())) {
return sdkerrors.Wrapf(ErrInvalidProof, "invalid proof for path: %s", path.String())
}
return nil

}

// IsEmpty returns true if the root is empty
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
func (proof MerkleProof) IsEmpty() bool {
return (proof == MerkleProof{}) || proof.Proof == nil
return proof.Proof == nil || proof.Spec == nil
}

// ValidateBasic checks if the proof is empty.
Expand Down