From 23e356ae12a3c6ec41fa4b9eca28ceb0486fca8d Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 19 Dec 2022 18:39:48 +0100 Subject: [PATCH 1/7] fix: do not emit module name if modules already do --- UPGRADING.md | 4 ++++ baseapp/baseapp.go | 32 ++++++++++++++++++++++++-------- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 1258a0ea6690..f096e68a8e12 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -111,6 +111,10 @@ ctx.EventManager().EmitEvent( ) ``` +The module name is assumed by `baseapp` to be the second element of the message route: `"cosmos.bank.v1beta1.MsgSend" -> "bank"`. +In case a module does not follow the standard message path, (e.g. IBC), it is adviced to keep emit the module name event. +`Baseapp` only emits that event if the module have not already done so. + #### `x/gov` ##### Minimum Proposal Deposit At Time of Submission diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index de164579f3f3..fd36c2fd8a93 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -796,7 +796,7 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s } // create message events - msgEvents := createEvents(msg).AppendEvents(msgResult.GetEvents()) + msgEvents := createEvents(msgResult.GetEvents(), msg) // append message events, data and logs // @@ -838,7 +838,7 @@ func makeABCIData(msgResponses []*codectypes.Any) ([]byte, error) { return proto.Marshal(&sdk.TxMsgData{MsgResponses: msgResponses}) } -func createEvents(msg sdk.Msg) sdk.Events { +func createEvents(events sdk.Events, msg sdk.Msg) sdk.Events { eventMsgName := sdk.MsgTypeURL(msg) msgEvent := sdk.NewEvent(sdk.EventTypeMessage, sdk.NewAttribute(sdk.AttributeKeyAction, eventMsgName)) @@ -847,14 +847,30 @@ func createEvents(msg sdk.Msg) sdk.Events { msgEvent = msgEvent.AppendAttributes(sdk.NewAttribute(sdk.AttributeKeySender, msg.GetSigners()[0].String())) } - // here we assume that routes module name is the second element of the route - // e.g. "cosmos.bank.v1beta1.MsgSend" => "bank" - moduleName := strings.Split(eventMsgName, ".") - if len(moduleName) > 1 { - msgEvent = msgEvent.AppendAttributes(sdk.NewAttribute(sdk.AttributeKeyModule, moduleName[1])) + // we verify the no module name events have been emitted + hasModuleEvent := false + for _, event := range events { + if event.Type != sdk.EventTypeMessage { + continue + } + + for _, attr := range event.Attributes { + if attr.Key == sdk.AttributeKeyModule { + hasModuleEvent = true + } + } + } + + if !hasModuleEvent { + // here we assume that routes module name is the second element of the route + // e.g. "cosmos.bank.v1beta1.MsgSend" => "bank" + moduleName := strings.Split(eventMsgName, ".") + if len(moduleName) > 1 { + msgEvent = msgEvent.AppendAttributes(sdk.NewAttribute(sdk.AttributeKeyModule, moduleName[1])) + } } - return sdk.Events{msgEvent} + return sdk.Events{msgEvent}.AppendEvents(events) } // DefaultPrepareProposal returns the default implementation for processing an From ea8d632d460f4f174b35e173abb0b678a8a2b68f Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 19 Dec 2022 18:43:46 +0100 Subject: [PATCH 2/7] improve msg --- baseapp/baseapp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index fd36c2fd8a93..157a73bbc16f 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -847,7 +847,7 @@ func createEvents(events sdk.Events, msg sdk.Msg) sdk.Events { msgEvent = msgEvent.AppendAttributes(sdk.NewAttribute(sdk.AttributeKeySender, msg.GetSigners()[0].String())) } - // we verify the no module name events have been emitted + // we verify the the events have no module attribute hasModuleEvent := false for _, event := range events { if event.Type != sdk.EventTypeMessage { From 4752f4118f60ddc78d18e6de66f4754f7f6b551e Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 19 Dec 2022 18:48:46 +0100 Subject: [PATCH 3/7] updates --- baseapp/baseapp.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 157a73bbc16f..d703d5ccbc9a 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -848,7 +848,7 @@ func createEvents(events sdk.Events, msg sdk.Msg) sdk.Events { } // we verify the the events have no module attribute - hasModuleEvent := false + hasModuleNameEvent := false for _, event := range events { if event.Type != sdk.EventTypeMessage { continue @@ -856,12 +856,12 @@ func createEvents(events sdk.Events, msg sdk.Msg) sdk.Events { for _, attr := range event.Attributes { if attr.Key == sdk.AttributeKeyModule { - hasModuleEvent = true + hasModuleNameEvent = true } } } - if !hasModuleEvent { + if !hasModuleNameEvent { // here we assume that routes module name is the second element of the route // e.g. "cosmos.bank.v1beta1.MsgSend" => "bank" moduleName := strings.Split(eventMsgName, ".") From 87a45db4aed20e2c60fd1a140463a59f117ce76f Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 19 Dec 2022 21:30:31 +0100 Subject: [PATCH 4/7] typos --- UPGRADING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPGRADING.md b/UPGRADING.md index f096e68a8e12..c190d9253c56 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -112,7 +112,7 @@ ctx.EventManager().EmitEvent( ``` The module name is assumed by `baseapp` to be the second element of the message route: `"cosmos.bank.v1beta1.MsgSend" -> "bank"`. -In case a module does not follow the standard message path, (e.g. IBC), it is adviced to keep emit the module name event. +In case a module does not follow the standard message path, (e.g. IBC), it is advised to keep emitting the module name event. `Baseapp` only emits that event if the module have not already done so. #### `x/gov` From 7fde0bd42accb8e83dab2ea97a131cf2b3920416 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 19 Dec 2022 22:48:40 +0100 Subject: [PATCH 5/7] feedback --- CHANGELOG.md | 2 ++ baseapp/baseapp.go | 17 ++--------------- types/events.go | 24 ++++++++++++++++++++++++ types/events_test.go | 15 +++++++++++++++ 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e550e231ba88..081f1877545d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,8 +56,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/consensus) [#12905](https://github.com/cosmos/cosmos-sdk/pull/12905) Create a new `x/consensus` module that is now responsible for maintaining Tendermint consensus parameters instead of `x/param`. Legacy types remain in order to facilitate parameter migration from the deprecated `x/params`. App developers should ensure that they execute `baseapp.MigrateParams` during their chain upgrade. These legacy types will be removed in a future release. * (client/tx) [#13670](https://github.com/cosmos/cosmos-sdk/pull/13670) Add validation in `BuildUnsignedTx` to prevent simple inclusion of valid mnemonics * [#13473](https://github.com/cosmos/cosmos-sdk/pull/13473) ADR-038: Go plugin system proposal +* [#14356](https://github.com/cosmos/cosmos-sdk/pull/14356) Add `events.GetAttribute` and `event.GetAttribute` methods to simplify the retrieval of an attribute from an event. ### Improvements + * (types) [#14354](https://github.com/cosmos/cosmos-sdk/pull/14354) - improve performance on Context.KVStore and Context.TransientStore by 40% * (crypto/keyring) [#14151](https://github.com/cosmos/cosmos-sdk/pull/14151) Move keys presentation from `crypto/keyring` to `client/keys` * (types) [#14163](https://github.com/cosmos/cosmos-sdk/pull/14163) Refactor `(coins Coins) Validate()` to avoid unnecessary map. diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index d703d5ccbc9a..04c5deffb567 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -847,21 +847,8 @@ func createEvents(events sdk.Events, msg sdk.Msg) sdk.Events { msgEvent = msgEvent.AppendAttributes(sdk.NewAttribute(sdk.AttributeKeySender, msg.GetSigners()[0].String())) } - // we verify the the events have no module attribute - hasModuleNameEvent := false - for _, event := range events { - if event.Type != sdk.EventTypeMessage { - continue - } - - for _, attr := range event.Attributes { - if attr.Key == sdk.AttributeKeyModule { - hasModuleNameEvent = true - } - } - } - - if !hasModuleNameEvent { + // verify that events have no module attribute set + if _, found := events.GetAttributes(sdk.AttributeKeyModule); !found { // here we assume that routes module name is the second element of the route // e.g. "cosmos.bank.v1beta1.MsgSend" => "bank" moduleName := strings.Split(eventMsgName, ".") diff --git a/types/events.go b/types/events.go index 3415927516b4..0d8fafc02c5e 100644 --- a/types/events.go +++ b/types/events.go @@ -197,6 +197,17 @@ func (e Event) AppendAttributes(attrs ...Attribute) Event { return e } +// GetAttribute returns an attribute for a given key present in an event. +// If the key is not found, the boolean value will be false. +func (e Event) GetAttribute(key string) (Attribute, bool) { + for _, attr := range e.Attributes { + if attr.Key == key { + return Attribute{Key: attr.Key, Value: attr.Value}, true + } + } + return Attribute{}, false +} + // AppendEvent adds an Event to a slice of events. func (e Events) AppendEvent(event Event) Events { return append(e, event) @@ -218,6 +229,19 @@ func (e Events) ToABCIEvents() []abci.Event { return res } +// GetAttributes returns all attributes matching a given key present in events. +// If the key is not found, the boolean value will be false. +func (e Events) GetAttributes(key string) ([]Attribute, bool) { + attrs := make([]Attribute, 0) + for _, event := range e { + if attr, found := event.GetAttribute(key); found { + attrs = append(attrs, attr) + } + } + + return attrs, len(attrs) > 0 +} + // Common event types and attribute keys const ( EventTypeTx = "tx" diff --git a/types/events_test.go b/types/events_test.go index a9aad208398c..c17c5eab6c13 100644 --- a/types/events_test.go +++ b/types/events_test.go @@ -39,6 +39,21 @@ func (s *eventsTestSuite) TestAppendAttributes() { s.Require().Equal(e, sdk.NewEvent("transfer", sdk.NewAttribute("sender", "foo"), sdk.NewAttribute("recipient", "bar"))) } +func (s *eventsTestSuite) TestGetAttributes() { + e := sdk.NewEvent("transfer", sdk.NewAttribute("sender", "foo")) + e = e.AppendAttributes(sdk.NewAttribute("recipient", "bar")) + attr, found := e.GetAttribute("recipient") + s.Require().True(found) + s.Require().Equal(attr, sdk.NewAttribute("recipient", "bar")) + + events := sdk.Events{e}.AppendEvent(sdk.NewEvent("message", sdk.NewAttribute("sender", "bar"))) + attrs, found := events.GetAttributes("sender") + s.Require().True(found) + s.Require().Len(attrs, 2) + s.Require().Equal(attrs[0], sdk.NewAttribute("sender", "foo")) + s.Require().Equal(attrs[1], sdk.NewAttribute("sender", "bar")) +} + func (s *eventsTestSuite) TestEmptyEvents() { s.Require().Equal(sdk.EmptyEvents(), sdk.Events{}) } From adce5bd33317b5012b5779a57e1a1b2810306118 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 19 Dec 2022 22:49:28 +0100 Subject: [PATCH 6/7] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 081f1877545d..718fb529f3f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,7 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/consensus) [#12905](https://github.com/cosmos/cosmos-sdk/pull/12905) Create a new `x/consensus` module that is now responsible for maintaining Tendermint consensus parameters instead of `x/param`. Legacy types remain in order to facilitate parameter migration from the deprecated `x/params`. App developers should ensure that they execute `baseapp.MigrateParams` during their chain upgrade. These legacy types will be removed in a future release. * (client/tx) [#13670](https://github.com/cosmos/cosmos-sdk/pull/13670) Add validation in `BuildUnsignedTx` to prevent simple inclusion of valid mnemonics * [#13473](https://github.com/cosmos/cosmos-sdk/pull/13473) ADR-038: Go plugin system proposal -* [#14356](https://github.com/cosmos/cosmos-sdk/pull/14356) Add `events.GetAttribute` and `event.GetAttribute` methods to simplify the retrieval of an attribute from an event. +* [#14356](https://github.com/cosmos/cosmos-sdk/pull/14356) Add `events.GetAttributes` and `event.GetAttribute` methods to simplify the retrieval of an attribute from event(s). ### Improvements From 384a9ba6bb268cd8ed6f39de3a6c526e392a115a Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 19 Dec 2022 22:50:40 +0100 Subject: [PATCH 7/7] check false --- types/events_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/types/events_test.go b/types/events_test.go index c17c5eab6c13..bcd242d714de 100644 --- a/types/events_test.go +++ b/types/events_test.go @@ -45,6 +45,8 @@ func (s *eventsTestSuite) TestGetAttributes() { attr, found := e.GetAttribute("recipient") s.Require().True(found) s.Require().Equal(attr, sdk.NewAttribute("recipient", "bar")) + _, found = e.GetAttribute("foo") + s.Require().False(found) events := sdk.Events{e}.AppendEvent(sdk.NewEvent("message", sdk.NewAttribute("sender", "bar"))) attrs, found := events.GetAttributes("sender") @@ -52,6 +54,8 @@ func (s *eventsTestSuite) TestGetAttributes() { s.Require().Len(attrs, 2) s.Require().Equal(attrs[0], sdk.NewAttribute("sender", "foo")) s.Require().Equal(attrs[1], sdk.NewAttribute("sender", "bar")) + _, found = events.GetAttributes("foo") + s.Require().False(found) } func (s *eventsTestSuite) TestEmptyEvents() {