Skip to content

Commit

Permalink
fix: Don't emit events from failed transactions (#2806)
Browse files Browse the repository at this point in the history
<!-- please provide a detailed description of the changes made in this
pull request. -->
I noticed that we emit events from the VM even when a transaction fails.
This is very difficult to write tests for because we don't display
events when a transaction fails, but I was able to verify the following
behavior BEFORE this fix:
1. Events emitted for failing transactions are stored in the block
results
2. Events emitted by `r/sys/validators` will be processed and the state
updated as normal, even if the transaction that emitted them fails

Correct me if I'm wrong, but I don't think we want to persist or take
any other actions on events sourced from failing transactions.

I'm open to suggestions on how to write tests for this, but the fix
should be self-explanatory.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
- [x] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
  • Loading branch information
deelawn committed Sep 19, 2024
1 parent f87ba5d commit 5450f64
Showing 1 changed file with 9 additions and 4 deletions.
13 changes: 9 additions & 4 deletions tm2/pkg/sdk/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,11 +625,12 @@ func (app *BaseApp) runMsgs(ctx Context, msgs []Msg, mode RunTxMode) (result Res
ctx = ctx.WithEventLogger(NewEventLogger())

msgLogs := make([]string, 0, len(msgs))

data := make([]byte, 0, len(msgs))
err := error(nil)

events := []Event{}
var (
err error
events = []Event{}
)

// NOTE: GasWanted is determined by ante handler and GasUsed by the GasMeter.
for i, msg := range msgs {
Expand Down Expand Up @@ -660,14 +661,18 @@ func (app *BaseApp) runMsgs(ctx Context, msgs []Msg, mode RunTxMode) (result Res
fmt.Sprintf("msg:%d,success:%v,log:%s,events:%v",
i, false, msgResult.Log, events))
err = msgResult.Error
events = nil
break
}

msgLogs = append(msgLogs,
fmt.Sprintf("msg:%d,success:%v,log:%s,events:%v",
i, true, msgResult.Log, events))
}
events = append(events, ctx.EventLogger().Events()...)

if err == nil {
events = append(events, ctx.EventLogger().Events()...)
}

result.Error = ABCIError(err)
result.Data = data
Expand Down

0 comments on commit 5450f64

Please sign in to comment.