From 99c0a54c54a0b17c2ab8893f053759e8185623ec Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Thu, 15 Feb 2024 23:37:11 -0600 Subject: [PATCH] refactor(abci): add msg index to event (#15845) (#526) * refactor(abci): add msg index to event (#15845) Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Co-authored-by: Facundo Medica * fix e2e tests * another e2e fix * e2e * reset groupID * e2e * switch check to events * e2e * e2e --------- Co-authored-by: Marko Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Co-authored-by: Facundo Medica --- baseapp/abci_test.go | 2 +- baseapp/baseapp.go | 11 +++++++---- docs/docs/core/08-events.md | 1 + tests/e2e/auth/suite.go | 21 +++++++++++++++++---- tests/e2e/group/tx.go | 15 +++++++-------- tests/e2e/staking/suite.go | 2 +- tests/e2e/tx/service_test.go | 15 ++++++++------- 7 files changed, 42 insertions(+), 25 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index c3d606db018a..691b6c94b8cf 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -676,7 +676,7 @@ func TestABCI_DeliverTx(t *testing.T) { events := res.GetEvents() require.Len(t, events, 3, "should contain ante handler, message type and counter events respectively") require.Equal(t, sdk.MarkEventsToIndex(counterEvent("ante_handler", counter).ToABCIEvents(), map[string]struct{}{})[0], events[0], "ante handler event") - require.Equal(t, sdk.MarkEventsToIndex(counterEvent(sdk.EventTypeMessage, counter).ToABCIEvents(), map[string]struct{}{})[0], events[2], "msg handler update counter event") + require.Equal(t, sdk.MarkEventsToIndex(counterEvent(sdk.EventTypeMessage, counter).ToABCIEvents(), map[string]struct{}{})[0].Attributes[0], events[2].Attributes[0], "msg handler update counter event") } suite.baseApp.EndBlock(abci.RequestEndBlock{}) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 9b4faa5376ed..704732714746 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "sort" + "strconv" "strings" dbm "github.com/cometbft/cometbft-db" @@ -781,7 +782,6 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // Handler does not exist for a given message route. Otherwise, a reference to a // Result is returned. The caller must not commit state if an error is returned. func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*sdk.Result, error) { - msgLogs := make(sdk.ABCIMessageLogs, 0, len(msgs)) events := sdk.EmptyEvents() var msgResponses []*codectypes.Any @@ -805,10 +805,15 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s // create message events msgEvents := createEvents(msgResult.GetEvents(), msg) - // append message events, data and logs + // append message events and data // // Note: Each message result's data must be length-prefixed in order to // separate each result. + for j, event := range msgEvents { + // append message index to all events + msgEvents[j] = event.AppendAttributes(sdk.NewAttribute("msg_index", strconv.Itoa(i))) + } + events = events.AppendEvents(msgEvents) // Each individual sdk.Result that went through the MsgServiceRouter @@ -824,7 +829,6 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s msgResponses = append(msgResponses, msgResponse) } - msgLogs = append(msgLogs, sdk.NewABCIMessageLog(uint32(i), msgResult.Log, msgEvents)) } data, err := makeABCIData(msgResponses) @@ -834,7 +838,6 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s return &sdk.Result{ Data: data, - Log: strings.TrimSpace(msgLogs.String()), Events: events.ToABCIEvents(), MsgResponses: msgResponses, }, nil diff --git a/docs/docs/core/08-events.md b/docs/docs/core/08-events.md index e8b1d5c33384..be19d9e3e907 100644 --- a/docs/docs/core/08-events.md +++ b/docs/docs/core/08-events.md @@ -27,6 +27,7 @@ An Event contains: * A `type` to categorize the Event at a high-level; for example, the Cosmos SDK uses the `"message"` type to filter Events by `Msg`s. * A list of `attributes` are key-value pairs that give more information about the categorized Event. For example, for the `"message"` type, we can filter Events by key-value pairs using `message.action={some_action}`, `message.module={some_module}` or `message.sender={some_sender}`. +* A `msg_index` to identify which messages relate to the same transaction :::tip To parse the attribute values as strings, make sure to add `'` (single quotes) around each attribute value. diff --git a/tests/e2e/auth/suite.go b/tests/e2e/auth/suite.go index e103f46052ae..489e7d6ac9bb 100644 --- a/tests/e2e/auth/suite.go +++ b/tests/e2e/auth/suite.go @@ -10,13 +10,13 @@ import ( "strings" "testing" + "cosmossdk.io/depinject" + "cosmossdk.io/math" + abci "github.com/cometbft/cometbft/abci/types" tmcli "github.com/cometbft/cometbft/libs/cli" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - "cosmossdk.io/depinject" - "cosmossdk.io/math" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/crypto/hd" @@ -545,7 +545,9 @@ func (s *E2ETestSuite) TestCLIQueryTxCmdByHash() { var result sdk.TxResponse s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &result)) s.Require().NotNil(result.Height) - s.Require().Contains(result.RawLog, tc.rawLogContains) + if ok := s.deepContains(result.Events, tc.rawLogContains); !ok { + s.Require().Fail("raw log does not contain the expected value, expected value: %s", tc.rawLogContains) + } } }) } @@ -2062,3 +2064,14 @@ func (s *E2ETestSuite) getBalances(clientCtx client.Context, addr sdk.AccAddress startTokens := balRes.Balances.AmountOf(denom) return startTokens } + +func (s *E2ETestSuite) deepContains(events []abci.Event, value string) bool { + for _, e := range events { + for _, attr := range e.Attributes { + if strings.Contains(attr.Value, value) { + return true + } + } + } + return false +} diff --git a/tests/e2e/group/tx.go b/tests/e2e/group/tx.go index bd54fc9b2baf..9c660ccbf465 100644 --- a/tests/e2e/group/tx.go +++ b/tests/e2e/group/tx.go @@ -1970,9 +1970,9 @@ func (s *E2ETestSuite) TestTxWithdrawProposal() { } func (s *E2ETestSuite) getProposalIDFromTxResponse(txResp sdk.TxResponse) string { - s.Require().Greater(len(txResp.Logs), 0) - s.Require().NotNil(txResp.Logs[0].Events) - events := txResp.Logs[0].Events + s.Require().Greater(len(txResp.Events), 0) + s.Require().NotNil(txResp.Events[0]) + events := txResp.Events createProposalEvent, _ := sdk.TypedEventToEvent(&group.EventSubmitProposal{}) for _, e := range events { @@ -2479,16 +2479,16 @@ func (s *E2ETestSuite) TestExecProposalsWhenMemberLeavesOrIsUpdated() { s.Require().NoError(err) if tc.expectLogErr { - s.Require().Contains(execResp.RawLog, tc.errMsg) + s.Require().True(strings.Contains(execResp.Events[len(execResp.Events)-1].String(), tc.errMsg)) } }) } } func (s *E2ETestSuite) getGroupIDFromTxResponse(txResp sdk.TxResponse) string { - s.Require().Greater(len(txResp.Logs), 0) - s.Require().NotNil(txResp.Logs[0].Events) - events := txResp.Logs[0].Events + s.Require().Greater(len(txResp.Events), 0) + s.Require().NotNil(txResp.Events[0]) + events := txResp.Events createProposalEvent, _ := sdk.TypedEventToEvent(&group.EventCreateGroup{}) for _, e := range events { @@ -2496,7 +2496,6 @@ func (s *E2ETestSuite) getGroupIDFromTxResponse(txResp sdk.TxResponse) string { return strings.ReplaceAll(e.Attributes[0].Value, "\"", "") } } - return "" } diff --git a/tests/e2e/staking/suite.go b/tests/e2e/staking/suite.go index 1b60e726b04f..a04a2debcb6b 100644 --- a/tests/e2e/staking/suite.go +++ b/tests/e2e/staking/suite.go @@ -229,7 +229,7 @@ func (s *E2ETestSuite) TestNewCreateValidatorCmd() { s.Require().Equal(tc.expectedCode, txResp.Code, out.String()) var hadEvent bool - events := txResp.Logs[0].GetEvents() + events := txResp.Events for i := 0; i < len(events); i++ { if events[i].GetType() == "create_validator" { attributes := events[i].GetAttributes() diff --git a/tests/e2e/tx/service_test.go b/tests/e2e/tx/service_test.go index f174344aabe5..a89ac9b7daab 100644 --- a/tests/e2e/tx/service_test.go +++ b/tests/e2e/tx/service_test.go @@ -337,10 +337,10 @@ func (s E2ETestSuite) TestGetTxEvents_GRPC() { "with pagination", &tx.GetTxsEventRequest{ Events: []string{bankMsgSendEventAction}, - Page: 2, + Page: 1, Limit: 2, }, - false, "", 1, + false, "", 2, }, { "with multi events", @@ -361,12 +361,13 @@ func (s E2ETestSuite) TestGetTxEvents_GRPC() { s.Require().NoError(err) s.Require().GreaterOrEqual(len(grpcRes.Txs), 1) s.Require().Equal("foobar", grpcRes.Txs[0].Body.Memo) - s.Require().Equal(len(grpcRes.Txs), tc.expLen) + s.Require().Equal(tc.expLen, len(grpcRes.Txs)) + // Make sure fields are populated. // ref: https://github.com/cosmos/cosmos-sdk/issues/8680 // ref: https://github.com/cosmos/cosmos-sdk/issues/8681 s.Require().NotEmpty(grpcRes.TxResponses[0].Timestamp) - s.Require().NotEmpty(grpcRes.TxResponses[0].RawLog) + s.Require().Empty(grpcRes.TxResponses[0].RawLog) // logs are empty if the transactions are successful } }) } @@ -395,9 +396,9 @@ func (s E2ETestSuite) TestGetTxEvents_GRPCGateway() { }, { "with pagination", - fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s&page=%d&limit=%d", val.APIAddress, bankMsgSendEventAction, 2, 2), + fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s&page=%d&limit=%d", val.APIAddress, bankMsgSendEventAction, 1, 2), false, - "", 1, + "", 2, }, { "valid request: order by asc", @@ -517,7 +518,7 @@ func (s E2ETestSuite) TestGetTx_GRPCGateway() { // ref: https://github.com/cosmos/cosmos-sdk/issues/8680 // ref: https://github.com/cosmos/cosmos-sdk/issues/8681 s.Require().NotEmpty(result.TxResponse.Timestamp) - s.Require().NotEmpty(result.TxResponse.RawLog) + s.Require().Empty(result.TxResponse.RawLog) // logs are empty on successful transactions } }) }