-
Notifications
You must be signed in to change notification settings - Fork 625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Emit event upon setting upgrade consensus state #1741
Changes from 6 commits
5943f62
572cc40
d05b931
bb717f1
41b4e35
92db553
4688051
09afa77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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" | ||
|
@@ -74,3 +77,55 @@ func (suite *ClientTestSuite) TestBeginBlockerConsensusState() { | |
suite.Require().NoError(err) | ||
suite.Require().Equal(bz, consState) | ||
} | ||
|
||
func (suite *ClientTestSuite) TestBeginBlockerWithUpgradePlan_EmitsUpgradeChainEvent() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I don't think underscores in test names are convention in Go, right? But I mean its probably fine to be honest, the names are meaningful as is! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah no harm simplifying the name a bit, great suggestion! |
||
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) | ||
|
||
cacheCtx, writeCache := suite.chainA.GetContext().CacheContext() | ||
|
||
client.BeginBlocker(cacheCtx, suite.chainA.App.GetIBCKeeper().ClientKeeper) | ||
writeCache() | ||
|
||
suite.requireContainsEvent(cacheCtx.EventManager().Events(), types.EventTypeUpgradeChain, true) | ||
Comment on lines
+99
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just as a note, the caching of the context should be unnecessary in this situation. You should be able to get the event via |
||
} | ||
|
||
func (suite *ClientTestSuite) TestBeginBlockerWithoutUpgradePlan_DoesNotEmitUpgradeChainEvent() { | ||
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 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 | ||
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, ",")) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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))), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this PR Fede recommended to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The type is |
||
sdk.NewAttribute(types.AttributeKeyUpgradeStore, upgradetypes.StoreKey), // which store to query proof of consensus state from | ||
), | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should we group
upgradetypes
with the above?