gno icon indicating copy to clipboard operation
gno copied to clipboard

fix(tm2/gnovm): multi-msg overwrites previous event(s)

Open r3v4s opened this issue 1 year ago • 4 comments

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

{
  "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)

{
  "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.


Contributors' checklist...
  • [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, if any. More info here.

r3v4s avatar May 03 '24 10:05 r3v4s

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 49.26%. Comparing base (f3ddc44) to head (c990b49). Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2030      +/-   ##
==========================================
+ Coverage   49.14%   49.26%   +0.11%     
==========================================
  Files         576      576              
  Lines       77597    78318     +721     
==========================================
+ Hits        38137    38585     +448     
- Misses      36368    36611     +243     
- Partials     3092     3122      +30     
Flag Coverage Δ
gno.land 61.38% <ø> (-0.31%) :arrow_down:
gnovm 42.20% <ø> (+0.01%) :arrow_up:
tm2 54.62% <100.00%> (-0.08%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 03 '24 10:05 codecov[bot]

@r3v4s

Can you provide the code you used to get the block output from the PR description?

leohhhn avatar May 03 '24 16:05 leohhhn

@r3v4s

Can you provide the code you used to get the block output from the PR description?

oh result is actual chain result (:26657/block_results)

r3v4s avatar May 03 '24 17:05 r3v4s

Could you add a test verifying the behavior? Thanks!

ajnavarro avatar May 15 '24 07:05 ajnavarro