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 forwarding information in transfer events #6585

Closed
crodriguezvega opened this issue Jun 13, 2024 · 7 comments
Closed

Emit forwarding information in transfer events #6585

crodriguezvega opened this issue Jun 13, 2024 · 7 comments
Assignees
Labels
20-transfer type: feature New features, sub-features or integrations

Comments

@crodriguezvega
Copy link
Contributor

Emit forwarding information in transfer events:

As we have done for tokens and denom, we can emit the JSON-encoded string of the field.

@crodriguezvega crodriguezvega added 20-transfer type: feature New features, sub-features or integrations labels Jun 13, 2024
@gjermundgaraba
Copy link
Contributor

I bumped into this a little when doing #6556, as the current OnRecvPacket does not emit any events on async ack. What should the events look like in that scenario?

@damiannolan
Copy link
Member

damiannolan commented Jun 13, 2024

I'd personally lean towards having a separate event for forwarding, at least that is what comes to mind initially...

What do others think? Maybe we can discuss this on Monday?

@crodriguezvega crodriguezvega added the needs discussion Issues that need discussion before they can be worked on label Jun 13, 2024
@DimitrisJim
Copy link
Contributor

agree with Damian, thinking these fields will most likely be empty most of the time considering simple transfers will be majority of cases.

@crodriguezvega
Copy link
Contributor Author

agree with Damian, thinking these fields will most likely be empty most of the time considering simple transfers will be majority of cases.

Is it not possible to not emit the attribute if the value is empty/nil?

@DimitrisJim
Copy link
Contributor

ah right, guess so. think that would make for more conditional code for us and for event parsers but could do the trick if this information is important for each of the events.

@DimitrisJim
Copy link
Contributor

going to do this after #6618

@crodriguezvega crodriguezvega removed the needs discussion Issues that need discussion before they can be worked on label Jun 18, 2024
@crodriguezvega
Copy link
Contributor Author

Just for reference: we decided to add forwarding information for now to the events mentioned in the description of the issue. We will revisit once we have more context about how these events are used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
20-transfer type: feature New features, sub-features or integrations
Projects
Archived in project
Development

No branches or pull requests

4 participants