blockstore: add merkle root to ErasureMeta column family

[AshwinSekar]

Problem

In order to detect duplicate blocks with conflicting merkle roots we need to be able to access the merkle root for each FEC set.

We store the merkle root along with the existing ErasureMeta for each FEC set.

Summary of Changes

This change allows us to read blockstore in both formats, with or without the merkle root. It is intended to be backported.

Contributes to #33644

[AshwinSekar]

next change in master will populate this from the coding shred, but I don't think that needs to be part of this backport change.

[codecov]

Codecov Report

Merging #33848 (9d9252a) into master (a851670) will increase coverage by 0.0%.
Report is 2 commits behind head on master.
The diff coverage is 92.3%.

@@           Coverage Diff           @@
##           master   #33848   +/-   ##
=======================================
  Coverage    81.8%    81.8%           
=======================================
  Files         809      809           
  Lines      217708   217796   +88     
=======================================
+ Hits       178283   178357   +74     
- Misses      39425    39439   +14     

[behzadnouri]

Can we rename the old struct ErasureMetaLegacy and use ErasureMeta for the new one?

That would be more obvious and self-documenting that the new code should not bother with the legacy one, and there is only one type that matters.

[behzadnouri]

don't we need the coding shred index from which this erasure meta was populated from here?
for example in the case we want to generate a duplicate proof, we need the coding shred which contains this erasure meta.

[AshwinSekar]

yeah good point, was planning to scan using the first_coding_index, but we can just put the coding shred index and save some blockstore reads

[behzadnouri]

why do we need this pub(crate) here? is this just for tests?

[AshwinSekar]

yeah for tests, i just made a test impl instead.

[behzadnouri]

This type trait and its associated code is pretty verbose and actually adding more complexity here.

Might be simple just to do something like below:

diff --git a/ledger/src/blockstore_db.rs b/ledger/src/blockstore_db.rs
index b65df82ee0..eebf163400 100644
--- a/ledger/src/blockstore_db.rs
+++ b/ledger/src/blockstore_db.rs
@@ -1586,7 +1586,7 @@ where
     }
 }
 
-impl<C> LedgerColumn<C>
+impl<C: 'static> LedgerColumn<C>
 where
     C: TypedColumn + ColumnName,
 {
@@ -1635,7 +1635,12 @@ where
             &self.read_perf_status,
         );
         if let Some(pinnable_slice) = self.backend.get_pinned_cf(self.handle(), key)? {
-            let value = deserialize(pinnable_slice.as_ref())?;
+            use std::any::TypeId;
+            let value = deserialize(pinnable_slice.as_ref()).or_else(|err| {
+                if TypeId::of::<C>() == TypeId::of::<columns::MerkleErasureMeta>() {
+                    // retry with the old erasure meta type.
+                    // ...
+                }
+                Err(err)
+            })?;
             result = Ok(Some(value))
         }
 

[AshwinSekar]

I added it to provide an easy way to transition types of already existing columns in blockstore. Similar to ColumnIndexDeprecation.

All you have to do is implement the trait and TypedColumn. And after the old type is no longer in blockstore, you can just remove the trait impl and the associated migration test.

Figured its worth adding the complexity here to make future transitions frictionless.

[AshwinSekar]

After some offline discussion, it seems possible to detect merkle root conflicts without the requirement of an ErasureMeta, which we originally thought was not possible.

Given this, I am closing this in favor of the original idea of adding a new column. We can add a new MerkleRootMeta column that contains the merkle root and meta information instead of reusing the ErasureMeta. This will avoid the need for any backports as well.

will continue here #33979

[mergify]

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.