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!: prepare process proposal state #326

Merged

Conversation

evan-forbes
Copy link
Member

Description

cosmos#15683 and cosmos#15717 are included in v0.46.13, which force us to stop relying on a query context for a specific height since errors are now thrown if there is no matching height in the store.

This PR is adds a simplified, but identical, approach to loading a branch of the state for prepare and process proposal as v0.47.x. This approach doesn't rely on height, instead it just creates a branch from the lastest commit multistore.

NOTE: in our version of PrepareProposal we don't pass height or chain-id, both of which are requried for basic signature verification. For this approach to work, we need to be sure to not pass a completely empty header when creating the proposal context. It must at least contain a height (anything greater than zero will work tbh) and chain-id. This PR also add a mechanism that provides access to the chain-id so we can do just that.

Closes: #325
blocked by #324

Copy link

@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.

LGTM.

How does the SDK handle the Chain ID part?

@codecov-commenter
Copy link

Codecov Report

Merging #326 (31b3923) into evan/bump-upstream-v0.46.13 (643c2b8) will decrease coverage by 0.01%.
The diff coverage is 10.00%.

Impacted file tree graph

@@                       Coverage Diff                       @@
##           evan/bump-upstream-v0.46.13     #326      +/-   ##
===============================================================
- Coverage                        65.52%   65.51%   -0.01%     
===============================================================
  Files                              666      666              
  Lines                            71263    71271       +8     
===============================================================
+ Hits                             46693    46695       +2     
- Misses                           21976    21982       +6     
  Partials                          2594     2594              
Impacted Files Coverage Δ
baseapp/baseapp.go 76.45% <0.00%> (-1.18%) ⬇️
baseapp/abci.go 60.59% <100.00%> (+0.06%) ⬆️

... and 1 file with indirect coverage changes

Base automatically changed from evan/bump-upstream-v0.46.13 to release/v0.46.x-celestia June 13, 2023 13:24
@evan-forbes evan-forbes merged commit dfc04d1 into release/v0.46.x-celestia Jun 13, 2023
@evan-forbes evan-forbes deleted the evan/prepare-process-proposal-state branch June 13, 2023 13:25
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.

Fix Prepare/Process Proposal state access
4 participants