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

chore!: bump sdk v0.46.13 and core v0.34.28 #1903

Closed
wants to merge 1 commit into from

Conversation

evan-forbes
Copy link
Member

@evan-forbes evan-forbes commented Jun 11, 2023

Overview

bumps to our forks of the cosmos-sdk and comet to their latest versions. This is consensus breaking because an event was modified in the sdk, although that event may or may not have gotten hit on mocha-2 already.

aiming towards v1.x atm for max :shipit:, but we need this on main ofc

blocked by celestiaorg/cosmos-sdk#326, celestiaorg/cosmos-sdk#324, celestiaorg/celestia-core#1021

@evan-forbes evan-forbes added the consensus breaking modifies block validity rules in a way that will break consensus unless all nodes update their rules label Jun 11, 2023
@evan-forbes evan-forbes added this to the Mainnet milestone Jun 11, 2023
@evan-forbes evan-forbes requested a review from rach-id June 11, 2023 20:52
@evan-forbes evan-forbes self-assigned this Jun 11, 2023
@MSevey MSevey requested a review from a team June 11, 2023 20:53
@evan-forbes evan-forbes changed the base branch from main to v1.x June 11, 2023 20:53
Comment on lines -17 to +19
sdkCtx, err := app.NewProcessProposalQueryContext()
if err != nil {
panic(err)
}

// create a context using a branch of the state and loaded using the
// proposal height and chain-id
sdkCtx := app.NewProposalContext(core.Header{ChainID: app.GetChainID(), Height: app.LastBlockHeight() + 1})
Copy link
Member Author

Choose a reason for hiding this comment

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

for posteriety, if we don't do this, then the signatures of some txs will get rejected because the chain-id and height is not consistently loaded into the context, resulting in PrepareProposal filtering out those transactions, and therefore those txs get caught in the mempool, which always produces confusing errors.

@cmwaters
Copy link
Contributor

cmwaters commented Jun 12, 2023

aiming towards v1.x atm for max :shipit:, but we need this on main ofc

Why not main first and then backport to v1.x?

In the past, we've had a bot that you can use to open the same PR on a backported branch

@evan-forbes
Copy link
Member Author

ok, we can go through main closing in favor of #1912

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus breaking modifies block validity rules in a way that will break consensus unless all nodes update their rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants