Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce ProofSpecs in 23-Commitment #6374

Merged
merged 26 commits into from
Jun 12, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3d2795c
enforce spec ordering
AdityaSripal Jun 8, 2020
a716e20
modify clients to pass in specs to verify functions
AdityaSripal Jun 9, 2020
3a5bc6a
start fixing tests
AdityaSripal Jun 9, 2020
1723648
Apply suggestions from code review
fedekunze Jun 9, 2020
e0d574d
Merge branch 'aditya/ibc-commitment' of github.com:cosmos/cosmos-sdk …
AdityaSripal Jun 9, 2020
74dd904
enforce spec length and proof length match
AdityaSripal Jun 9, 2020
e2f3537
fix all tests
AdityaSripal Jun 9, 2020
57e0a84
add argument to constructor:
AdityaSripal Jun 9, 2020
5a9782d
Merge branch 'master' into aditya/ibc-commitment
AdityaSripal Jun 9, 2020
6d759cf
fixed msg client and tests
AdityaSripal Jun 9, 2020
8e8bfd8
appease linter
AdityaSripal Jun 9, 2020
761fa7b
Apply suggestions from code review
AdityaSripal Jun 10, 2020
4bb1e56
finish fixes from review
AdityaSripal Jun 10, 2020
bd71a76
add back proof-specific checks
AdityaSripal Jun 10, 2020
99c8700
fix merge
AdityaSripal Jun 10, 2020
10c978a
Apply suggestions from code review
fedekunze Jun 10, 2020
3e5b687
Merge branch 'master' of github.com:cosmos/cosmos-sdk into aditya/ibc…
AdityaSripal Jun 10, 2020
f170cf9
more robust proof spec checks
AdityaSripal Jun 10, 2020
f414ad8
Merge branch 'aditya/ibc-commitment' of github.com:cosmos/cosmos-sdk …
AdityaSripal Jun 10, 2020
236ab96
add CHANGELOG entries
AdityaSripal Jun 10, 2020
92e8315
Merge branch 'master' into aditya/ibc-commitment
fedekunze Jun 11, 2020
f441fb1
do not hardcode proofspecs in 23-commitment
AdityaSripal Jun 11, 2020
3731d8a
fix client modules
AdityaSripal Jun 11, 2020
3241a2d
fix tests
AdityaSripal Jun 11, 2020
f6749b5
fix merge
AdityaSripal Jun 11, 2020
0737e09
appease linter
AdityaSripal Jun 11, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x/ibc/02-client/exported/exported.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type ClientState interface {
GetLatestHeight() uint64
IsFrozen() bool
Validate() error
GetProofSpecs() []string

// State verification functions

Expand Down
31 changes: 20 additions & 11 deletions x/ibc/07-tendermint/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,16 @@ 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
func InitializeFromMsg(msg MsgCreateClient) (ClientState, error) {
return Initialize(
msg.GetClientID(), msg.TrustLevel,
msg.TrustingPeriod, msg.UnbondingPeriod, msg.MaxClockDrift,
msg.Header,
msg.Header, msg.ProofSpecs,
)
}

Expand All @@ -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 {
Expand All @@ -73,15 +75,15 @@ func Initialize(
)
}

clientState := NewClientState(id, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header)
clientState := NewClientState(id, trustLevel, trustingPeriod, ubdPeriod, maxClockDrift, header, specs)
return clientState, nil
}

// NewClientState creates a new ClientState instance
func NewClientState(
id string, trustLevel tmmath.Fraction,
trustingPeriod, ubdPeriod, maxClockDrift time.Duration,
header Header,
header Header, specs []string,
) ClientState {
return ClientState{
ID: id,
Expand All @@ -91,6 +93,7 @@ func NewClientState(
MaxClockDrift: maxClockDrift,
LastHeader: header,
FrozenHeight: 0,
ProofSpecs: specs,
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -147,6 +150,12 @@ func (cs ClientState) Validate() error {
return cs.LastHeader.ValidateBasic(cs.GetChainID())
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
}

// 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(
Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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())
}

Expand Down
1 change: 1 addition & 0 deletions x/ibc/07-tendermint/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
Signer sdk.AccAddress `json:"address" yaml:"address"`
}

Expand Down
5 changes: 5 additions & 0 deletions x/ibc/09-localhost/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions x/ibc/23-commitment/exported/exported.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 70 additions & 2 deletions x/ibc/23-commitment/types/merkle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
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
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
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
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
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{})
Expand Down
6 changes: 4 additions & 2 deletions x/ibc/23-commitment/types/merkle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
abci "github.com/tendermint/tendermint/abci/types"
)

var specs = []string{"ics23:iavl", "simple:v"}
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved

func (suite *MerkleTestSuite) TestVerifyMembership() {
suite.iavlStore.Set([]byte("MYKEY"), []byte("MYVALUE"))
cid := suite.store.Commit()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
65 changes: 0 additions & 65 deletions x/ibc/23-commitment/verify.go

This file was deleted.