From ee44cc06d0a6c8a35c5657cb765f6c6ba39902fa Mon Sep 17 00:00:00 2001 From: Assaf Morami Date: Thu, 28 Jul 2022 12:52:06 +0300 Subject: [PATCH 1/4] Don't pass non-wasm events to reply() Output events are not part of consensus and can be non-deterministic --- x/wasm/keeper/msg_dispatcher.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x/wasm/keeper/msg_dispatcher.go b/x/wasm/keeper/msg_dispatcher.go index a3e7e3a142..1a32af409f 100644 --- a/x/wasm/keeper/msg_dispatcher.go +++ b/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 From 9671a21430e5a0e64c4adf5dc7d273c1a976a7ff Mon Sep 17 00:00:00 2001 From: Assaf Morami Date: Thu, 28 Jul 2022 12:53:28 +0300 Subject: [PATCH 2/4] Oops --- x/wasm/keeper/msg_dispatcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/wasm/keeper/msg_dispatcher.go b/x/wasm/keeper/msg_dispatcher.go index 1a32af409f..51650d85ef 100644 --- a/x/wasm/keeper/msg_dispatcher.go +++ b/x/wasm/keeper/msg_dispatcher.go @@ -107,7 +107,7 @@ 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 { + if msg.Msg.Wasm == nil { filteredEvents = []sdk.Event{} } } // on failure, revert state from sandbox, and ignore events (just skip doing the above) From f7211b8dd05d040d6fbe67fb20980dcd2bb0e6f4 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 29 Jul 2022 07:54:13 +0200 Subject: [PATCH 3/4] Add changelog for fix --- CHANGELOG.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a718846617..9467435b3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,20 @@ ## [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)) + ## [v0.27.0](https://github.com/CosmWasm/wasmd/tree/v0.27.0) (2022-05-19) From ce1cf7446dc80c06cbdd54c3e003a7f6e49f56e4 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 29 Jul 2022 08:09:39 +0200 Subject: [PATCH 4/4] Fix tests in keeper --- CHANGELOG.md | 6 ++++ x/wasm/keeper/msg_dispatcher_test.go | 47 ++++++++++++++++++++++++++-- x/wasm/keeper/submsg_test.go | 15 +++------ 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9467435b3e..fedf6aafec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,12 @@ No - 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) diff --git a/x/wasm/keeper/msg_dispatcher_test.go b/x/wasm/keeper/msg_dispatcher_test.go index 529a208ff0..e514ae4119 100644 --- a/x/wasm/keeper/msg_dispatcher_test.go +++ b/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) { diff --git a/x/wasm/keeper/submsg_test.go b/x/wasm/keeper/submsg_test.go index 7d83c8ffcd..4d7075761f 100644 --- a/x/wasm/keeper/submsg_test.go +++ b/x/wasm/keeper/submsg_test.go @@ -96,15 +96,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) { @@ -247,7 +240,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, @@ -267,7 +260,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,