Send custody data column to DataAvailabilityChecker for determining block importability

[jimmygchen]

Issue Addressed

Send custody data column to DataAvailabilityChecker for determining block importability.

Proposed Changes

• Add --subscribe-all-data-column-subnets flag, and provide custody_column_count to da_checker.availability_cache.
• Make PeerDAS activate on a soft fork PEER_DAS_EPOCH and start separating DAS from deneb logic.
• Update PendingComponent.is_available to check for number of expected columns.
• When publishing a block, send only the custody data columns to the DataAvailabilityChecker.
• Database migration for OverflowLRUCache if necessary. (Moved to tracking issue)

Additional Information

Custody Columns from gossip

• After receiving a gossip verfieid data column, we wrap it in a CustodyDataColumn object and send it to DAChecker. (nodes only subscribe to subnets they custody)
• DAChecker is aware of how many columns it's custodying, so once it collects enough distinct CustodyDataColumns then custody requirement is fullfiled.

Custody Columns from RPC (out of scope for this PR as RPC not yet implemented)

• If we receive an attestation with unknown block, this triggers a block and data column lookup for the root from sync (sync can compute the custody columns)
• When response for this request type (RequestId::SingleDataColumn) is received, sync wraps the data in a CustodyDataColumn and send it to DAChecker
• DAChecker is aware of how many columns it's custodying, so once it collects enough distinct CustodyDataColumns then custody requirement is fullfiled.
• For sampling responses, I guess we would have a different RequestId to differentiate, e.g. RequestId::Sampling?

Custody Columns from HTTP API

• http_api has access to network_globals and can calculate which columns need to custody, so we can wrap them the same way and send them to the beacon chain.

[jimmygchen]

It looks like updating the is_available/is_importable function has opened up a can of worms and broken a bunch of tests, because the current branch allows blobs & columns to coexist, which would require updating a bunch of code that we eventually don't need. I'll open a new PR to implement activating PeerDAS on a specific PEER_DAS_EPOCH. (thanks to @dapplion for the suggestion)

[jimmygchen]

Tested this on a local devnet, fixed a few bugs and I think this is now working!

I was able to see the regular node persisting its custody columns during import, and supernodes (--subscribe-all-data-column-subnet persisting all custody columns during import.

[dapplion]

This debug here appears like it could become very noisy. It's not a response to a state-changing event, but just a getter

[jimmygchen]

Yep agree that it could be become noisy, and I thought about downgrading it too - it was quite useful when I was debugging issues with block importability. I guess we could get this somehow with other logs though. Will downgrade to trace. (Also not opposed to just removing it)

[jimmygchen]

Addressed in 3424dcd

[dapplion]

Why not necessary to key by column_index?

[jimmygchen]

Good catch, wasn't sure what I was thinking!
The index is no longer used for inserting it into PendingComponents but is still important for persistence. I'll revert this.

[jimmygchen]

Addressed in 3424dcd

[dapplion]

Should you import all the columns when producing a block? I see that there's no mechanism for others to retrieve data you created but don't declare to custody. However it frightens me a little that network liveness depends on those very few peers of the block proposer that are meant to custody specific rows. If a proposer happens to not be peered at the time of publication with a peer on each subnet there will be data loss.

[jimmygchen]

Yeah that's a good point. I initially run into an issue when sending all columns one-by-one to the beacon_chain because it wasn't expecting that additional data columns, and it basically imported the block after receiving min_custody_requirement_columns, and return BlockIsAlreadyKnown error for additional columns:

lighthouse/beacon_node/beacon_chain/src/beacon_chain.rs

Lines 2950 to 2964 in 41d6225

	pub async fn process_gossip_data_column(
	self: &Arc<Self>,
	data_column: GossipVerifiedDataColumn<T>,
	) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
	let block_root = data_column.block_root();
	// If this block has already been imported to forkchoice it must have been available, so
	// we don't need to process its samples again.
	if self
	.canonical_head
	.fork_choice_read_lock()
	.contains_block(&block_root)
	{
	return Err(BlockError::BlockIsAlreadyKnown(block_root));
	}

Perhaps we could add an additional function dedicated for proposer - that adds all the columns before importing? e.g. chain.process_proposer_gossip_data_columns?

[jimmygchen]

Added this to tracking issue #4983

[dapplion]

Is this error necessary? The import requirements are not complete until the block shows up, so you can assume that it will always eventually show up. Instead of returning an error here, you can return something variant signaling that requirements are not known yet.

[jimmygchen]

Good question - I thought about this and my reason was that pending_components.epoch should always return an epoch unless there's a bug - there should be at least one component in pending_components when we're checking availability, and any component (block/blob/column) would have a slot / epoch attached to it.

I think it's safer to return an error here, in case if there's a bug in pending_components.epoch() - if we've already received all necessary components, this would never be called again and we end up having stuck pending components.

[jimmygchen]

@dapplion Thanks for the review! I've addressed some of your comments, and raised some questions to the remaining.

[dapplion]

Looks great! Happy to merge as is, and continue improving once the rest of the moving parts land