Skip to content

Commit

Permalink
fix(tm2/gnovm): multi-msg overwrites previous event(s) (#2030)
Browse files Browse the repository at this point in the history
Closes #2028 

root cause: `defer` was used inside of loop that handles multi-msg. It
was causing vm to use only last appended event value

---
## AS-IS
```json
{
  "jsonrpc": "2.0",
  "id": "",
  "result": {
    "height": "43",
    "results": {
      "deliver_tx": [
        {
          "ResponseBase": {
            "Error": null,
            "Data": null,
            "Events": [
              {
                "@type": "/tm.gnoEvent",
                "pkg_path": "gno.land/r/demo/event",
                "type": "other",
                "func": "Other",
                "attrs": [
                  {
                    "key": "k",
                    "value": "v"
                  }
                ]
              },
              {
                "@type": "/tm.gnoEvent",
                "pkg_path": "gno.land/r/demo/event",
                "type": "other",
                "func": "Other",
                "attrs": [
                  {
                    "key": "k",
                    "value": "v"
                  }
                ]
              }
            ],
            "Log": "msg:0,success:true,log:,events:[]\nmsg:1,success:true,log:,events:[]",
            "Info": ""
          },
          "GasWanted": "2000000",
          "GasUsed": "270168"
        }
      ],
      "end_block": {
        "ResponseBase": {
          "Error": null,
          "Data": null,
          "Events": null,
          "Log": "",
          "Info": ""
        },
        "ValidatorUpdates": null,
        "ConsensusParams": null,
        "Events": null
      },
      "begin_block": {
        "ResponseBase": {
          "Error": null,
          "Data": null,
          "Events": null,
          "Log": "",
          "Info": ""
        }
      }
    }
  }
}
```
---

## TO-BE (in this PR)
```json
{
  "jsonrpc": "2.0",
  "id": "",
  "result": {
    "height": "6",
    "results": {
      "deliver_tx": [
        {
          "ResponseBase": {
            "Error": null,
            "Data": null,
            "Events": [
              {
                "@type": "/tm.gnoEvent",
                "pkg_path": "gno.land/r/demo/event2",
                "type": "t",
                "func": "inner",
                "attrs": [
                  {
                    "key": "k",
                    "value": "v"
                  }
                ]
              },
              {
                "@type": "/tm.gnoEvent",
                "pkg_path": "gno.land/r/demo/event2",
                "type": "t",
                "func": "Hello",
                "attrs": [
                  {
                    "key": "k",
                    "value": "v"
                  }
                ]
              },
              {
                "@type": "/tm.gnoEvent",
                "pkg_path": "gno.land/r/demo/event",
                "type": "t",
                "func": "inner",
                "attrs": [
                  {
                    "key": "k",
                    "value": "v"
                  }
                ]
              },
              {
                "@type": "/tm.gnoEvent",
                "pkg_path": "gno.land/r/demo/event",
                "type": "t",
                "func": "Hello",
                "attrs": [
                  {
                    "key": "k",
                    "value": "v"
                  }
                ]
              },
              {
                "@type": "/tm.gnoEvent",
                "pkg_path": "gno.land/r/demo/event",
                "type": "other",
                "func": "Other",
                "attrs": [
                  {
                    "key": "k",
                    "value": "v"
                  }
                ]
              }
            ],
            "Log": "msg:0,success:true,log:,events:[{gno.land/r/demo/event2 t inner [{k v}] \u003cnil\u003e} {gno.land/r/demo/event2 t Hello [{k v}] \u003cnil\u003e} {gno.land/r/demo/event t inner [{k v}] \u003cnil\u003e} {gno.land/r/demo/event t Hello [{k v}] \u003cnil\u003e}]\nmsg:1,success:true,log:,events:[{gno.land/r/demo/event2 t inner [{k v}] \u003cnil\u003e} {gno.land/r/demo/event2 t Hello [{k v}] \u003cnil\u003e} {gno.land/r/demo/event t inner [{k v}] \u003cnil\u003e} {gno.land/r/demo/event t Hello [{k v}] \u003cnil\u003e} {gno.land/r/demo/event other Other [{k v}] \u003cnil\u003e}]",
            "Info": ""
          },
          "GasWanted": "2000000",
          "GasUsed": "270168"
        }
      ],
      "end_block": {
        "ResponseBase": {
          "Error": null,
          "Data": null,
          "Events": null,
          "Log": "",
          "Info": ""
        },
        "ValidatorUpdates": null,
        "ConsensusParams": null,
        "Events": null
      },
      "begin_block": {
        "ResponseBase": {
          "Error": null,
          "Data": null,
          "Events": null,
          "Log": "",
          "Info": ""
        }
      }
    }
  }
}
```

FYI, unable to provide any test cases(unit test, integration test or
txtar) due to lack of multi-msg testing.

---




<!-- please provide a detailed description of the changes made in this
pull request. -->

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

- [x] Added new tests, or not needed, or not feasible
- [ ] 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
- [ ] Provided any useful hints for running manual tests
- [ ] 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
r3v4s authored May 24, 2024
1 parent 90aa89c commit a548238
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 10 deletions.
9 changes: 9 additions & 0 deletions examples/gno.land/r/demo/event/event.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package event

import (
"std"
)

func Emit(value string) {
std.Emit("TAG", "key", value)
}
1 change: 1 addition & 0 deletions examples/gno.land/r/demo/event/gno.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module gno.land/r/demo/event
43 changes: 43 additions & 0 deletions gno.land/cmd/gnoland/testdata/event_multi_msg.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# load the package from $WORK directory
loadpkg gno.land/r/demo/simple_event $WORK/event

# start a new node
gnoland start

## test1 account should be available on default
gnokey query auth/accounts/${USER_ADDR_test1}
stdout 'height: 0'
stdout 'data: {'
stdout ' "BaseAccount": {'
stdout ' "address": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",'
stdout ' "coins": "[0-9]*ugnot",' # dynamic
stdout ' "public_key": null,'
stdout ' "account_number": "0",'
stdout ' "sequence": "0"'
stdout ' }'
stdout '}'
! stderr '.+' # empty


## sign
gnokey sign -tx-path $WORK/multi/multi_msg.tx -chainid=tendermint_test -account-number 0 -account-sequence 0 test1
stdout 'Tx successfully signed and saved to '

## broadcast
gnokey broadcast $WORK/multi/multi_msg.tx -quiet=false


-- event/simple_event.gno --
package simple_event

import (
"std"
)

func Event(value string) {
std.Emit("TAG", "KEY", value)
}

-- multi/multi_msg.tx --
{"msg":[{"@type":"/vm.m_call","caller":"g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5","send":"","pkg_path":"gno.land/r/demo/simple_event","func":"Event","args":["value11"]},{"@type":"/vm.m_call","caller":"g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5","send":"","pkg_path":"gno.land/r/demo/simple_event","func":"Event","args":["value22"]}],"fee":{"gas_wanted":"2000000","gas_fee":"1000000ugnot"},"signatures":null,"memo":""}

4 changes: 2 additions & 2 deletions gno.land/pkg/gnoclient/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,12 @@ func TestSendMultiple_Integration(t *testing.T) {

// Execute send
res, err := client.Send(baseCfg, msg1, msg2)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Equal(t, "", string(res.DeliverTx.Data))

// Get the new account balance
account, _, err := client.QueryAccount(toAddress)
assert.Nil(t, err)
assert.NoError(t, err)

expected := std.Coins{{"ugnot", int64(amount1 + amount2)}}
got := account.GetCoins()
Expand Down
4 changes: 4 additions & 0 deletions gnovm/stdlibs/stdshim/stdshim.gno
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,7 @@ func DecodeBech32(addr Address) (prefix string, bytes [20]byte, ok bool) {
func DerivePkgAddr(pkgPath string) (addr Address) {
panic(shimWarn)
}

func Emit(tag, key, value string) {
panic(shimWarn)
}
13 changes: 5 additions & 8 deletions tm2/pkg/sdk/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,8 @@ func (app *BaseApp) getContextForTx(mode RunTxMode, txBytes []byte) (ctx Context

// / runMsgs iterates through all the messages and executes them.
func (app *BaseApp) runMsgs(ctx Context, msgs []Msg, mode RunTxMode) (result Result) {
ctx = ctx.WithEventLogger(NewEventLogger())

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

data := make([]byte, 0, len(msgs))
Expand All @@ -641,23 +643,17 @@ func (app *BaseApp) runMsgs(ctx Context, msgs []Msg, mode RunTxMode) (result Res
}

var msgResult Result
ctx = ctx.WithEventLogger(NewEventLogger())

// run the message!
// skip actual execution for CheckTx mode
if mode != RunTxModeCheck {
msgResult = handler.Process(ctx, msg)
msgResult = handler.Process(ctx, msg) // ctx event logger being updated in handler
}

// Each message result's Data must be length prefixed in order to separate
// each result.
data = append(data, msgResult.Data...)
events = append(events, msgResult.Events...)
defer func() {
events = append(events, ctx.EventLogger().Events()...)
result.Events = events
}()
// TODO append msgevent from ctx. XXX XXX

// stop execution and return on first failed message
if !msgResult.IsOK() {
Expand All @@ -672,12 +668,13 @@ 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()...)

result.Error = ABCIError(err)
result.Data = data
result.Events = events
result.Log = strings.Join(msgLogs, "\n")
result.GasUsed = ctx.GasMeter().GasConsumed()
result.Events = events
return result
}

Expand Down

0 comments on commit a548238

Please sign in to comment.