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

blockstore: relax backwards chained merkle root check for upgrades #1163

Merged
merged 2 commits into from
May 6, 2024

Conversation

AshwinSekar
Copy link

@AshwinSekar AshwinSekar commented May 3, 2024

Problem

If a validator upgrades from a version that does not contain the MerkleRootMeta column in blockstore in the middle of receiving shreds for a slot we cannot assume that the MerkleRootMeta exists.

Specifically, if a coding shred is received from FEC set N before the upgrade, when the first shred from FEC set N+1 is received post upgrade, we will perform the backwards chained merkle root check.
This check assumes that both the erasure meta and merkle root meta exists for FEC set N.

Summary of Changes

Relax this check to warn and succeed if the MerkleRootMeta is missing for the previous FEC set.
The corresponding forward check already handles this case correctly.

Note: the actual results of this check are not processed until the chained_merkle_conflict_duplicate_proofs feature is activated. This feature will only be activated several epochs after the cluster has upgraded ensuring that there are no missing MerkleRootMetas

@AshwinSekar AshwinSekar marked this pull request as ready for review May 3, 2024 04:18
ledger/src/blockstore.rs Outdated Show resolved Hide resolved
Co-authored-by: Trent Nelson <490004+t-nelson@users.noreply.github.com>
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.1%. Comparing base (f291ca4) to head (f1f9dbe).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #1163     +/-   ##
=========================================
- Coverage    82.1%    82.1%   -0.1%     
=========================================
  Files         886      886             
  Lines      236392   236450     +58     
=========================================
- Hits       194274   194264     -10     
- Misses      42118    42186     +68     

@AshwinSekar AshwinSekar merged commit a645d07 into anza-xyz:master May 6, 2024
38 checks passed
Copy link

mergify bot commented May 6, 2024

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

@AshwinSekar AshwinSekar deleted the chain-dup-prev-panic branch May 6, 2024 02:21
mergify bot pushed a commit that referenced this pull request May 6, 2024
…1163)

* blockstore: relax backwards chained merkle root check for upgrades

* s/v1.18.12/v1.18.13

Co-authored-by: Trent Nelson <490004+t-nelson@users.noreply.github.com>

---------

Co-authored-by: Trent Nelson <490004+t-nelson@users.noreply.github.com>
(cherry picked from commit a645d07)
AshwinSekar added a commit that referenced this pull request May 9, 2024
…ades (backport of #1163) (#1196)

blockstore: relax backwards chained merkle root check for upgrades (#1163)

* blockstore: relax backwards chained merkle root check for upgrades

* s/v1.18.12/v1.18.13

Co-authored-by: Trent Nelson <490004+t-nelson@users.noreply.github.com>

---------

Co-authored-by: Trent Nelson <490004+t-nelson@users.noreply.github.com>
(cherry picked from commit a645d07)

Co-authored-by: Ashwin Sekar <ashwin@anza.xyz>
anwayde pushed a commit to firedancer-io/agave that referenced this pull request Jul 23, 2024
…ades (backport of anza-xyz#1163) (anza-xyz#1196)

blockstore: relax backwards chained merkle root check for upgrades (anza-xyz#1163)

* blockstore: relax backwards chained merkle root check for upgrades

* s/v1.18.12/v1.18.13

Co-authored-by: Trent Nelson <490004+t-nelson@users.noreply.github.com>

---------

Co-authored-by: Trent Nelson <490004+t-nelson@users.noreply.github.com>
(cherry picked from commit a645d07)

Co-authored-by: Ashwin Sekar <ashwin@anza.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants