From 5450f64a0b8d2ea482df781bbd5360ea6c348987 Mon Sep 17 00:00:00 2001 From: deelawn Date: Thu, 19 Sep 2024 18:15:58 +0100 Subject: [PATCH] fix: Don't emit events from failed transactions (#2806) 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.
Contributors' checklist... - [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).
--- tm2/pkg/sdk/baseapp.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tm2/pkg/sdk/baseapp.go b/tm2/pkg/sdk/baseapp.go index 867a38d680a..671f18cf058 100644 --- a/tm2/pkg/sdk/baseapp.go +++ b/tm2/pkg/sdk/baseapp.go @@ -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 { @@ -660,6 +661,7 @@ 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 } @@ -667,7 +669,10 @@ func (app *BaseApp) runMsgs(ctx Context, msgs []Msg, mode RunTxMode) (result Res 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