db: fix save blob sidecar using wrong prefix

[terencechain]

Background:

Bolt DB struggles with pruning, which has been evident during the development of the slasher. With the introduction of EIP-4844 in Prysm, there will be a significant amount of blob data. In the last design, we implemented a rotating buffer database schema for blobs. This schema will overwrite old data once the expiration period for blobs is reached, eliminating the need for pruning or deletion routines.

How It Works:

• Store a maximum of N epochs worth of blobs.
• Once epoch N is reached, overwrite the contents of epoch 0 using a modulo operation.
• Given the possibility of multiple blob sidecars per slot, a straightforward overwriting algorithm won't suffice.
  Saving blobs involves:
• Convert the slot using a modulo operator to a range of [0, maxSlots], where maxSlots = MAX_BLOB_EPOCHS * SLOTS_PER_EPOCH
• Compute the key for the blob using the formula: bytes(slot_to_rotating_buffer(blob.slot)) + bytes(blob.slot) + blob.block_root.
• Overwrite all elements if the incoming blob has a slot more significant than the last saved slot in the rotating keys buffer.
  This PR ensures efficient data storage and retrieval without frequent pruning, thus keeping the database size bounded.

Bug fix:

When checking for a pre-existing blob sidecar to override (ex: if blob sidecar slot is > 4096 epoch),
it is supposed to use bytes(blob.slot), which is [8:16].
The bug is it was using: [:8], which is bytes(slot_to_rotating_buffer(blob.slot))

[james-prysm]

is there a comment that can be added to direct readers on this

[prestonvanloon]

@james-prysm it's in the godoc for this method.

//  2. Compute key for blob as bytes(slot_to_rotating_buffer(blob.slot)) ++ bytes(blob.slot) ++ blob.block_root

Terence wants bytes(blob.slot) here, which is the second quadword

[prestonvanloon]

@james-prysm what do you think about this? #12855

Would that the PR make it easier to understand?

[rauljordan]

hi @terencechain, @saolyn and I were looking at this PR and felt the description and fix did not have enough context. A few questions we had coming it blind:

• Why is the structure of a DB key like this? Why is the first part a rotating buffer?
• What is newRetentionSlot in the unit test?
• What about other edge cases, such as saving a blob exactly at the modulo 0?

In terms of testing, we think the unit tests can be made a lot simpler. They should be more human-readable. For example, we should override the config to store 10 slots of blobs, and then do something like

maxBlobTimeToLive := types.Slot(10)
for i := 0; i < maxBlobTimeToLive; i++ {
  db.SaveBlob(generateBlobAtSlot(i))
}

// Then check a few things about the stored blobs..

// Then, override the first 3 blobs, check they were overwritten

// Override the whole thing, save at the boundaries

This way, we can better audit for edge-cases

[terencechain]

@rauljordan @saolyn I'll answer them here

Why is the structure of a DB key like this? Why is the first part a rotating buffer?

The rotating buffer is to handle blobs outside of the retention window ~4096 epochs. It enables a fast lookup without having to unmarshal the blob sidecar and check. It was motivated by your PR here :) terencechain#3
Another option is to eliminate the rotating buffer and use the slot. Open to this, and it can be done in another PR

What is newRetentionSlot in the unit test?

It's a slot that's higher than 4096 * 32. The idea is we want to trigger the bug which only happens higher than the first retention window

What about other edge cases, such as saving a blob exactly at the modulo 0?

Can add this

For example, we should override the config to store 10 slots of blobs, and then do something like

I believe there is already a unit test for this