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

Transfer events backport #4875

Merged
merged 3 commits into from
Oct 7, 2024
Merged

Conversation

cronokirby
Copy link
Contributor

A backport of #4874 to v0.79.X, for reindexing purposes.

This adds events for outbound, inbound, and refunds.
This is sufficient for most indexing needs for now.
Copy link
Contributor

@aubrika aubrika left a comment

Choose a reason for hiding this comment

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

LGTM

@conorsch conorsch mentioned this pull request Oct 1, 2024
3 tasks
@cronokirby cronokirby force-pushed the transfer-events-backport branch from 3aefc9e to 3df778e Compare October 1, 2024 21:14
conorsch added a commit to penumbra-zone/reindexer that referenced this pull request Oct 1, 2024
Updates the cargo deps to use the most recent feature branches from the
penumbra protocol repo, while we drive toward a dashboard.

Also sorts the deps to make them a bit easier to read and edit.

Refs [0], [1], [2].

[0] penumbra-zone/penumbra#4871
[1] penumbra-zone/penumbra#4875
[2] penumbra-zone/penumbra#4877
@conorsch
Copy link
Contributor

conorsch commented Oct 4, 2024

#4874 has been merged into main; @cronokirby can I trouble you to cherry-pick that squashed commit onto this branch? I tried myself, and wasn't sure about the conflict resolution, so I defer to you with your familiarity of the patch.

Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

requesting cherry-pick of recently merged commit

@cronokirby
Copy link
Contributor Author

The commit will have to be different because the IBC code changed in the meantime to have refunds for both transfers and error acks, so the logic of where you put the event emission is different; I think squashing the commits on this branch achieves the same effect as cherry picking and then having to modify the commit on main, idk

@conorsch
Copy link
Contributor

conorsch commented Oct 4, 2024

Thanks, I accept that interpretation. I guess what I really care about is: do you believe this PR, #4875, is right now 100% suitable for squash-merge as into backport branch? We can discuss in detail on Monday, just wanted to confirm it's good to go, given both the revisions and the need for conflict resolution when re-generating the patch.

Copy link
Contributor Author

@cronokirby cronokirby left a comment

Choose a reason for hiding this comment

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

I checked the tricky bit again @conorsch, this has everything we need.

@conorsch conorsch self-requested a review October 7, 2024 20:55
Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

let's do it 👍

@conorsch conorsch merged commit 67989ab into release/v0.79.x-contd Oct 7, 2024
11 checks passed
@conorsch conorsch deleted the transfer-events-backport branch October 7, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants