Merge pull request #917 from CosmWasm/v0.27.0-dont-pass-non-wasm-events-to-reply

Don't pass non-wasm events to reply()

CHANGELOG.md

@@ -2,7 +2,26 @@
 
 ## [Unreleased](https://github.com/CosmWasm/wasmd/tree/HEAD)
 
-[Full Changelog](https://github.com/CosmWasm/wasmd/compare/v0.27.0...HEAD)
+[Full Changelog](https://github.com/CosmWasm/wasmd/compare/v0.28.0...HEAD)
+
+## [v0.27.0](https://github.com/CosmWasm/wasmd/tree/v0.28.0) (2022-07-29)
+
+[Full Changelog](https://github.com/CosmWasm/wasmd/compare/v0.27.0...v0.28.0)
+
+**API Breaking**
+
+No
+
+**Fixed Bugs**
+
+- Fix: Make events in reply completely determinisitic by stripping out anything coming from Cosmos SDK (not CosmWasm codebase) [\#917](https://github.com/CosmWasm/wasmd/pull/917) ([assafmo](https://github.com/assafmo))
+
+Migration notes:
+
+* Contracts can no longer parse events from any calls except if they call another contract (or instantiate it, migrate it, etc).
+The main issue here is likely "Custom" queries from a blockchain, which want to send info (eg. how many tokens were swapped).
+Since those custom bindings are maintained by the chain, they can use the data field to pass any deterministic information
+back to the contract. We recommend using JSON encoding there with some documented format the contracts can parse out easily.
 
 ## [v0.27.0](https://github.com/CosmWasm/wasmd/tree/v0.27.0) (2022-05-19)
 

x/wasm/keeper/msg_dispatcher.go

@@ -107,6 +107,9 @@ func (d MessageDispatcher) DispatchSubmessages(ctx sdk.Context, contractAddr sdk
 			commit()
 			filteredEvents = filterEvents(append(em.Events(), events...))
 			ctx.EventManager().EmitEvents(filteredEvents)
+			if msg.Msg.Wasm == nil {
+				filteredEvents = []sdk.Event{}
+			}
 		} // 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

x/wasm/keeper/msg_dispatcher_test.go

@@ -296,8 +296,9 @@ func TestDispatchSubmessages(t *testing.T) {
 			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}},
+		"wasm reply gets proper events": {
+			// put fake wasmmsg in here to show where it comes from
+			msgs: []wasmvmtypes.SubMsg{{ID: 1, ReplyOn: wasmvmtypes.ReplyAlways, Msg: wasmvmtypes.CosmosMsg{Wasm: &wasmvmtypes.WasmMsg{}}}},
 			replyer: &mockReplyer{
 				replyFn: func(ctx sdk.Context, contractAddress sdk.AccAddress, reply wasmvmtypes.Reply) ([]byte, error) {
 					if reply.Result.Err != "" {
@@ -343,6 +344,48 @@ func TestDispatchSubmessages(t *testing.T) {
 				sdk.NewEvent("wasm-reply"),
 			},
 		},
+		"non-wasm reply events get filtered": {
+			// show events from a stargate message gets filtered out
+			msgs: []wasmvmtypes.SubMsg{{ID: 1, ReplyOn: wasmvmtypes.ReplyAlways, Msg: wasmvmtypes.CosmosMsg{Stargate: &wasmvmtypes.StargateMsg{}}}},
+			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) != 0 {
+						return nil, errors.New("events not filtered out")
+					}
+
+					// let's add a custom event here and see if it makes it out
+					ctx.EventManager().EmitEvent(sdk.NewEvent("stargate-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{
+						// this is filtered out
+						sdk.NewEvent("message", sdk.NewAttribute("stargate", "something-something")),
+						// we still emit this to the client, but not the contract
+						sdk.NewEvent("non-determinstic"),
+					}
+					return events, [][]byte{[]byte("subData")}, nil
+				},
+			},
+			expData:    []byte("subData"),
+			expCommits: []bool{true},
+			expEvents: []sdk.Event{
+				sdk.NewEvent("non-determinstic"),
+				// the event from reply is also exposed
+				sdk.NewEvent("stargate-reply"),
+			},
+		},
 	}
 	for name, spec := range specs {
 		t.Run(name, func(t *testing.T) {

x/wasm/keeper/submsg_test.go

@@ -94,15 +94,8 @@ 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)
-	assert.Equal(t, "coin_spent", sub.Events[0].Type)
-	assert.Equal(t, "coin_received", sub.Events[1].Type)
-	transfer := sub.Events[2]
-	assert.Equal(t, "transfer", transfer.Type)
-	assert.Equal(t, wasmvmtypes.EventAttribute{
-		Key:   "recipient",
-		Value: fred.String(),
-	}, transfer.Attributes[0])
+	// as of v0.28.0 we strip out all events that don't come from wasm contracts. can't trust the sdk.
+	require.Len(t, sub.Events, 0)
 }
 
 func TestDispatchSubMsgErrorHandling(t *testing.T) {
@@ -243,7 +236,7 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) {
 		"send tokens": {
 			submsgID:         5,
 			msg:              validBankSend,
-			resultAssertions: []assertion{assertReturnedEvents(3), assertGasUsed(112000, 112900)},
+			resultAssertions: []assertion{assertReturnedEvents(0), assertGasUsed(95000, 96000)},
 		},
 		"not enough tokens": {
 			submsgID:    6,
@@ -263,7 +256,7 @@ func TestDispatchSubMsgErrorHandling(t *testing.T) {
 			msg:      validBankSend,
 			gasLimit: &subGasLimit,
 			// uses same gas as call without limit (note we do not charge the 40k on reply)
-			resultAssertions: []assertion{assertReturnedEvents(3), assertGasUsed(112000, 113000)},
+			resultAssertions: []assertion{assertReturnedEvents(0), assertGasUsed(95000, 96000)},
 		},
 		"not enough tokens with limit": {
 			submsgID:    16,