-
Notifications
You must be signed in to change notification settings - Fork 382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bug(tm2/gnovm): multi-msg call overwrites previous event #2028
Labels
🐞 bug
Something isn't working
investigating
This behavior is still being tested out
📦 🤖 gnovm
Issues or PRs gnovm related
Milestone
Comments
Maybe fixed |
This was referenced May 3, 2024
leohhhn
changed the title
bug: multi-msg call overwrites previous event
bug(tm2/gnovm): multi-msg call overwrites previous event
May 3, 2024
zivkovicmilos
pushed a commit
that referenced
this issue
May 24, 2024
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>
github-project-automation
bot
moved this from In Progress
to Done
in 🧙♂️gno.land core team
May 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
🐞 bug
Something isn't working
investigating
This behavior is still being tested out
📦 🤖 gnovm
Issues or PRs gnovm related
Bug: multi-msg call overwrites previous event
related pr #1653
Description
event gets overwrite in multi-msg transaction call
Steps to reproduce
deploy first contract
deploy second contract
call with multi-msg like below (with adena)
Expected behaviour
should print each function's events
Actual behaviour
number of event looks good, but contents are being over-write by last called function's event
The text was updated successfully, but these errors were encountered: