From 5943f62b29b547fb91b6f026e6767536259819d5 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Wed, 20 Jul 2022 10:12:28 +0100 Subject: [PATCH 1/7] chore: emitting event on chain upgrade --- modules/core/02-client/abci.go | 1 + modules/core/02-client/keeper/events.go | 13 +++++++++++++ modules/core/02-client/types/events.go | 15 +++++++++------ 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/modules/core/02-client/abci.go b/modules/core/02-client/abci.go index ec209b4fdd0..7caa94886b4 100644 --- a/modules/core/02-client/abci.go +++ b/modules/core/02-client/abci.go @@ -26,6 +26,7 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper) { bz := k.MustMarshalConsensusState(upgradedConsState) k.SetUpgradedConsensusState(ctx, plan.Height, bz) + keeper.EmitUpgradeChainEvent(ctx, plan.Height) } } } diff --git a/modules/core/02-client/keeper/events.go b/modules/core/02-client/keeper/events.go index ec15ca41f4e..4497937184f 100644 --- a/modules/core/02-client/keeper/events.go +++ b/modules/core/02-client/keeper/events.go @@ -2,10 +2,12 @@ package keeper import ( "encoding/hex" + "strconv" "strings" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" "github.com/cosmos/ibc-go/v3/modules/core/exported" @@ -101,3 +103,14 @@ func EmitSubmitMisbehaviourEvent(ctx sdk.Context, clientID string, clientState e ), ) } + +// EmitUpgradeChainEvent emits an upgrade chain event. +func EmitUpgradeChainEvent(ctx sdk.Context, height int64) { + ctx.EventManager().EmitEvents(sdk.Events{ + sdk.NewEvent( + types.EventTypeUpgradeChain, + sdk.NewAttribute(types.AttributeKeyUpgradePlanHeight, strconv.Itoa(int(height))), + sdk.NewAttribute(types.AttributeKeyUpgradeStore, upgradetypes.StoreKey), // which store to query proof of consensus state from + ), + }) +} diff --git a/modules/core/02-client/types/events.go b/modules/core/02-client/types/events.go index 05708a84859..6975bcd6e66 100644 --- a/modules/core/02-client/types/events.go +++ b/modules/core/02-client/types/events.go @@ -8,12 +8,14 @@ import ( // IBC client events const ( - AttributeKeyClientID = "client_id" - AttributeKeySubjectClientID = "subject_client_id" - AttributeKeyClientType = "client_type" - AttributeKeyConsensusHeight = "consensus_height" - AttributeKeyConsensusHeights = "consensus_heights" - AttributeKeyHeader = "header" + AttributeKeyClientID = "client_id" + AttributeKeySubjectClientID = "subject_client_id" + AttributeKeyClientType = "client_type" + AttributeKeyConsensusHeight = "consensus_height" + AttributeKeyConsensusHeights = "consensus_heights" + AttributeKeyHeader = "header" + AttributeKeyUpgradeStore = "upgrade_store" + AttributeKeyUpgradePlanHeight = "upgrade_plan_height" ) // IBC client events vars @@ -23,6 +25,7 @@ var ( EventTypeUpgradeClient = "upgrade_client" EventTypeSubmitMisbehaviour = "client_misbehaviour" EventTypeUpdateClientProposal = "update_client_proposal" + EventTypeUpgradeChain = "upgrade_chain" AttributeValueCategory = fmt.Sprintf("%s_%s", host.ModuleName, SubModuleName) ) From 572cc4074cea5f5959a858adc37a10f94301db0b Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Wed, 20 Jul 2022 10:51:23 +0100 Subject: [PATCH 2/7] chore: adding callback to EmitUpgradeChainEvent for use in tests --- modules/core/02-client/abci.go | 4 +-- modules/core/02-client/abci_test.go | 48 +++++++++++++++++++++++++ modules/core/02-client/keeper/events.go | 10 ++++-- 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/modules/core/02-client/abci.go b/modules/core/02-client/abci.go index 7caa94886b4..3372be26030 100644 --- a/modules/core/02-client/abci.go +++ b/modules/core/02-client/abci.go @@ -8,7 +8,7 @@ import ( ) // BeginBlocker is used to perform IBC client upgrades -func BeginBlocker(ctx sdk.Context, k keeper.Keeper) { +func BeginBlocker(ctx sdk.Context, k keeper.Keeper, cbs ...func(sdk.Events)) { plan, found := k.GetUpgradePlan(ctx) if found { // Once we are at the last block this chain will commit, set the upgraded consensus state @@ -26,7 +26,7 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper) { bz := k.MustMarshalConsensusState(upgradedConsState) k.SetUpgradedConsensusState(ctx, plan.Height, bz) - keeper.EmitUpgradeChainEvent(ctx, plan.Height) + keeper.EmitUpgradeChainEvent(ctx, plan.Height, cbs...) } } } diff --git a/modules/core/02-client/abci_test.go b/modules/core/02-client/abci_test.go index 5d220f53a42..7b5683ead75 100644 --- a/modules/core/02-client/abci_test.go +++ b/modules/core/02-client/abci_test.go @@ -1,13 +1,16 @@ package client_test import ( + "strings" "testing" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" + client "github.com/cosmos/ibc-go/v3/modules/core/02-client" "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types" @@ -74,3 +77,48 @@ func (suite *ClientTestSuite) TestBeginBlockerConsensusState() { suite.Require().NoError(err) suite.Require().Equal(bz, consState) } + +func (suite *ClientTestSuite) TestBeginBlockerWithUpgradePlan_EmitsUpgradeChainEvent() { + plan := &upgradetypes.Plan{ + Name: "test", + Height: suite.chainA.GetContext().BlockHeight() + 1, + } + // set upgrade plan in the upgrade store + store := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetKey(upgradetypes.StoreKey)) + bz := suite.chainA.App.AppCodec().MustMarshal(plan) + store.Set(upgradetypes.PlanKey(), bz) + + nextValsHash := []byte("nextValsHash") + newCtx := suite.chainA.GetContext().WithBlockHeader(tmproto.Header{ + Height: suite.chainA.GetContext().BlockHeight(), + NextValidatorsHash: nextValsHash, + }) + + err := suite.chainA.GetSimApp().UpgradeKeeper.SetUpgradedClient(newCtx, plan.Height, []byte("client state")) + suite.Require().NoError(err) + + client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper, suite.requireContainsEvent(types.EventTypeUpgradeChain, true)) +} + +func (suite *ClientTestSuite) TestBeginBlockerWithoutUpgradePlan_DoesNotEmitUpgradeChainEvent() { + client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper, suite.requireContainsEvent(types.EventTypeUpgradeChain, false)) +} + +// requireContainsEvent returns a callback function which can be used to verify an event of a specific type was emitted. +func (suite *ClientTestSuite) requireContainsEvent(eventType string, shouldContain bool) func(events sdk.Events) { + return func(events sdk.Events) { + found := false + var eventTypes []string + for _, e := range events { + eventTypes = append(eventTypes, e.Type) + if e.Type == eventType { + found = true + } + } + if shouldContain { + suite.Require().True(found, "event type %s was not found in %s", eventType, strings.Join(eventTypes, ",")) + } else { + suite.Require().False(found, "event type %s was found in %s", eventType, strings.Join(eventTypes, ",")) + } + } +} diff --git a/modules/core/02-client/keeper/events.go b/modules/core/02-client/keeper/events.go index 4497937184f..2f993c5e7b0 100644 --- a/modules/core/02-client/keeper/events.go +++ b/modules/core/02-client/keeper/events.go @@ -105,12 +105,16 @@ func EmitSubmitMisbehaviourEvent(ctx sdk.Context, clientID string, clientState e } // EmitUpgradeChainEvent emits an upgrade chain event. -func EmitUpgradeChainEvent(ctx sdk.Context, height int64) { - ctx.EventManager().EmitEvents(sdk.Events{ +func EmitUpgradeChainEvent(ctx sdk.Context, height int64, cbs ...func(sdk.Events)) { + events := sdk.Events{ sdk.NewEvent( types.EventTypeUpgradeChain, sdk.NewAttribute(types.AttributeKeyUpgradePlanHeight, strconv.Itoa(int(height))), sdk.NewAttribute(types.AttributeKeyUpgradeStore, upgradetypes.StoreKey), // which store to query proof of consensus state from ), - }) + } + ctx.EventManager().EmitEvents(events) + for _, fn := range cbs { + fn(events) + } } From d05b93106b92f6a8215c94cb3ed05d2b3938251c Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Wed, 20 Jul 2022 11:02:15 +0100 Subject: [PATCH 3/7] chore: updating changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1459234ad64..a33e7a2a84b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (modules/core/02-client) [\#1195](https://github.com/cosmos/ibc-go/pull/1210) Removing `CheckHeaderAndUpdateState` from `ClientState` interface & associated light client implementations. * (modules/core/02-client) [\#1189](https://github.com/cosmos/ibc-go/pull/1212) Removing `CheckMisbehaviourAndUpdateState` from `ClientState` interface & associated light client implementations. * (modules/core/exported) [\#1206](https://github.com/cosmos/ibc-go/pull/1206) Adding new method `UpdateState` to `ClientState` interface. -* (testing) [\#1302](https://github.com/cosmos/ibc-go/pull/1302) Change testchain default behaviour to use a chainID in the revision format. Set `ChainIDSuffix` to an empty string to disable this functionality. +* (testing) [\#1302](https://github.com/cosmos/ibc-go/pull/1302) Change testchain default behaviour to use a chainID in the revision format. Set `ChainIDSuffix` to an empty string to disable this functionality. +* (modules/core/02-client) [\#1741](https://github.com/cosmos/ibc-go/pull/1741) Emitting a new `upgrade_chain` event upon setting upgrade consensus state. ### Features From 41b4e351b15a13de92cbb23369c0a16c87d98c5a Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 25 Jul 2022 12:13:07 +0100 Subject: [PATCH 4/7] chore: using writeCache to write events to cache to test they occurred --- modules/core/02-client/abci.go | 4 +-- modules/core/02-client/abci_test.go | 39 +++++++++++++++---------- modules/core/02-client/keeper/events.go | 10 ++----- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/modules/core/02-client/abci.go b/modules/core/02-client/abci.go index a1d2fbcce1c..0fb1cb5233a 100644 --- a/modules/core/02-client/abci.go +++ b/modules/core/02-client/abci.go @@ -8,7 +8,7 @@ import ( ) // BeginBlocker is used to perform IBC client upgrades -func BeginBlocker(ctx sdk.Context, k keeper.Keeper, cbs ...func(sdk.Events)) { +func BeginBlocker(ctx sdk.Context, k keeper.Keeper) { plan, found := k.GetUpgradePlan(ctx) if found { // Once we are at the last block this chain will commit, set the upgraded consensus state @@ -26,7 +26,7 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper, cbs ...func(sdk.Events)) { bz := k.MustMarshalConsensusState(upgradedConsState) k.SetUpgradedConsensusState(ctx, plan.Height, bz) - keeper.EmitUpgradeChainEvent(ctx, plan.Height, cbs...) + keeper.EmitUpgradeChainEvent(ctx, plan.Height) } } } diff --git a/modules/core/02-client/abci_test.go b/modules/core/02-client/abci_test.go index 2be0882a21a..7ccacc66ef6 100644 --- a/modules/core/02-client/abci_test.go +++ b/modules/core/02-client/abci_test.go @@ -97,28 +97,35 @@ func (suite *ClientTestSuite) TestBeginBlockerWithUpgradePlan_EmitsUpgradeChainE err := suite.chainA.GetSimApp().UpgradeKeeper.SetUpgradedClient(newCtx, plan.Height, []byte("client state")) suite.Require().NoError(err) - client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper, suite.requireContainsEvent(types.EventTypeUpgradeChain, true)) + cacheCtx, writeCache := suite.chainA.GetContext().CacheContext() + + client.BeginBlocker(cacheCtx, suite.chainA.App.GetIBCKeeper().ClientKeeper) + writeCache() + + suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, true) } func (suite *ClientTestSuite) TestBeginBlockerWithoutUpgradePlan_DoesNotEmitUpgradeChainEvent() { - client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper, suite.requireContainsEvent(types.EventTypeUpgradeChain, false)) + cacheCtx, writeCache := suite.chainA.GetContext().CacheContext() + client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) + writeCache() + suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, false) } // requireContainsEvent returns a callback function which can be used to verify an event of a specific type was emitted. -func (suite *ClientTestSuite) requireContainsEvent(eventType string, shouldContain bool) func(events sdk.Events) { - return func(events sdk.Events) { - found := false - var eventTypes []string - for _, e := range events { - eventTypes = append(eventTypes, e.Type) - if e.Type == eventType { - found = true - } - } - if shouldContain { - suite.Require().True(found, "event type %s was not found in %s", eventType, strings.Join(eventTypes, ",")) - } else { - suite.Require().False(found, "event type %s was found in %s", eventType, strings.Join(eventTypes, ",")) +func (suite *ClientTestSuite) requireContainsEvent(events sdk.Events, eventType string, shouldContain bool) { + found := false + var eventTypes []string + for _, e := range events { + eventTypes = append(eventTypes, e.Type) + if e.Type == eventType { + found = true + break } } + if shouldContain { + suite.Require().True(found, "event type %s was not found in %s", eventType, strings.Join(eventTypes, ",")) + } else { + suite.Require().False(found, "event type %s was found in %s", eventType, strings.Join(eventTypes, ",")) + } } diff --git a/modules/core/02-client/keeper/events.go b/modules/core/02-client/keeper/events.go index 2f993c5e7b0..4497937184f 100644 --- a/modules/core/02-client/keeper/events.go +++ b/modules/core/02-client/keeper/events.go @@ -105,16 +105,12 @@ func EmitSubmitMisbehaviourEvent(ctx sdk.Context, clientID string, clientState e } // EmitUpgradeChainEvent emits an upgrade chain event. -func EmitUpgradeChainEvent(ctx sdk.Context, height int64, cbs ...func(sdk.Events)) { - events := sdk.Events{ +func EmitUpgradeChainEvent(ctx sdk.Context, height int64) { + ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeUpgradeChain, sdk.NewAttribute(types.AttributeKeyUpgradePlanHeight, strconv.Itoa(int(height))), sdk.NewAttribute(types.AttributeKeyUpgradeStore, upgradetypes.StoreKey), // which store to query proof of consensus state from ), - } - ctx.EventManager().EmitEvents(events) - for _, fn := range cbs { - fn(events) - } + }) } From 92db55351896616206fc58652c23aa1605bf835b Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 25 Jul 2022 12:14:00 +0100 Subject: [PATCH 5/7] chore: corrected incorrect comment --- modules/core/02-client/abci_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/02-client/abci_test.go b/modules/core/02-client/abci_test.go index 7ccacc66ef6..ef9d2b239be 100644 --- a/modules/core/02-client/abci_test.go +++ b/modules/core/02-client/abci_test.go @@ -112,7 +112,7 @@ func (suite *ClientTestSuite) TestBeginBlockerWithoutUpgradePlan_DoesNotEmitUpgr suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, false) } -// requireContainsEvent returns a callback function which can be used to verify an event of a specific type was emitted. +// requireContainsEvent verifies if an event of a specific type was emitted. func (suite *ClientTestSuite) requireContainsEvent(events sdk.Events, eventType string, shouldContain bool) { found := false var eventTypes []string From 46880519339e3c8356c0d508832c2682669ce1db Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Tue, 26 Jul 2022 10:44:07 +0100 Subject: [PATCH 6/7] chore: addressing PR feedback --- modules/core/02-client/keeper/events.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/02-client/keeper/events.go b/modules/core/02-client/keeper/events.go index 4497937184f..56c537b224b 100644 --- a/modules/core/02-client/keeper/events.go +++ b/modules/core/02-client/keeper/events.go @@ -109,7 +109,7 @@ func EmitUpgradeChainEvent(ctx sdk.Context, height int64) { ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeUpgradeChain, - sdk.NewAttribute(types.AttributeKeyUpgradePlanHeight, strconv.Itoa(int(height))), + sdk.NewAttribute(types.AttributeKeyUpgradePlanHeight, strconv.FormatInt(height, 10)), sdk.NewAttribute(types.AttributeKeyUpgradeStore, upgradetypes.StoreKey), // which store to query proof of consensus state from ), }) From 09afa77cafcefdd5db1cce9bd7e39a3604912113 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Wed, 27 Jul 2022 10:28:41 +0100 Subject: [PATCH 7/7] chore: addressing PR feedback --- modules/core/02-client/abci_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/core/02-client/abci_test.go b/modules/core/02-client/abci_test.go index ef9d2b239be..72e9fbe44dd 100644 --- a/modules/core/02-client/abci_test.go +++ b/modules/core/02-client/abci_test.go @@ -5,12 +5,11 @@ import ( "testing" sdk "github.com/cosmos/cosmos-sdk/types" + upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" - upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" - client "github.com/cosmos/ibc-go/v3/modules/core/02-client" "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint" @@ -78,7 +77,7 @@ func (suite *ClientTestSuite) TestBeginBlockerConsensusState() { suite.Require().Equal(bz, consState) } -func (suite *ClientTestSuite) TestBeginBlockerWithUpgradePlan_EmitsUpgradeChainEvent() { +func (suite *ClientTestSuite) TestBeginBlockerUpgradeEvents() { plan := &upgradetypes.Plan{ Name: "test", Height: suite.chainA.GetContext().BlockHeight() + 1, @@ -105,7 +104,7 @@ func (suite *ClientTestSuite) TestBeginBlockerWithUpgradePlan_EmitsUpgradeChainE suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, true) } -func (suite *ClientTestSuite) TestBeginBlockerWithoutUpgradePlan_DoesNotEmitUpgradeChainEvent() { +func (suite *ClientTestSuite) TestBeginBlockerUpgradeEventsAbsence() { cacheCtx, writeCache := suite.chainA.GetContext().CacheContext() client.BeginBlocker(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) writeCache()