Skip to content

Conversation

@mergify
Copy link

@mergify mergify bot commented May 6, 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


This is an automatic backport of pull request #1163 done by Mergify.

…1163)

* blockstore: relax backwards chained merkle root check for upgrades

* s/v1.18.12/v1.18.13

Co-authored-by: Trent Nelson <[email protected]>

---------

Co-authored-by: Trent Nelson <[email protected]>
(cherry picked from commit a645d07)
@mergify mergify bot requested review from a team as code owners May 6, 2024 02:22
@AshwinSekar AshwinSekar requested a review from behzadnouri May 6, 2024 02:22
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.6%. Comparing base (a73b6cb) to head (7c40a77).

Additional details and impacted files
@@           Coverage Diff           @@
##            v1.18    #1196   +/-   ##
=======================================
  Coverage    81.6%    81.6%           
=======================================
  Files         827      827           
  Lines      225352   225410   +58     
=======================================
+ Hits       184043   184132   +89     
+ Misses      41309    41278   -31     

Copy link

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

are there any considerations toward a downgrade from 1.18 to 1.17 with this change? i'm thinking no, but figure we should get anything into 1.17 asap if needed

@AshwinSekar
Copy link

AshwinSekar commented May 9, 2024

are there any considerations toward a downgrade from 1.18 to 1.17 with this change? i'm thinking no, but figure we should get anything into 1.17 asap if needed

Although v1.17 does not populate the new column MerkleRootMeta, the stub for reading the column exists: solana-labs#34028

Additionally we also have solana-labs#34287 in v1.17.

I verified this works by opening a v1.18.12 populated blockstore in v1.17.28.

This specific change does not change anything about the column format and should not impact a downgrade.

@AshwinSekar AshwinSekar merged commit 24155e7 into v1.18 May 9, 2024
@AshwinSekar AshwinSekar deleted the mergify/bp/v1.18/pr-1163 branch May 9, 2024 03:17
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 <[email protected]>

---------

Co-authored-by: Trent Nelson <[email protected]>
(cherry picked from commit a645d07)

Co-authored-by: Ashwin Sekar <[email protected]>
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.

6 participants