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

Implement ADR 032: Typed Events #7563

Closed
1 of 13 tasks
akhilkumarpilli opened this issue Oct 15, 2020 · 15 comments
Closed
1 of 13 tasks

Implement ADR 032: Typed Events #7563

akhilkumarpilli opened this issue Oct 15, 2020 · 15 comments
Labels
S:needs architecture review To discuss on the next architecture review call to come to alignment

Comments

@akhilkumarpilli
Copy link
Contributor

akhilkumarpilli commented Oct 15, 2020

Summary

Implement ADR 032: Typed Events as documented in #7474

Pros

Cons

Roadmap

@colin-axner
Copy link
Contributor

Resolve road blocks outlined in this comment

@amaury1093
Copy link
Contributor

Hey @akhilkumarpilli, what's the status of this issue? What are the next steps so that all modules only emit typed events?

@akhilkumarpilli
Copy link
Contributor Author

Hey @akhilkumarpilli, what's the status of this issue? What are the next steps so that all modules only emit typed events?

This issue tendermint/tendermint#5978 needs to be resolved first. Once after that, we can start implementing ADR-032 in all modules.

@tac0turtle
Copy link
Member

This issue tendermint/tendermint#5978 needs to be resolved first. Once after that, we can start implementing ADR-032 in all modules.

We will be providing a postgres indexer, can this be done through that?

As for the current indexer, we may say it is feature complete and not work on it anymore.

@akhilkumarpilli
Copy link
Contributor Author

This issue tendermint/tendermint#5978 needs to be resolved first. Once after that, we can start implementing ADR-032 in all modules.

We will be providing a postgres indexer, can this be done through that?

As for the current indexer, we may say it is feature complete and not work on it anymore.

Hope it will support. We will try to implement and test this once after postgres indexer is integrated. Thanks.

@clevinson
Copy link
Contributor

@marbar3778 can we discuss this next friday in our architecture call to align on if there's any tendermint blockers here?

@clevinson clevinson added Status: Ice Box S:needs architecture review To discuss on the next architecture review call to come to alignment and removed Status: Backlog labels Apr 30, 2021
@albertchon
Copy link
Contributor

I've been doing some benchmarking with emitting very large typed events and have found that the TypedEventToEvent takes a significant amount of time to run due to high amount of allocations in the ProtoMarshalJSON call.

Can a more efficient solution for typed events be worked on?

@aaronc
Copy link
Member

aaronc commented May 21, 2021

Based on on last architecture call, it sounds like the path forward for this may be to create a custom event system within the SDK that bypasses Tendermint. With that approach, event indexing could happen in a separate thread reducing the impact of marshaling JSON. A custom approach would also allow us to tune things to the data model we're using (i.e. dealing with quotes properly).

@clevinson
Copy link
Contributor

@akhilkumarpilli We'd like to discuss this at our next architecture call on Friday July 2. @anilcse will invite you to that call.

@akhilkumarpilli
Copy link
Contributor Author

@akhilkumarpilli We'd like to discuss this at our next architecture call on Friday July 2. @anilcse will invite you to that call.

@clevinson I am not sure whether I can be available for call as I need to undergo a minor surgery on friday. @anilcse will help in discussions regarding typed events incase of my absence.

@francas
Copy link

francas commented Feb 21, 2023

Hi, I have a question on Typed Events.

I noticed because of unneeded marshalling we get strangely quoted values in the event log:

- attributes:
    - key: appId
      value: '"62d449fd27193b2aff8aeee5322381b40ed05a3cd2904c2de4b2c65983372882"'
    type: poc.poc.EventRegisterApp

Main reason I'm asking is because we're subscribing to those events and now have to "unquote" them for no reason. Maybe there is another way of subscribing that lets read Typed Events somehow. But current one (via RPC - Tendermint RPC) gives access to already parsed (quoted) data.

@tac0turtle tac0turtle closed this as not planned Won't fix, can't repro, duplicate, stale Oct 1, 2024
@anilcse
Copy link
Collaborator

anilcse commented Oct 1, 2024

@tac0turtle should we remove TypedEvents as we are not really using them widely?

@julienrbrt
Copy link
Member

@tac0turtle should we remove TypedEvents as we are not really using them widely?

Typed events are still supported in core (https://github.com/cosmos/cosmos-sdk/blob/main/core/event/service.go#L19-L22), so we shouldn't kill them. We just don't need to migrate all existing modules to typed event.

@anilcse
Copy link
Collaborator

anilcse commented Oct 1, 2024

@julienrbrt
Copy link
Member

What about this: https://github.com/cosmos/cosmos-sdk/blob/main/types/events.go#L21-L22

That is equivalent to the snippet above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:needs architecture review To discuss on the next architecture review call to come to alignment
Projects
None yet
Development

No branches or pull requests

10 participants