From 051b4848b46d0763497b23d1a8262e5e58b744b8 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Thu, 17 Aug 2023 00:11:53 +0000 Subject: [PATCH] fix: do not add `module` attribute in case of ibc messages (#1079) * Do not add `module` attribute in case of ibc * Update CHANGELOG.md * Add test * Apply suggestions from code review --- CHANGELOG.md | 1 + baseapp/baseapp.go | 7 ++- baseapp/baseapp_test.go | 45 ++++++++++++++++ baseapp/xxx_msg_test.go | 111 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 baseapp/xxx_msg_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index af85fd988a..f95f3ddbbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (ledger) [\#1040](https://github.com/Finschia/finschia-sdk/pull/1040) fix a bug(unable to connect nano S plus ledger on ubuntu) * (x/foundation) [\#1053](https://github.com/Finschia/finschia-sdk/pull/1053) Make x/foundation MsgExec propagate events * (baseapp) [\#1091](https://github.com/cosmos/cosmos-sdk/pull/1091) Add `events.GetAttributes` and `event.GetAttribute` methods to simplify the retrieval of an attribute from event(s) (backport #1075) +* (baseapp) [\#1079](https://github.com/cosmos/cosmos-sdk/pull/1079) Do not add `module` attribute in case of ibc messages ### Removed diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 98479a400a..cd0798dcc7 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -850,10 +850,13 @@ func createEvents(events sdk.Events, msg sdk.Msg) sdk.Events { // 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" + // e.g. "/cosmos.bank.v1beta1.MsgSend" => "bank" moduleName := strings.Split(eventMsgName, ".") if len(moduleName) > 1 { - msgEvent = msgEvent.AppendAttributes(sdk.NewAttribute(sdk.AttributeKeyModule, moduleName[1])) + // NOTE(0Tech): please remove this condition check after applying ibc-go v7, because it's hard coding for the ibc + if moduleName[0] != "/ibc" { + msgEvent = msgEvent.AppendAttributes(sdk.NewAttribute(sdk.AttributeKeyModule, moduleName[1])) + } } } diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 4dea4bc6bf..c9366eff08 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" @@ -229,3 +230,47 @@ func TestSetChanCheckTxSize(t *testing.T) { app = NewBaseApp(t.Name(), logger, db, nil) require.Equal(t, config.DefaultChanCheckTxSize, cap(app.chCheckTx)) } + +func TestCreateEvents(t *testing.T) { + testCases := map[string]struct { + eventsIn sdk.Events + messageName string + eventsOut sdk.Events + }{ + "cosmos": { + messageName: "cosmos.foo.v1beta1.MsgFoo", + eventsOut: sdk.Events{{Type: "message", Attributes: []abci.EventAttribute{{Key: []uint8{0x61, 0x63, 0x74, 0x69, 0x6f, 0x6e}, Value: []uint8{0x2f, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x66, 0x6f, 0x6f, 0x2e, 0x76, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0x2e, 0x4d, 0x73, 0x67, 0x46, 0x6f, 0x6f}, Index: false}, {Key: []uint8{0x73, 0x65, 0x6e, 0x64, 0x65, 0x72}, Value: []uint8{0x6c, 0x69, 0x6e, 0x6b, 0x31, 0x76, 0x33, 0x6a, 0x6b, 0x7a, 0x65, 0x72, 0x7a, 0x76, 0x34, 0x6a, 0x6b, 0x76, 0x6b, 0x75, 0x64, 0x32, 0x63, 0x6d}, Index: false}, {Key: []uint8{0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65}, Value: []uint8{0x66, 0x6f, 0x6f}, Index: false}}}}, + }, + "ibc without module attribute": { + messageName: "ibc.foo.bar.v1.MsgBar", + eventsOut: sdk.Events{{Type: "message", Attributes: []abci.EventAttribute{{Key: []uint8{0x61, 0x63, 0x74, 0x69, 0x6f, 0x6e}, Value: []uint8{0x2f, 0x69, 0x62, 0x63, 0x2e, 0x66, 0x6f, 0x6f, 0x2e, 0x62, 0x61, 0x72, 0x2e, 0x76, 0x31, 0x2e, 0x4d, 0x73, 0x67, 0x42, 0x61, 0x72}, Index: false}, {Key: []uint8{0x73, 0x65, 0x6e, 0x64, 0x65, 0x72}, Value: []uint8{0x6c, 0x69, 0x6e, 0x6b, 0x31, 0x76, 0x33, 0x6a, 0x6b, 0x7a, 0x65, 0x72, 0x7a, 0x76, 0x34, 0x6a, 0x6b, 0x76, 0x6b, 0x75, 0x64, 0x32, 0x63, 0x6d}, Index: false}}}}, + }, + "ibc with module attribute": { + eventsIn: sdk.Events{{Type: "message", Attributes: []abci.EventAttribute{{Key: []uint8{0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65}, Value: []uint8{0x66, 0x6f, 0x6f, 0x53, 0x62, 0x61, 0x72}, Index: false}}}}, + messageName: "ibc.foo.bar.v1.MsgBar", + eventsOut: sdk.Events{{Type: "message", Attributes: []abci.EventAttribute{{Key: []uint8{0x61, 0x63, 0x74, 0x69, 0x6f, 0x6e}, Value: []uint8{0x2f, 0x69, 0x62, 0x63, 0x2e, 0x66, 0x6f, 0x6f, 0x2e, 0x62, 0x61, 0x72, 0x2e, 0x76, 0x31, 0x2e, 0x4d, 0x73, 0x67, 0x42, 0x61, 0x72}, Index: false}, {Key: []uint8{0x73, 0x65, 0x6e, 0x64, 0x65, 0x72}, Value: []uint8{0x6c, 0x69, 0x6e, 0x6b, 0x31, 0x76, 0x33, 0x6a, 0x6b, 0x7a, 0x65, 0x72, 0x7a, 0x76, 0x34, 0x6a, 0x6b, 0x76, 0x6b, 0x75, 0x64, 0x32, 0x63, 0x6d}, Index: false}}}, {Type: "message", Attributes: []abci.EventAttribute{{Key: []uint8{0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65}, Value: []uint8{0x66, 0x6f, 0x6f, 0x53, 0x62, 0x61, 0x72}, Index: false}}}}, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + msg := NewMockXXXMessage(ctrl) + + signer := sdk.AccAddress([]byte("deadbeef")) + msg. + EXPECT(). + GetSigners(). + Return([]sdk.AccAddress{signer}). + AnyTimes() + msg. + EXPECT(). + XXX_MessageName(). + Return(tc.messageName). + AnyTimes() + + eventsOut := createEvents(tc.eventsIn, msg) + require.Equal(t, tc.eventsOut, eventsOut) + }) + } +} diff --git a/baseapp/xxx_msg_test.go b/baseapp/xxx_msg_test.go new file mode 100644 index 0000000000..79ed2873f1 --- /dev/null +++ b/baseapp/xxx_msg_test.go @@ -0,0 +1,111 @@ +package baseapp + +import ( + reflect "reflect" + + types "github.com/Finschia/finschia-sdk/types" + gomock "github.com/golang/mock/gomock" +) + +// MockXXXMessage is a mock of XXXMessage interface. +type MockXXXMessage struct { + ctrl *gomock.Controller + recorder *MockXXXMessageMockRecorder +} + +// MockXXXMessageMockRecorder is the mock recorder for MockXXXMessage. +type MockXXXMessageMockRecorder struct { + mock *MockXXXMessage +} + +// NewMockXXXMessage creates a new mock instance. +func NewMockXXXMessage(ctrl *gomock.Controller) *MockXXXMessage { + mock := &MockXXXMessage{ctrl: ctrl} + mock.recorder = &MockXXXMessageMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockXXXMessage) EXPECT() *MockXXXMessageMockRecorder { + return m.recorder +} + +// GetSigners mocks base method. +func (m *MockXXXMessage) GetSigners() []types.AccAddress { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSigners") + ret0, _ := ret[0].([]types.AccAddress) + return ret0 +} + +// GetSigners indicates an expected call of GetSigners. +func (mr *MockXXXMessageMockRecorder) GetSigners() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSigners", reflect.TypeOf((*MockXXXMessage)(nil).GetSigners)) +} + +// ProtoMessage mocks base method. +func (m *MockXXXMessage) ProtoMessage() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "ProtoMessage") +} + +// ProtoMessage indicates an expected call of ProtoMessage. +func (mr *MockXXXMessageMockRecorder) ProtoMessage() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ProtoMessage", reflect.TypeOf((*MockXXXMessage)(nil).ProtoMessage)) +} + +// Reset mocks base method. +func (m *MockXXXMessage) Reset() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Reset") +} + +// Reset indicates an expected call of Reset. +func (mr *MockXXXMessageMockRecorder) Reset() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Reset", reflect.TypeOf((*MockXXXMessage)(nil).Reset)) +} + +// String mocks base method. +func (m *MockXXXMessage) String() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "String") + ret0, _ := ret[0].(string) + return ret0 +} + +// String indicates an expected call of String. +func (mr *MockXXXMessageMockRecorder) String() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "String", reflect.TypeOf((*MockXXXMessage)(nil).String)) +} + +// ValidateBasic mocks base method. +func (m *MockXXXMessage) ValidateBasic() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ValidateBasic") + ret0, _ := ret[0].(error) + return ret0 +} + +// ValidateBasic indicates an expected call of ValidateBasic. +func (mr *MockXXXMessageMockRecorder) ValidateBasic() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateBasic", reflect.TypeOf((*MockXXXMessage)(nil).ValidateBasic)) +} + +// XXX_MessageName mocks base method. +func (m *MockXXXMessage) XXX_MessageName() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "XXX_MessageName") + ret0, _ := ret[0].(string) + return ret0 +} + +// XXX_MessageName indicates an expected call of XXX_MessageName. +func (mr *MockXXXMessageMockRecorder) XXX_MessageName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "XXX_MessageName", reflect.TypeOf((*MockXXXMessage)(nil).XXX_MessageName)) +}