diff --git a/x/wasm/keeper/keeper.go b/x/wasm/keeper/keeper.go index 020c2b0838..2448122850 100644 --- a/x/wasm/keeper/keeper.go +++ b/x/wasm/keeper/keeper.go @@ -1072,19 +1072,12 @@ func NewDefaultWasmVMContractResponseHandler(md msgDispatcher) *DefaultWasmVMCon // Handle processes the data returned by a contract invocation. func (h DefaultWasmVMContractResponseHandler) Handle(ctx sdk.Context, contractAddr sdk.AccAddress, ibcPort string, messages []wasmvmtypes.SubMsg, origRspData []byte) ([]byte, error) { - em := sdk.NewEventManager() result := origRspData - switch rsp, err := h.md.DispatchSubmessages(ctx.WithEventManager(em), contractAddr, ibcPort, messages); { + switch rsp, err := h.md.DispatchSubmessages(ctx, contractAddr, ibcPort, messages); { case err != nil: return nil, sdkerrors.Wrap(err, "submessages") case rsp != nil: result = rsp } - // emit non message type events only - for _, e := range em.Events() { - if e.Type != sdk.EventTypeMessage { - ctx.EventManager().EmitEvent(e) - } - } return result, nil } diff --git a/x/wasm/keeper/keeper_test.go b/x/wasm/keeper/keeper_test.go index 041a991cc4..0cf5628ff7 100644 --- a/x/wasm/keeper/keeper_test.go +++ b/x/wasm/keeper/keeper_test.go @@ -1563,15 +1563,6 @@ func TestNewDefaultWasmVMContractResponseHandler(t *testing.T) { }, expErr: true, }, - "message events filtered out": { - setup: func(m *wasmtesting.MockMsgDispatcher) { - m.DispatchSubmessagesFn = func(ctx sdk.Context, contractAddr sdk.AccAddress, ibcPort string, msgs []wasmvmtypes.SubMsg) ([]byte, error) { - ctx.EventManager().EmitEvent(sdk.NewEvent(sdk.EventTypeMessage)) - return nil, nil - } - }, - expEvts: sdk.Events{}, - }, "message emit non message events": { setup: func(m *wasmtesting.MockMsgDispatcher) { m.DispatchSubmessagesFn = func(ctx sdk.Context, contractAddr sdk.AccAddress, ibcPort string, msgs []wasmvmtypes.SubMsg) ([]byte, error) { diff --git a/x/wasm/keeper/msg_dispatcher.go b/x/wasm/keeper/msg_dispatcher.go index 18e774ac86..7ed63b1729 100644 --- a/x/wasm/keeper/msg_dispatcher.go +++ b/x/wasm/keeper/msg_dispatcher.go @@ -99,10 +99,11 @@ func (d MessageDispatcher) DispatchSubmessages(ctx sdk.Context, contractAddr sdk } // if it succeeds, commit state changes from submessage, and pass on events to Event Manager + var filteredEvents []sdk.Event if err == nil { commit() - ctx.EventManager().EmitEvents(em.Events()) - ctx.EventManager().EmitEvents(events) + filteredEvents = filterEvents(append(em.Events(), events...)) + ctx.EventManager().EmitEvents(filteredEvents) } // on failure, revert state from sandbox, and ignore events (just skip doing the above) // we only callback if requested. Short-circuit here the cases we don't want to @@ -124,7 +125,7 @@ func (d MessageDispatcher) DispatchSubmessages(ctx sdk.Context, contractAddr sdk } result = wasmvmtypes.SubcallResult{ Ok: &wasmvmtypes.SubcallResponse{ - Events: sdkEventsToWasmVmEvents(events), + Events: sdkEventsToWasmVmEvents(filteredEvents), Data: responseData, }, } @@ -153,6 +154,17 @@ func (d MessageDispatcher) DispatchSubmessages(ctx sdk.Context, contractAddr sdk return rsp, nil } +func filterEvents(events []sdk.Event) []sdk.Event { + // pre-allocate space for efficiency + res := make([]sdk.Event, 0, len(events)) + for _, ev := range events { + if ev.Type != "message" { + res = append(res, ev) + } + } + return res +} + func sdkEventsToWasmVmEvents(events []sdk.Event) []wasmvmtypes.Event { res := make([]wasmvmtypes.Event, len(events)) for i, ev := range events { diff --git a/x/wasm/keeper/msg_dispatcher_test.go b/x/wasm/keeper/msg_dispatcher_test.go index 94d6f322ad..42f9eab0c4 100644 --- a/x/wasm/keeper/msg_dispatcher_test.go +++ b/x/wasm/keeper/msg_dispatcher_test.go @@ -85,6 +85,7 @@ func TestDispatchSubmessages(t *testing.T) { }}, replyer: &mockReplyer{ replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) { + ctx.EventManager().EmitEvent(sdk.NewEvent("wasm-reply")) return []byte("myReplyData"), nil }, }, @@ -99,7 +100,9 @@ func TestDispatchSubmessages(t *testing.T) { expEvents: []sdk.Event{{ Type: "myEvent", Attributes: []abci.EventAttribute{{Key: []byte("foo"), Value: []byte("bar")}}, - }}, + }, + sdk.NewEvent("wasm-reply"), + }, }, "with context events - released on commit": { msgs: []wasmvmtypes.SubMsg{{ @@ -266,6 +269,75 @@ func TestDispatchSubmessages(t *testing.T) { expData: []byte{}, expCommits: []bool{false, false}, }, + "message event filtered without reply": { + msgs: []wasmvmtypes.SubMsg{{ + ReplyOn: wasmvmtypes.ReplyNever, + }}, + replyer: &mockReplyer{ + replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) { + return nil, errors.New("should never be called") + }, + }, + msgHandler: &wasmtesting.MockMessageHandler{ + DispatchMsgFn: func(ctx sdk.Context, contractAddr sdk.AccAddress, contractIBCPortID string, msg wasmvmtypes.CosmosMsg) (events []sdk.Event, data [][]byte, err error) { + myEvents := []sdk.Event{ + sdk.NewEvent("message"), + sdk.NewEvent("execute", sdk.NewAttribute("foo", "bar")), + } + return myEvents, [][]byte{[]byte("myData")}, nil + }, + }, + expData: nil, + expCommits: []bool{true}, + expEvents: []sdk.Event{sdk.NewEvent("execute", sdk.NewAttribute("foo", "bar"))}, + }, + "reply gets proper events": { + msgs: []wasmvmtypes.SubMsg{{ID: 1, ReplyOn: wasmvmtypes.ReplyAlways}}, + replyer: &mockReplyer{ + replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) { + if reply.Result.Err != "" { + return nil, errors.New(reply.Result.Err) + } + res := reply.Result.Ok + + // ensure the input events are what we expect + // I didn't use require.Equal() to act more like a contract... but maybe that would be better + if len(res.Events) != 2 { + return nil, fmt.Errorf("event count: %#v", res.Events) + } + if res.Events[0].Type != "execute" { + return nil, fmt.Errorf("event0: %#v", res.Events[0]) + } + if res.Events[1].Type != "wasm" { + return nil, fmt.Errorf("event1: %#v", res.Events[1]) + } + + // let's add a custom event here and see if it makes it out + ctx.EventManager().EmitEvent(sdk.NewEvent("wasm-reply")) + + // update data from what we got in + return res.Data, nil + }, + }, + msgHandler: &wasmtesting.MockMessageHandler{ + DispatchMsgFn: func(ctx sdk.Context, contractAddr sdk.AccAddress, contractIBCPortID string, msg wasmvmtypes.CosmosMsg) (events []sdk.Event, data [][]byte, err error) { + events = []sdk.Event{ + sdk.NewEvent("message", sdk.NewAttribute("_contract_address", contractAddr.String())), + // we don't know what the contarctAddr will be so we can't use it in the final tests + sdk.NewEvent("execute", sdk.NewAttribute("_contract_address", "placeholder-random-addr")), + sdk.NewEvent("wasm", sdk.NewAttribute("random", "data")), + } + return events, [][]byte{[]byte("subData")}, nil + }, + }, + expData: []byte("subData"), + expCommits: []bool{true}, + expEvents: []sdk.Event{ + sdk.NewEvent("execute", sdk.NewAttribute("_contract_address", "placeholder-random-addr")), + sdk.NewEvent("wasm", sdk.NewAttribute("random", "data")), + sdk.NewEvent("wasm-reply"), + }, + }, } for name, spec := range specs { t.Run(name, func(t *testing.T) { diff --git a/x/wasm/keeper/submsg_test.go b/x/wasm/keeper/submsg_test.go index 820d4e1349..11c1647ced 100644 --- a/x/wasm/keeper/submsg_test.go +++ b/x/wasm/keeper/submsg_test.go @@ -93,7 +93,7 @@ func TestDispatchSubMsgSuccessCase(t *testing.T) { require.NotNil(t, res.Result.Ok) sub := res.Result.Ok assert.Empty(t, sub.Data) - require.Len(t, sub.Events, 3) + require.Len(t, sub.Events, 1) transfer := sub.Events[0] assert.Equal(t, "transfer", transfer.Type) @@ -101,22 +101,6 @@ func TestDispatchSubMsgSuccessCase(t *testing.T) { Key: "recipient", Value: fred.String(), }, transfer.Attributes[0]) - - sender := sub.Events[1] - assert.Equal(t, "message", sender.Type) - assert.Equal(t, wasmvmtypes.EventAttribute{ - Key: "sender", - Value: contractAddr.String(), - }, sender.Attributes[0]) - - // where does this come from? - module := sub.Events[2] - assert.Equal(t, "message", module.Type) - assert.Equal(t, wasmvmtypes.EventAttribute{ - Key: "module", - Value: "bank", - }, module.Attributes[0]) - } func TestDispatchSubMsgErrorHandling(t *testing.T) { @@ -262,7 +246,7 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) { submsgID: 5, msg: validBankSend, // note we charge another 40k for the reply call - resultAssertions: []assertion{assertReturnedEvents(3), assertGasUsed(123000, 125000)}, + resultAssertions: []assertion{assertReturnedEvents(1), assertGasUsed(116000, 121000)}, }, "not enough tokens": { submsgID: 6, @@ -282,7 +266,7 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) { msg: validBankSend, gasLimit: &subGasLimit, // uses same gas as call without limit - resultAssertions: []assertion{assertReturnedEvents(3), assertGasUsed(123000, 125000)}, + resultAssertions: []assertion{assertReturnedEvents(1), assertGasUsed(116000, 121000)}, }, "not enough tokens with limit": { submsgID: 16, @@ -300,7 +284,6 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) { // uses all the subGasLimit, plus the 92k or so for the main contract resultAssertions: []assertion{assertGasUsed(subGasLimit+92000, subGasLimit+94000), assertErrorString("out of gas")}, }, - "instantiate contract gets address in data and events": { submsgID: 21, msg: instantiateContract,