Skip to content

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

Merged
mergify[bot] merged 5 commits intomasterfrom
erik/initchain-app-hash
Aug 11, 2020
Merged

abci: add ResponseInitChain.app_hash, check and record it#5227
mergify[bot] merged 5 commits intomasterfrom
erik/initchain-app-hash

Conversation

@erikgrinaker
Copy link
Contributor

@erikgrinaker erikgrinaker commented Aug 11, 2020

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

tac0turtle commented Aug 11, 2020

could we get spec updates prior to merging this PR?

@erikgrinaker
Copy link
Contributor Author

erikgrinaker commented Aug 11, 2020

Yup, will prepare them shortly.

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

erikgrinaker commented Aug 11, 2020

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

@melekes melekes Awaiting requested review from melekes

@tessr tessr Awaiting requested review from tessr

1 more reviewer

@tac0turtle tac0turtle tac0turtle approved these changes

Reviewers whose approvals may not affect merge requirements

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