From 3d2795cd519460903a00e1af3467ad449e06252b Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 8 Jun 2020 18:28:02 -0400 Subject: [PATCH 01/19] enforce spec ordering --- x/ibc/23-commitment/exported/exported.go | 4 +- x/ibc/23-commitment/types/merkle.go | 72 +++++++++++++++++++++++- x/ibc/23-commitment/types/merkle_test.go | 6 +- x/ibc/23-commitment/verify.go | 65 --------------------- 4 files changed, 76 insertions(+), 71 deletions(-) delete mode 100644 x/ibc/23-commitment/verify.go diff --git a/x/ibc/23-commitment/exported/exported.go b/x/ibc/23-commitment/exported/exported.go index c6fbbce3ada1..53de156c459c 100644 --- a/x/ibc/23-commitment/exported/exported.go +++ b/x/ibc/23-commitment/exported/exported.go @@ -39,8 +39,8 @@ type Path interface { // Proofs includes key but value is provided dynamically at the verification time. type Proof interface { GetCommitmentType() Type - VerifyMembership(Root, Path, []byte) error - VerifyNonMembership(Root, Path) error + VerifyMembership([]string, Root, Path, []byte) error + VerifyNonMembership([]string, Root, Path) error IsEmpty() bool ValidateBasic() error diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index 2040b1179dd1..225f2b1bbf9d 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -128,25 +128,93 @@ 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 { +func (proof MerkleProof) VerifyMembership(specs []string, root exported.Root, path exported.Path, value []byte) error { if proof.IsEmpty() || root == nil || root.IsEmpty() || path == nil || path.IsEmpty() || len(value) == 0 { return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") } + for i, op := range proof.Proof.Ops { + if op.Type != specs[i] { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "proof does not match expected format at position %d, expected: %s, got: %s", + i, specs[i], op.Type) + } + } + runtime := rootmulti.DefaultProofRuntime() return runtime.VerifyValue(proof.Proof, root.GetHash(), path.String(), value) } // 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 { +func (proof MerkleProof) VerifyNonMembership(specs []string, root exported.Root, path exported.Path) error { if proof.IsEmpty() || root == nil || root.IsEmpty() || path == nil || path.IsEmpty() { return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") } + for i, op := range proof.Proof.Ops { + if op.Type != specs[i] { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "proof does not match expected format at position %d, expected: %s, got: %s", + i, specs[i], op.Type) + } + } + runtime := rootmulti.DefaultProofRuntime() return runtime.VerifyAbsence(proof.Proof, root.GetHash(), path.String()) } +// BatchVerifyMembership verifies a group of key value pairs against the given root +// NOTE: Untested +func (proof MerkleProof) BatchVerifyMembership(specs []string, root exported.Root, items map[string][]byte) error { + if proof.IsEmpty() || root == nil || root.IsEmpty() { + return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") + } + + for i, op := range proof.Proof.Ops { + if op.Type != specs[i] { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "proof does not match expected format at position %d, expected: %s, got: %s", + i, specs[i], op.Type) + } + } + + // Verify each item seperately against same proof + runtime := rootmulti.DefaultProofRuntime() + for path, value := range items { + if err := runtime.VerifyValue(proof.Proof, root.GetHash(), path, value); err != nil { + return sdkerrors.Wrapf(ErrInvalidProof, "verification failed for path: %s, value: %x. Error: %v", + path, value, err) + } + } + return nil +} + +// BatchVerifyNonMembership verifies absence of a group of keys against the given root +// NOTE: Untested +func (proof MerkleProof) BatchVerifyNonMembership(specs []string, root exported.Root, items []string) error { + if proof.IsEmpty() || root == nil || root.IsEmpty() { + return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") + } + + for i, op := range proof.Proof.Ops { + if op.Type != specs[i] { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "proof does not match expected format at position %d, expected: %s, got: %s", + i, specs[i], op.Type) + } + } + + // Verify each item seperately against same proof + runtime := rootmulti.DefaultProofRuntime() + for _, path := range items { + if err := runtime.VerifyAbsence(proof.Proof, root.GetHash(), path); err != nil { + return sdkerrors.Wrapf(ErrInvalidProof, "absence verification failed for path: %s. Error: %v", + path, err) + } + } + return nil +} + // IsEmpty returns true if the root is empty func (proof MerkleProof) IsEmpty() bool { return proof.Proof.Equal(nil) || proof.Equal(MerkleProof{}) || proof.Proof.Equal(nil) || proof.Proof.Equal(merkle.Proof{}) diff --git a/x/ibc/23-commitment/types/merkle_test.go b/x/ibc/23-commitment/types/merkle_test.go index 50685c2a8efb..6c8d457238f9 100644 --- a/x/ibc/23-commitment/types/merkle_test.go +++ b/x/ibc/23-commitment/types/merkle_test.go @@ -11,6 +11,8 @@ import ( abci "github.com/tendermint/tendermint/abci/types" ) +var specs = []string{"ics23:iavl", "simple:v"} + func (suite *MerkleTestSuite) TestVerifyMembership() { suite.iavlStore.Set([]byte("MYKEY"), []byte("MYVALUE")) cid := suite.store.Commit() @@ -53,7 +55,7 @@ func (suite *MerkleTestSuite) TestVerifyMembership() { root := types.NewMerkleRoot(tc.root) path := types.NewMerklePath(tc.pathArr) - err := proof.VerifyMembership(&root, path, tc.value) + err := proof.VerifyMembership(specs, &root, path, tc.value) if tc.shouldPass { // nolint: scopelint @@ -108,7 +110,7 @@ func (suite *MerkleTestSuite) TestVerifyNonMembership() { root := types.NewMerkleRoot(tc.root) path := types.NewMerklePath(tc.pathArr) - err := proof.VerifyNonMembership(&root, path) + err := proof.VerifyNonMembership(specs, &root, path) if tc.shouldPass { // nolint: scopelint diff --git a/x/ibc/23-commitment/verify.go b/x/ibc/23-commitment/verify.go deleted file mode 100644 index a18cac3a85c4..000000000000 --- a/x/ibc/23-commitment/verify.go +++ /dev/null @@ -1,65 +0,0 @@ -package commitment - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/exported" - "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types" -) - -// CalculateRoot returns the application Hash at the curretn block height as a commitment -// root for proof verification. -func CalculateRoot(ctx sdk.Context) exported.Root { - root := types.NewMerkleRoot(ctx.BlockHeader().AppHash) - return &root -} - -// BatchVerifyMembership verifies a proof that many paths have been set to -// specific values in a commitment. It calls the proof's VerifyMembership method -// with the calculated root and the provided paths. -// Returns false on the first failed membership verification. -func BatchVerifyMembership( - ctx sdk.Context, - proof exported.Proof, - prefix exported.Prefix, - items map[string][]byte, -) error { - root := CalculateRoot(ctx) - - for pathStr, value := range items { - path, err := types.ApplyPrefix(prefix, pathStr) - if err != nil { - return err - } - - if err := proof.VerifyMembership(root, path, value); err != nil { - return err - } - } - - return nil -} - -// BatchVerifyNonMembership verifies a proof that many paths have not been set -// to any value in a commitment. It calls the proof's VerifyNonMembership method -// with the calculated root and the provided paths. -// Returns false on the first failed non-membership verification. -func BatchVerifyNonMembership( - ctx sdk.Context, - proof exported.Proof, - prefix exported.Prefix, - paths []string, -) error { - root := CalculateRoot(ctx) - for _, pathStr := range paths { - path, err := types.ApplyPrefix(prefix, pathStr) - if err != nil { - return err - } - - if err := proof.VerifyNonMembership(root, path); err != nil { - return err - } - } - - return nil -} From a716e2025bd3f504779807ad762af04e4028eac1 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 8 Jun 2020 21:25:15 -0400 Subject: [PATCH 02/19] modify clients to pass in specs to verify functions --- x/ibc/02-client/exported/exported.go | 1 + x/ibc/07-tendermint/types/client_state.go | 31 +++++++++++++++-------- x/ibc/07-tendermint/types/msgs.go | 1 + x/ibc/09-localhost/types/client_state.go | 5 ++++ 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/x/ibc/02-client/exported/exported.go b/x/ibc/02-client/exported/exported.go index 304a62430c8d..fcac8adfa133 100644 --- a/x/ibc/02-client/exported/exported.go +++ b/x/ibc/02-client/exported/exported.go @@ -21,6 +21,7 @@ type ClientState interface { GetLatestHeight() uint64 IsFrozen() bool Validate() error + GetProofSpecs() []string // State verification functions diff --git a/x/ibc/07-tendermint/types/client_state.go b/x/ibc/07-tendermint/types/client_state.go index dec469d5a5d4..54b6a0b2488a 100644 --- a/x/ibc/07-tendermint/types/client_state.go +++ b/x/ibc/07-tendermint/types/client_state.go @@ -47,6 +47,8 @@ type ClientState struct { // Last Header that was stored by client LastHeader Header `json:"last_header" yaml:"last_header"` + + ProofSpecs []string `json:"proof_specs" yaml:"proof_specs"` } // InitializeFromMsg creates a tendermint client state from a CreateClientMsg @@ -54,7 +56,7 @@ func InitializeFromMsg(msg MsgCreateClient) (ClientState, error) { return Initialize( msg.GetClientID(), msg.TrustLevel, msg.TrustingPeriod, msg.UnbondingPeriod, msg.MaxClockDrift, - msg.Header, + msg.Header, msg.ProofSpecs, ) } @@ -63,7 +65,7 @@ func InitializeFromMsg(msg MsgCreateClient) (ClientState, error) { func Initialize( id string, trustLevel tmmath.Fraction, trustingPeriod, ubdPeriod, maxClockDrift time.Duration, - header Header, + header Header, specs []string, ) (ClientState, error) { if trustingPeriod >= ubdPeriod { @@ -73,7 +75,7 @@ func Initialize( ) } - clientState := NewClientState(id, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header) + clientState := NewClientState(id, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs) return clientState, nil } @@ -81,7 +83,7 @@ func Initialize( func NewClientState( id string, trustLevel tmmath.Fraction, trustingPeriod, ubdPeriod, maxClockDrift time.Duration, - header Header, + header Header, specs []string, ) ClientState { return ClientState{ ID: id, @@ -91,6 +93,7 @@ func NewClientState( MaxClockDrift: maxClockDrift, LastHeader: header, FrozenHeight: 0, + ProofSpecs: specs, } } @@ -147,6 +150,12 @@ func (cs ClientState) Validate() error { return cs.LastHeader.ValidateBasic(cs.GetChainID()) } +// GetProofSpecs returns the format the client expects for proof verification +// as a string array specifying the proof type for each position in chained proof +func (cs ClientState) GetProofSpecs() []string { + return cs.ProofSpecs +} + // VerifyClientConsensusState verifies a proof of the consensus state of the // Tendermint client stored on the target machine. func (cs ClientState) VerifyClientConsensusState( @@ -175,7 +184,7 @@ func (cs ClientState) VerifyClientConsensusState( return err } - if err := proof.VerifyMembership(provingRoot, path, bz); err != nil { + if err := proof.VerifyMembership(cs.ProofSpecs, provingRoot, path, bz); err != nil { return sdkerrors.Wrap(clienttypes.ErrFailedClientConsensusStateVerification, err.Error()) } @@ -213,7 +222,7 @@ func (cs ClientState) VerifyConnectionState( return err } - if err := proof.VerifyMembership(consensusState.GetRoot(), path, bz); err != nil { + if err := proof.VerifyMembership(cs.ProofSpecs, consensusState.GetRoot(), path, bz); err != nil { return sdkerrors.Wrap(clienttypes.ErrFailedConnectionStateVerification, err.Error()) } @@ -252,7 +261,7 @@ func (cs ClientState) VerifyChannelState( return err } - if err := proof.VerifyMembership(consensusState.GetRoot(), path, bz); err != nil { + if err := proof.VerifyMembership(cs.ProofSpecs, consensusState.GetRoot(), path, bz); err != nil { return sdkerrors.Wrap(clienttypes.ErrFailedChannelStateVerification, err.Error()) } @@ -281,7 +290,7 @@ func (cs ClientState) VerifyPacketCommitment( return err } - if err := proof.VerifyMembership(consensusState.GetRoot(), path, commitmentBytes); err != nil { + if err := proof.VerifyMembership(cs.ProofSpecs, consensusState.GetRoot(), path, commitmentBytes); err != nil { return sdkerrors.Wrap(clienttypes.ErrFailedPacketCommitmentVerification, err.Error()) } @@ -310,7 +319,7 @@ func (cs ClientState) VerifyPacketAcknowledgement( return err } - if err := proof.VerifyMembership(consensusState.GetRoot(), path, channeltypes.CommitAcknowledgement(acknowledgement)); err != nil { + if err := proof.VerifyMembership(cs.ProofSpecs, consensusState.GetRoot(), path, channeltypes.CommitAcknowledgement(acknowledgement)); err != nil { return sdkerrors.Wrap(clienttypes.ErrFailedPacketAckVerification, err.Error()) } @@ -339,7 +348,7 @@ func (cs ClientState) VerifyPacketAcknowledgementAbsence( return err } - if err := proof.VerifyNonMembership(consensusState.GetRoot(), path); err != nil { + if err := proof.VerifyNonMembership(cs.ProofSpecs, consensusState.GetRoot(), path); err != nil { return sdkerrors.Wrap(clienttypes.ErrFailedPacketAckAbsenceVerification, err.Error()) } @@ -369,7 +378,7 @@ func (cs ClientState) VerifyNextSequenceRecv( bz := sdk.Uint64ToBigEndian(nextSequenceRecv) - if err := proof.VerifyMembership(consensusState.GetRoot(), path, bz); err != nil { + if err := proof.VerifyMembership(cs.ProofSpecs, consensusState.GetRoot(), path, bz); err != nil { return sdkerrors.Wrap(clienttypes.ErrFailedNextSeqRecvVerification, err.Error()) } diff --git a/x/ibc/07-tendermint/types/msgs.go b/x/ibc/07-tendermint/types/msgs.go index 67f4a6aab86b..fae964c13e03 100644 --- a/x/ibc/07-tendermint/types/msgs.go +++ b/x/ibc/07-tendermint/types/msgs.go @@ -36,6 +36,7 @@ type MsgCreateClient struct { TrustingPeriod time.Duration `json:"trusting_period" yaml:"trusting_period"` UnbondingPeriod time.Duration `json:"unbonding_period" yaml:"unbonding_period"` MaxClockDrift time.Duration `json:"max_clock_drift" yaml:"max_clock_drift"` + ProofSpecs []string `json:"proof_specs" yaml:"proof_specs"` Signer sdk.AccAddress `json:"address" yaml:"address"` } diff --git a/x/ibc/09-localhost/types/client_state.go b/x/ibc/09-localhost/types/client_state.go index ab613be1dd25..54fb4c86ad86 100644 --- a/x/ibc/09-localhost/types/client_state.go +++ b/x/ibc/09-localhost/types/client_state.go @@ -74,6 +74,11 @@ func (cs ClientState) Validate() error { return host.ClientIdentifierValidator(cs.ID) } +// GetProofSpecs returns nil since localhost does not have to verify proofs +func (cs ClientState) GetProofSpecs() []string { + return nil +} + // VerifyClientConsensusState verifies a proof of the consensus // state of the loop-back client. // VerifyClientConsensusState verifies a proof of the consensus state of the From 3a5bc6a3ef05d60f81cd1bd8cdecb71215f05607 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 8 Jun 2020 22:33:31 -0400 Subject: [PATCH 03/19] start fixing tests --- x/ibc/02-client/types/genesis_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/x/ibc/02-client/types/genesis_test.go b/x/ibc/02-client/types/genesis_test.go index 4d494b199833..b342f9eba9b9 100644 --- a/x/ibc/02-client/types/genesis_test.go +++ b/x/ibc/02-client/types/genesis_test.go @@ -6,9 +6,11 @@ import ( "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto/merkle" lite "github.com/tendermint/tendermint/lite2" tmtypes "github.com/tendermint/tendermint/types" + storetypes "github.com/cosmos/cosmos-sdk/store/types" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types" ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types" @@ -24,6 +26,8 @@ const ( maxClockDrift time.Duration = time.Second * 10 ) +var specs = []string{storetypes.ProofOpIAVLCommitment, merkle.ProofOpSimpleValue} + func TestValidateGenesis(t *testing.T) { privVal := tmtypes.NewMockPV() pubKey, err := privVal.GetPubKey() @@ -50,7 +54,7 @@ func TestValidateGenesis(t *testing.T) { name: "valid genesis", genState: types.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs), localhosttypes.NewClientState("chaindID", 10), }, []types.ClientConsensusStates{ @@ -71,7 +75,7 @@ func TestValidateGenesis(t *testing.T) { name: "invalid client", genState: types.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs), localhosttypes.NewClientState("chaindID", 0), }, nil, @@ -83,7 +87,7 @@ func TestValidateGenesis(t *testing.T) { name: "invalid consensus state", genState: types.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs), localhosttypes.NewClientState("chaindID", 10), }, []types.ClientConsensusStates{ @@ -104,7 +108,7 @@ func TestValidateGenesis(t *testing.T) { name: "invalid consensus state", genState: types.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs), localhosttypes.NewClientState("chaindID", 10), }, []types.ClientConsensusStates{ From 1723648a588da195a8a8305b209dcfe50ece4efe Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Tue, 9 Jun 2020 12:09:54 +0200 Subject: [PATCH 04/19] Apply suggestions from code review --- x/ibc/23-commitment/types/merkle.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index 225f2b1bbf9d..d2c9d3a1d072 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -178,7 +178,7 @@ func (proof MerkleProof) BatchVerifyMembership(specs []string, root exported.Roo } } - // Verify each item seperately against same proof + // Verify each item separately against same proof runtime := rootmulti.DefaultProofRuntime() for path, value := range items { if err := runtime.VerifyValue(proof.Proof, root.GetHash(), path, value); err != nil { @@ -204,7 +204,7 @@ func (proof MerkleProof) BatchVerifyNonMembership(specs []string, root exported. } } - // Verify each item seperately against same proof + // Verify each item separately against same proof runtime := rootmulti.DefaultProofRuntime() for _, path := range items { if err := runtime.VerifyAbsence(proof.Proof, root.GetHash(), path); err != nil { From 74dd9042178672ea064b7857578217ec5edec802 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 9 Jun 2020 13:22:45 -0400 Subject: [PATCH 05/19] enforce spec length and proof length match --- x/ibc/23-commitment/types/merkle.go | 24 ++++++++++ x/ibc/23-commitment/types/merkle_test.go | 61 ++++++++++++++++-------- 2 files changed, 66 insertions(+), 19 deletions(-) diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index d2c9d3a1d072..a7b23d0c635d 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -133,6 +133,12 @@ func (proof MerkleProof) VerifyMembership(specs []string, root exported.Root, pa return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") } + if len(specs) != len(proof.Proof.Ops) { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "length of specs: %d not equal to length of proof: %d", + len(specs), len(proof.Proof.Ops)) + } + for i, op := range proof.Proof.Ops { if op.Type != specs[i] { return sdkerrors.Wrapf(ErrInvalidMerkleProof, @@ -151,6 +157,12 @@ func (proof MerkleProof) VerifyNonMembership(specs []string, root exported.Root, return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") } + if len(specs) != len(proof.Proof.Ops) { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "length of specs: %d not equal to length of proof: %d", + len(specs), len(proof.Proof.Ops)) + } + for i, op := range proof.Proof.Ops { if op.Type != specs[i] { return sdkerrors.Wrapf(ErrInvalidMerkleProof, @@ -170,6 +182,12 @@ func (proof MerkleProof) BatchVerifyMembership(specs []string, root exported.Roo return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") } + if len(specs) != len(proof.Proof.Ops) { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "length of specs: %d not equal to length of proof: %d", + len(specs), len(proof.Proof.Ops)) + } + for i, op := range proof.Proof.Ops { if op.Type != specs[i] { return sdkerrors.Wrapf(ErrInvalidMerkleProof, @@ -196,6 +214,12 @@ func (proof MerkleProof) BatchVerifyNonMembership(specs []string, root exported. return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") } + if len(specs) != len(proof.Proof.Ops) { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "length of specs: %d not equal to length of proof: %d", + len(specs), len(proof.Proof.Ops)) + } + for i, op := range proof.Proof.Ops { if op.Type != specs[i] { return sdkerrors.Wrapf(ErrInvalidMerkleProof, diff --git a/x/ibc/23-commitment/types/merkle_test.go b/x/ibc/23-commitment/types/merkle_test.go index 6c8d457238f9..688100e18c40 100644 --- a/x/ibc/23-commitment/types/merkle_test.go +++ b/x/ibc/23-commitment/types/merkle_test.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/crypto/merkle" ) var specs = []string{"ics23:iavl", "simple:v"} @@ -35,23 +36,34 @@ func (suite *MerkleTestSuite) TestVerifyMembership() { root []byte pathArr []string value []byte + malleate func() shouldPass bool }{ - {"valid proof", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, []byte("MYVALUE"), true}, // valid proof - {"wrong value", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, []byte("WRONGVALUE"), false}, // invalid proof with wrong value - {"nil value", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, []byte(nil), false}, // invalid proof with nil value - {"wrong key", cid.Hash, []string{suite.storeKey.Name(), "NOTMYKEY"}, []byte("MYVALUE"), false}, // invalid proof with wrong key - {"wrong path 1", cid.Hash, []string{suite.storeKey.Name(), "MYKEY", "MYKEY"}, []byte("MYVALUE"), false}, // invalid proof with wrong path - {"wrong path 2", cid.Hash, []string{suite.storeKey.Name()}, []byte("MYVALUE"), false}, // invalid proof with wrong path - {"wrong path 3", cid.Hash, []string{"MYKEY"}, []byte("MYVALUE"), false}, // invalid proof with wrong path - {"wrong storekey", cid.Hash, []string{"otherStoreKey", "MYKEY"}, []byte("MYVALUE"), false}, // invalid proof with wrong store prefix - {"wrong root", []byte("WRONGROOT"), []string{suite.storeKey.Name(), "MYKEY"}, []byte("MYVALUE"), false}, // invalid proof with wrong root - {"nil root", []byte(nil), []string{suite.storeKey.Name(), "MYKEY"}, []byte("MYVALUE"), false}, // invalid proof with nil root + {"valid proof", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, []byte("MYVALUE"), func() {}, true}, // valid proof + {"wrong value", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, []byte("WRONGVALUE"), func() {}, false}, // invalid proof with wrong value + {"nil value", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, []byte(nil), func() {}, false}, // invalid proof with nil value + {"wrong key", cid.Hash, []string{suite.storeKey.Name(), "NOTMYKEY"}, []byte("MYVALUE"), func() {}, false}, // invalid proof with wrong key + {"wrong path 1", cid.Hash, []string{suite.storeKey.Name(), "MYKEY", "MYKEY"}, []byte("MYVALUE"), func() {}, false}, // invalid proof with wrong path + {"wrong path 2", cid.Hash, []string{suite.storeKey.Name()}, []byte("MYVALUE"), func() {}, false}, // invalid proof with wrong path + {"wrong path 3", cid.Hash, []string{"MYKEY"}, []byte("MYVALUE"), func() {}, false}, // invalid proof with wrong path + {"wrong storekey", cid.Hash, []string{"otherStoreKey", "MYKEY"}, []byte("MYVALUE"), func() {}, false}, // invalid proof with wrong store prefix + {"wrong root", []byte("WRONGROOT"), []string{suite.storeKey.Name(), "MYKEY"}, []byte("MYVALUE"), func() {}, false}, // invalid proof with wrong root + {"nil root", []byte(nil), []string{suite.storeKey.Name(), "MYKEY"}, []byte("MYVALUE"), func() {}, false}, // invalid proof with nil root + {"proof is wrong length", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, []byte("MYVALUE"), func() { + proof = types.MerkleProof{ + Proof: &merkle.Proof{ + Ops: res.Proof.Ops[1:], + }, + } + }, false}, // invalid proof with wrong length + } for i, tc := range cases { tc := tc suite.Run(tc.name, func() { + tc.malleate() + root := types.NewMerkleRoot(tc.root) path := types.NewMerklePath(tc.pathArr) @@ -90,23 +102,34 @@ func (suite *MerkleTestSuite) TestVerifyNonMembership() { name string root []byte pathArr []string + malleate func() shouldPass bool }{ - {"valid proof", cid.Hash, []string{suite.storeKey.Name(), "MYABSENTKEY"}, true}, // valid proof - {"wrong key", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, false}, // invalid proof with existent key - {"wrong path 1", cid.Hash, []string{suite.storeKey.Name(), "MYKEY", "MYABSENTKEY"}, false}, // invalid proof with wrong path - {"wrong path 2", cid.Hash, []string{suite.storeKey.Name(), "MYABSENTKEY", "MYKEY"}, false}, // invalid proof with wrong path - {"wrong path 3", cid.Hash, []string{suite.storeKey.Name()}, false}, // invalid proof with wrong path - {"wrong path 4", cid.Hash, []string{"MYABSENTKEY"}, false}, // invalid proof with wrong path - {"wrong storeKey", cid.Hash, []string{"otherStoreKey", "MYABSENTKEY"}, false}, // invalid proof with wrong store prefix - {"wrong root", []byte("WRONGROOT"), []string{suite.storeKey.Name(), "MYABSENTKEY"}, false}, // invalid proof with wrong root - {"nil root", []byte(nil), []string{suite.storeKey.Name(), "MYABSENTKEY"}, false}, // invalid proof with nil root + {"valid proof", cid.Hash, []string{suite.storeKey.Name(), "MYABSENTKEY"}, func() {}, true}, // valid proof + {"wrong key", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, func() {}, false}, // invalid proof with existent key + {"wrong path 1", cid.Hash, []string{suite.storeKey.Name(), "MYKEY", "MYABSENTKEY"}, func() {}, false}, // invalid proof with wrong path + {"wrong path 2", cid.Hash, []string{suite.storeKey.Name(), "MYABSENTKEY", "MYKEY"}, func() {}, false}, // invalid proof with wrong path + {"wrong path 3", cid.Hash, []string{suite.storeKey.Name()}, func() {}, false}, // invalid proof with wrong path + {"wrong path 4", cid.Hash, []string{"MYABSENTKEY"}, func() {}, false}, // invalid proof with wrong path + {"wrong storeKey", cid.Hash, []string{"otherStoreKey", "MYABSENTKEY"}, func() {}, false}, // invalid proof with wrong store prefix + {"wrong root", []byte("WRONGROOT"), []string{suite.storeKey.Name(), "MYABSENTKEY"}, func() {}, false}, // invalid proof with wrong root + {"nil root", []byte(nil), []string{suite.storeKey.Name(), "MYABSENTKEY"}, func() {}, false}, // invalid proof with nil root + {"proof is wrong length", cid.Hash, []string{suite.storeKey.Name(), "MYKEY"}, func() { + proof = types.MerkleProof{ + Proof: &merkle.Proof{ + Ops: res.Proof.Ops[1:], + }, + } + }, false}, // invalid proof with wrong length + } for i, tc := range cases { tc := tc suite.Run(tc.name, func() { + tc.malleate() + root := types.NewMerkleRoot(tc.root) path := types.NewMerklePath(tc.pathArr) From e2f35370426e2910bd674e90cec48b6b85744937 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 9 Jun 2020 17:21:56 -0400 Subject: [PATCH 06/19] fix all tests --- x/ibc-transfer/handler_test.go | 4 +- x/ibc-transfer/keeper/keeper_test.go | 4 +- x/ibc/02-client/keeper/client_test.go | 14 ++-- x/ibc/02-client/keeper/keeper_test.go | 10 +-- x/ibc/03-connection/keeper/keeper_test.go | 10 +-- x/ibc/04-channel/keeper/keeper_test.go | 4 +- x/ibc/07-tendermint/misbehaviour_test.go | 12 ++-- .../07-tendermint/types/client_state_test.go | 70 +++++++++---------- x/ibc/07-tendermint/update_test.go | 18 ++--- x/ibc/23-commitment/types/merkle.go | 9 +++ x/ibc/23-commitment/types/merkle_test.go | 6 +- x/ibc/ante/ante_test.go | 4 +- x/ibc/genesis_test.go | 4 +- 13 files changed, 88 insertions(+), 81 deletions(-) diff --git a/x/ibc-transfer/handler_test.go b/x/ibc-transfer/handler_test.go index f2f50c4cdcc2..7eafc98977ab 100644 --- a/x/ibc-transfer/handler_test.go +++ b/x/ibc-transfer/handler_test.go @@ -190,7 +190,7 @@ func (chain *TestChain) CreateClient(client *TestChain) error { ctxTarget := chain.GetContext() // create client - clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header) + clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } @@ -259,7 +259,7 @@ func (chain *TestChain) updateClient(client *TestChain) { ctxTarget, client.ClientID, uint64(client.Header.SignedHeader.Header.Height), consensusState, ) chain.App.IBCKeeper.ClientKeeper.SetClientState( - ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header), + ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()), ) // _, _, err := simapp.SignCheckDeliver( diff --git a/x/ibc-transfer/keeper/keeper_test.go b/x/ibc-transfer/keeper/keeper_test.go index d8782e1565f8..558e5a93a06d 100644 --- a/x/ibc-transfer/keeper/keeper_test.go +++ b/x/ibc-transfer/keeper/keeper_test.go @@ -165,7 +165,7 @@ func (chain *TestChain) CreateClient(client *TestChain) error { ctxTarget := chain.GetContext() // create client - clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header) + clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } @@ -234,7 +234,7 @@ func (chain *TestChain) updateClient(client *TestChain) { ctxTarget, client.ClientID, client.Header.GetHeight(), consensusState, ) chain.App.IBCKeeper.ClientKeeper.SetClientState( - ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header), + ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()), ) // _, _, err := simapp.SignCheckDeliver( diff --git a/x/ibc/02-client/keeper/client_test.go b/x/ibc/02-client/keeper/client_test.go index f20447cbd077..d343be644607 100644 --- a/x/ibc/02-client/keeper/client_test.go +++ b/x/ibc/02-client/keeper/client_test.go @@ -38,12 +38,12 @@ func (suite *KeeperTestSuite) TestCreateClient() { i := i if tc.expPanic { suite.Require().Panics(func() { - clientState, err := ibctmtypes.Initialize(tc.clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState, err := ibctmtypes.Initialize(tc.clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) suite.Require().NoError(err, "err on client state initialization") suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) }, "Msg %d didn't panic: %s", i, tc.msg) } else { - clientState, err := ibctmtypes.Initialize(tc.clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState, err := ibctmtypes.Initialize(tc.clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) if tc.expPass { suite.Require().NoError(err, "errored on initialization") suite.Require().NotNil(clientState, "valid test case %d failed: %s", i, tc.msg) @@ -80,7 +80,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { expPass bool }{ {"valid update", func() error { - clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } @@ -114,7 +114,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { return nil }, false}, {"invalid header", func() error { - clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } @@ -229,7 +229,7 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { }, func() error { suite.consensusState.ValidatorSet = bothValSet - clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } @@ -249,7 +249,7 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { }, func() error { suite.consensusState.ValidatorSet = bothValSet - clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } @@ -304,7 +304,7 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { ClientID: testClientID, }, func() error { - clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } diff --git a/x/ibc/02-client/keeper/keeper_test.go b/x/ibc/02-client/keeper/keeper_test.go index a959c7df3ab6..eda23a26f0ac 100644 --- a/x/ibc/02-client/keeper/keeper_test.go +++ b/x/ibc/02-client/keeper/keeper_test.go @@ -90,7 +90,7 @@ func TestKeeperTestSuite(t *testing.T) { } func (suite *KeeperTestSuite) TestSetClientState() { - clientState := ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}) + clientState := ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}, commitmenttypes.GetSDKSpecs()) suite.keeper.SetClientState(suite.ctx, clientState) retrievedState, found := suite.keeper.GetClientState(suite.ctx, testClientID) @@ -121,9 +121,9 @@ func (suite *KeeperTestSuite) TestSetClientConsensusState() { func (suite KeeperTestSuite) TestGetAllClients() { expClients := []exported.ClientState{ - ibctmtypes.NewClientState(testClientID2, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}), - ibctmtypes.NewClientState(testClientID3, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}), - ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}), + ibctmtypes.NewClientState(testClientID2, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}, commitmenttypes.GetSDKSpecs()), + ibctmtypes.NewClientState(testClientID3, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}, commitmenttypes.GetSDKSpecs()), + ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}, commitmenttypes.GetSDKSpecs()), } for i := range expClients { @@ -168,7 +168,7 @@ func (suite KeeperTestSuite) TestGetConsensusState() { func (suite KeeperTestSuite) TestConsensusStateHelpers() { // initial setup - clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState, err := ibctmtypes.Initialize(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) suite.Require().NoError(err) suite.keeper.SetClientState(suite.ctx, clientState) diff --git a/x/ibc/03-connection/keeper/keeper_test.go b/x/ibc/03-connection/keeper/keeper_test.go index 18284b929d8d..6af60c47b733 100644 --- a/x/ibc/03-connection/keeper/keeper_test.go +++ b/x/ibc/03-connection/keeper/keeper_test.go @@ -130,9 +130,9 @@ func (suite KeeperTestSuite) TestGetAllConnections() { func (suite KeeperTestSuite) TestGetAllClientConnectionPaths() { clients := []clientexported.ClientState{ - ibctmtypes.NewClientState(testClientIDA, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}), - ibctmtypes.NewClientState(testClientIDB, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}), - ibctmtypes.NewClientState(testClientID3, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}), + ibctmtypes.NewClientState(testClientIDA, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}, commitmenttypes.GetSDKSpecs()), + ibctmtypes.NewClientState(testClientIDB, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}, commitmenttypes.GetSDKSpecs()), + ibctmtypes.NewClientState(testClientID3, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}, commitmenttypes.GetSDKSpecs()), } for i := range clients { @@ -262,7 +262,7 @@ func (chain *TestChain) CreateClient(client *TestChain) error { ctxTarget := chain.GetContext() // create client - clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header) + clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } @@ -338,7 +338,7 @@ func (chain *TestChain) updateClient(client *TestChain) { ctxTarget, client.ClientID, client.Header.GetHeight(), consensusState, ) chain.App.IBCKeeper.ClientKeeper.SetClientState( - ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header), + ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()), ) // _, _, err := simapp.SignCheckDeliver( diff --git a/x/ibc/04-channel/keeper/keeper_test.go b/x/ibc/04-channel/keeper/keeper_test.go index 68845950068b..b41e7957ce1f 100644 --- a/x/ibc/04-channel/keeper/keeper_test.go +++ b/x/ibc/04-channel/keeper/keeper_test.go @@ -379,7 +379,7 @@ func (chain *TestChain) CreateClient(client *TestChain) error { ctxTarget := chain.GetContext() // create client - clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header) + clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } @@ -447,7 +447,7 @@ func (chain *TestChain) updateClient(client *TestChain) { ctxTarget, client.ClientID, client.Header.GetHeight(), consensusState, ) chain.App.IBCKeeper.ClientKeeper.SetClientState( - ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header), + ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()), ) // _, _, err := simapp.SignCheckDeliver( diff --git a/x/ibc/07-tendermint/misbehaviour_test.go b/x/ibc/07-tendermint/misbehaviour_test.go index d2237f2660ef..95e39b74648a 100644 --- a/x/ibc/07-tendermint/misbehaviour_test.go +++ b/x/ibc/07-tendermint/misbehaviour_test.go @@ -49,7 +49,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { }{ { "valid misbehavior evidence", - ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), ibctmtypes.ConsensusState{Timestamp: suite.now, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, ibctmtypes.Evidence{ Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, bothValSet, bothSigners), @@ -62,7 +62,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { }, { "valid misbehavior at height greater than last consensusState", - ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), ibctmtypes.ConsensusState{Timestamp: suite.now, Height: height - 1, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, ibctmtypes.Evidence{ Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, bothValSet, bothSigners), @@ -75,7 +75,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { }, { "consensus state's valset hash different from evidence should still pass", - ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), ibctmtypes.ConsensusState{Timestamp: suite.now, Height: height - 1, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: suite.valSet}, ibctmtypes.Evidence{ Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, bothValSet, bothSigners), @@ -88,7 +88,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { }, { "first valset has too much change", - ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), ibctmtypes.ConsensusState{Timestamp: suite.now, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, ibctmtypes.Evidence{ Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, altValSet, altSigners), @@ -101,7 +101,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { }, { "second valset has too much change", - ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), ibctmtypes.ConsensusState{Timestamp: suite.now, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, ibctmtypes.Evidence{ Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, bothValSet, bothSigners), @@ -114,7 +114,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { }, { "both valsets have too much change", - ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), ibctmtypes.ConsensusState{Timestamp: suite.now, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, ibctmtypes.Evidence{ Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, altValSet, altSigners), diff --git a/x/ibc/07-tendermint/types/client_state_test.go b/x/ibc/07-tendermint/types/client_state_test.go index bb6f284c04c6..0ddc11be1dbf 100644 --- a/x/ibc/07-tendermint/types/client_state_test.go +++ b/x/ibc/07-tendermint/types/client_state_test.go @@ -27,37 +27,37 @@ func (suite *TendermintTestSuite) TestValidate() { }{ { name: "valid client", - clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), expPass: true, }, { name: "invalid client id", - clientState: ibctmtypes.NewClientState("(testClientID)", lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState("(testClientID)", lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), expPass: false, }, { name: "invalid trust level", - clientState: ibctmtypes.NewClientState(testClientID, tmmath.Fraction{Numerator: 0, Denominator: 1}, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(testClientID, tmmath.Fraction{Numerator: 0, Denominator: 1}, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), expPass: false, }, { name: "invalid trusting period", - clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, 0, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, 0, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), expPass: false, }, { name: "invalid unbonding period", - clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, 0, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, 0, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), expPass: false, }, { name: "invalid max clock drift", - clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, 0, suite.header), + clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, 0, suite.header, commitmenttypes.GetSDKSpecs()), expPass: false, }, { name: "invalid header", - clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}), + clientState: ibctmtypes.NewClientState(testClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, ibctmtypes.Header{}, commitmenttypes.GetSDKSpecs()), expPass: false, }, } @@ -84,7 +84,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() { // FIXME: uncomment // { // name: "successful verification", - // clientState: ibctmtypes.NewClientState(chainID, chainID, height), + // clientState: ibctmtypes.NewClientState(chainID, chainID, height, commitmenttypes.GetSDKSpecs()), // consensusState: ibctmtypes.ConsensusState{ // Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), // }, @@ -93,7 +93,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() { // }, { name: "ApplyPrefix failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), }, @@ -102,7 +102,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() { }, { name: "latest client height < height", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), }, @@ -120,7 +120,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() { }, { name: "proof verification failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), ValidatorSet: suite.valSet, @@ -162,7 +162,7 @@ func (suite *TendermintTestSuite) TestVerifyConnectionState() { // FIXME: uncomment // { // name: "successful verification", - // clientState: ibctmtypes.NewClientState(chainID, chainID, height), + // clientState: ibctmtypes.NewClientState(chainID, chainID, height, commitmenttypes.GetSDKSpecs()), // connection: conn, // consensusState: ibctmtypes.ConsensusState{ // Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -172,7 +172,7 @@ func (suite *TendermintTestSuite) TestVerifyConnectionState() { // }, { name: "ApplyPrefix failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), connection: conn, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -182,7 +182,7 @@ func (suite *TendermintTestSuite) TestVerifyConnectionState() { }, { name: "latest client height < height", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), connection: conn, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -202,7 +202,7 @@ func (suite *TendermintTestSuite) TestVerifyConnectionState() { }, { name: "proof verification failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), connection: conn, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -245,7 +245,7 @@ func (suite *TendermintTestSuite) TestVerifyChannelState() { // FIXME: uncomment // { // name: "successful verification", - // clientState: ibctmtypes.NewClientState(chainID, height), + // clientState: ibctmtypes.NewClientState(chainID, height, commitmenttypes.GetSDKSpecs()), // connection: conn, // consensusState: ibctmtypes.ConsensusState{ // Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -255,7 +255,7 @@ func (suite *TendermintTestSuite) TestVerifyChannelState() { // }, { name: "ApplyPrefix failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), channel: ch, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -265,7 +265,7 @@ func (suite *TendermintTestSuite) TestVerifyChannelState() { }, { name: "latest client height < height", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), channel: ch, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -285,7 +285,7 @@ func (suite *TendermintTestSuite) TestVerifyChannelState() { }, { name: "proof verification failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), channel: ch, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -325,7 +325,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketCommitment() { // FIXME: uncomment // { // name: "successful verification", - // clientState: ibctmtypes.NewClientState(chainID, height), + // clientState: ibctmtypes.NewClientState(chainID, height, commitmenttypes.GetSDKSpecs()), // connection: conn, // consensusState: ibctmtypes.ConsensusState{ // Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -335,7 +335,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketCommitment() { // }, { name: "ApplyPrefix failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), commitment: []byte{}, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -345,7 +345,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketCommitment() { }, { name: "latest client height < height", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), commitment: []byte{}, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -365,7 +365,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketCommitment() { }, { name: "proof verification failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), commitment: []byte{}, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -405,7 +405,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgement() { // FIXME: uncomment // { // name: "successful verification", - // clientState: ibctmtypes.NewClientState(chainID, chainID, height), + // clientState: ibctmtypes.NewClientState(chainID, chainID, height, commitmenttypes.GetSDKSpecs()), // connection: conn, // consensusState: ibctmtypes.ConsensusState{ // Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -415,7 +415,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgement() { // }, { name: "ApplyPrefix failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), ack: []byte{}, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -425,7 +425,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgement() { }, { name: "latest client height < height", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), ack: []byte{}, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -445,7 +445,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgement() { }, { name: "proof verification failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), ack: []byte{}, consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -484,7 +484,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgementAbsence() { // FIXME: uncomment // { // name: "successful verification", - // clientState: ibctmtypes.NewClientState(chainID, chainID, height), + // clientState: ibctmtypes.NewClientState(chainID, chainID, height, commitmenttypes.GetSDKSpecs()), // connection: conn, // consensusState: ibctmtypes.ConsensusState{ // Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -494,7 +494,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgementAbsence() { // }, { name: "ApplyPrefix failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), }, @@ -503,7 +503,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgementAbsence() { }, { name: "latest client height < height", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), }, @@ -521,7 +521,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgementAbsence() { }, { name: "proof verification failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), ValidatorSet: suite.valSet, @@ -559,7 +559,7 @@ func (suite *TendermintTestSuite) TestVerifyNextSeqRecv() { // FIXME: uncomment // { // name: "successful verification", - // clientState: ibctmtypes.NewClientState(chainID, chainID, height), + // clientState: ibctmtypes.NewClientState(chainID, chainID, height, commitmenttypes.GetSDKSpecs()), // connection: conn, // consensusState: ibctmtypes.ConsensusState{ // Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), @@ -569,7 +569,7 @@ func (suite *TendermintTestSuite) TestVerifyNextSeqRecv() { // }, { name: "ApplyPrefix failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), }, @@ -578,7 +578,7 @@ func (suite *TendermintTestSuite) TestVerifyNextSeqRecv() { }, { name: "latest client height < height", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), }, @@ -596,7 +596,7 @@ func (suite *TendermintTestSuite) TestVerifyNextSeqRecv() { }, { name: "proof verification failed", - clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + clientState: ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), consensusState: ibctmtypes.ConsensusState{ Root: commitmenttypes.NewMerkleRoot(suite.header.AppHash), ValidatorSet: suite.valSet, diff --git a/x/ibc/07-tendermint/update_test.go b/x/ibc/07-tendermint/update_test.go index 57cf3741e31a..c2c48e135f90 100644 --- a/x/ibc/07-tendermint/update_test.go +++ b/x/ibc/07-tendermint/update_test.go @@ -54,7 +54,7 @@ func (suite *TendermintTestSuite) TestCheckValidity() { { name: "successful update with next height and same validator set", setup: func() { - clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) newHeader = ibctmtypes.CreateTestHeader(chainID, height+1, suite.headerTime, suite.valSet, signers) currentTime = suite.now }, @@ -63,7 +63,7 @@ func (suite *TendermintTestSuite) TestCheckValidity() { { name: "successful update with future height and different validator set", setup: func() { - clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) newHeader = ibctmtypes.CreateTestHeader(chainID, height+5, suite.headerTime, bothValSet, bothSigners) currentTime = suite.now }, @@ -72,7 +72,7 @@ func (suite *TendermintTestSuite) TestCheckValidity() { { name: "unsuccessful update with next height: update header mismatches nextValSetHash", setup: func() { - clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) newHeader = ibctmtypes.CreateTestHeader(chainID, height+1, suite.headerTime, bothValSet, bothSigners) currentTime = suite.now }, @@ -81,7 +81,7 @@ func (suite *TendermintTestSuite) TestCheckValidity() { { name: "unsuccessful update with future height: too much change in validator set", setup: func() { - clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) newHeader = ibctmtypes.CreateTestHeader(chainID, height+5, suite.headerTime, altValSet, altSigners) currentTime = suite.now }, @@ -90,7 +90,7 @@ func (suite *TendermintTestSuite) TestCheckValidity() { { name: "unsuccessful update: trusting period has passed since last client timestamp", setup: func() { - clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) newHeader = ibctmtypes.CreateTestHeader(chainID, height+1, suite.headerTime, suite.valSet, signers) // make current time pass trusting period from last timestamp on clientstate currentTime = suite.now.Add(ubdPeriod) @@ -100,7 +100,7 @@ func (suite *TendermintTestSuite) TestCheckValidity() { { name: "unsuccessful update: header timestamp is past current timestamp", setup: func() { - clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) newHeader = ibctmtypes.CreateTestHeader(chainID, height+1, suite.now.Add(time.Minute), suite.valSet, signers) currentTime = suite.now }, @@ -109,7 +109,7 @@ func (suite *TendermintTestSuite) TestCheckValidity() { { name: "unsuccessful update: header timestamp is not past last client timestamp", setup: func() { - clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) newHeader = ibctmtypes.CreateTestHeader(chainID, height+1, suite.clientTime, suite.valSet, signers) currentTime = suite.now }, @@ -118,7 +118,7 @@ func (suite *TendermintTestSuite) TestCheckValidity() { { name: "header basic validation failed", setup: func() { - clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) newHeader = ibctmtypes.CreateTestHeader(chainID, height+1, suite.headerTime, suite.valSet, signers) // cause new header to fail validatebasic by changing commit height to mismatch header height newHeader.SignedHeader.Commit.Height = height - 1 @@ -129,7 +129,7 @@ func (suite *TendermintTestSuite) TestCheckValidity() { { name: "header height < latest client height", setup: func() { - clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header) + clientState = ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()) // Make new header at height less than latest client state newHeader = ibctmtypes.CreateTestHeader(chainID, height-1, suite.headerTime, suite.valSet, signers) currentTime = suite.now diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index a7b23d0c635d..c1168c78693d 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -4,6 +4,7 @@ import ( "net/url" "github.com/cosmos/cosmos-sdk/store/rootmulti" + storetypes "github.com/cosmos/cosmos-sdk/store/types" 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" @@ -11,6 +12,9 @@ import ( "github.com/tendermint/tendermint/crypto/merkle" ) +// var representing the proofspecs for a SDK chain +var sdkSpecs = []string{storetypes.ProofOpIAVLCommitment, merkle.ProofOpSimpleValue} + // ICS 023 Merkle Types Implementation // // This file defines Merkle commitment types that implements ICS 023. @@ -19,6 +23,11 @@ import ( // Applied on SDK-based IBC implementation var _ exported.Root = (*MerkleRoot)(nil) +// GetSDKSpecs is a getter function for the proofspecs of an sdk chain +func GetSDKSpecs() []string { + return sdkSpecs +} + // NewMerkleRoot constructs a new MerkleRoot func NewMerkleRoot(hash []byte) MerkleRoot { return MerkleRoot{ diff --git a/x/ibc/23-commitment/types/merkle_test.go b/x/ibc/23-commitment/types/merkle_test.go index 688100e18c40..bc5fa341a664 100644 --- a/x/ibc/23-commitment/types/merkle_test.go +++ b/x/ibc/23-commitment/types/merkle_test.go @@ -12,8 +12,6 @@ import ( "github.com/tendermint/tendermint/crypto/merkle" ) -var specs = []string{"ics23:iavl", "simple:v"} - func (suite *MerkleTestSuite) TestVerifyMembership() { suite.iavlStore.Set([]byte("MYKEY"), []byte("MYVALUE")) cid := suite.store.Commit() @@ -67,7 +65,7 @@ func (suite *MerkleTestSuite) TestVerifyMembership() { root := types.NewMerkleRoot(tc.root) path := types.NewMerklePath(tc.pathArr) - err := proof.VerifyMembership(specs, &root, path, tc.value) + err := proof.VerifyMembership(types.GetSDKSpecs(), &root, path, tc.value) if tc.shouldPass { // nolint: scopelint @@ -133,7 +131,7 @@ func (suite *MerkleTestSuite) TestVerifyNonMembership() { root := types.NewMerkleRoot(tc.root) path := types.NewMerklePath(tc.pathArr) - err := proof.VerifyNonMembership(specs, &root, path) + err := proof.VerifyNonMembership(types.GetSDKSpecs(), &root, path) if tc.shouldPass { // nolint: scopelint diff --git a/x/ibc/ante/ante_test.go b/x/ibc/ante/ante_test.go index 727a82a751f1..5e13ffadea3d 100644 --- a/x/ibc/ante/ante_test.go +++ b/x/ibc/ante/ante_test.go @@ -241,7 +241,7 @@ func (chain *TestChain) CreateClient(client *TestChain) error { ctxTarget := chain.GetContext() // create client - clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header) + clientState, err := ibctmtypes.Initialize(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()) if err != nil { return err } @@ -309,7 +309,7 @@ func (chain *TestChain) updateClient(client *TestChain) { ctxTarget, client.ClientID, uint64(client.Header.Height-1), consensusState, ) chain.App.IBCKeeper.ClientKeeper.SetClientState( - ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header), + ctxTarget, ibctmtypes.NewClientState(client.ClientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, client.Header, commitmenttypes.GetSDKSpecs()), ) // _, _, err := simapp.SignCheckDeliver( diff --git a/x/ibc/genesis_test.go b/x/ibc/genesis_test.go index cd64b18dd799..4d39c43e2ebd 100644 --- a/x/ibc/genesis_test.go +++ b/x/ibc/genesis_test.go @@ -30,7 +30,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() { genState: ibc.GenesisState{ ClientGenesis: client.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), localhosttypes.NewClientState("chaindID", 10), }, []client.ConsensusStates{ @@ -86,7 +86,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() { genState: ibc.GenesisState{ ClientGenesis: client.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), localhosttypes.NewClientState("(chaindID)", 0), }, nil, From 57e0a8443b3516dfd80fd35ccbe829dbdcadb69d Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 9 Jun 2020 17:23:36 -0400 Subject: [PATCH 07/19] add argument to constructor: --- x/ibc/07-tendermint/types/msgs.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x/ibc/07-tendermint/types/msgs.go b/x/ibc/07-tendermint/types/msgs.go index fae964c13e03..65e0482ca50b 100644 --- a/x/ibc/07-tendermint/types/msgs.go +++ b/x/ibc/07-tendermint/types/msgs.go @@ -51,7 +51,8 @@ func (msg MsgCreateClient) ProtoMessage() {} // NewMsgCreateClient creates a new MsgCreateClient instance func NewMsgCreateClient( id string, header Header, trustLevel tmmath.Fraction, - trustingPeriod, unbondingPeriod, maxClockDrift time.Duration, signer sdk.AccAddress, + trustingPeriod, unbondingPeriod, maxClockDrift time.Duration, + specs []string, signer sdk.AccAddress, ) MsgCreateClient { return MsgCreateClient{ @@ -61,6 +62,7 @@ func NewMsgCreateClient( TrustingPeriod: trustingPeriod, UnbondingPeriod: unbondingPeriod, MaxClockDrift: maxClockDrift, + ProofSpecs: specs, Signer: signer, } } From 6d759cfb3717f185b77486d1f0c40d1b8b08d289 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 9 Jun 2020 18:03:15 -0400 Subject: [PATCH 08/19] fixed msg client and tests --- x/ibc/07-tendermint/client/cli/tx.go | 25 ++++++++++++++++++++----- x/ibc/07-tendermint/client/rest/rest.go | 1 + x/ibc/07-tendermint/client/rest/tx.go | 2 +- x/ibc/07-tendermint/types/errors.go | 1 + x/ibc/07-tendermint/types/msgs.go | 8 ++++++++ x/ibc/07-tendermint/types/msgs_test.go | 18 ++++++++++-------- 6 files changed, 41 insertions(+), 14 deletions(-) diff --git a/x/ibc/07-tendermint/client/cli/tx.go b/x/ibc/07-tendermint/client/cli/tx.go index c68b9564cc2b..a078ff9fb89e 100644 --- a/x/ibc/07-tendermint/client/cli/tx.go +++ b/x/ibc/07-tendermint/client/cli/tx.go @@ -25,17 +25,22 @@ import ( authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types" + commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types" ) -const flagTrustLevel = "trust-level" +const ( + flagTrustLevel = "trust-level" + flagProofSpecs = "proof-specs" +) // GetCmdCreateClient defines the command to create a new IBC Client as defined // in https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#create func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { cmd := &cobra.Command{ - Use: "create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift]", - Short: "create new tendermint client", - Long: "create new tendermint client. Trust level can be a fraction (eg: '1/3') or 'default'", + Use: "create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift]", + Short: "create new tendermint client", + Long: `create new tendermint client. Trust level can be a fraction (eg: '1/3') or 'default'. + Proof Specs can be a comma-seperated list of strings (eg: 'ics23:simple,ics23:iavl' or 'default'`, Example: fmt.Sprintf("%s tx ibc %s create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift] --trust-level default --from node0 --home ../node0/cli --chain-id $CID", version.ClientName, ibctmtypes.SubModuleName), Args: cobra.ExactArgs(5), RunE: func(cmd *cobra.Command, args []string) error { @@ -59,6 +64,7 @@ func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { var ( trustLevel tmmath.Fraction + specs []string err error ) @@ -88,8 +94,16 @@ func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { return err } + spc := viper.GetString(flagProofSpecs) + + if spc == "default" { + specs = commitmenttypes.GetSDKSpecs() + } else { + specs = strings.Split(spc, ",") + } + msg := ibctmtypes.NewMsgCreateClient( - clientID, header, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, clientCtx.GetFromAddress(), + clientID, header, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, specs, clientCtx.GetFromAddress(), ) if err := msg.ValidateBasic(); err != nil { @@ -100,6 +114,7 @@ func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { }, } cmd.Flags().String(flagTrustLevel, "default", "light client trust level fraction for header updates") + cmd.Flags().String(flagProofSpecs, "default", "proof specs format to be used for verification") return cmd } diff --git a/x/ibc/07-tendermint/client/rest/rest.go b/x/ibc/07-tendermint/client/rest/rest.go index 198fa8f82ec2..510ac501a157 100644 --- a/x/ibc/07-tendermint/client/rest/rest.go +++ b/x/ibc/07-tendermint/client/rest/rest.go @@ -34,6 +34,7 @@ type CreateClientReq struct { TrustingPeriod time.Duration `json:"trusting_period" yaml:"trusting_period"` UnbondingPeriod time.Duration `json:"unbonding_period" yaml:"unbonding_period"` MaxClockDrift time.Duration `json:"max_clock_drift" yaml:"max_clock_drift"` + ProofSpecs []string `json:"proof_specs" yaml:"proof_specs"` } // UpdateClientReq defines the properties of a update client request's body. diff --git a/x/ibc/07-tendermint/client/rest/tx.go b/x/ibc/07-tendermint/client/rest/tx.go index a6298b79c7ab..699e8fbced27 100644 --- a/x/ibc/07-tendermint/client/rest/tx.go +++ b/x/ibc/07-tendermint/client/rest/tx.go @@ -52,7 +52,7 @@ func createClientHandlerFn(clientCtx client.Context) http.HandlerFunc { msg := ibctmtypes.NewMsgCreateClient( req.ClientID, req.Header, req.TrustLevel, req.TrustingPeriod, req.UnbondingPeriod, req.MaxClockDrift, - fromAddr, + req.ProofSpecs, fromAddr, ) if err := msg.ValidateBasic(); err != nil { diff --git a/x/ibc/07-tendermint/types/errors.go b/x/ibc/07-tendermint/types/errors.go index 0c659acbddf5..8a6890c8e7c1 100644 --- a/x/ibc/07-tendermint/types/errors.go +++ b/x/ibc/07-tendermint/types/errors.go @@ -16,4 +16,5 @@ var ( ErrInvalidMaxClockDrift = sdkerrors.Register(SubModuleName, 5, "invalid max clock drift") ErrTrustingPeriodExpired = sdkerrors.Register(SubModuleName, 6, "time since latest trusted state has passed the trusting period") ErrUnbondingPeriodExpired = sdkerrors.Register(SubModuleName, 7, "time since latest trusted state has passed the unbonding period") + ErrInvalidProofSpecs = sdkerrors.Register(SubModuleName, 8, "proof specs are invalid") ) diff --git a/x/ibc/07-tendermint/types/msgs.go b/x/ibc/07-tendermint/types/msgs.go index 65e0482ca50b..c3ac2d6e3cd6 100644 --- a/x/ibc/07-tendermint/types/msgs.go +++ b/x/ibc/07-tendermint/types/msgs.go @@ -98,6 +98,14 @@ func (msg MsgCreateClient) ValidateBasic() error { if err := msg.Header.ValidateBasic(msg.Header.ChainID); err != nil { return sdkerrors.Wrapf(ErrInvalidHeader, "header failed validatebasic with its own chain-id: %v", err) } + // Validate ProofSpecs if provided + if msg.ProofSpecs != nil { + for _, spec := range msg.ProofSpecs { + if spec == "" { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "Invalid empty spec") + } + } + } return host.ClientIdentifierValidator(msg.ClientID) } diff --git a/x/ibc/07-tendermint/types/msgs_test.go b/x/ibc/07-tendermint/types/msgs_test.go index f55c87fca0c4..dcb5713ed710 100644 --- a/x/ibc/07-tendermint/types/msgs_test.go +++ b/x/ibc/07-tendermint/types/msgs_test.go @@ -9,6 +9,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types" + commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types" ) func (suite *TendermintTestSuite) TestMsgCreateClientValidateBasic() { @@ -22,14 +23,15 @@ func (suite *TendermintTestSuite) TestMsgCreateClientValidateBasic() { expPass bool errMsg string }{ - {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, signer), true, "success msg should pass"}, - {ibctmtypes.NewMsgCreateClient("(BADCHAIN)", suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, signer), false, "invalid client id passed"}, - {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, math.Fraction{Numerator: 0, Denominator: 1}, trustingPeriod, ubdPeriod, maxClockDrift, signer), false, "invalid trust level"}, - {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, 0, ubdPeriod, maxClockDrift, signer), false, "zero trusting period passed"}, - {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, 0, maxClockDrift, signer), false, "zero unbonding period passed"}, - {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, nil), false, "Empty address passed"}, - {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, ibctmtypes.Header{}, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, signer), false, "nil header"}, - {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, invalidHeader, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, signer), false, "invalid header"}, + {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), signer), true, "success msg should pass"}, + {ibctmtypes.NewMsgCreateClient("(BADCHAIN)", suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), signer), false, "invalid client id passed"}, + {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, math.Fraction{Numerator: 0, Denominator: 1}, trustingPeriod, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), signer), false, "invalid trust level"}, + {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, 0, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), signer), false, "zero trusting period passed"}, + {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, 0, maxClockDrift, commitmenttypes.GetSDKSpecs(), signer), false, "zero unbonding period passed"}, + {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), nil), false, "Empty address passed"}, + {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, ibctmtypes.Header{}, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), signer), false, "nil header"}, + {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, invalidHeader, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), signer), false, "invalid header"}, + {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, []string{"", "spec"}, signer), false, "invalid proof specs"}, } for i, tc := range cases { From 8e8bfd86d2e1e845bf55f7aa079a4bfca2376ee8 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 9 Jun 2020 18:19:31 -0400 Subject: [PATCH 09/19] appease linter --- x/ibc/07-tendermint/client/cli/tx.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/ibc/07-tendermint/client/cli/tx.go b/x/ibc/07-tendermint/client/cli/tx.go index a078ff9fb89e..4a96ab3ea636 100644 --- a/x/ibc/07-tendermint/client/cli/tx.go +++ b/x/ibc/07-tendermint/client/cli/tx.go @@ -40,7 +40,7 @@ func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { Use: "create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift]", Short: "create new tendermint client", Long: `create new tendermint client. Trust level can be a fraction (eg: '1/3') or 'default'. - Proof Specs can be a comma-seperated list of strings (eg: 'ics23:simple,ics23:iavl' or 'default'`, + Proof Specs can be a comma-separated list of strings (eg: 'ics23:simple,ics23:iavl' or 'default'`, Example: fmt.Sprintf("%s tx ibc %s create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift] --trust-level default --from node0 --home ../node0/cli --chain-id $CID", version.ClientName, ibctmtypes.SubModuleName), Args: cobra.ExactArgs(5), RunE: func(cmd *cobra.Command, args []string) error { From 761fa7bc42fdc1a810ac4d6ed9793537e791d0ca Mon Sep 17 00:00:00 2001 From: Aditya Date: Wed, 10 Jun 2020 10:57:58 -0400 Subject: [PATCH 10/19] Apply suggestions from code review Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- x/ibc/07-tendermint/client/cli/tx.go | 5 +++-- x/ibc/07-tendermint/types/errors.go | 2 +- x/ibc/07-tendermint/types/msgs.go | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/x/ibc/07-tendermint/client/cli/tx.go b/x/ibc/07-tendermint/client/cli/tx.go index 4a96ab3ea636..e3344aaa0bef 100644 --- a/x/ibc/07-tendermint/client/cli/tx.go +++ b/x/ibc/07-tendermint/client/cli/tx.go @@ -39,8 +39,9 @@ func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift]", Short: "create new tendermint client", - Long: `create new tendermint client. Trust level can be a fraction (eg: '1/3') or 'default'. - Proof Specs can be a comma-separated list of strings (eg: 'ics23:simple,ics23:iavl' or 'default'`, + Long: `Create a new tendermint IBC client. + - 'trust-level' flag can be a fraction (eg: '1/3') or 'default' + - 'proof-specs' flag can be a comma-separated list of strings (eg: 'ics23:simple,ics23:iavl' or 'default'`, Example: fmt.Sprintf("%s tx ibc %s create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift] --trust-level default --from node0 --home ../node0/cli --chain-id $CID", version.ClientName, ibctmtypes.SubModuleName), Args: cobra.ExactArgs(5), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/x/ibc/07-tendermint/types/errors.go b/x/ibc/07-tendermint/types/errors.go index 8a6890c8e7c1..81a176491bfc 100644 --- a/x/ibc/07-tendermint/types/errors.go +++ b/x/ibc/07-tendermint/types/errors.go @@ -16,5 +16,5 @@ var ( ErrInvalidMaxClockDrift = sdkerrors.Register(SubModuleName, 5, "invalid max clock drift") ErrTrustingPeriodExpired = sdkerrors.Register(SubModuleName, 6, "time since latest trusted state has passed the trusting period") ErrUnbondingPeriodExpired = sdkerrors.Register(SubModuleName, 7, "time since latest trusted state has passed the unbonding period") - ErrInvalidProofSpecs = sdkerrors.Register(SubModuleName, 8, "proof specs are invalid") + ErrInvalidProofSpecs = sdkerrors.Register(SubModuleName, 8, "invalid proof specs") ) diff --git a/x/ibc/07-tendermint/types/msgs.go b/x/ibc/07-tendermint/types/msgs.go index c3ac2d6e3cd6..ea8046daaa9c 100644 --- a/x/ibc/07-tendermint/types/msgs.go +++ b/x/ibc/07-tendermint/types/msgs.go @@ -101,8 +101,8 @@ func (msg MsgCreateClient) ValidateBasic() error { // Validate ProofSpecs if provided if msg.ProofSpecs != nil { for _, spec := range msg.ProofSpecs { - if spec == "" { - return sdkerrors.Wrap(ErrInvalidProofSpecs, "Invalid empty spec") + if strings.TrimSpace(spec) == "" { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec cannot be blank") } } } From 4bb1e56aa3d525c81f2a9c06eef1b7d3640cec9f Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 10 Jun 2020 11:25:10 -0400 Subject: [PATCH 11/19] finish fixes from review --- x/ibc/02-client/keeper/keeper_test.go | 2 +- x/ibc/07-tendermint/types/client_state.go | 27 +++++-- x/ibc/07-tendermint/types/msgs.go | 15 ++-- x/ibc/23-commitment/types/merkle.go | 94 ++++++++--------------- 4 files changed, 60 insertions(+), 78 deletions(-) diff --git a/x/ibc/02-client/keeper/keeper_test.go b/x/ibc/02-client/keeper/keeper_test.go index eda23a26f0ac..1c4562849869 100644 --- a/x/ibc/02-client/keeper/keeper_test.go +++ b/x/ibc/02-client/keeper/keeper_test.go @@ -23,7 +23,7 @@ import ( ) const ( - testClientID = "gaia" + testClientID = "gaiachain" testClientID2 = "ethbridge" testClientID3 = "ethermint" diff --git a/x/ibc/07-tendermint/types/client_state.go b/x/ibc/07-tendermint/types/client_state.go index 54b6a0b2488a..546afe6e448e 100644 --- a/x/ibc/07-tendermint/types/client_state.go +++ b/x/ibc/07-tendermint/types/client_state.go @@ -2,6 +2,7 @@ package types import ( "fmt" + "strings" "time" tmmath "github.com/tendermint/tendermint/libs/math" @@ -67,15 +68,11 @@ func Initialize( trustingPeriod, ubdPeriod, maxClockDrift time.Duration, header Header, specs []string, ) (ClientState, error) { + clientState := NewClientState(id, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs) - if trustingPeriod >= ubdPeriod { - return ClientState{}, sdkerrors.Wrapf( - ErrInvalidTrustingPeriod, - "trusting period (%s) should be < unbonding period (%s)", trustingPeriod, ubdPeriod, - ) + if err := clientState.Validate(); err != nil { + return ClientState{}, err } - - clientState := NewClientState(id, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs) return clientState, nil } @@ -147,6 +144,22 @@ func (cs ClientState) Validate() error { if cs.MaxClockDrift == 0 { return sdkerrors.Wrap(ErrInvalidMaxClockDrift, "max clock drift cannot be zero") } + if cs.TrustingPeriod >= cs.UnbondingPeriod { + return sdkerrors.Wrapf( + ErrInvalidTrustingPeriod, + "trusting period (%s) should be < unbonding period (%s)", cs.TrustingPeriod, cs.UnbondingPeriod, + ) + } + // Validate ProofSpecs + if cs.ProofSpecs == nil { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof specs cannot be nil for tm client") + } + for _, spec := range cs.ProofSpecs { + if strings.TrimSpace(spec) == "" { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec cannot be blank") + } + } + return cs.LastHeader.ValidateBasic(cs.GetChainID()) } diff --git a/x/ibc/07-tendermint/types/msgs.go b/x/ibc/07-tendermint/types/msgs.go index ea8046daaa9c..34c1a066a392 100644 --- a/x/ibc/07-tendermint/types/msgs.go +++ b/x/ibc/07-tendermint/types/msgs.go @@ -1,6 +1,7 @@ package types import ( + "strings" "time" tmmath "github.com/tendermint/tendermint/libs/math" @@ -98,12 +99,14 @@ func (msg MsgCreateClient) ValidateBasic() error { if err := msg.Header.ValidateBasic(msg.Header.ChainID); err != nil { return sdkerrors.Wrapf(ErrInvalidHeader, "header failed validatebasic with its own chain-id: %v", err) } - // Validate ProofSpecs if provided - if msg.ProofSpecs != nil { - for _, spec := range msg.ProofSpecs { - if strings.TrimSpace(spec) == "" { - return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec cannot be blank") - } + + // Validate ProofSpecs + if msg.ProofSpecs == nil { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof specs cannot be nil") + } + for _, spec := range msg.ProofSpecs { + if strings.TrimSpace(spec) == "" { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec cannot be blank") } } return host.ClientIdentifierValidator(msg.ClientID) diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index c1168c78693d..4d3e5286e156 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -138,22 +138,8 @@ func (MerkleProof) GetCommitmentType() exported.Type { // VerifyMembership verifies the membership pf a merkle proof against the given root, path, and value. func (proof MerkleProof) VerifyMembership(specs []string, root exported.Root, path exported.Path, value []byte) error { - if proof.IsEmpty() || root == nil || root.IsEmpty() || path == nil || path.IsEmpty() || len(value) == 0 { - return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") - } - - if len(specs) != len(proof.Proof.Ops) { - return sdkerrors.Wrapf(ErrInvalidMerkleProof, - "length of specs: %d not equal to length of proof: %d", - len(specs), len(proof.Proof.Ops)) - } - - for i, op := range proof.Proof.Ops { - if op.Type != specs[i] { - return sdkerrors.Wrapf(ErrInvalidMerkleProof, - "proof does not match expected format at position %d, expected: %s, got: %s", - i, specs[i], op.Type) - } + if err := proof.validateVerificationArgs(specs, root); err != nil { + return err } runtime := rootmulti.DefaultProofRuntime() @@ -162,22 +148,8 @@ func (proof MerkleProof) VerifyMembership(specs []string, root exported.Root, pa // VerifyNonMembership verifies the absence of a merkle proof against the given root and path. func (proof MerkleProof) VerifyNonMembership(specs []string, root exported.Root, path exported.Path) error { - if proof.IsEmpty() || root == nil || root.IsEmpty() || path == nil || path.IsEmpty() { - return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") - } - - if len(specs) != len(proof.Proof.Ops) { - return sdkerrors.Wrapf(ErrInvalidMerkleProof, - "length of specs: %d not equal to length of proof: %d", - len(specs), len(proof.Proof.Ops)) - } - - for i, op := range proof.Proof.Ops { - if op.Type != specs[i] { - return sdkerrors.Wrapf(ErrInvalidMerkleProof, - "proof does not match expected format at position %d, expected: %s, got: %s", - i, specs[i], op.Type) - } + if err := proof.validateVerificationArgs(specs, root); err != nil { + return err } runtime := rootmulti.DefaultProofRuntime() @@ -187,22 +159,8 @@ func (proof MerkleProof) VerifyNonMembership(specs []string, root exported.Root, // BatchVerifyMembership verifies a group of key value pairs against the given root // NOTE: Untested func (proof MerkleProof) BatchVerifyMembership(specs []string, root exported.Root, items map[string][]byte) error { - if proof.IsEmpty() || root == nil || root.IsEmpty() { - return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") - } - - if len(specs) != len(proof.Proof.Ops) { - return sdkerrors.Wrapf(ErrInvalidMerkleProof, - "length of specs: %d not equal to length of proof: %d", - len(specs), len(proof.Proof.Ops)) - } - - for i, op := range proof.Proof.Ops { - if op.Type != specs[i] { - return sdkerrors.Wrapf(ErrInvalidMerkleProof, - "proof does not match expected format at position %d, expected: %s, got: %s", - i, specs[i], op.Type) - } + if err := proof.validateVerificationArgs(specs, root); err != nil { + return err } // Verify each item separately against same proof @@ -219,22 +177,8 @@ func (proof MerkleProof) BatchVerifyMembership(specs []string, root exported.Roo // BatchVerifyNonMembership verifies absence of a group of keys against the given root // NOTE: Untested func (proof MerkleProof) BatchVerifyNonMembership(specs []string, root exported.Root, items []string) error { - if proof.IsEmpty() || root == nil || root.IsEmpty() { - return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") - } - - if len(specs) != len(proof.Proof.Ops) { - return sdkerrors.Wrapf(ErrInvalidMerkleProof, - "length of specs: %d not equal to length of proof: %d", - len(specs), len(proof.Proof.Ops)) - } - - for i, op := range proof.Proof.Ops { - if op.Type != specs[i] { - return sdkerrors.Wrapf(ErrInvalidMerkleProof, - "proof does not match expected format at position %d, expected: %s, got: %s", - i, specs[i], op.Type) - } + if err := proof.validateVerificationArgs(specs, root); err != nil { + return err } // Verify each item separately against same proof @@ -260,3 +204,25 @@ func (proof MerkleProof) ValidateBasic() error { } return nil } + +// validateVerificationArgs verifies the proof arguments are valid +func (proof MerkleProof) validateVerificationArgs(specs []string, root exported.Root) error { + if proof.IsEmpty() || root == nil || root.IsEmpty() { + return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") + } + + if len(specs) != len(proof.Proof.Ops) { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "length of specs: %d not equal to length of proof: %d", + len(specs), len(proof.Proof.Ops)) + } + + for i, op := range proof.Proof.Ops { + if op.Type != specs[i] { + return sdkerrors.Wrapf(ErrInvalidMerkleProof, + "proof does not match expected format at position %d, expected: %s, got: %s", + i, specs[i], op.Type) + } + } + return nil +} From bd71a76e5f26d060a00686b827c93a9cf47e1eeb Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 10 Jun 2020 11:36:27 -0400 Subject: [PATCH 12/19] add back proof-specific checks --- x/ibc/23-commitment/types/merkle.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index 4d3e5286e156..0b88b156b5f8 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -142,6 +142,14 @@ func (proof MerkleProof) VerifyMembership(specs []string, root exported.Root, pa return err } + // VerifyMembership specific argument validation + if path == nil || path.IsEmpty() { + return sdkerrors.Wrap(ErrInvalidProof, "empty path") + } + if len(value) == 0 { + return sdkerrors.Wrap(ErrInvalidProof, "empty value in membership proof") + } + runtime := rootmulti.DefaultProofRuntime() return runtime.VerifyValue(proof.Proof, root.GetHash(), path.String(), value) } @@ -152,6 +160,11 @@ func (proof MerkleProof) VerifyNonMembership(specs []string, root exported.Root, return err } + // VerifyNonMembership specific argument validation + if path == nil || path.IsEmpty() { + return sdkerrors.Wrap(ErrInvalidProof, "empty path") + } + runtime := rootmulti.DefaultProofRuntime() return runtime.VerifyAbsence(proof.Proof, root.GetHash(), path.String()) } @@ -166,6 +179,11 @@ func (proof MerkleProof) BatchVerifyMembership(specs []string, root exported.Roo // Verify each item separately against same proof runtime := rootmulti.DefaultProofRuntime() for path, value := range items { + // check value is not empty + if len(value) == 0 { + return sdkerrors.Wrap(ErrInvalidProof, "empty value in batched membership proof") + } + if err := runtime.VerifyValue(proof.Proof, root.GetHash(), path, value); err != nil { return sdkerrors.Wrapf(ErrInvalidProof, "verification failed for path: %s, value: %x. Error: %v", path, value, err) From 10c978a0c75e911f7b3bee6114091d43f51545df Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Wed, 10 Jun 2020 17:55:03 +0200 Subject: [PATCH 13/19] Apply suggestions from code review --- x/ibc/07-tendermint/client/cli/tx.go | 4 ++-- x/ibc/07-tendermint/types/client_state.go | 3 --- x/ibc/07-tendermint/types/msgs.go | 7 ++++++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/x/ibc/07-tendermint/client/cli/tx.go b/x/ibc/07-tendermint/client/cli/tx.go index e3344aaa0bef..be05bf0c5c63 100644 --- a/x/ibc/07-tendermint/client/cli/tx.go +++ b/x/ibc/07-tendermint/client/cli/tx.go @@ -40,8 +40,8 @@ func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { Use: "create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift]", Short: "create new tendermint client", Long: `Create a new tendermint IBC client. - - 'trust-level' flag can be a fraction (eg: '1/3') or 'default' - - 'proof-specs' flag can be a comma-separated list of strings (eg: 'ics23:simple,ics23:iavl' or 'default'`, + - 'trust-level' flag can be a fraction (eg: '1/3') or 'default' + - 'proof-specs' flag can be a comma-separated list of strings (eg: 'ics23:simple,ics23:iavl') or 'default'`, Example: fmt.Sprintf("%s tx ibc %s create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift] --trust-level default --from node0 --home ../node0/cli --chain-id $CID", version.ClientName, ibctmtypes.SubModuleName), Args: cobra.ExactArgs(5), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/x/ibc/07-tendermint/types/client_state.go b/x/ibc/07-tendermint/types/client_state.go index 6048b5c6c002..a3449834854f 100644 --- a/x/ibc/07-tendermint/types/client_state.go +++ b/x/ibc/07-tendermint/types/client_state.go @@ -69,9 +69,6 @@ func Initialize( ) (ClientState, error) { clientState := NewClientState(id, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs) - if err := clientState.Validate(); err != nil { - return ClientState{}, err - } return clientState, nil } diff --git a/x/ibc/07-tendermint/types/msgs.go b/x/ibc/07-tendermint/types/msgs.go index ae6c971220e3..ec53fb9a33bd 100644 --- a/x/ibc/07-tendermint/types/msgs.go +++ b/x/ibc/07-tendermint/types/msgs.go @@ -99,7 +99,12 @@ func (msg MsgCreateClient) ValidateBasic() error { if err := msg.Header.ValidateBasic(msg.Header.ChainID); err != nil { return sdkerrors.Wrapf(ErrInvalidHeader, "header failed validatebasic with its own chain-id: %v", err) } - + if msg.TrustingPeriod >= msg.UnbondingPeriod { + return sdkerrors.Wrapf( + ErrInvalidTrustingPeriod, + "trusting period (%s) should be < unbonding period (%s)", msg.TrustingPeriod, msg.UnbondingPeriod, + ) + } // Validate ProofSpecs if msg.ProofSpecs == nil { return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof specs cannot be nil") From f170cf9af6786540b04b72bc4bee438379df740d Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 10 Jun 2020 13:43:32 -0400 Subject: [PATCH 14/19] more robust proof spec checks --- x/ibc/02-client/types/genesis_test.go | 12 ++++-------- x/ibc/07-tendermint/types/client_state.go | 4 ++-- x/ibc/07-tendermint/types/msgs.go | 4 ++-- x/ibc/23-commitment/types/merkle.go | 2 +- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/x/ibc/02-client/types/genesis_test.go b/x/ibc/02-client/types/genesis_test.go index b342f9eba9b9..55aca4088095 100644 --- a/x/ibc/02-client/types/genesis_test.go +++ b/x/ibc/02-client/types/genesis_test.go @@ -6,11 +6,9 @@ import ( "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto/merkle" lite "github.com/tendermint/tendermint/lite2" tmtypes "github.com/tendermint/tendermint/types" - storetypes "github.com/cosmos/cosmos-sdk/store/types" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types" ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types" @@ -26,8 +24,6 @@ const ( maxClockDrift time.Duration = time.Second * 10 ) -var specs = []string{storetypes.ProofOpIAVLCommitment, merkle.ProofOpSimpleValue} - func TestValidateGenesis(t *testing.T) { privVal := tmtypes.NewMockPV() pubKey, err := privVal.GetPubKey() @@ -54,7 +50,7 @@ func TestValidateGenesis(t *testing.T) { name: "valid genesis", genState: types.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, commitmenttypes.GetSDKSpecs()), localhosttypes.NewClientState("chaindID", 10), }, []types.ClientConsensusStates{ @@ -75,7 +71,7 @@ func TestValidateGenesis(t *testing.T) { name: "invalid client", genState: types.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, commitmenttypes.GetSDKSpecs()), localhosttypes.NewClientState("chaindID", 0), }, nil, @@ -87,7 +83,7 @@ func TestValidateGenesis(t *testing.T) { name: "invalid consensus state", genState: types.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, commitmenttypes.GetSDKSpecs()), localhosttypes.NewClientState("chaindID", 10), }, []types.ClientConsensusStates{ @@ -108,7 +104,7 @@ func TestValidateGenesis(t *testing.T) { name: "invalid consensus state", genState: types.NewGenesisState( []exported.ClientState{ - ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs), + ibctmtypes.NewClientState(clientID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, commitmenttypes.GetSDKSpecs()), localhosttypes.NewClientState("chaindID", 10), }, []types.ClientConsensusStates{ diff --git a/x/ibc/07-tendermint/types/client_state.go b/x/ibc/07-tendermint/types/client_state.go index 6048b5c6c002..046df7f60de7 100644 --- a/x/ibc/07-tendermint/types/client_state.go +++ b/x/ibc/07-tendermint/types/client_state.go @@ -154,8 +154,8 @@ func (cs ClientState) Validate() error { return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof specs cannot be nil for tm client") } for _, spec := range cs.ProofSpecs { - if strings.TrimSpace(spec) == "" { - return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec cannot be blank") + if !strings.HasPrefix(spec, "ics23:") { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec must be an ics23 spec") } } diff --git a/x/ibc/07-tendermint/types/msgs.go b/x/ibc/07-tendermint/types/msgs.go index ae6c971220e3..1009c0eb081a 100644 --- a/x/ibc/07-tendermint/types/msgs.go +++ b/x/ibc/07-tendermint/types/msgs.go @@ -105,8 +105,8 @@ func (msg MsgCreateClient) ValidateBasic() error { return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof specs cannot be nil") } for _, spec := range msg.ProofSpecs { - if strings.TrimSpace(spec) == "" { - return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec cannot be blank") + if !strings.HasPrefix(spec, "ics23:") { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec must be an ics23 spec") } } return host.ClientIdentifierValidator(msg.ClientID) diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index 0b88b156b5f8..c979392904d8 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -13,7 +13,7 @@ import ( ) // var representing the proofspecs for a SDK chain -var sdkSpecs = []string{storetypes.ProofOpIAVLCommitment, merkle.ProofOpSimpleValue} +var sdkSpecs = []string{storetypes.ProofOpIAVLCommitment, storetypes.ProofOpSimpleMerkleCommitment} // ICS 023 Merkle Types Implementation // From 236ab966d423269b749f63300b8e28dbd61f870e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 10 Jun 2020 13:51:58 -0400 Subject: [PATCH 15/19] add CHANGELOG entries --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28e01abc42e2..7c21f6e318d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ balances or a single balance by denom when the `denom` query parameter is present. * (client) [\#5640](https://github.com/cosmos/cosmos-sdk/pull/5640) The rest server endpoint `/swagger-ui/` is replaced by ´/´. * (x/auth) [\#5702](https://github.com/cosmos/cosmos-sdk/pull/5702) The `x/auth` querier route has changed from `"acc"` to `"auth"`. +* (store/rootmulti) [\#6390](https://github.com/cosmos/cosmos-sdk/pull/6390) Proofs of empty stores are no longer supported. * (store/types) [\#5730](https://github.com/cosmos/cosmos-sdk/pull/5730) store.types.Cp() is removed in favour of types.CopyBytes(). * (client) [\#5640](https://github.com/cosmos/cosmos-sdk/issues/5783) Unify all coins representations on JSON client requests for governance proposals. * [\#5785](https://github.com/cosmos/cosmos-sdk/issues/5785) JSON strings coerced to valid UTF-8 bytes at JSON marshalling time @@ -132,6 +133,7 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa * (client/lcd) [\#6290](https://github.com/cosmos/cosmos-sdk/pull/6290) `CliCtx` of struct `RestServer` in package client/lcd has been renamed to `ClientCtx`. * (types) [\#6327](https://github.com/cosmos/cosmos-sdk/pull/6327) `sdk.Msg` now inherits `proto.Message`, as a result all `sdk.Msg` types now use pointer semantics. * (codec) [\#6330](https://github.com/cosmos/cosmos-sdk/pull/6330) `codec.RegisterCrypto` has been moved to the `crypto/codec` package and the global `codec.Cdc` Amino instance has been deprecated and moved to the `codec/legacy_global` package. +* (x/ibc) [\#6374](https://github.com/cosmos/cosmos-sdk/pull/6374) `VerifyMembership` and `VerifyNonMembership` now take a `specs []string` argument to specify the proof format used for verification. Most SDK chains can simply use `commitmenttypes.GetSDKSpecs()` for this argument. ### Features @@ -150,6 +152,7 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa * (x/capability) [\#5828](https://github.com/cosmos/cosmos-sdk/pull/5828) Capability module integration as outlined in [ADR 3 - Dynamic Capability Store](https://github.com/cosmos/tree/master/docs/architecture/adr-003-dynamic-capability-store.md). * (x/params) [\#6005](https://github.com/cosmos/cosmos-sdk/pull/6005) Add new CLI command for querying raw x/params parameters by subspace and key. * (x/ibc) [\#5769](https://github.com/cosmos/cosmos-sdk/pull/5769) [ICS 009 - Loopback Client](https://github.com/cosmos/ics/tree/master/spec/ics-009-loopback-client) subpackage +* (x/ibc) [\#6374](https://github.com/cosmos/cosmos-sdk/pull/6374) ICS-23 Verify functions will now accept and verify ics23 CommitmentProofs exclusively * (store) [\#6324](https://github.com/cosmos/cosmos-sdk/pull/6324) IAVL store query proofs now return CommitmentOp which wraps an ics23 CommitmentProof * (store) [\#6390](https://github.com/cosmos/cosmos-sdk/pull/6390) `RootMulti` store query proofs now return `CommitmentOp` which wraps `CommitmentProofs` * `store.Query` now only returns chained `ics23.CommitmentProof` wrapped in `merkle.Proof` From f441fb17cc805a8f12631d12b8dce67d905aca54 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 11 Jun 2020 19:09:07 -0400 Subject: [PATCH 16/19] do not hardcode proofspecs in 23-commitment --- x/ibc/02-client/exported/exported.go | 3 +- x/ibc/23-commitment/exported/exported.go | 6 +- x/ibc/23-commitment/types/key_path.go | 12 ++ x/ibc/23-commitment/types/merkle.go | 225 +++++++++++++++++++---- 4 files changed, 206 insertions(+), 40 deletions(-) diff --git a/x/ibc/02-client/exported/exported.go b/x/ibc/02-client/exported/exported.go index fcac8adfa133..b733815bad9d 100644 --- a/x/ibc/02-client/exported/exported.go +++ b/x/ibc/02-client/exported/exported.go @@ -3,6 +3,7 @@ package exported import ( "encoding/json" + ics23 "github.com/confio/ics23/go" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -21,7 +22,7 @@ type ClientState interface { GetLatestHeight() uint64 IsFrozen() bool Validate() error - GetProofSpecs() []string + GetProofSpecs() []*ics23.ProofSpec // State verification functions diff --git a/x/ibc/23-commitment/exported/exported.go b/x/ibc/23-commitment/exported/exported.go index 53de156c459c..e476c6ab431c 100644 --- a/x/ibc/23-commitment/exported/exported.go +++ b/x/ibc/23-commitment/exported/exported.go @@ -1,5 +1,7 @@ package exported +import ics23 "github.com/confio/ics23/go" + // ICS 023 Types Implementation // // This file includes types defined under @@ -39,8 +41,8 @@ type Path interface { // Proofs includes key but value is provided dynamically at the verification time. type Proof interface { GetCommitmentType() Type - VerifyMembership([]string, Root, Path, []byte) error - VerifyNonMembership([]string, Root, Path) error + VerifyMembership([]*ics23.ProofSpec, Root, Path, []byte) error + VerifyNonMembership([]*ics23.ProofSpec, Root, Path) error IsEmpty() bool ValidateBasic() error diff --git a/x/ibc/23-commitment/types/key_path.go b/x/ibc/23-commitment/types/key_path.go index 4d79e2e726e2..7482d3eb0beb 100644 --- a/x/ibc/23-commitment/types/key_path.go +++ b/x/ibc/23-commitment/types/key_path.go @@ -26,3 +26,15 @@ func (pth *KeyPath) String() string { } return res } + +// GetKey returns the bytes representation of key at given index +// Passing in a positive index return the key at index in forward order +// from the highest key to the lowest key +// Passing in a negative index will return the key at index in reverse order +// from the lowest key to the highest key. This is the order for proof verification, +// since we prove lowest key first before moving to key of higher subtrees +func (pth *KeyPath) GetKey(i int) []byte { + total := len(pth.Keys) + index := (total + i) % total + return []byte(pth.Keys[index].name) +} diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index c979392904d8..7e196e59ed3d 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -1,10 +1,10 @@ package types import ( + "bytes" "net/url" - "github.com/cosmos/cosmos-sdk/store/rootmulti" - storetypes "github.com/cosmos/cosmos-sdk/store/types" + 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" @@ -13,7 +13,7 @@ import ( ) // var representing the proofspecs for a SDK chain -var sdkSpecs = []string{storetypes.ProofOpIAVLCommitment, storetypes.ProofOpSimpleMerkleCommitment} +var sdkSpecs = []*ics23.ProofSpec{ics23.IavlSpec, ics23.TendermintSpec} // ICS 023 Merkle Types Implementation // @@ -24,7 +24,7 @@ var sdkSpecs = []string{storetypes.ProofOpIAVLCommitment, storetypes.ProofOpSimp var _ exported.Root = (*MerkleRoot)(nil) // GetSDKSpecs is a getter function for the proofspecs of an sdk chain -func GetSDKSpecs() []string { +func GetSDKSpecs() []*ics23.ProofSpec { return sdkSpecs } @@ -137,79 +137,232 @@ func (MerkleProof) GetCommitmentType() exported.Type { } // VerifyMembership verifies the membership pf a merkle proof against the given root, path, and value. -func (proof MerkleProof) VerifyMembership(specs []string, root exported.Root, path exported.Path, value []byte) error { +func (proof MerkleProof) VerifyMembership(specs []*ics23.ProofSpec, root exported.Root, path exported.Path, value []byte) error { if err := proof.validateVerificationArgs(specs, root); err != nil { return err } // VerifyMembership specific argument validation - if path == nil || path.IsEmpty() { - return sdkerrors.Wrap(ErrInvalidProof, "empty path") + mpath, ok := path.(MerklePath) + if !ok { + return sdkerrors.Wrapf(ErrInvalidProof, "path %v is not of type MerkleProof", path) + } + if len(mpath.KeyPath.Keys) != len(specs) { + return sdkerrors.Wrapf(ErrInvalidProof, "path length %d not same as proof %d", + len(mpath.KeyPath.Keys), len(specs)) } if len(value) == 0 { return sdkerrors.Wrap(ErrInvalidProof, "empty value in membership proof") } - runtime := rootmulti.DefaultProofRuntime() - return runtime.VerifyValue(proof.Proof, root.GetHash(), path.String(), value) + // Convert Proof to []CommitmentProof + proofs, err := convertProofs(proof) + if err != nil { + return err + } + + // Since every proof in chain is a membership proof we can chain from index 0 + if err := verifyChainedMembershipProof(root.GetHash(), specs, proofs, mpath.KeyPath, value, 0); err != nil { + return err + } + return nil } // VerifyNonMembership verifies the absence of a merkle proof against the given root and path. -func (proof MerkleProof) VerifyNonMembership(specs []string, root exported.Root, path exported.Path) error { +// VerifyNonMembership verifies a chained proof where the absence of a given path is proven +// at the lowest subtree and then each subtree's inclusion is proved up to the final root. +func (proof MerkleProof) VerifyNonMembership(specs []*ics23.ProofSpec, root exported.Root, path exported.Path) error { if err := proof.validateVerificationArgs(specs, root); err != nil { return err } // VerifyNonMembership specific argument validation - if path == nil || path.IsEmpty() { - return sdkerrors.Wrap(ErrInvalidProof, "empty path") + mpath, ok := path.(MerklePath) + if !ok { + return sdkerrors.Wrapf(ErrInvalidProof, "path %v is not of type MerkleProof", path) + } + if len(mpath.KeyPath.Keys) != len(specs) { + return sdkerrors.Wrapf(ErrInvalidProof, "path length %d not same as proof %d", + len(mpath.KeyPath.Keys), len(specs)) } - runtime := rootmulti.DefaultProofRuntime() - return runtime.VerifyAbsence(proof.Proof, root.GetHash(), path.String()) + // Convert Proof to []CommitmentProof + proofs, err := convertProofs(proof) + if err != nil { + return err + } + + // VerifyNonMembership will verify the absence of key in lowest subtree, and then chain inclusion proofs + // of all subroots up to final root + subroot, err := proofs[0].Calculate() + if err != nil { + sdkerrors.Wrapf(ErrInvalidProof, "could not calculate root for proof index 0. %v", err) + } + key := mpath.KeyPath.GetKey(-1) + if ok := ics23.VerifyNonMembership(specs[0], subroot, proofs[0], key); !ok { + return sdkerrors.Wrapf(ErrInvalidProof, "could not verify absence of key %s", string(key)) + } + + // Verify chained membership proof starting from index 1 with value = subroot + if err := verifyChainedMembershipProof(root.GetHash(), specs, proofs, mpath.KeyPath, subroot, 1); err != nil { + return err + } + return nil } // BatchVerifyMembership verifies a group of key value pairs against the given root +// NOTE: All items must be part of a batch proof in the first chained proof, i.e. items must all be part of smallest subtree in the chained proof +// NOTE: The path passed in must be the path from the root to the smallest subtree in the chained proof // NOTE: Untested -func (proof MerkleProof) BatchVerifyMembership(specs []string, root exported.Root, items map[string][]byte) error { +func (proof MerkleProof) BatchVerifyMembership(specs []*ics23.ProofSpec, root exported.Root, path exported.Path, items map[string][]byte) error { if err := proof.validateVerificationArgs(specs, root); err != nil { return err } - // Verify each item separately against same proof - runtime := rootmulti.DefaultProofRuntime() - for path, value := range items { - // check value is not empty - if len(value) == 0 { - return sdkerrors.Wrap(ErrInvalidProof, "empty value in batched membership proof") + // Convert Proof to []CommitmentProof + proofs, err := convertProofs(proof) + if err != nil { + return err + } + + // VerifyNonMembership will verify the absence of key in lowest subtree, and then chain inclusion proofs + // of all subroots up to final root + subroot, err := proofs[0].Calculate() + if err != nil { + sdkerrors.Wrapf(ErrInvalidProof, "could not calculate root for proof index 0. %v", err) + } + if ok := ics23.BatchVerifyMembership(specs[0], subroot, proofs[0], items); !ok { + return sdkerrors.Wrapf(ErrInvalidProof, "could not verify batch items") + } + + // BatchVerifyMembership specific argument validation + // Path must only be defined if this is a chained proof + if len(specs) > 1 { + mpath, ok := path.(MerklePath) + if !ok { + return sdkerrors.Wrapf(ErrInvalidProof, "path %v is not of type MerkleProof", path) + } + // path length should be one less than specs, since lowest proof keys are in items + if len(mpath.KeyPath.Keys) != len(specs)-1 { + return sdkerrors.Wrapf(ErrInvalidProof, "path length %d not same as proof %d", + len(mpath.KeyPath.Keys), len(specs)) } - if err := runtime.VerifyValue(proof.Proof, root.GetHash(), path, value); err != nil { - return sdkerrors.Wrapf(ErrInvalidProof, "verification failed for path: %s, value: %x. Error: %v", - path, value, err) + // Since BatchedProof path does not include lowest subtree, exclude first proof from specs and proofs and start + // chaining at index 0 + if err := verifyChainedMembershipProof(root.GetHash(), specs[1:], proofs[1:], mpath.KeyPath, subroot, 0); err != nil { + return err } + } else if !bytes.Equal(root.GetHash(), subroot) { + // Since we are not chaining proofs, we must check first subroot equals given root + return sdkerrors.Wrapf(ErrInvalidProof, "batched proof did not commit to expected root: %X, got: %X", root.GetHash(), subroot) } + return nil } // BatchVerifyNonMembership verifies absence of a group of keys against the given root +// NOTE: All items must be part of a batch proof in the first chained proof, i.e. items must all be part of smallest subtree in the chained proof +// NOTE: The path passed in must be the path from the root to the smallest subtree in the chained proof // NOTE: Untested -func (proof MerkleProof) BatchVerifyNonMembership(specs []string, root exported.Root, items []string) error { +func (proof MerkleProof) BatchVerifyNonMembership(specs []*ics23.ProofSpec, root exported.Root, path exported.Path, items [][]byte) error { if err := proof.validateVerificationArgs(specs, root); err != nil { return err } - // Verify each item separately against same proof - runtime := rootmulti.DefaultProofRuntime() - for _, path := range items { - if err := runtime.VerifyAbsence(proof.Proof, root.GetHash(), path); err != nil { - return sdkerrors.Wrapf(ErrInvalidProof, "absence verification failed for path: %s. Error: %v", - path, err) + // Convert Proof to []CommitmentProof + proofs, err := convertProofs(proof) + if err != nil { + return err + } + + // VerifyNonMembership will verify the absence of key in lowest subtree, and then chain inclusion proofs + // of all subroots up to final root + subroot, err := proofs[0].Calculate() + if err != nil { + sdkerrors.Wrapf(ErrInvalidProof, "could not calculate root for proof index 0. %v", err) + } + if ok := ics23.BatchVerifyNonMembership(specs[0], subroot, proofs[0], items); !ok { + return sdkerrors.Wrapf(ErrInvalidProof, "could not verify batch items") + } + + // BatchVerifyNonMembership specific argument validation + // Path must only be defined if this is a chained proof + if len(specs) > 1 { + mpath, ok := path.(MerklePath) + if !ok { + return sdkerrors.Wrapf(ErrInvalidProof, "path %v is not of type MerkleProof", path) + } + // path length should be one less than specs, since lowest proof keys are in items + if len(mpath.KeyPath.Keys) != len(specs)-1 { + return sdkerrors.Wrapf(ErrInvalidProof, "path length %d not same as proof %d", + len(mpath.KeyPath.Keys), len(specs)) + } + + // Since BatchedProof path does not include lowest subtree, exclude first proof from specs and proofs and start + // chaining at index 0 + if err := verifyChainedMembershipProof(root.GetHash(), specs[1:], proofs[1:], mpath.KeyPath, subroot, 0); err != nil { + return err + } + } else if !bytes.Equal(root.GetHash(), subroot) { + // Since we are not chaining proofs, we must check first subroot equals given root + return sdkerrors.Wrapf(ErrInvalidProof, "batched proof did not commit to expected root: %X, got: %X", root.GetHash(), subroot) + } + + return nil +} + +// verifyChainedMembershipProof takes a list of proofs and specs and verifies each proof sequentially ensuring that the value is committed to +// by first proof and each subsequent subroot is committed to by the next subroot and checking that the final calculated root is equal to the given roothash. +// The proofs and specs are passed in from lowest subtree to the highest subtree, but the keys are passed in from highest subtree to lowest. +// The index specifies what index to start chaining the membership proofs, this is useful since the lowest proof may not be a membership proof, thus we +// will want to start the membership proof chaining from index 1 with value being the lowest subroot +func verifyChainedMembershipProof(root []byte, specs []*ics23.ProofSpec, proofs []*ics23.CommitmentProof, keys KeyPath, value []byte, index int) error { + var ( + subroot []byte + err error + ) + // Initialize subroot to value since the proofs list may be empty. + // This may happen if this call is verifying intermediate proofs after the lowest proof has been executed. + // In this case, there may be no intermediate proofs to verify and we just check that lowest proof root equals final root + subroot = value + for i := index; i < len(proofs); i++ { + subroot, err = proofs[i].Calculate() + if err != nil { + return sdkerrors.Wrapf(ErrInvalidProof, "could not calculate proof root at index %d. %v", i, err) } + // Since keys are passed in from highest to lowest, we must grab their indices in reverse order + // from the proofs and specs which are lowest to highest + key := keys.GetKey(-1 * (i + 1)) + if ok := ics23.VerifyMembership(specs[i], subroot, proofs[i], key, value); !ok { + return sdkerrors.Wrapf(ErrInvalidProof, "chained membership proof failed to verify membership of value: %X in subroot %X at index %d", + value, subroot, i) + } + // Set value to subroot so that we verify next proof in chain commits to this subroot + value = subroot + } + // Check that chained proof root equals passed-in root + if !bytes.Equal(root, subroot) { + return sdkerrors.Wrapf(ErrInvalidProof, "proof did not commit to expected root: %X, got: %X", root, subroot) } return nil } +// convertProofs converts a MerkleProof into []*ics23.CommitmentProof +func convertProofs(mproof MerkleProof) ([]*ics23.CommitmentProof, error) { + // Unmarshal all proof ops to CommitmentProof + proofs := make([]*ics23.CommitmentProof, len(mproof.Proof.Ops)) + for i, op := range mproof.Proof.Ops { + var p ics23.CommitmentProof + err := p.Unmarshal(op.Data) + if err != nil { + return nil, sdkerrors.Wrapf(ErrInvalidMerkleProof, "could not unmarshal proof op into CommitmentProof at index: %d", i) + } + proofs[i] = &p + } + return proofs, nil +} + // IsEmpty returns true if the root is empty func (proof MerkleProof) IsEmpty() bool { return proof.Proof.Equal(nil) || proof.Equal(MerkleProof{}) || proof.Proof.Equal(nil) || proof.Proof.Equal(merkle.Proof{}) @@ -224,7 +377,7 @@ func (proof MerkleProof) ValidateBasic() error { } // validateVerificationArgs verifies the proof arguments are valid -func (proof MerkleProof) validateVerificationArgs(specs []string, root exported.Root) error { +func (proof MerkleProof) validateVerificationArgs(specs []*ics23.ProofSpec, root exported.Root) error { if proof.IsEmpty() || root == nil || root.IsEmpty() { return sdkerrors.Wrap(ErrInvalidMerkleProof, "empty params or proof") } @@ -235,11 +388,9 @@ func (proof MerkleProof) validateVerificationArgs(specs []string, root exported. len(specs), len(proof.Proof.Ops)) } - for i, op := range proof.Proof.Ops { - if op.Type != specs[i] { - return sdkerrors.Wrapf(ErrInvalidMerkleProof, - "proof does not match expected format at position %d, expected: %s, got: %s", - i, specs[i], op.Type) + for i, spec := range specs { + if spec == nil { + return sdkerrors.Wrapf(ErrInvalidProof, "spec at position %d is nil", i) } } return nil From 3731d8a9655e40da8cb1c8551821bcfa070df8fc Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 11 Jun 2020 19:18:06 -0400 Subject: [PATCH 17/19] fix client modules --- x/ibc/07-tendermint/client/cli/tx.go | 13 ++++++++---- x/ibc/07-tendermint/client/rest/rest.go | 19 +++++++++--------- x/ibc/07-tendermint/types/client_state.go | 14 ++++++------- x/ibc/07-tendermint/types/msgs.go | 24 +++++++++++------------ x/ibc/09-localhost/types/client_state.go | 3 ++- 5 files changed, 40 insertions(+), 33 deletions(-) diff --git a/x/ibc/07-tendermint/client/cli/tx.go b/x/ibc/07-tendermint/client/cli/tx.go index be05bf0c5c63..899213e268be 100644 --- a/x/ibc/07-tendermint/client/cli/tx.go +++ b/x/ibc/07-tendermint/client/cli/tx.go @@ -8,6 +8,7 @@ import ( "strings" "time" + ics23 "github.com/confio/ics23/go" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -65,7 +66,7 @@ func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { var ( trustLevel tmmath.Fraction - specs []string + specs []*ics23.ProofSpec err error ) @@ -97,10 +98,14 @@ func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { spc := viper.GetString(flagProofSpecs) - if spc == "default" { + // Currently supports SDK chain or simple kvstore tendermint chain + switch spc { + case "default": specs = commitmenttypes.GetSDKSpecs() - } else { - specs = strings.Split(spc, ",") + case "simple": + specs = []*ics23.ProofSpec{ics23.TendermintSpec} + default: + return fmt.Errorf("Proof Spec: %s not supported", spc) } msg := ibctmtypes.NewMsgCreateClient( diff --git a/x/ibc/07-tendermint/client/rest/rest.go b/x/ibc/07-tendermint/client/rest/rest.go index 510ac501a157..8e428ff03c4a 100644 --- a/x/ibc/07-tendermint/client/rest/rest.go +++ b/x/ibc/07-tendermint/client/rest/rest.go @@ -3,6 +3,7 @@ package rest import ( "time" + ics23 "github.com/confio/ics23/go" "github.com/gorilla/mux" tmmath "github.com/tendermint/tendermint/libs/math" @@ -26,15 +27,15 @@ func RegisterRoutes(clientCtx client.Context, r *mux.Router, queryRoute string) // CreateClientReq defines the properties of a create client request's body. type CreateClientReq struct { - BaseReq rest.BaseReq `json:"base_req" yaml:"base_req"` - ClientID string `json:"client_id" yaml:"client_id"` - ChainID string `json:"chain_id" yaml:"chain_id"` - Header ibctmtypes.Header `json:"header" yaml:"header"` - TrustLevel tmmath.Fraction `json:"trust_level" yaml:"trust_level"` - TrustingPeriod time.Duration `json:"trusting_period" yaml:"trusting_period"` - UnbondingPeriod time.Duration `json:"unbonding_period" yaml:"unbonding_period"` - MaxClockDrift time.Duration `json:"max_clock_drift" yaml:"max_clock_drift"` - ProofSpecs []string `json:"proof_specs" yaml:"proof_specs"` + BaseReq rest.BaseReq `json:"base_req" yaml:"base_req"` + ClientID string `json:"client_id" yaml:"client_id"` + ChainID string `json:"chain_id" yaml:"chain_id"` + Header ibctmtypes.Header `json:"header" yaml:"header"` + TrustLevel tmmath.Fraction `json:"trust_level" yaml:"trust_level"` + TrustingPeriod time.Duration `json:"trusting_period" yaml:"trusting_period"` + UnbondingPeriod time.Duration `json:"unbonding_period" yaml:"unbonding_period"` + MaxClockDrift time.Duration `json:"max_clock_drift" yaml:"max_clock_drift"` + ProofSpecs []*ics23.ProofSpec `json:"proof_specs" yaml:"proof_specs"` } // UpdateClientReq defines the properties of a update client request's body. diff --git a/x/ibc/07-tendermint/types/client_state.go b/x/ibc/07-tendermint/types/client_state.go index cc9d2fe7d70a..eed2977637e1 100644 --- a/x/ibc/07-tendermint/types/client_state.go +++ b/x/ibc/07-tendermint/types/client_state.go @@ -1,9 +1,9 @@ package types import ( - "strings" "time" + ics23 "github.com/confio/ics23/go" tmmath "github.com/tendermint/tendermint/libs/math" lite "github.com/tendermint/tendermint/lite2" @@ -48,7 +48,7 @@ type ClientState struct { // Last Header that was stored by client LastHeader Header `json:"last_header" yaml:"last_header"` - ProofSpecs []string `json:"proof_specs" yaml:"proof_specs"` + ProofSpecs []*ics23.ProofSpec `json:"proof_specs" yaml:"proof_specs"` } // InitializeFromMsg creates a tendermint client state from a CreateClientMsg @@ -65,7 +65,7 @@ func InitializeFromMsg(msg MsgCreateClient) (ClientState, error) { func Initialize( id string, trustLevel tmmath.Fraction, trustingPeriod, ubdPeriod, maxClockDrift time.Duration, - header Header, specs []string, + header Header, specs []*ics23.ProofSpec, ) (ClientState, error) { clientState := NewClientState(id, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs) @@ -76,7 +76,7 @@ func Initialize( func NewClientState( id string, trustLevel tmmath.Fraction, trustingPeriod, ubdPeriod, maxClockDrift time.Duration, - header Header, specs []string, + header Header, specs []*ics23.ProofSpec, ) ClientState { return ClientState{ ID: id, @@ -151,8 +151,8 @@ func (cs ClientState) Validate() error { return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof specs cannot be nil for tm client") } for _, spec := range cs.ProofSpecs { - if !strings.HasPrefix(spec, "ics23:") { - return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec must be an ics23 spec") + if spec == nil { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec cannot be nil") } } @@ -161,7 +161,7 @@ func (cs ClientState) Validate() error { // GetProofSpecs returns the format the client expects for proof verification // as a string array specifying the proof type for each position in chained proof -func (cs ClientState) GetProofSpecs() []string { +func (cs ClientState) GetProofSpecs() []*ics23.ProofSpec { return cs.ProofSpecs } diff --git a/x/ibc/07-tendermint/types/msgs.go b/x/ibc/07-tendermint/types/msgs.go index d9a7e77912c2..6cc16c234781 100644 --- a/x/ibc/07-tendermint/types/msgs.go +++ b/x/ibc/07-tendermint/types/msgs.go @@ -1,9 +1,9 @@ package types import ( - "strings" "time" + ics23 "github.com/confio/ics23/go" tmmath "github.com/tendermint/tendermint/libs/math" lite "github.com/tendermint/tendermint/lite2" @@ -31,14 +31,14 @@ var ( // MsgCreateClient defines a message to create an IBC client type MsgCreateClient struct { - ClientID string `json:"client_id" yaml:"client_id"` - Header Header `json:"header" yaml:"header"` - TrustLevel tmmath.Fraction `json:"trust_level" yaml:"trust_level"` - TrustingPeriod time.Duration `json:"trusting_period" yaml:"trusting_period"` - UnbondingPeriod time.Duration `json:"unbonding_period" yaml:"unbonding_period"` - MaxClockDrift time.Duration `json:"max_clock_drift" yaml:"max_clock_drift"` - ProofSpecs []string `json:"proof_specs" yaml:"proof_specs"` - Signer sdk.AccAddress `json:"address" yaml:"address"` + ClientID string `json:"client_id" yaml:"client_id"` + Header Header `json:"header" yaml:"header"` + TrustLevel tmmath.Fraction `json:"trust_level" yaml:"trust_level"` + TrustingPeriod time.Duration `json:"trusting_period" yaml:"trusting_period"` + UnbondingPeriod time.Duration `json:"unbonding_period" yaml:"unbonding_period"` + MaxClockDrift time.Duration `json:"max_clock_drift" yaml:"max_clock_drift"` + ProofSpecs []*ics23.ProofSpec `json:"proof_specs" yaml:"proof_specs"` + Signer sdk.AccAddress `json:"address" yaml:"address"` } // this is a constant to satisfy the linter @@ -53,7 +53,7 @@ func (msg MsgCreateClient) ProtoMessage() {} func NewMsgCreateClient( id string, header Header, trustLevel tmmath.Fraction, trustingPeriod, unbondingPeriod, maxClockDrift time.Duration, - specs []string, signer sdk.AccAddress, + specs []*ics23.ProofSpec, signer sdk.AccAddress, ) MsgCreateClient { return MsgCreateClient{ @@ -110,8 +110,8 @@ func (msg MsgCreateClient) ValidateBasic() error { return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof specs cannot be nil") } for _, spec := range msg.ProofSpecs { - if !strings.HasPrefix(spec, "ics23:") { - return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec must be an ics23 spec") + if spec == nil { + return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof spec cannot be nil") } } return host.ClientIdentifierValidator(msg.ClientID) diff --git a/x/ibc/09-localhost/types/client_state.go b/x/ibc/09-localhost/types/client_state.go index 54fb4c86ad86..22e0f84a2a7d 100644 --- a/x/ibc/09-localhost/types/client_state.go +++ b/x/ibc/09-localhost/types/client_state.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" + ics23 "github.com/confio/ics23/go" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -75,7 +76,7 @@ func (cs ClientState) Validate() error { } // GetProofSpecs returns nil since localhost does not have to verify proofs -func (cs ClientState) GetProofSpecs() []string { +func (cs ClientState) GetProofSpecs() []*ics23.ProofSpec { return nil } From 3241a2dcf016e66e51f35fc03860baa63750b748 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 11 Jun 2020 19:20:11 -0400 Subject: [PATCH 18/19] fix tests --- x/ibc/07-tendermint/types/msgs_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/ibc/07-tendermint/types/msgs_test.go b/x/ibc/07-tendermint/types/msgs_test.go index dcb5713ed710..cd354ea0b2b3 100644 --- a/x/ibc/07-tendermint/types/msgs_test.go +++ b/x/ibc/07-tendermint/types/msgs_test.go @@ -1,6 +1,7 @@ package types_test import ( + ics23 "github.com/confio/ics23/go" "github.com/tendermint/tendermint/crypto/secp256k1" "github.com/tendermint/tendermint/libs/math" lite "github.com/tendermint/tendermint/lite2" @@ -31,7 +32,7 @@ func (suite *TendermintTestSuite) TestMsgCreateClientValidateBasic() { {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), nil), false, "Empty address passed"}, {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, ibctmtypes.Header{}, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), signer), false, "nil header"}, {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, invalidHeader, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, commitmenttypes.GetSDKSpecs(), signer), false, "invalid header"}, - {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, []string{"", "spec"}, signer), false, "invalid proof specs"}, + {ibctmtypes.NewMsgCreateClient(exported.ClientTypeTendermint, suite.header, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, []*ics23.ProofSpec{nil}, signer), false, "invalid proof specs"}, } for i, tc := range cases { From 0737e09a10b632b0a6315aa45bef345506a16df0 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 11 Jun 2020 19:32:19 -0400 Subject: [PATCH 19/19] appease linter --- x/ibc/07-tendermint/client/cli/tx.go | 2 +- x/ibc/23-commitment/types/key_path.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/ibc/07-tendermint/client/cli/tx.go b/x/ibc/07-tendermint/client/cli/tx.go index 899213e268be..f22b3d91440c 100644 --- a/x/ibc/07-tendermint/client/cli/tx.go +++ b/x/ibc/07-tendermint/client/cli/tx.go @@ -105,7 +105,7 @@ func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command { case "simple": specs = []*ics23.ProofSpec{ics23.TendermintSpec} default: - return fmt.Errorf("Proof Spec: %s not supported", spc) + return fmt.Errorf("proof Spec: %s not supported", spc) } msg := ibctmtypes.NewMsgCreateClient( diff --git a/x/ibc/23-commitment/types/key_path.go b/x/ibc/23-commitment/types/key_path.go index 7482d3eb0beb..2dc4c47c5b4f 100644 --- a/x/ibc/23-commitment/types/key_path.go +++ b/x/ibc/23-commitment/types/key_path.go @@ -36,5 +36,5 @@ func (pth *KeyPath) String() string { func (pth *KeyPath) GetKey(i int) []byte { total := len(pth.Keys) index := (total + i) % total - return []byte(pth.Keys[index].name) + return pth.Keys[index].name }