From 8c1c48d7d64bd1f29a6593c6791c149d9cc8bd67 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 4 Aug 2023 08:24:17 +0000 Subject: [PATCH] refactor(x/bank): remove message events including `sender` attribute whose information is already present in the relevant events (#17273) --- CHANGELOG.md | 1 + tests/e2e/tx/service_test.go | 10 +++---- x/bank/keeper/keeper_test.go | 57 ++++++++++++------------------------ x/bank/keeper/send.go | 14 ++------- 4 files changed, 26 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a44429c8db..0ca0b3f5c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -353,6 +353,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (abci) [#15845](https://github.com/cosmos/cosmos-sdk/pull/15845) Remove duplicating events in `logs` * (baseapp) [#15519](https://github.com/cosmos/cosmos-sdk/pull/15519/files) BeginBlock & EndBlock events have begin or endblock in the events in order to identify which stage they are emitted from since they are returned to comet as FinalizeBlock events, * (store/streaming) [#15519](https://github.com/cosmos/cosmos-sdk/pull/15519/files) State Streaming removed emitting of beginblock, endblock and delivertx in favour of emitting FinalizeBlock. +* (x/bank) [#17273](https://github.com/cosmos/cosmos-sdk/pull/17273) Remove message events including `sender` attribute whose information is already present in the relevant events ### CLI Breaking Changes diff --git a/tests/e2e/tx/service_test.go b/tests/e2e/tx/service_test.go index cee50c6a5a..fa878a901f 100644 --- a/tests/e2e/tx/service_test.go +++ b/tests/e2e/tx/service_test.go @@ -187,11 +187,11 @@ func (s *E2ETestSuite) TestSimulateTx_GRPC() { // Check the result and gas used are correct. // // The 12 events are: - // - Sending Fee to the pool: coin_spent, coin_received, transfer and message.sender= + // - Sending Fee to the pool: coin_spent, coin_received and transfer // - tx.* events: tx.fee, tx.acc_seq, tx.signature - // - Sending Amount to recipient: coin_spent, coin_received, transfer and message.sender= - // - Msg events: message.module=bank and message.action=/cosmos.bank.v1beta1.MsgSend (in one message) - s.Require().Equal(12, len(res.GetResult().GetEvents())) + // - Sending Amount to recipient: coin_spent, coin_received and transfer + // - Msg events: message.module=bank, message.action=/cosmos.bank.v1beta1.MsgSend and message.sender= (in one message) + s.Require().Equal(10, len(res.GetResult().GetEvents())) s.Require().True(res.GetGasInfo().GetGasUsed() > 0) // Gas used sometimes change, just check it's not empty. } }) @@ -233,7 +233,7 @@ func (s *E2ETestSuite) TestSimulateTx_GRPCGateway() { s.Require().NoError(err) // Check the result and gas used are correct. s.Require().Len(result.GetResult().MsgResponses, 1) - s.Require().Equal(12, len(result.GetResult().GetEvents())) // See TestSimulateTx_GRPC for the 12 events. + s.Require().Equal(10, len(result.GetResult().GetEvents())) // See TestSimulateTx_GRPC for the 10 events. s.Require().True(result.GetGasInfo().GetGasUsed() > 0) // Gas used sometimes change, just check it's not empty. } }) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index cacd7db69e..49688b906d 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -844,20 +844,10 @@ func (suite *KeeperTestSuite) TestMsgSendEvents() { abci.EventAttribute{Key: sdk.AttributeKeyAmount, Value: newCoins.String()}, ) - event2 := sdk.Event{ - Type: sdk.EventTypeMessage, - Attributes: []abci.EventAttribute{}, - } - event2.Attributes = append( - event2.Attributes, - abci.EventAttribute{Key: banktypes.AttributeKeySender, Value: accAddrs[0].String()}, - ) - // events are shifted due to the funding account events events := ctx.EventManager().ABCIEvents() - require.Equal(10, len(events)) - require.Equal(abci.Event(event1), events[8]) - require.Equal(abci.Event(event2), events[9]) + require.Equal(8, len(events)) + require.Equal(abci.Event(event1), events[7]) } func (suite *KeeperTestSuite) TestMsgMultiSendEvents() { @@ -893,17 +883,7 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() { require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs)) events = ctx.EventManager().ABCIEvents() - require.Equal(12, len(events)) // 12 events because account funding causes extra minting + coin_spent + coin_recv events - - event1 := sdk.Event{ - Type: sdk.EventTypeMessage, - Attributes: []abci.EventAttribute{}, - } - event1.Attributes = append( - event1.Attributes, - abci.EventAttribute{Key: banktypes.AttributeKeySender, Value: accAddrs[0].String()}, - ) - require.Equal(abci.Event(event1), events[7]) + require.Equal(10, len(events)) // 10 events because account funding causes extra minting + coin_spent + coin_recv events // Set addr's coins and accAddrs[1]'s coins suite.mockFundAccount(accAddrs[0]) @@ -918,35 +898,34 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() { require.NoError(suite.bankKeeper.InputOutputCoins(ctx, input, outputs)) events = ctx.EventManager().ABCIEvents() - require.Equal(30, len(events)) // 27 due to account funding + coin_spent + coin_recv events + require.Equal(25, len(events)) // 25 due to account funding + coin_spent + coin_recv events + event1 := sdk.Event{ + Type: banktypes.EventTypeTransfer, + Attributes: []abci.EventAttribute{}, + } + event1.Attributes = append( + event1.Attributes, + abci.EventAttribute{Key: banktypes.AttributeKeyRecipient, Value: accAddrs[2].String()}, + ) + event1.Attributes = append( + event1.Attributes, + abci.EventAttribute{Key: sdk.AttributeKeyAmount, Value: newCoins.String()}) event2 := sdk.Event{ Type: banktypes.EventTypeTransfer, Attributes: []abci.EventAttribute{}, } event2.Attributes = append( event2.Attributes, - abci.EventAttribute{Key: banktypes.AttributeKeyRecipient, Value: accAddrs[2].String()}, + abci.EventAttribute{Key: banktypes.AttributeKeyRecipient, Value: accAddrs[3].String()}, ) event2.Attributes = append( event2.Attributes, - abci.EventAttribute{Key: sdk.AttributeKeyAmount, Value: newCoins.String()}) - event3 := sdk.Event{ - Type: banktypes.EventTypeTransfer, - Attributes: []abci.EventAttribute{}, - } - event3.Attributes = append( - event3.Attributes, - abci.EventAttribute{Key: banktypes.AttributeKeyRecipient, Value: accAddrs[3].String()}, - ) - event3.Attributes = append( - event3.Attributes, abci.EventAttribute{Key: sdk.AttributeKeyAmount, Value: newCoins2.String()}, ) // events are shifted due to the funding account events - require.Equal(abci.Event(event1), events[25]) - require.Equal(abci.Event(event2), events[27]) - require.Equal(abci.Event(event3), events[29]) + require.Equal(abci.Event(event1), events[22]) + require.Equal(abci.Event(event2), events[24]) } func (suite *KeeperTestSuite) TestSpendableCoins() { diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 09ad850af8..0963f34a6b 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -136,12 +136,6 @@ func (k BaseSendKeeper) InputOutputCoins(ctx context.Context, input types.Input, } sdkCtx := sdk.UnwrapSDKContext(ctx) - sdkCtx.EventManager().EmitEvent( - sdk.NewEvent( - sdk.EventTypeMessage, - sdk.NewAttribute(types.AttributeKeySender, input.Address), - ), - ) for _, out := range outputs { outAddress, err := k.ak.AddressCodec().StringToBytes(out.Address) @@ -201,18 +195,14 @@ func (k BaseSendKeeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccA // bech32 encoding is expensive! Only do it once for fromAddr fromAddrString := fromAddr.String() sdkCtx := sdk.UnwrapSDKContext(ctx) - sdkCtx.EventManager().EmitEvents(sdk.Events{ + sdkCtx.EventManager().EmitEvent( sdk.NewEvent( types.EventTypeTransfer, sdk.NewAttribute(types.AttributeKeyRecipient, toAddr.String()), sdk.NewAttribute(types.AttributeKeySender, fromAddrString), sdk.NewAttribute(sdk.AttributeKeyAmount, amt.String()), ), - sdk.NewEvent( - sdk.EventTypeMessage, - sdk.NewAttribute(types.AttributeKeySender, fromAddr.String()), - ), - }) + ) return nil }