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

fix: address serialization in PayForBlobs event #2793

Merged
merged 5 commits into from
Nov 6, 2023
Merged

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Oct 31, 2023

Closes #2781

Note this shouldn't be consensus breaking b/c the ABCI response results hash shouldn't depend on events. deterministicResponseDeliverTx filters events from the response. Thanks @cmwaters for identifying.

Testing

I tested on a Robusta devnet named app-v130-node-v0110. See #2794 (comment) and #2794 (comment)

@rootulp
Copy link
Collaborator Author

rootulp commented Oct 31, 2023

Ohh @Bidon15 I know why this failed during Robusta testing. This PR is based on main which already contains other consensus breaking changes. I want to re-test this change on top of the v1.x branch :)

@rootulp rootulp marked this pull request as ready for review November 1, 2023 12:47
Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

nice spot and fix

@rootulp rootulp merged commit f5fd791 into main Nov 6, 2023
31 checks passed
@rootulp rootulp deleted the rp/fix-event-signer branch November 6, 2023 09:30
rootulp added a commit that referenced this pull request Nov 6, 2023
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.

Address serialization in PayForBlobs event
3 participants