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

runtime/consensus/tendermint/verifier: Correctly compare headers #5068

Merged
merged 2 commits into from
Nov 22, 2022

Conversation

kostko
Copy link
Member

@kostko kostko commented Nov 22, 2022

Since the store may have an earlier (non-canonical, but valid) version of the block available, we need to only compare the actual header and not the commits/signatures.

This is because it can happen that during the immediate sync the light block does not yet contain all of the commits (but only just enough to be valid, e.g. 2/3+) and this gets stored in the light block store. Later on (e.g. during a query) the presented light block may have the full set of commits.

@kostko kostko added the c:bug Category: bug label Nov 22, 2022
Since the store may have an earlier (non-canonical, but valid) version
of the block available, we need to only compare the actual header and
not the commits/signatures.

This is because it can happen that during the immediate sync the light
block does not yet contain all of the commits (but only just enough to
be valid, e.g. 2/3+) and this gets stored in the light block store.
Later on (e.g. during a query) the presented light block may have the
full set of commits.
@kostko kostko force-pushed the kostko/fix/rt-verifier-hdreq branch from de577f2 to 131c4d3 Compare November 22, 2022 08:02
@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Merging #5068 (fc1a00b) into master (17f363d) will increase coverage by 0.02%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master    #5068      +/-   ##
==========================================
+ Coverage   66.87%   66.90%   +0.02%     
==========================================
  Files         490      490              
  Lines       52395    52402       +7     
==========================================
+ Hits        35041    35059      +18     
+ Misses      13068    13056      -12     
- Partials     4286     4287       +1     
Impacted Files Coverage Δ
go/scheduler/api/api.go 57.60% <ø> (-2.18%) ⬇️
go/staking/api/api.go 64.54% <ø> (-0.61%) ⬇️
.../consensus/tendermint/apps/staking/transactions.go 73.73% <60.00%> (-0.15%) ⬇️
go/consensus/tendermint/apps/scheduler/genesis.go 36.75% <100.00%> (+1.66%) ⬆️
go/consensus/tendermint/apps/scheduler/query.go 76.92% <100.00%> (+1.92%) ⬆️
...o/consensus/tendermint/apps/scheduler/scheduler.go 72.86% <100.00%> (+0.24%) ⬆️
...consensus/tendermint/apps/scheduler/state/state.go 67.92% <100.00%> (ø)
...nsus/tendermint/apps/supplementarysanity/checks.go 47.08% <100.00%> (ø)
go/staking/api/sanity_check.go 51.80% <100.00%> (+1.34%) ⬆️
go/roothash/api/api.go 80.35% <0.00%> (-5.36%) ⬇️
... and 26 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kostko kostko merged commit 2d4aa8c into master Nov 22, 2022
@kostko kostko deleted the kostko/fix/rt-verifier-hdreq branch November 22, 2022 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:bug Category: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants