Skip to content
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

Emit generic events from tx wasms #3088

Merged
merged 67 commits into from
May 9, 2024
Merged

Conversation

sug0
Copy link
Collaborator

@sug0 sug0 commented Apr 16, 2024

Describe your changes

Emit core events (i.e. namada_core::event::Event) from tx wasms. To this end, we leverage IBC's existing wasm infrastructure.

NOTE: IBC e2e tests are expected to fail, just like in the base PR.

Indicate on which release or other PRs this topic is based on

#3032

Diff for review: https://github.com/anoma/namada/compare/tiago/generalize-events..tiago/emit-generic-events-wasm
(https://github.com/anoma/namada/pull/3088/files/2fc3cc8bbb6302c1bbb3ccdee26d271ed6bae092..0ac23e8a2d514d30a94abb42eafc8c0d6f5ccebd)

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

Copy link

codecov bot commented Apr 16, 2024

Codecov Report

Attention: Patch coverage is 60.36543% with 564 lines in your changes are missing coverage. Please review.

Project coverage is 59.61%. Comparing base (ea843f7) to head (9a3e15e).
Report is 7 commits behind head on main.

Files Patch % Lines
crates/core/src/ibc/event.rs 37.83% 115 Missing ⚠️
...tes/apps/src/lib/node/ledger/shell/testing/node.rs 0.00% 88 Missing ⚠️
crates/core/src/event/extend.rs 68.90% 88 Missing ⚠️
crates/core/src/event.rs 72.82% 50 Missing ⚠️
crates/namada/src/ledger/governance/utils.rs 54.73% 43 Missing ⚠️
crates/sdk/src/rpc.rs 0.00% 32 Missing ⚠️
...s/apps/src/lib/node/ledger/shell/finalize_block.rs 29.41% 24 Missing ⚠️
crates/sdk/src/events/log/dumb_queries.rs 82.75% 20 Missing ⚠️
crates/sdk/src/masp.rs 0.00% 18 Missing ⚠️
crates/ibc/src/actions.rs 0.00% 14 Missing ⚠️
... and 16 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3088      +/-   ##
==========================================
+ Coverage   59.41%   59.61%   +0.20%     
==========================================
  Files         298      298              
  Lines       92326    92934     +608     
==========================================
+ Hits        54853    55401     +548     
- Misses      37473    37533      +60     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sug0 sug0 force-pushed the tiago/emit-generic-events-wasm branch 2 times, most recently from 4a481d9 to 1ea7a77 Compare April 19, 2024 08:40
@sug0 sug0 mentioned this pull request Apr 19, 2024
2 tasks
@sug0 sug0 force-pushed the tiago/emit-generic-events-wasm branch 2 times, most recently from 7220e48 to 0ffd059 Compare April 19, 2024 12:28
@sug0 sug0 changed the title Tiago/emit generic events wasm Emit generic events from tx wasms Apr 19, 2024
sug0 added a commit that referenced this pull request Apr 19, 2024
@sug0 sug0 marked this pull request as ready for review April 19, 2024 12:40
@sug0 sug0 mentioned this pull request Apr 22, 2024
2 tasks
tzemanovic
tzemanovic previously approved these changes Apr 22, 2024
Copy link
Member

@tzemanovic tzemanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes look great! Will the e2e tests work after hermes update like in the base?

@sug0
Copy link
Collaborator Author

sug0 commented Apr 22, 2024

changes look great! Will the e2e tests work after hermes update like in the base?

haven't tested, but I think nothing broke between the base and the tip of this branch. I'll make sure to run them again tho, manually

Fraccaman pushed a commit that referenced this pull request Apr 22, 2024
sug0 added a commit that referenced this pull request Apr 23, 2024
@sug0 sug0 force-pushed the tiago/emit-generic-events-wasm branch from 0ac23e8 to d3d93fc Compare April 23, 2024 08:09
sug0 added a commit that referenced this pull request Apr 23, 2024
@sug0 sug0 force-pushed the tiago/emit-generic-events-wasm branch from d3d93fc to b23270a Compare April 23, 2024 08:14
sug0 added a commit that referenced this pull request Apr 23, 2024
@sug0 sug0 force-pushed the tiago/emit-generic-events-wasm branch from b23270a to a123582 Compare April 23, 2024 08:35
batconjurer
batconjurer previously approved these changes Apr 23, 2024
yito88
yito88 previously approved these changes Apr 25, 2024
@sug0 sug0 dismissed stale reviews from yito88 and batconjurer via 9a3e15e April 29, 2024 12:45
@sug0 sug0 force-pushed the tiago/emit-generic-events-wasm branch from a123582 to 9a3e15e Compare April 29, 2024 12:45
brentstone added a commit that referenced this pull request May 8, 2024
* origin/tiago/emit-generic-events-wasm:
  Changelog for #3088
  Extend WASM events with the current height in FinalizeBlock
  Fix IBC unit tests
  Keep all IBC events in VP pseudo exec ctx
  Query arbitrary events from VPs
  Emit and query arbitrary events from wasm txs
  Change TxEnv def to allow emitting/querying arbitrary events
  Further remove IBC specific event logic from write log
  Attempt to unify event kinds under the write log
  Docstr fixes
  Match the prefix of an event type
  Allow extending query matcher with new attrs
  Write a more accurate docstr on attributes map
  Impl Hash on events
  Implement Ord on events
  Switch to BTreeMap in event attributes
  Implement AttributesMap on BTreeMap
  Derive serde traits on events
@brentstone brentstone merged commit 349274b into main May 9, 2024
17 of 19 checks passed
@brentstone brentstone deleted the tiago/emit-generic-events-wasm branch May 9, 2024 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants