From 2dfd27522258d13b824405c914f1ea0d0d51370a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 14 Nov 2022 17:32:15 +0100 Subject: [PATCH 1/9] build(deps): bump golangci/golangci-lint-action from 3.3.0 to 3.3.1 (#2738) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.3.0 to 3.3.1. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](https://github.com/golangci/golangci-lint-action/compare/v3.3.0...v3.3.1) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci.yml b/.github/workflows/golangci.yml index bd865fcc2b8..e032640dd2d 100644 --- a/.github/workflows/golangci.yml +++ b/.github/workflows/golangci.yml @@ -20,7 +20,7 @@ jobs: go-version: 1.18 - uses: actions/checkout@v3 - name: golangci-lint - uses: golangci/golangci-lint-action@v3.3.0 + uses: golangci/golangci-lint-action@v3.3.1 with: version: latest args: --timeout 5m \ No newline at end of file From d46f59bf5621cd1d30636bdb2d185b7cb4dab2c3 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 14 Nov 2022 16:59:55 +0000 Subject: [PATCH 2/9] Compare signature data in misbehaviours only if path differs (#2744) fix(statemachine)!: (06-solomachine) [#2744] Misbehaviour.ValidateBasic() now only enforces that signature data does not match when the signature paths are different. --- CHANGELOG.md | 1 + modules/light-clients/06-solomachine/misbehaviour.go | 7 ++++--- .../light-clients/06-solomachine/misbehaviour_test.go | 10 +++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76a3cc30065..e413de28dbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking * (light-clients/07-tendermint) [\#2554](https://github.com/cosmos/ibc-go/pull/2554) Forbid negative values for `TrustingPeriod`, `UnbondingPeriod` and `MaxClockDrift` (as specified in ICS-07). +* (06-solomachine) [\#2744](https://github.com/cosmos/ibc-go/pull/2744) `Misbehaviour.ValidateBasic()` now only enforces that signature data does not match when the signature paths are different. ### Improvements diff --git a/modules/light-clients/06-solomachine/misbehaviour.go b/modules/light-clients/06-solomachine/misbehaviour.go index 9e17a09010b..76294641998 100644 --- a/modules/light-clients/06-solomachine/misbehaviour.go +++ b/modules/light-clients/06-solomachine/misbehaviour.go @@ -40,13 +40,14 @@ func (misbehaviour Misbehaviour) ValidateBasic() error { return sdkerrors.Wrap(err, "signature two failed basic validation") } - // misbehaviour signatures cannot be identical + // misbehaviour signatures cannot be identical. if bytes.Equal(misbehaviour.SignatureOne.Signature, misbehaviour.SignatureTwo.Signature) { return sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "misbehaviour signatures cannot be equal") } - // message data signed cannot be identical - if bytes.Equal(misbehaviour.SignatureOne.Data, misbehaviour.SignatureTwo.Data) { + // message data signed cannot be identical if both paths are the same. + if bytes.Equal(misbehaviour.SignatureOne.Path, misbehaviour.SignatureTwo.Path) && + bytes.Equal(misbehaviour.SignatureOne.Data, misbehaviour.SignatureTwo.Data) { return sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "misbehaviour signature data must be signed over different messages") } diff --git a/modules/light-clients/06-solomachine/misbehaviour_test.go b/modules/light-clients/06-solomachine/misbehaviour_test.go index 0a27eba3466..f8b6afe764f 100644 --- a/modules/light-clients/06-solomachine/misbehaviour_test.go +++ b/modules/light-clients/06-solomachine/misbehaviour_test.go @@ -76,10 +76,18 @@ func (suite *SoloMachineTestSuite) TestMisbehaviourValidateBasic() { false, }, { - "data signed is identical", + "data signed is identical but path differs", func(misbehaviour *solomachine.Misbehaviour) { misbehaviour.SignatureTwo.Data = misbehaviour.SignatureOne.Data }, + true, + }, + { + "data signed and path are identical", + func(misbehaviour *solomachine.Misbehaviour) { + misbehaviour.SignatureTwo.Path = misbehaviour.SignatureOne.Path + misbehaviour.SignatureTwo.Data = misbehaviour.SignatureOne.Data + }, false, }, { From 68ba37553336b39cfba605ca3f068d776b42b60b Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 14 Nov 2022 17:15:23 +0000 Subject: [PATCH 3/9] chore: rename all occurances of smtypes and solomachinetypes import aliases to solomachine (#2743) --- modules/core/02-client/keeper/client_test.go | 4 +-- modules/core/02-client/keeper/keeper_test.go | 4 +-- modules/core/02-client/legacy/v100/store.go | 8 ++--- modules/core/02-client/types/genesis_test.go | 4 +-- modules/core/02-client/types/msgs_test.go | 10 +++---- modules/core/types/codec.go | 4 +-- .../07-tendermint/misbehaviour_handle_test.go | 6 ++-- testing/solomachine.go | 30 +++++++++---------- 8 files changed, 35 insertions(+), 35 deletions(-) diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index 5f3ce33dc55..12acddb4ff1 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -11,7 +11,7 @@ import ( clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v6/modules/core/23-commitment/types" "github.com/cosmos/ibc-go/v6/modules/core/exported" - solomachinetypes "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" + solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v6/testing" ) @@ -23,7 +23,7 @@ func (suite *KeeperTestSuite) TestCreateClient() { expPass bool }{ {"success", ibctm.NewClientState(testChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), true}, - {"client type not supported", solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}), false}, + {"client type not supported", solomachine.NewClientState(0, &solomachine.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}), false}, } for i, tc := range cases { diff --git a/modules/core/02-client/keeper/keeper_test.go b/modules/core/02-client/keeper/keeper_test.go index a97a599c049..f02da108b20 100644 --- a/modules/core/02-client/keeper/keeper_test.go +++ b/modules/core/02-client/keeper/keeper_test.go @@ -19,7 +19,7 @@ import ( "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v6/modules/core/23-commitment/types" "github.com/cosmos/ibc-go/v6/modules/core/exported" - solomachinetypes "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" + solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v6/testing" ibctestingmock "github.com/cosmos/ibc-go/v6/testing/mock" @@ -190,7 +190,7 @@ func (suite *KeeperTestSuite) TestValidateSelfClient() { }, { "invalid client type", - solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}), + solomachine.NewClientState(0, &solomachine.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}), false, }, { diff --git a/modules/core/02-client/legacy/v100/store.go b/modules/core/02-client/legacy/v100/store.go index 8e09a0a72be..a7bac3529f5 100644 --- a/modules/core/02-client/legacy/v100/store.go +++ b/modules/core/02-client/legacy/v100/store.go @@ -14,7 +14,7 @@ import ( clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" host "github.com/cosmos/ibc-go/v6/modules/core/24-host" "github.com/cosmos/ibc-go/v6/modules/core/exported" - smtypes "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" + solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" ) @@ -106,15 +106,15 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar } // migrateSolomachine migrates the solomachine from v1 to v2 solo machine protobuf definition. -func migrateSolomachine(clientState *ClientState) *smtypes.ClientState { +func migrateSolomachine(clientState *ClientState) *solomachine.ClientState { isFrozen := clientState.FrozenSequence != 0 - consensusState := &smtypes.ConsensusState{ + consensusState := &solomachine.ConsensusState{ PublicKey: clientState.ConsensusState.PublicKey, Diversifier: clientState.ConsensusState.Diversifier, Timestamp: clientState.ConsensusState.Timestamp, } - return &smtypes.ClientState{ + return &solomachine.ClientState{ Sequence: clientState.Sequence, IsFrozen: isFrozen, ConsensusState: consensusState, diff --git a/modules/core/02-client/types/genesis_test.go b/modules/core/02-client/types/genesis_test.go index 33e3b8d4adb..4ab4fd90b98 100644 --- a/modules/core/02-client/types/genesis_test.go +++ b/modules/core/02-client/types/genesis_test.go @@ -9,7 +9,7 @@ import ( "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v6/modules/core/23-commitment/types" "github.com/cosmos/ibc-go/v6/modules/core/exported" - solomachinetypes "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" + solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v6/testing" ibctestingmock "github.com/cosmos/ibc-go/v6/testing/mock" @@ -114,7 +114,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { types.NewIdentifiedClientState( soloMachineClientID, ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), ), - types.NewIdentifiedClientState(tmClientID0, solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time})), + types.NewIdentifiedClientState(tmClientID0, solomachine.NewClientState(0, &solomachine.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time})), }, nil, nil, diff --git a/modules/core/02-client/types/msgs_test.go b/modules/core/02-client/types/msgs_test.go index a04430f82f4..37617434e75 100644 --- a/modules/core/02-client/types/msgs_test.go +++ b/modules/core/02-client/types/msgs_test.go @@ -9,7 +9,7 @@ import ( "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v6/modules/core/23-commitment/types" - solomachinetypes "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" + solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v6/testing" ) @@ -152,7 +152,7 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { "invalid solomachine client", func() { soloMachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2) - msg, err = types.NewMsgCreateClient(&solomachinetypes.ClientState{}, soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress().String()) + msg, err = types.NewMsgCreateClient(&solomachine.ClientState{}, soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress().String()) suite.Require().NoError(err) }, false, @@ -161,7 +161,7 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { "invalid solomachine consensus state", func() { soloMachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2) - msg, err = types.NewMsgCreateClient(soloMachine.ClientState(), &solomachinetypes.ConsensusState{}, suite.chainA.SenderAccount.GetAddress().String()) + msg, err = types.NewMsgCreateClient(soloMachine.ClientState(), &solomachine.ConsensusState{}, suite.chainA.SenderAccount.GetAddress().String()) suite.Require().NoError(err) }, false, @@ -301,7 +301,7 @@ func (suite *TypesTestSuite) TestMsgUpdateClient_ValidateBasic() { { "invalid solomachine header", func() { - msg, err = types.NewMsgUpdateClient("solomachine", &solomachinetypes.Header{}, suite.chainA.SenderAccount.GetAddress().String()) + msg, err = types.NewMsgUpdateClient("solomachine", &solomachine.Header{}, suite.chainA.SenderAccount.GetAddress().String()) suite.Require().NoError(err) }, false, @@ -583,7 +583,7 @@ func (suite *TypesTestSuite) TestMsgSubmitMisbehaviour_ValidateBasic() { { "invalid solomachine misbehaviour", func() { - msg, err = types.NewMsgSubmitMisbehaviour("solomachine", &solomachinetypes.Misbehaviour{}, suite.chainA.SenderAccount.GetAddress().String()) + msg, err = types.NewMsgSubmitMisbehaviour("solomachine", &solomachine.Misbehaviour{}, suite.chainA.SenderAccount.GetAddress().String()) suite.Require().NoError(err) }, false, diff --git a/modules/core/types/codec.go b/modules/core/types/codec.go index daaadfd7246..4c688e6ca28 100644 --- a/modules/core/types/codec.go +++ b/modules/core/types/codec.go @@ -7,7 +7,7 @@ import ( connectiontypes "github.com/cosmos/ibc-go/v6/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" commitmenttypes "github.com/cosmos/ibc-go/v6/modules/core/23-commitment/types" - solomachinetypes "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" + solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" ) @@ -16,7 +16,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { clienttypes.RegisterInterfaces(registry) connectiontypes.RegisterInterfaces(registry) channeltypes.RegisterInterfaces(registry) - solomachinetypes.RegisterInterfaces(registry) + solomachine.RegisterInterfaces(registry) ibctm.RegisterInterfaces(registry) commitmenttypes.RegisterInterfaces(registry) } diff --git a/modules/light-clients/07-tendermint/misbehaviour_handle_test.go b/modules/light-clients/07-tendermint/misbehaviour_handle_test.go index 2dc9216e987..29f96e08a5d 100644 --- a/modules/light-clients/07-tendermint/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/misbehaviour_handle_test.go @@ -9,7 +9,7 @@ import ( clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" "github.com/cosmos/ibc-go/v6/modules/core/exported" - smtypes "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" + solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v6/testing" ibctestingmock "github.com/cosmos/ibc-go/v6/testing/mock" @@ -284,7 +284,7 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { }, { "invalid tendermint misbehaviour", func() { - misbehaviour = &smtypes.Misbehaviour{} + misbehaviour = &solomachine.Misbehaviour{} }, false, }, { @@ -603,7 +603,7 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviourNonRevisionChainID() { }, { "invalid tendermint misbehaviour", func() { - misbehaviour = &smtypes.Misbehaviour{} + misbehaviour = &solomachine.Misbehaviour{} }, false, }, { diff --git a/testing/solomachine.go b/testing/solomachine.go index baac72df70a..56de68683ea 100644 --- a/testing/solomachine.go +++ b/testing/solomachine.go @@ -16,7 +16,7 @@ import ( commitmenttypes "github.com/cosmos/ibc-go/v6/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v6/modules/core/24-host" "github.com/cosmos/ibc-go/v6/modules/core/exported" - solomachinetypes "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" + solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ) // Solomachine is a testing helper used to simulate a counterparty @@ -83,16 +83,16 @@ func GenerateKeys(t *testing.T, n uint64) ([]cryptotypes.PrivKey, []cryptotypes. } // ClientState returns a new solo machine ClientState instance. -func (solo *Solomachine) ClientState() *solomachinetypes.ClientState { - return solomachinetypes.NewClientState(solo.Sequence, solo.ConsensusState()) +func (solo *Solomachine) ClientState() *solomachine.ClientState { + return solomachine.NewClientState(solo.Sequence, solo.ConsensusState()) } // ConsensusState returns a new solo machine ConsensusState instance -func (solo *Solomachine) ConsensusState() *solomachinetypes.ConsensusState { +func (solo *Solomachine) ConsensusState() *solomachine.ConsensusState { publicKey, err := codectypes.NewAnyWithValue(solo.PublicKey) require.NoError(solo.t, err) - return &solomachinetypes.ConsensusState{ + return &solomachine.ConsensusState{ PublicKey: publicKey, Diversifier: solo.Diversifier, Timestamp: solo.Time, @@ -107,14 +107,14 @@ func (solo *Solomachine) GetHeight() exported.Height { // CreateHeader generates a new private/public key pair and creates the // necessary signature to construct a valid solo machine header. // A new diversifier will be used as well -func (solo *Solomachine) CreateHeader(newDiversifier string) *solomachinetypes.Header { +func (solo *Solomachine) CreateHeader(newDiversifier string) *solomachine.Header { // generate new private keys and signature for header newPrivKeys, newPubKeys, newPubKey := GenerateKeys(solo.t, uint64(len(solo.PrivateKeys))) publicKey, err := codectypes.NewAnyWithValue(newPubKey) require.NoError(solo.t, err) - data := &solomachinetypes.HeaderData{ + data := &solomachine.HeaderData{ NewPubKey: publicKey, NewDiversifier: newDiversifier, } @@ -122,7 +122,7 @@ func (solo *Solomachine) CreateHeader(newDiversifier string) *solomachinetypes.H dataBz, err := solo.cdc.Marshal(data) require.NoError(solo.t, err) - signBytes := &solomachinetypes.SignBytes{ + signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, @@ -135,7 +135,7 @@ func (solo *Solomachine) CreateHeader(newDiversifier string) *solomachinetypes.H sig := solo.GenerateSignature(bz) - header := &solomachinetypes.Header{ + header := &solomachine.Header{ Sequence: solo.Sequence, Timestamp: solo.Time, Signature: sig, @@ -155,7 +155,7 @@ func (solo *Solomachine) CreateHeader(newDiversifier string) *solomachinetypes.H // CreateMisbehaviour constructs testing misbehaviour for the solo machine client // by signing over two different data bytes at the same sequence. -func (solo *Solomachine) CreateMisbehaviour() *solomachinetypes.Misbehaviour { +func (solo *Solomachine) CreateMisbehaviour() *solomachine.Misbehaviour { merklePath := solo.GetClientStatePath("counterparty") path, err := solo.cdc.Marshal(&merklePath) require.NoError(solo.t, err) @@ -163,7 +163,7 @@ func (solo *Solomachine) CreateMisbehaviour() *solomachinetypes.Misbehaviour { data, err := solo.cdc.Marshal(solo.ClientState()) require.NoError(solo.t, err) - signBytes := &solomachinetypes.SignBytes{ + signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, @@ -175,7 +175,7 @@ func (solo *Solomachine) CreateMisbehaviour() *solomachinetypes.Misbehaviour { require.NoError(solo.t, err) sig := solo.GenerateSignature(bz) - signatureOne := solomachinetypes.SignatureAndData{ + signatureOne := solomachine.SignatureAndData{ Signature: sig, Path: path, Data: data, @@ -192,7 +192,7 @@ func (solo *Solomachine) CreateMisbehaviour() *solomachinetypes.Misbehaviour { data, err = solo.cdc.Marshal(solo.ConsensusState()) require.NoError(solo.t, err) - signBytes = &solomachinetypes.SignBytes{ + signBytes = &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, @@ -204,14 +204,14 @@ func (solo *Solomachine) CreateMisbehaviour() *solomachinetypes.Misbehaviour { require.NoError(solo.t, err) sig = solo.GenerateSignature(bz) - signatureTwo := solomachinetypes.SignatureAndData{ + signatureTwo := solomachine.SignatureAndData{ Signature: sig, Path: path, Data: data, Timestamp: solo.Time, } - return &solomachinetypes.Misbehaviour{ + return &solomachine.Misbehaviour{ ClientId: solo.ClientID, Sequence: solo.Sequence, SignatureOne: &signatureOne, From f54143e91fdda13f2138bf8d99fa8e5cbd8c9de1 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 15 Nov 2022 10:53:52 +0100 Subject: [PATCH 4/9] add missing set order functions for ica (#2740) Co-authored-by: Carlos Rodriguez --- docs/apps/interchain-accounts/integration.md | 16 +++++++++++++++- docs/middleware/ics29-fee/integration.md | 6 +++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/docs/apps/interchain-accounts/integration.md b/docs/apps/interchain-accounts/integration.md index 8105d684276..bd3d9404500 100644 --- a/docs/apps/interchain-accounts/integration.md +++ b/docs/apps/interchain-accounts/integration.md @@ -117,8 +117,22 @@ app.moduleManager = module.NewManager( ... +// Add fee middleware to begin blocker logic +app.moduleManager.SetOrderBeginBlockers( + ... + icatypes.ModuleName, + ... +) + +// Add fee middleware to end blocker logic +app.moduleManager.SetOrderEndBlockers( + ... + icatypes.ModuleName, + ... +) + // Add Interchain Accounts module InitGenesis logic -app.mm.SetOrderInitGenesis( +app.moduleManager.SetOrderInitGenesis( ... icatypes.ModuleName, ... diff --git a/docs/middleware/ics29-fee/integration.md b/docs/middleware/ics29-fee/integration.md index 8f11998c456..b1d13fef065 100644 --- a/docs/middleware/ics29-fee/integration.md +++ b/docs/middleware/ics29-fee/integration.md @@ -77,21 +77,21 @@ app.moduleManager = module.NewManager( ... // Add fee middleware to begin blocker logic -app.mm.SetOrderBeginBlockers( +app.moduleManager.SetOrderBeginBlockers( ... ibcfeetypes.ModuleName, ... ) // Add fee middleware to end blocker logic -app.mm.SetOrderEndBlockers( +app.moduleManager.SetOrderEndBlockers( ... ibcfeetypes.ModuleName, ... ) // Add fee middleware to init genesis logic -app.mm.SetOrderInitGenesis( +app.moduleManager.SetOrderInitGenesis( ... ibcfeetypes.ModuleName, ... From 510afbe0e6218a06d62f23ddfa30ac7c2536fc77 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Tue, 15 Nov 2022 10:02:38 +0000 Subject: [PATCH 5/9] test: added test to verify signbytes marshal correctly for single signature solomachine --- .../06-solomachine/client_state_test.go | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/modules/light-clients/06-solomachine/client_state_test.go b/modules/light-clients/06-solomachine/client_state_test.go index d28374d54bb..71a2a898e07 100644 --- a/modules/light-clients/06-solomachine/client_state_test.go +++ b/modules/light-clients/06-solomachine/client_state_test.go @@ -1,6 +1,8 @@ package solomachine_test import ( + "bytes" + sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" @@ -603,6 +605,34 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { } } +func (suite *SoloMachineTestSuite) TestSignBytesMarshalling() { + sm := suite.solomachine + merklePath := commitmenttypes.NewMerklePath("ibc", "solomachine") + signBytesNilData := solomachine.SignBytes{ + Sequence: sm.GetHeight().GetRevisionHeight(), + Timestamp: sm.Time, + Diversifier: sm.Diversifier, + Path: []byte(merklePath.String()), + Data: nil, + } + + signBytesEmptyArray := solomachine.SignBytes{ + Sequence: sm.GetHeight().GetRevisionHeight(), + Timestamp: sm.Time, + Diversifier: sm.Diversifier, + Path: []byte(merklePath.String()), + Data: []byte{}, + } + + signBzNil, err := suite.chainA.Codec.Marshal(&signBytesNilData) + suite.Require().NoError(err) + + signBzEmptyArray, err := suite.chainA.Codec.Marshal(&signBytesEmptyArray) + suite.Require().NoError(err) + + suite.Require().True(bytes.Equal(signBzNil, signBzEmptyArray)) +} + func (suite *SoloMachineTestSuite) TestVerifyNonMembership() { // test singlesig and multisig public keys for _, sm := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} { From 1b7745f30ff1e35640d121d449087aa5b8910793 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 15 Nov 2022 12:54:10 +0100 Subject: [PATCH 6/9] docs: fix migration/docs for ICA controller middleware (#2737) * update docs/migration with the change to middleware for ICA controller * improve variable naming * alignment Co-authored-by: Carlos Rodriguez --- docs/apps/interchain-accounts/integration.md | 20 +++++++++++--------- docs/middleware/ics29-fee/integration.md | 2 +- docs/migrations/v3-to-v4.md | 13 ++++++++++++- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/docs/apps/interchain-accounts/integration.md b/docs/apps/interchain-accounts/integration.md index bd3d9404500..f1139d31df2 100644 --- a/docs/apps/interchain-accounts/integration.md +++ b/docs/apps/interchain-accounts/integration.md @@ -97,14 +97,15 @@ icaAuthModule := icaauth.NewAppModule(appCodec, app.ICAAuthKeeper) // ICA auth IBC Module icaAuthIBCModule := icaauth.NewIBCModule(app.ICAAuthKeeper) -// Create host and controller IBC Modules as desired -icaControllerIBCModule := icacontroller.NewIBCModule(app.ICAControllerKeeper, icaAuthIBCModule) +// Create controller IBC application stack and host IBC module as desired +icaControllerStack := icacontroller.NewIBCMiddleware(icaAuthIBCModule, app.ICAControllerKeeper) icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper) // Register host and authentication routes -ibcRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerIBCModule). - AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). - AddRoute(icaauthtypes.ModuleName, icaControllerIBCModule) // Note, the authentication module is routed to the top level of the middleware stack +ibcRouter. + AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). + AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). + AddRoute(icaauthtypes.ModuleName, icaControllerStack) // Note, the authentication module is routed to the top level of the middleware stack ... @@ -178,10 +179,11 @@ app.ICAAuthKeeper = icaauthkeeper.NewKeeper(appCodec, keys[icaauthtypes.StoreKey icaAuthModule := icaauth.NewAppModule(appCodec, app.ICAAuthKeeper) icaAuthIBCModule := icaauth.NewIBCModule(app.ICAAuthKeeper) -// Create controller IBC Module -icaControllerIBCModule := icacontroller.NewIBCModule(app.ICAControllerKeeper, icaAuthIBCModule) +// Create controller IBC application stack +icaControllerStack := icacontroller.NewIBCMiddleware(icaAuthIBCModule, app.ICAControllerKeeper) // Register controller and authentication routes -ibcRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerIBCModule) -ibcRouter.AddRoute(icaauthtypes.ModuleName, icaControllerIBCModule) // Note, the authentication module is routed to the top level of the middleware stack +ibcRouter. + AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). + AddRoute(icaauthtypes.ModuleName, icaControllerStack) // Note, the authentication module is routed to the top level of the middleware stack ``` diff --git a/docs/middleware/ics29-fee/integration.md b/docs/middleware/ics29-fee/integration.md index b1d13fef065..1a663daa446 100644 --- a/docs/middleware/ics29-fee/integration.md +++ b/docs/middleware/ics29-fee/integration.md @@ -16,7 +16,7 @@ For Cosmos SDK chains this setup is done via the `app/app.go` file, where module ## Example integration of the Fee Middleware module -``` +```go // app.go // Register the AppModule for the fee middleware module diff --git a/docs/migrations/v3-to-v4.md b/docs/migrations/v3-to-v4.md index b6ab165d066..e1f99de04f3 100644 --- a/docs/migrations/v3-to-v4.md +++ b/docs/migrations/v3-to-v4.md @@ -18,7 +18,18 @@ No genesis or in-place migrations required when upgrading from v1 or v2 of ibc-g ## Chains -### Fee Middleware +### ICS27 - Interchain Accounts + +The controller submodule implements now the 05-port `Middleware` interface instead of the 05-port `IBCModule` interface. Chains that integrate the controller submodule, need to create it with the `NewIBCMiddleware` constructor function. For example: + +```diff +- icacontroller.NewIBCModule(app.ICAControllerKeeper, icaAuthIBCModule) ++ icacontroller.NewIBCMiddleware(icaAuthIBCModule, app.ICAControllerKeeper) +``` + +where `icaAuthIBCModule` is the Interchain Accounts authentication IBC Module. + +### ICS29 - Fee Middleware The Fee Middleware module, as the name suggests, plays the role of an IBC middleware and as such must be configured by chain developers to route and handle IBC messages correctly. From fb9dedd706e5835cdd607392320df79fcea7ca7f Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 15 Nov 2022 14:12:35 +0100 Subject: [PATCH 7/9] chore: updating `VerifyMembership` and `VerifyNonMembership` methods to use `Path` interface (#2736) * updating VerifyMembership and VerifyNonMembership interfaces to use exported.Path in favour of []byte * adding mock struct KeyPath to ibcmock testing pkg, adding additional tests to solomachine and tm clients --- .../core/02-client/legacy/v100/solomachine.go | 4 +- modules/core/03-connection/keeper/verify.go | 56 ++------- modules/core/exported/client.go | 4 +- .../06-solomachine/client_state.go | 16 +-- .../06-solomachine/client_state_test.go | 100 +++++----------- .../07-tendermint/client_state.go | 16 +-- .../07-tendermint/client_state_test.go | 107 +++++------------- testing/mock/mock.go | 16 +++ 8 files changed, 106 insertions(+), 213 deletions(-) diff --git a/modules/core/02-client/legacy/v100/solomachine.go b/modules/core/02-client/legacy/v100/solomachine.go index 8fa99104316..c953fc419b6 100644 --- a/modules/core/02-client/legacy/v100/solomachine.go +++ b/modules/core/02-client/legacy/v100/solomachine.go @@ -227,7 +227,7 @@ func (cs *ClientState) VerifyMembership( delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, - path []byte, + path exported.Path, value []byte, ) error { panic("legacy solo machine is deprecated!") @@ -242,7 +242,7 @@ func (cs *ClientState) VerifyNonMembership( delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, - path []byte, + path exported.Path, ) error { panic("legacy solo machine is deprecated") } diff --git a/modules/core/03-connection/keeper/verify.go b/modules/core/03-connection/keeper/verify.go index 64c6cdb43cf..ad9cfac718f 100644 --- a/modules/core/03-connection/keeper/verify.go +++ b/modules/core/03-connection/keeper/verify.go @@ -41,11 +41,6 @@ func (k Keeper) VerifyClientState( return err } - path, err := k.cdc.Marshal(&merklePath) - if err != nil { - return err - } - bz, err := k.cdc.MarshalInterface(clientState) if err != nil { return err @@ -54,7 +49,7 @@ func (k Keeper) VerifyClientState( if err := targetClient.VerifyMembership( ctx, clientStore, k.cdc, height, 0, 0, // skip delay period checks for non-packet processing verification - proof, path, bz, + proof, merklePath, bz, ); err != nil { return sdkerrors.Wrapf(err, "failed client state verification for target client: %s", clientID) } @@ -90,11 +85,6 @@ func (k Keeper) VerifyClientConsensusState( return err } - path, err := k.cdc.Marshal(&merklePath) - if err != nil { - return err - } - bz, err := k.cdc.MarshalInterface(consensusState) if err != nil { return err @@ -103,7 +93,7 @@ func (k Keeper) VerifyClientConsensusState( if err := clientState.VerifyMembership( ctx, clientStore, k.cdc, height, 0, 0, // skip delay period checks for non-packet processing verification - proof, path, bz, + proof, merklePath, bz, ); err != nil { return sdkerrors.Wrapf(err, "failed consensus state verification for client (%s)", clientID) } @@ -139,11 +129,6 @@ func (k Keeper) VerifyConnectionState( return err } - path, err := k.cdc.Marshal(&merklePath) - if err != nil { - return err - } - connectionEnd, ok := counterpartyConnection.(connectiontypes.ConnectionEnd) if !ok { return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "invalid connection type %T", counterpartyConnection) @@ -157,7 +142,7 @@ func (k Keeper) VerifyConnectionState( if err := clientState.VerifyMembership( ctx, clientStore, k.cdc, height, 0, 0, // skip delay period checks for non-packet processing verification - proof, path, bz, + proof, merklePath, bz, ); err != nil { return sdkerrors.Wrapf(err, "failed connection state verification for client (%s)", clientID) } @@ -194,11 +179,6 @@ func (k Keeper) VerifyChannelState( return err } - path, err := k.cdc.Marshal(&merklePath) - if err != nil { - return err - } - channelEnd, ok := channel.(channeltypes.Channel) if !ok { return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "invalid channel type %T", channel) @@ -212,7 +192,7 @@ func (k Keeper) VerifyChannelState( if err := clientState.VerifyMembership( ctx, clientStore, k.cdc, height, 0, 0, // skip delay period checks for non-packet processing verification - proof, path, bz, + proof, merklePath, bz, ); err != nil { return sdkerrors.Wrapf(err, "failed channel state verification for client (%s)", clientID) } @@ -254,15 +234,10 @@ func (k Keeper) VerifyPacketCommitment( return err } - path, err := k.cdc.Marshal(&merklePath) - if err != nil { - return err - } - if err := clientState.VerifyMembership( ctx, clientStore, k.cdc, height, timeDelay, blockDelay, - proof, path, commitmentBytes, + proof, merklePath, commitmentBytes, ); err != nil { return sdkerrors.Wrapf(err, "failed packet commitment verification for client (%s)", clientID) } @@ -304,15 +279,10 @@ func (k Keeper) VerifyPacketAcknowledgement( return err } - path, err := k.cdc.Marshal(&merklePath) - if err != nil { - return err - } - if err := clientState.VerifyMembership( ctx, clientStore, k.cdc, height, timeDelay, blockDelay, - proof, path, channeltypes.CommitAcknowledgement(acknowledgement), + proof, merklePath, channeltypes.CommitAcknowledgement(acknowledgement), ); err != nil { return sdkerrors.Wrapf(err, "failed packet acknowledgement verification for client (%s)", clientID) } @@ -354,15 +324,10 @@ func (k Keeper) VerifyPacketReceiptAbsence( return err } - path, err := k.cdc.Marshal(&merklePath) - if err != nil { - return err - } - if err := clientState.VerifyNonMembership( ctx, clientStore, k.cdc, height, timeDelay, blockDelay, - proof, path, + proof, merklePath, ); err != nil { return sdkerrors.Wrapf(err, "failed packet receipt absence verification for client (%s)", clientID) } @@ -403,15 +368,10 @@ func (k Keeper) VerifyNextSequenceRecv( return err } - path, err := k.cdc.Marshal(&merklePath) - if err != nil { - return err - } - if err := clientState.VerifyMembership( ctx, clientStore, k.cdc, height, timeDelay, blockDelay, - proof, path, sdk.Uint64ToBigEndian(nextSequenceRecv), + proof, merklePath, sdk.Uint64ToBigEndian(nextSequenceRecv), ); err != nil { return sdkerrors.Wrapf(err, "failed next sequence receive verification for client (%s)", clientID) } diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index ac76a647dc5..81dea3373c5 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -74,7 +74,7 @@ type ClientState interface { delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, - path []byte, + path Path, value []byte, ) error @@ -88,7 +88,7 @@ type ClientState interface { delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, - path []byte, + path Path, ) error // VerifyClientMessage must verify a ClientMessage. A ClientMessage could be a Header, Misbehaviour, or batch update. diff --git a/modules/light-clients/06-solomachine/client_state.go b/modules/light-clients/06-solomachine/client_state.go index 17b07364927..36cb1135540 100644 --- a/modules/light-clients/06-solomachine/client_state.go +++ b/modules/light-clients/06-solomachine/client_state.go @@ -112,7 +112,7 @@ func (cs *ClientState) VerifyMembership( delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, - path []byte, + path exported.Path, value []byte, ) error { publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, proof) @@ -120,9 +120,9 @@ func (cs *ClientState) VerifyMembership( return err } - var merklePath commitmenttypes.MerklePath - if err := cdc.Unmarshal(path, &merklePath); err != nil { - return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal path into ICS 23 commitment merkle path") + merklePath, ok := path.(commitmenttypes.MerklePath) + if !ok { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path) } signBytes := &SignBytes{ @@ -159,16 +159,16 @@ func (cs *ClientState) VerifyNonMembership( delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, - path []byte, + path exported.Path, ) error { publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, proof) if err != nil { return err } - var merklePath commitmenttypes.MerklePath - if err := cdc.Unmarshal(path, &merklePath); err != nil { - return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal path into ICS 23 commitment merkle path") + merklePath, ok := path.(commitmenttypes.MerklePath) + if !ok { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path) } signBytes := &SignBytes{ diff --git a/modules/light-clients/06-solomachine/client_state_test.go b/modules/light-clients/06-solomachine/client_state_test.go index 71a2a898e07..3fabbb21783 100644 --- a/modules/light-clients/06-solomachine/client_state_test.go +++ b/modules/light-clients/06-solomachine/client_state_test.go @@ -12,6 +12,7 @@ import ( solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v6/testing" + ibcmock "github.com/cosmos/ibc-go/v6/testing/mock" ) const ( @@ -147,7 +148,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { clientState *solomachine.ClientState err error height clienttypes.Height - path []byte + path exported.Path proof []byte testingPath *ibctesting.Path signBytes solomachine.SignBytes @@ -170,12 +171,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { clientStateBz, err := suite.chainA.Codec.Marshal(clientState) suite.Require().NoError(err) - merklePath := suite.solomachine.GetClientStatePath(counterpartyClientIdentifier) + path = suite.solomachine.GetClientStatePath(counterpartyClientIdentifier) signBytes = solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: clientStateBz, } @@ -189,9 +190,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) }, @@ -205,12 +203,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { consensusStateBz, err := suite.chainA.Codec.Marshal(consensusState) suite.Require().NoError(err) - merklePath := sm.GetConsensusStatePath(counterpartyClientIdentifier, height) + path = sm.GetConsensusStatePath(counterpartyClientIdentifier, height) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: consensusStateBz, } @@ -224,9 +222,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) }, @@ -243,12 +238,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { connectionEndBz, err := suite.chainA.Codec.Marshal(&connectionEnd) suite.Require().NoError(err) - merklePath := sm.GetConnectionStatePath(ibctesting.FirstConnectionID) + path = sm.GetConnectionStatePath(ibctesting.FirstConnectionID) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: connectionEndBz, } @@ -262,9 +257,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) }, @@ -282,12 +274,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { channelEndBz, err := suite.chainA.Codec.Marshal(&channelEnd) suite.Require().NoError(err) - merklePath := sm.GetChannelStatePath(ibctesting.MockPort, ibctesting.FirstChannelID) + path = sm.GetChannelStatePath(ibctesting.MockPort, ibctesting.FirstChannelID) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: channelEndBz, } @@ -301,9 +293,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) }, @@ -318,12 +307,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { nextSeqRecv, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainA.GetContext(), ibctesting.MockPort, ibctesting.FirstChannelID) suite.Require().True(found) - merklePath := sm.GetNextSequenceRecvPath(ibctesting.MockPort, ibctesting.FirstChannelID) + path = sm.GetNextSequenceRecvPath(ibctesting.MockPort, ibctesting.FirstChannelID) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: sdk.Uint64ToBigEndian(nextSeqRecv), } @@ -337,9 +326,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) }, @@ -360,12 +346,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { ) commitmentBz := channeltypes.CommitPacket(suite.chainA.Codec, packet) - merklePath := sm.GetPacketCommitmentPath(ibctesting.MockPort, ibctesting.FirstChannelID) + path = sm.GetPacketCommitmentPath(ibctesting.MockPort, ibctesting.FirstChannelID) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: commitmentBz, } @@ -379,9 +365,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) }, @@ -390,12 +373,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { { "success: packet acknowledgement verification", func() { - merklePath := sm.GetPacketAcknowledgementPath(ibctesting.MockPort, ibctesting.FirstChannelID) + path = sm.GetPacketAcknowledgementPath(ibctesting.MockPort, ibctesting.FirstChannelID) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: ibctesting.MockAcknowledgement, } @@ -409,9 +392,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) }, @@ -420,12 +400,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { { "success: packet receipt verification", func() { - merklePath := sm.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID) + path = sm.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: []byte{byte(1)}, // packet receipt is stored as a single byte } @@ -439,9 +419,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) }, @@ -467,19 +444,16 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { false, }, { - "malformed merkle path fails to unmarshal", + "invalid path type", func() { - path = []byte("invalid path") + path = ibcmock.KeyPath{} }, false, }, { "malformed proof fails to unmarshal", func() { - merklePath := suite.solomachine.GetClientStatePath(counterpartyClientIdentifier) - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - + path = suite.solomachine.GetClientStatePath(counterpartyClientIdentifier) proof = []byte("invalid proof") }, false, @@ -555,12 +529,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { clientState = sm.ClientState() height = clienttypes.NewHeight(sm.GetHeight().GetRevisionNumber(), sm.GetHeight().GetRevisionHeight()) - merklePath := commitmenttypes.NewMerklePath("ibc", "solomachine") + path = commitmenttypes.NewMerklePath("ibc", "solomachine") signBytes = solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: []byte("solomachine"), } @@ -574,9 +548,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) @@ -641,7 +612,7 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() { clientState *solomachine.ClientState err error height clienttypes.Height - path []byte + path exported.Path proof []byte signBytes solomachine.SignBytes ) @@ -659,12 +630,12 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() { { "success: packet receipt absence verification", func() { - merklePath := suite.solomachine.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID) + path = suite.solomachine.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID) signBytes = solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: nil, } @@ -678,9 +649,6 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) }, @@ -706,19 +674,16 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() { false, }, { - "malformed merkle path fails to unmarshal", + "invalid path type", func() { - path = []byte("invalid path") + path = ibcmock.KeyPath{} }, false, }, { "malformed proof fails to unmarshal", func() { - merklePath := suite.solomachine.GetClientStatePath(counterpartyClientIdentifier) - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - + path = suite.solomachine.GetClientStatePath(counterpartyClientIdentifier) proof = []byte("invalid proof") }, false, @@ -804,12 +769,12 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() { clientState = sm.ClientState() height = clienttypes.NewHeight(sm.GetHeight().GetRevisionNumber(), sm.GetHeight().GetRevisionHeight()) - merklePath := commitmenttypes.NewMerklePath("ibc", "solomachine") + path = commitmenttypes.NewMerklePath("ibc", "solomachine") signBytes = solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: []byte(path.String()), Data: nil, } @@ -823,9 +788,6 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() { Timestamp: sm.Time, } - path, err = suite.chainA.Codec.Marshal(&merklePath) - suite.Require().NoError(err) - proof, err = suite.chainA.Codec.Marshal(signatureDoc) suite.Require().NoError(err) diff --git a/modules/light-clients/07-tendermint/client_state.go b/modules/light-clients/07-tendermint/client_state.go index dc434912505..b1de29b44b7 100644 --- a/modules/light-clients/07-tendermint/client_state.go +++ b/modules/light-clients/07-tendermint/client_state.go @@ -210,7 +210,7 @@ func (cs ClientState) VerifyMembership( delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, - path []byte, + path exported.Path, value []byte, ) error { if cs.GetLatestHeight().LT(height) { @@ -229,9 +229,9 @@ func (cs ClientState) VerifyMembership( return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal proof into ICS 23 commitment merkle proof") } - var merklePath commitmenttypes.MerklePath - if err := cdc.Unmarshal(path, &merklePath); err != nil { - return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal path into ICS 23 commitment merkle path") + merklePath, ok := path.(commitmenttypes.MerklePath) + if !ok { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path) } consensusState, found := GetConsensusState(clientStore, cdc, height) @@ -256,7 +256,7 @@ func (cs ClientState) VerifyNonMembership( delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, - path []byte, + path exported.Path, ) error { if cs.GetLatestHeight().LT(height) { return sdkerrors.Wrapf( @@ -274,9 +274,9 @@ func (cs ClientState) VerifyNonMembership( return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal proof into ICS 23 commitment merkle proof") } - var merklePath commitmenttypes.MerklePath - if err := cdc.Unmarshal(path, &merklePath); err != nil { - return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal path into ICS 23 commitment merkle path") + merklePath, ok := path.(commitmenttypes.MerklePath) + if !ok { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path) } consensusState, found := GetConsensusState(clientStore, cdc, height) diff --git a/modules/light-clients/07-tendermint/client_state_test.go b/modules/light-clients/07-tendermint/client_state_test.go index ba3d04f337e..64ffff0fd50 100644 --- a/modules/light-clients/07-tendermint/client_state_test.go +++ b/modules/light-clients/07-tendermint/client_state_test.go @@ -214,9 +214,10 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { testingpath *ibctesting.Path delayTimePeriod uint64 delayBlockPeriod uint64 + err error proofHeight exported.Height proof []byte - path []byte + path exported.Path value []byte ) @@ -236,10 +237,7 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { "successful ConsensusState verification", func() { key := host.FullConsensusStateKey(testingpath.EndpointB.ClientID, testingpath.EndpointB.GetClientState().GetLatestHeight()) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = suite.chainB.QueryProof(key) @@ -254,10 +252,7 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { "successful Connection verification", func() { key := host.ConnectionKey(testingpath.EndpointB.ConnectionID) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = suite.chainB.QueryProof(key) @@ -272,10 +267,7 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { "successful Channel verification", func() { key := host.ChannelKey(testingpath.EndpointB.ChannelConfig.PortID, testingpath.EndpointB.ChannelID) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = suite.chainB.QueryProof(key) @@ -296,10 +288,7 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { packet := channeltypes.NewPacket(ibctesting.MockPacketData, sequence, testingpath.EndpointB.ChannelConfig.PortID, testingpath.EndpointB.ChannelID, testingpath.EndpointA.ChannelConfig.PortID, testingpath.EndpointA.ChannelID, clienttypes.NewHeight(1, 100), 0) key := host.PacketCommitmentKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = testingpath.EndpointB.QueryProof(key) @@ -320,10 +309,7 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { key := host.PacketAcknowledgementKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = testingpath.EndpointB.QueryProof(key) @@ -347,10 +333,7 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { key := host.NextSequenceRecvKey(packet.GetSourcePort(), packet.GetSourceChannel()) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = testingpath.EndpointB.QueryProof(key) @@ -363,10 +346,7 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { "successful verification outside IBC store", func() { key := transfertypes.PortKey merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(commitmenttypes.NewMerklePrefix([]byte(transfertypes.StoreKey)), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(commitmenttypes.NewMerklePrefix([]byte(transfertypes.StoreKey)), merklePath) suite.Require().NoError(err) clientState := testingpath.EndpointA.GetClientState() @@ -407,9 +387,11 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { }, false, }, { - "failed to unmarshal merkle path", func() { - path = []byte("invalid merkle path") - }, false, + "invalid path type", + func() { + path = ibcmock.KeyPath{} + }, + false, }, { "failed to unmarshal merkle proof", func() { @@ -446,10 +428,7 @@ func (suite *TendermintTestSuite) TestVerifyMembership() { // may be overwritten by malleate() key := host.FullClientStateKey(testingpath.EndpointB.ClientID) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainA.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = suite.chainB.QueryProof(key) @@ -484,9 +463,10 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { testingpath *ibctesting.Path delayTimePeriod uint64 delayBlockPeriod uint64 + err error proofHeight exported.Height + path exported.Path proof []byte - path []byte invalidClientID = "09-tendermint" invalidConnectionID = "connection-100" invalidChannelID = "channel-800" @@ -509,10 +489,7 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { "successful ConsensusState verification of non membership", func() { key := host.FullConsensusStateKey(invalidClientID, testingpath.EndpointB.GetClientState().GetLatestHeight()) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = suite.chainB.QueryProof(key) @@ -523,10 +500,7 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { "successful Connection verification of non membership", func() { key := host.ConnectionKey(invalidConnectionID) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = suite.chainB.QueryProof(key) @@ -537,10 +511,7 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { "successful Channel verification of non membership", func() { key := host.ChannelKey(testingpath.EndpointB.ChannelConfig.PortID, invalidChannelID) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = suite.chainB.QueryProof(key) @@ -552,10 +523,7 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { // make packet commitment proof key := host.PacketCommitmentKey(invalidPortID, invalidChannelID, 1) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = testingpath.EndpointB.QueryProof(key) @@ -565,10 +533,7 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { "successful Acknowledgement verification of non membership", func() { key := host.PacketAcknowledgementKey(invalidPortID, invalidChannelID, 1) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = testingpath.EndpointB.QueryProof(key) @@ -579,10 +544,7 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { "successful NextSequenceRecv verification of non membership", func() { key := host.NextSequenceRecvKey(invalidPortID, invalidChannelID) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = testingpath.EndpointB.QueryProof(key) @@ -593,10 +555,7 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { "successful verification of non membership outside IBC store", func() { key := []byte{0x08} merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(commitmenttypes.NewMerklePrefix([]byte(transfertypes.StoreKey)), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainB.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(commitmenttypes.NewMerklePrefix([]byte(transfertypes.StoreKey)), merklePath) suite.Require().NoError(err) clientState := testingpath.EndpointA.GetClientState() @@ -634,9 +593,11 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { }, false, }, { - "failed to unmarshal merkle path", func() { - path = []byte("invalid merkle path") - }, false, + "invalid path type", + func() { + path = ibcmock.KeyPath{} + }, + false, }, { "failed to unmarshal merkle proof", func() { @@ -653,10 +614,7 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { // change the value being proved key := host.FullClientStateKey(testingpath.EndpointB.ClientID) merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainA.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = suite.chainB.QueryProof(key) @@ -682,10 +640,7 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() { key := host.FullClientStateKey("invalid-client-id") merklePath := commitmenttypes.NewMerklePath(string(key)) - merklePath, err := commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) - suite.Require().NoError(err) - - path, err = suite.chainA.Codec.Marshal(&merklePath) + path, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath) suite.Require().NoError(err) proof, proofHeight = suite.chainB.QueryProof(key) diff --git a/testing/mock/mock.go b/testing/mock/mock.go index d8fa60806a9..f99fac85830 100644 --- a/testing/mock/mock.go +++ b/testing/mock/mock.go @@ -17,6 +17,7 @@ import ( channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v6/modules/core/05-port/types" host "github.com/cosmos/ibc-go/v6/modules/core/24-host" + "github.com/cosmos/ibc-go/v6/modules/core/exported" ) const ( @@ -150,3 +151,18 @@ func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { func (am AppModule) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) []abci.ValidatorUpdate { return []abci.ValidatorUpdate{} } + +var _ exported.Path = KeyPath{} + +// KeyPath defines a placeholder struct which implements the exported.Path interface +type KeyPath struct{} + +// String implements the exported.Path interface +func (KeyPath) String() string { + return "" +} + +// Empty implements the exported.Path interface +func (KeyPath) Empty() bool { + return false +} From 65b3ddfa1d5a5adf46635077c7506342172b1569 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Tue, 15 Nov 2022 14:07:06 +0000 Subject: [PATCH 8/9] e2e: updated compatibility tests to support running against all unreleased versions (#2680) --- .../unreleased/client.json | 10 ++++ .../unreleased/connection.json | 10 ++++ .../unreleased/incentivized-transfer.json | 15 +++++ .../unreleased/transfer.json | 15 +++++ .../e2e-compatibility-workflow-call.yaml | 6 +- .github/workflows/e2e-compatibility.yaml | 60 ++++++++++++------- 6 files changed, 92 insertions(+), 24 deletions(-) create mode 100644 .github/compatibility-test-matrices/unreleased/client.json create mode 100644 .github/compatibility-test-matrices/unreleased/connection.json create mode 100644 .github/compatibility-test-matrices/unreleased/incentivized-transfer.json create mode 100644 .github/compatibility-test-matrices/unreleased/transfer.json diff --git a/.github/compatibility-test-matrices/unreleased/client.json b/.github/compatibility-test-matrices/unreleased/client.json new file mode 100644 index 00000000000..d6859f4b370 --- /dev/null +++ b/.github/compatibility-test-matrices/unreleased/client.json @@ -0,0 +1,10 @@ +{ + "chain-a": ["release-v6.0.x", "release-v5.1.x", "release-v5.0.x", "release-v4.2.x", "release-v3.4.x", "release-v2.5.x"], + "chain-b": ["release-v6.0.x", "release-v5.1.x", "release-v5.0.x", "release-v4.2.x", "release-v3.4.x", "release-v2.5.x"], + "entrypoint": ["TestClientTestSuite"], + "test": [ + "TestClientUpdateProposal_Succeeds" + ], + "chain-binary": ["simd"], + "chain-image": ["ghcr.io/cosmos/ibc-go-simd"] +} diff --git a/.github/compatibility-test-matrices/unreleased/connection.json b/.github/compatibility-test-matrices/unreleased/connection.json new file mode 100644 index 00000000000..9594c65fb0d --- /dev/null +++ b/.github/compatibility-test-matrices/unreleased/connection.json @@ -0,0 +1,10 @@ +{ + "chain-a": ["release-v6.0.x", "release-v5.1.x", "release-v5.0.x", "release-v4.2.x", "release-v3.4.x", "release-v2.5.x"], + "chain-b": ["release-v6.0.x", "release-v5.1.x", "release-v5.0.x", "release-v4.2.x", "release-v3.4.x", "release-v2.5.x"], + "entrypoint": ["TestConnectionTestSuite"], + "test": [ + "TestMaxExpectedTimePerBlockParam" + ], + "chain-binary": ["simd"], + "chain-image": ["ghcr.io/cosmos/ibc-go-simd"] +} diff --git a/.github/compatibility-test-matrices/unreleased/incentivized-transfer.json b/.github/compatibility-test-matrices/unreleased/incentivized-transfer.json new file mode 100644 index 00000000000..b20429a2b64 --- /dev/null +++ b/.github/compatibility-test-matrices/unreleased/incentivized-transfer.json @@ -0,0 +1,15 @@ +{ + "chain-a": ["release-v6.0.x", "release-v5.1.x", "release-v5.0.x", "release-v4.2.x"], + "chain-b": ["release-v6.0.x", "release-v5.1.x", "release-v5.0.x", "release-v4.2.x"], + "entrypoint": ["TestIncentivizedTransferTestSuite"], + "test": [ + "TestMsgPayPacketFee_AsyncSingleSender_Succeeds", + "TestMsgPayPacketFee_InvalidReceiverAccount", + "TestMultiMsg_MsgPayPacketFeeSingleSender", + "TestMsgPayPacketFee_SingleSender_TimesOut", + "TestPayPacketFeeAsync_SingleSender_NoCounterPartyAddress", + "TestMsgPayPacketFee_AsyncMultipleSenders_Succeeds" + ], + "chain-binary": ["simd"], + "chain-image": ["ghcr.io/cosmos/ibc-go-simd"] +} diff --git a/.github/compatibility-test-matrices/unreleased/transfer.json b/.github/compatibility-test-matrices/unreleased/transfer.json new file mode 100644 index 00000000000..1183d941ba9 --- /dev/null +++ b/.github/compatibility-test-matrices/unreleased/transfer.json @@ -0,0 +1,15 @@ +{ + "chain-a": ["release-v6.0.x", "release-v5.1.x", "release-v4.2.x", "release-v3.4.x", "release-v2.5.x"], + "chain-b": ["release-v6.0.x", "release-v5.1.x", "release-v4.2.x", "release-v3.4.x", "release-v2.5.x"], + "entrypoint": ["TestTransferTestSuite"], + "test": [ + "TestMsgTransfer_Succeeds_Nonincentivized", + "TestMsgTransfer_Fails_InvalidAddress", + "TestMsgTransfer_Timeout_Nonincentivized", + "TestMsgTransfer_WithMemo", + "TestSendEnabledParam", + "TestReceiveEnabledParam" + ], + "chain-binary": ["simd"], + "chain-image": ["ghcr.io/cosmos/ibc-go-simd"] +} diff --git a/.github/workflows/e2e-compatibility-workflow-call.yaml b/.github/workflows/e2e-compatibility-workflow-call.yaml index 9aa82416c69..2c141c5b028 100644 --- a/.github/workflows/e2e-compatibility-workflow-call.yaml +++ b/.github/workflows/e2e-compatibility-workflow-call.yaml @@ -1,8 +1,8 @@ on: workflow_call: inputs: - docker-tag: - description: 'Docker tag being used' + test-file-directory: + description: 'Directory containing compatibility matrices' required: true type: string test-suite: @@ -21,7 +21,7 @@ jobs: - run: | # use jq -c to put the full json contents on a single line. This is required when using the json body # to create the matrix in the following job. - test_matrix="$(cat .github/compatibility-test-matrices/${{ inputs.docker-tag }}/${{ inputs.test-suite }}.json | jq -c)" + test_matrix="$(cat .github/compatibility-test-matrices/${{ inputs.test-file-directory }}/${{ inputs.test-suite }}.json | jq -c)" echo $test_matrix echo "::set-output name=test-matrix::$test_matrix" id: set-test-matrix diff --git a/.github/workflows/e2e-compatibility.yaml b/.github/workflows/e2e-compatibility.yaml index 02311c3a261..950ed677cd0 100644 --- a/.github/workflows/e2e-compatibility.yaml +++ b/.github/workflows/e2e-compatibility.yaml @@ -3,7 +3,7 @@ on: workflow_dispatch: inputs: release-branch: - description: 'Release branch to test' + description: 'Release branch to test (unreleased to run all release branches against each other)' required: true type: choice options: @@ -13,6 +13,7 @@ on: - release/v5.0.x - release/v5.1.x - release/v6.0.x + - unreleased env: REGISTRY: ghcr.io @@ -21,49 +22,66 @@ env: RELEASE_BRANCH: '${{ inputs.release-branch }}' jobs: - determine-docker-tag: + determine-test-directory: runs-on: ubuntu-latest outputs: - docker-tag: ${{ steps.set-docker-tag.outputs.docker-tag }} + test-directory: ${{ steps.set-test-dir.outputs.test-directory }} steps: - run: | - docker_tag="$(echo $RELEASE_BRANCH | sed 's/\//-/')" - echo $docker_tag - echo "::set-output name=docker-tag::$docker_tag" - id: set-docker-tag + test_dir="$(echo $RELEASE_BRANCH | sed 's/\//-/')" + echo $test_dir + echo "::set-output name=test-directory::$test_dir" + id: set-test-dir - build-release-image: + # build-release-images builds all docker images that are relevant for the compatibility tests. If a single release + # branch is specified, only that image will be built, e.g. release-v6.0.x but if 'unreleased' is specified + # every image will be built. + build-release-images: runs-on: ubuntu-latest - needs: determine-docker-tag + strategy: + matrix: + release-branch: + - release/v2.5.x + - release/v3.4.x + - release/v4.2.x + - release/v5.0.x + - release/v5.1.x + - release/v6.0.x steps: - uses: actions/checkout@v3 + if: env.RELEASE_BRANCH == 'unreleased' || env.RELEASE_BRANCH == matrix.release-branch with: - ref: "${{ env.RELEASE_BRANCH }}" + ref: "${{ matrix.release-branch }}" fetch-depth: 0 - name: Log in to the Container registry + if: env.RELEASE_BRANCH == 'unreleased' || env.RELEASE_BRANCH == matrix.release-branch uses: docker/login-action@f4ef78c080cd8ba55a85445d5b36e214a81df20a with: registry: ${{ env.REGISTRY }} username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Fetch Makefile dependencies + if: env.RELEASE_BRANCH == 'unreleased' || env.RELEASE_BRANCH == matrix.release-branch run: | mkdir -p contrib/devtools curl https://raw.githubusercontent.com/cosmos/ibc-go/main/contrib/devtools/Makefile -o contrib/devtools/Makefile - name: Fetch latest Dockerfile + if: env.RELEASE_BRANCH == 'unreleased' || env.RELEASE_BRANCH == matrix.release-branch run: curl https://raw.githubusercontent.com/cosmos/ibc-go/main/Dockerfile -o Dockerfile - name: Build image + if: env.RELEASE_BRANCH == 'unreleased' || env.RELEASE_BRANCH == matrix.release-branch run: | - docker build . -t "${REGISTRY}/${ORG}/${IMAGE_NAME}:${{ needs.determine-docker-tag.outputs.docker-tag }}" - docker push "${REGISTRY}/${ORG}/${IMAGE_NAME}:${{ needs.determine-docker-tag.outputs.docker-tag }}" + docker_tag="$(echo ${{ matrix.release-branch }} | sed 's/\//-/')" + docker build . -t "${REGISTRY}/${ORG}/${IMAGE_NAME}:$docker_tag" + docker push "${REGISTRY}/${ORG}/${IMAGE_NAME}:$docker_tag" transfer: needs: - - build-release-image - - determine-docker-tag + - build-release-images + - determine-test-directory uses: ./.github/workflows/e2e-compatibility-workflow-call.yaml with: - docker-tag: "${{ needs.determine-docker-tag.outputs.docker-tag }}" + test-file-directory: "${{ needs.determine-test-directory.outputs.test-directory }}" test-suite: "transfer" transfer-params: @@ -86,18 +104,18 @@ jobs: client: needs: - - build-release-image - - determine-docker-tag + - build-release-images + - determine-test-directory uses: ./.github/workflows/e2e-compatibility-workflow-call.yaml with: - docker-tag: "${{ needs.determine-docker-tag.outputs.docker-tag }}" + test-file-directory: "${{ needs.determine-test-directory.outputs.test-directory }}" test-suite: "client" incentivized-transfer: needs: - - build-release-image - - determine-docker-tag + - build-release-images + - determine-test-directory uses: ./.github/workflows/e2e-compatibility-workflow-call.yaml with: - docker-tag: "${{ needs.determine-docker-tag.outputs.docker-tag }}" + test-file-directory: "${{ needs.determine-test-directory.outputs.test-directory }}" test-suite: "incentivized-transfer" From 3e072a131a23808c871b0a3f2a7e3b9ab834414b Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Tue, 15 Nov 2022 14:33:24 +0000 Subject: [PATCH 9/9] chore: integrated git cliff into the code base to automate generation of changelogs --- .github/PULL_REQUEST_TEMPLATE.md | 35 +++++++-- CHANGELOG.md | 2 +- CONTRIBUTING.md | 23 ++++++ Makefile | 4 ++ cliff.toml | 119 +++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 7 deletions(-) create mode 100644 cliff.toml diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 4479c1cfc8b..c517591a095 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -12,18 +12,41 @@ are the most critical to review. closes: #XXXX + +### Commit Message / Changelog Entry + +```bash +type: commit message +``` + +see the [guidelines](../CONTRIBUTING.md#commit-messages) for commit messages. (view raw markdown for examples) + + + + --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. -- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting)) +- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting)). - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md). -- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing) -- [ ] Updated relevant documentation (`docs/`) or specification (`x//spec/`) +- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing). +- [ ] Updated relevant documentation (`docs/`) or specification (`x//spec/`). - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). -- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` -- [ ] Re-reviewed `Files changed` in the Github PR explorer -- [ ] Review `Codecov Report` in the comment section below once CI passes +- [ ] Provide a [commit message](../CONTRIBUTING.md#commit-messages) to be used for the changelog entry in the PR description for review. +- [ ] Re-reviewed `Files changed` in the Github PR explorer. +- [ ] Review `Codecov Report` in the comment section below once CI passes. diff --git a/CHANGELOG.md b/CHANGELOG.md index e413de28dbf..350e65b67e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ Types of changes (Stanzas): "Bug Fixes" for any bug fixes. "Client Breaking" for breaking CLI commands and REST routes used by end-users. "API Breaking" for breaking exported APIs used by developers building on SDK. -"State Machine Breaking" for any changes that result in a different AppState given same genesisState and txList. +"State Machine Breaking" for any changes that result in a different AppState given the same genesisState and txList. Ref: https://keepachangelog.com/en/1.0.0/ --> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b1111fc6d10..292a924d06b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -85,6 +85,29 @@ All PRs require an approval from at least one CODEOWNER before merge. PRs which - If you sat down with the PR submitter and did a pairing review please note that in the `Approval`, or your PR comments. - If you are only making "surface level" reviews, submit any notes as `Comments` without adding a review. +### Commit Messages + +Commit messages should be [conventional](https://www.conventionalcommits.org/en/v1.0.0/). + +If opening a PR, include the proposed commit message in the PR description. + +The commit message type should be one of: + +* `feat` / `feature` for feature work. +* `bug` / `fix` for bug fixes. +* `imp` / `improvements` for improvements. +* `doc` / `docs` / `documentation` for any documentation changes. +* `test` / `e2e` for addition or improvements of unit, integration and e2e tests or their corresponding infrastructure. +* `deprecated` for deprecation changes. +* `deps` / `build` for changes to dependencies. +* `chore` / `misc` / `nit` for any miscellaneous changes that don't fit into another category. + +**Note**: If any change is breaking, the following format must be used: +* `type` + `(api)!` for api breaking changes, e.g. `fix(api)!: api breaking fix` +* `type` + `(statemachine)!` for state machine breaking changes, e.g. `fix(statemachine)!: state machine breaking fix` + +**`api` breaking changes take precedence over `statemachine` breaking changes.** + ### Updating Documentation If you open a PR on ibc-go, it is mandatory to update the relevant documentation in /docs. diff --git a/Makefile b/Makefile index dd4785ce10b..12735069e29 100644 --- a/Makefile +++ b/Makefile @@ -171,6 +171,10 @@ view-docs: @cd docs && \ npm install && npm run serve + +changelog: + docker run --rm -v "$$(pwd)"/.git:/app/ -v "$$(pwd)/cliff.toml":/app/cliff.toml orhunp/git-cliff:latest --unreleased --tag $(tag) + .PHONY: build-docs ############################################################################### diff --git a/cliff.toml b/cliff.toml new file mode 100644 index 00000000000..c29518f5bbf --- /dev/null +++ b/cliff.toml @@ -0,0 +1,119 @@ +# configuration file for git-cliff (0.1.0) + +[changelog] +# changelog header +header = """ + + +# Changelog +All notable changes to this project will be documented in this file. +""" +# template for the changelog body +# https://tera.netlify.app/docs/#introduction +body = """ +{% if version %}\ + ## [{{ version | trim_start_matches(pat="v") }}] - {{ timestamp | date(format="%Y-%m-%d") }} +{% else %}\ + ## [unreleased] +{% endif %}\ +{% for group, commits in commits | group_by(attribute="group") %} + ### {{ group | striptags | trim | upper_first }} + {% for commit in commits %} + * {{ commit.message | upper_first }}\ + {% endfor %} +{% endfor %}\n +""" +# remove the leading and trailing whitespace from the template +trim = true +# changelog footer +footer = """ + +""" + +[git] +# parse the commits based on https://www.conventionalcommits.org +conventional_commits = true +# filter out the commits that are not conventional +filter_unconventional = true +# process each line of a commit as an individual commit +split_commits = true +# regex for preprocessing the commit messages +commit_preprocessors = [ + # A reference to an issue is appened to commits that looks like "(#1234)", this will be replaced + # with a link to that issue, e.g. "[#$1234](https://github.com/cosmos/ibc-go/issues/1234)". + { pattern = '\((\w+\s)?#([0-9]+)\)', replace = "([#${2}](https://github.com/cosmos/ibc-go/issues/${2}))" }, + # any reference to a pr like "pr-1234" will be replaced with a link to the PR. + { pattern = '\(pr-([0-9]+)\)', replace = "([#${1}](https://github.com/cosmos/ibc-go/pulls/${1}))" }, + + # the following patterns only exist because "split_commits" is set to true, and we are processesing + # each line of the commit as a separate message. + # these exist to filter out common messages that appear in commit messages that are technically + # conventional, but we do not way to include in the changelog. + { pattern = '^Signed-off-by:.*', replace='' }, + { pattern = '^Co-authored-by:.*', replace='' }, + # don't include references to issues as changelog entries. + { pattern = '^ref:.*', replace='' }, + # exclude CVSS format, CVE can still be included in regular conventinal commits. + { pattern = 'CVSS:.*', replace='' }, + # don't include dependabot auto merge entries. + { pattern = '.*dependabot-automerge-.*', replace='' }, + # don't include statements saying which issue is closed. + { pattern = '^closes:.*', replace='' }, + # remove standalone links in the commit messages. + { pattern = '^https://.*', replace='' }, + # remove lines with html. + { pattern = '^<.*', replace='' }, +] + +# regex for parsing and grouping commits +commit_parsers = [ + # specifying the number in a comment is a workaround to enable ordering of groups. + # these comments are stripped out of the markdown with the filter "{{ group | striptags | trim | upper_first }}" + # above in the body template. + { message = "^((?i)deps|(?i)dep|(?i)build)", group = "Dependencies" }, + { message = '^.*\(api\)!', group = "API Breaking" }, + { message = '^.*\(statemachine\)!', group = "State Machine Breaking" }, + { message = "^((?i)improvements|(?i)imp)", group = "Improvements" }, + { message = "^((?i)feature|(?i)feat)", group = "Features" }, + { message = "^((?i)fix|(?i)bug)", group = "Bug Fixes" }, + { message = "^((?i)doc|(?i)docs|(?i)documentation)", group = "Documentation" }, + { message = "^((?i)test|(?i)e2e)", group = "Testing" }, + { message = "^((?i)deprecated)", group = "Deprecated" }, + { message = "^((?i)chore|(?i)misc|(?i)nit)", group = "Miscellaneous Tasks" }, +] +# filter out the commits that are not matched by commit parsers +filter_commits = false +# glob pattern for matching git tags +tag_pattern = "v[0-9]*" +# regex for skipping tags +skip_tags = "" +# regex for ignoring tags +ignore_tags = "" +# sort the tags chronologically +date_order = false +# sort the commits inside sections by oldest/newest order +sort_commits = "oldest"