Skip to content

Commit

Permalink
Merge pull request CosmWasm#917 from CosmWasm/v0.27.0-dont-pass-non-w…
Browse files Browse the repository at this point in the history
…asm-events-to-reply

Don't pass non-wasm events to reply()
  • Loading branch information
ethanfrey authored and mattverse committed Aug 12, 2022
1 parent a575bce commit fc467ce
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 13 deletions.
26 changes: 26 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,32 @@
# Changelog

## [Unreleased](https://github.com/CosmWasm/wasmd/tree/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)

[Full Changelog](https://github.com/CosmWasm/wasmd/compare/v0.26.0...v0.27.0)

**API Breaking**
Yes

Expand Down
3 changes: 3 additions & 0 deletions x/wasm/keeper/msg_dispatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 45 additions & 2 deletions x/wasm/keeper/msg_dispatcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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) {
Expand Down
15 changes: 4 additions & 11 deletions x/wasm/keeper/submsg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,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) {
Expand Down Expand Up @@ -249,7 +242,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,
Expand All @@ -269,7 +262,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,
Expand Down

0 comments on commit fc467ce

Please sign in to comment.