add merkle root column in blockstore

[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

This is the preliminary change to add a merkle root column for use in further changes to store the merkle root for shreds received.

Summary of Changes

Add merkle root column

Contributes to #33644

[codecov]

Codecov Report

Merging #33807 (70b60ab) into master (5a96352) will increase coverage by 0.0%.
The diff coverage is 53.3%.

@@           Coverage Diff           @@
##           master   #33807   +/-   ##
=======================================
  Coverage    81.8%    81.8%           
=======================================
  Files         807      807           
  Lines      217492   217522   +30     
=======================================
+ Hits       178091   178118   +27     
- Misses      39401    39404    +3     

[AshwinSekar]

@behzadnouri I decided to add a new column instead of modifying the existing ErasureMeta because ErasureMeta is only populated once we insert a coding shred.

This approach will allow us to submit a duplicate proof if we observe 2 data shreds with differing merkle roots.

[behzadnouri]

• If you have 2 data shreds with the same index and they have different merkle roots, then they already are identified as duplicate shreds regardless of their merkle root because their payload is different.
• If they have different index and different merkle roots, then you can't determine if they belong to the same FEC set or not without receiving a coding shred from the same FEC set, because data shreds don't have the erasure set info.
  • But if you have a coding shred from the same FEC set, then the coding shred + one of the data shreds is already enough to prove mismatching merkle roots.

[AshwinSekar]

ah you're right 🤦 i'll stick to the original approach of modifying the erasure meta instead.

[AshwinSekar]

If they have different index and different merkle roots, then you can't determine if they belong to the same FEC set or not without receiving a coding shred from the same FEC set

Out of curiosity, why is it not enough to compare (shred.slot(), shred.fec_set_index())?

[behzadnouri]

Discussed this offline.
@AshwinSekar is right. (shred.slot(), shred.fec_set_index()) will identify the FEC set.

The 2 cases below are left out but they can be tracked separately.

• another scenario for duplicate blocks is that you receive a coding shred which indicates data shreds with indices [10..20] belong to the same FEC set with fec_set_index = 10, but then you have a data shred with index 17 (i.e. within that range) but different fec_set_index.
• Another case is that we have 2 coding shreds which indicate overlapping FEC sets.

[AshwinSekar]

This is a copypasta from ShredCode, ShredData, and ErasureMeta. Happy to consolidate but am unsure about the effects of

 fn as_index(slot: Slot) -> Self::Index {
        (slot, 0)
    }

Is it more beneficial to cache a fec_set_index and use it instead of 0 here?

[steviez]

What you have for as_index() is fine; see this comment in the trail declaration:

solana/ledger/src/blockstore_db.rs

Lines 710 to 713 in 56ccffd

	// This trait method is primarily used by `Database::delete_range_cf()`, and is therefore only
	// relevant for columns keyed by Slot: ie. SlotColumns and columns that feature a Slot as the
	// first item in the key.
	fn as_index(slot: Slot) -> Self::Index;

And yeah, looks like you might be able to reuse some impl's

[steviez]

In general, I think you have things plumbed correctly everywhere (metrics, cleanup, etc). What are you thinking in terms of rolling this broader change out? I see this PR adds a getter/setter, but it is not actually plugged in anywhere yet. Would the actual meat of the logic be added later ?

As mentioned on Discord, we ideally have a minimal change that we can backport for the sake of compatibility between versions; is that this PR?

Also, what happens to the old ErasureMeta column; would this change essentially make that column obsolete ?

[AshwinSekar]

As mentioned on Discord, we ideally have a minimal change that we can backport for the sake of compatibility between versions; is that this PR?

Also, what happens to the old ErasureMeta column; would this change essentially make that column obsolete ?

Was trying to be smart and avoid having to change the ErasureMeta column, but as behzad pointed out this doesn't work. I'll follow the original plan mentioned in Discord instead and have a minimal change that we can backport.