Skip to content

GetBlobSidecars DA period check compares slot against itself #3127

Description

@NikhilSharmaWe

Problem

GetBlobSidecars is supposed to reject requests for slots outside MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS. It calls WithinDAPeriod with the requested slot as both arguments:

https://github.com/berachain/beacon-kit/blob/main/node-api/handlers/beacon/blobs.go#L73-L79

if !h.cs.WithinDAPeriod(slot, slot) {

WithinDAPeriod(block, current) is defined as:

https://github.com/berachain/beacon-kit/blob/main/chain/helpers.go#L57-L61

func (s spec) WithinDAPeriod(block, current math.Slot) bool {
	return s.SlotToEpoch(block)+s.MinEpochsForBlobsSidecarsRequest() >= s.SlotToEpoch(current)
}

When block == current, that's just epoch(slot) + N >= epoch(slot), which is always true. The check can never fail.

We already use this correctly on the consensus side — FinalizeSidecars passes the block slot and chain head:

https://github.com/berachain/beacon-kit/blob/main/beacon/blockchain/finalize_block.go#L97

s.chainSpec.WithinDAPeriod(blk.GetSlot(), math.Slot(syncingToHeight))

Looks like the API handler just wired up the wrong second arg. GetSyncData() is already available in the same function (used a few lines above for head block id).

Expected

If someone asks for blob sidecars at a slot that's fallen outside the DA window, we should fail the request up front — before we touch the availability store.

The logic for this already exists in WithinDAPeriod. There's a test for it in chain/helpers_test.go with MinEpochsForBlobsSidecarsRequest = 5:

  • slot 0 vs head at 160 → still in window
  • slot 0 vs head at 192 → outside window, should reject

So for a concrete example: chain head at slot 192, client requests sidecars for slot 0 → API should return an error. Right now it doesn't.

Actual

The guard at line 74 never trips because both args are the same slot. Every request gets through to GetBlobSidecarsAtSlot.

What the caller gets back is basically luck — if pruning hasn't cleaned up old blobs yet, they'll get data for a slot we shouldn't be serving. If pruning already ran, they get empty. Either way the API isn't enforcing the retention policy itself.

Suggested fix

Use chain head as the second arg. GetSyncData() is already called in this handler for the head block id case, so it's just wiring:

headHeight, _ := h.backend.GetSyncData()
if headHeight < 0 {
    return nil, errors.New("invalid negative block height")
}
if !h.cs.WithinDAPeriod(slot, math.Slot(headHeight)) {
    return nil, fmt.Errorf(
        "requested slot (%d) is not within Data Availability Period (previous %d epochs)",
        slotID, h.cs.MinEpochsForBlobsSidecarsRequest(),
    )
}

Would also add a test in blob_test.go — mock head at something like slot 192, request slot 0, assert we get an error. Currently only the happy path with head is covered.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions