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.

Sign up for GitHub

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

Jump to bottom

abci: add ResponseInitChain.app_hash, check and record it #5227

Merged
merged 5 commits into from
Aug 11, 2020
Merged

abci: add ResponseInitChain.app_hash, check and record it #5227

merged 5 commits into from
Aug 11, 2020

Conversation

erikgrinaker
Copy link
Contributor

Fixes #5177.

@erikgrinaker erikgrinaker added C:abci Component: Application Blockchain Interface T:breaking Type: Breaking Change labels Aug 11, 2020
@erikgrinaker erikgrinaker self-assigned this Aug 11, 2020
@erikgrinaker erikgrinaker added the S:automerge Automatically merge PR when requirements pass label Aug 11, 2020
@codecov
Copy link

codecov bot commented Aug 11, 2020 •
edited
Loading

Codecov Report

Merging #5227 into master will decrease coverage by 0.16%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master    #5227      +/-   ##
==========================================
- Coverage   62.66%   62.50%   -0.17%     
==========================================
  Files         259      259              
  Lines       27125    27131       +6     
==========================================
- Hits        16999    16959      -40     
- Misses       8653     8699      +46     
  Partials     1473     1473
Impacted Files Coverage Δ
consensus/replay.go 72.54% <33.33%> (-0.15%) ⬇️
crypto/sr25519/pubkey.go 37.93% <0.00%> (-6.90%) ⬇️
consensus/reactor.go 75.69% <0.00%> (-3.96%) ⬇️
blockchain/v0/pool.go 79.29% <0.00%> (-2.23%) ⬇️
p2p/switch.go 67.46% <0.00%> (-1.87%) ⬇️
statesync/snapshots.go 92.59% <0.00%> (-1.49%) ⬇️
consensus/state.go 72.81% <0.00%> (ø)
blockchain/v0/reactor.go 64.18% <0.00%> (ø)
p2p/conn/connection.go 80.00% <0.00%> (+0.24%) ⬆️
statesync/syncer.go 80.23% <0.00%> (+0.79%) ⬆️
... and 3 more

@tac0turtle
Copy link
Contributor

could we get spec updates prior to merging this PR?

@erikgrinaker
Copy link
Contributor Author

Yup, will prepare them shortly.

@erikgrinaker erikgrinaker mentioned this pull request Aug 11, 2020
Merged
@erikgrinaker
Copy link
Contributor Author

Spec changes in tendermint/spec#140, this should be good to go.

tac0turtle
tac0turtle approved these changes Aug 11, 2020
View reviewed changes
Copy link
Contributor

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

utACK

@mergify mergify bot merged commit 08ffe13 into master Aug 11, 2020
@mergify mergify bot deleted the erik/initchain-app-hash branch August 11, 2020 14:28
This was referenced Aug 12, 2020
Closed
Merged
mergify bot pushed a commit that referenced this pull request Aug 13, 2020
@erikgrinaker
consensus: don't check InitChain app hash vs genesis app hash, replac…
e1a1395 
…e it (#5237)

Followup from #5227. Instead of checking `ResponseInitChain.app_hash` against the genesis doc app hash, we instead replace it. We should probably remove the genesis doc app hash completely, and rely solely on the one from `InitChain`, I'll open a separate issue to discuss this.
@erikgrinaker erikgrinaker mentioned this pull request Feb 21, 2022
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tac0turtle tac0turtle tac0turtle approved these changes

@melekes melekes Awaiting requested review from melekes

@tessr tessr Awaiting requested review from tessr

Assignees

@erikgrinaker erikgrinaker

Labels
C:abci Component: Application Blockchain Interface S:automerge Automatically merge PR when requirements pass T:breaking Type: Breaking Change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

InitChain should return and record app hash
2 participants
@erikgrinaker @tac0turtle