Skip to content

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

Merged
AshwinSekar merged 2 commits intoanza-xyz:masterfrom
AshwinSekar:chain-dup-prev-panic
May 6, 2024
Merged

blockstore: relax backwards chained merkle root check for upgrades#1163
AshwinSekar merged 2 commits intoanza-xyz:masterfrom
AshwinSekar:chain-dup-prev-panic

Conversation

@AshwinSekar
Copy link
Copy Markdown

@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
Comment thread ledger/src/blockstore.rs Outdated
Co-authored-by: Trent Nelson <490004+t-nelson@users.noreply.github.com>
@codecov-commenter
Copy link
Copy Markdown

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 requested a review from behzadnouri May 4, 2024 23:03
@AshwinSekar AshwinSekar merged commit a645d07 into anza-xyz:master May 6, 2024
@mergify
Copy link
Copy Markdown

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants