Spec without peer sampling

[fradamt]

A first spec for this proposed simplification of the first version of PeerDAS. This includes:

1. Splitting the peer sampling part of the spec into a separate file, peer-sampling.md
2. Specifying "subnet sampling" as participating in max(custody_subnet_count, SAMPLES_PER_SLOT) subnets, only custody_subnet_count of which are advertised. custody_subnet_count is required to be at least CUSTODY_REQUIREMENT, which is less than SAMPLES_PER_SLOT, so only a portion of the subnet sampling is enforced by peers.
3. A simple fork-choice spec: is_data_available is entirely based on subnet sampling, meaning that something is available as long as all subnet sampling columns (again, a superset of custody) are available. This is used as a fork-choice filter in get_head rather than as a block import filter in on_block. We always import blocks regardless of whether they are available, but we never follow unavailable branches in the fork-choice. Possibly moving this to its own separate PR and keeping the Deneb-style fork-choice (don't import unavailable blocks) for now, see Spec without peer sampling #3870 (comment)

[jtraglia]

LGTM 👍 I would like another review before merging this.

[nisdas]

A simple fork-choice spec: is_data_available is entirely based on subnet sampling, meaning that something is available as long as all subnet sampling columns (again, a superset of custody) are available. This is used as a fork-choice filter in get_head rather than as a block import filter in on_block. We always import blocks regardless of whether they are available, but we never follow unavailable branches in the fork-choice.

What is the reason for changing this ? Any reason we can't hold off on importing blocks till we verify that all custodied data is there . Importing unavailable blocks into forkchoice and the db, will require some special case handling

[fradamt]

A simple fork-choice spec: is_data_available is entirely based on subnet sampling, meaning that something is available as long as all subnet sampling columns (again, a superset of custody) are available. This is used as a fork-choice filter in get_head rather than as a block import filter in on_block. We always import blocks regardless of whether they are available, but we never follow unavailable branches in the fork-choice.

What is the reason for changing this ? Any reason we can't hold off on importing blocks till we verify that all custodied data is there . Importing unavailable blocks into forkchoice and the db, will require some special case handling

I don't like this about the current behavior, applied to a system with sharded distribution:

1. At slot n there's an uncontroversial block A, with a lot of voting weight
2. At slot n+1, someone builds B on top of A, and only makes some of the columns available
3. Some validators in the slot n+1 committee vote for B, because they see it as available
4. Anyone that does not see B as available (and hasn't imported it) will ignore those votes as well, even though they should at the very least count for A

Basically A ends up missing out on voting weight by no fault of its own. Of course, if B is actually not available (< 50%), the amount of "missed weight" would be low due to sampling. And if it is available, reconstruction should quickly enough make sure that everyone imports it and sees all weight for A. Therefore, we could also go with the simpler option of just not importing, at a small price.

In principle I struggle to see why not importing should be simpler than this other solution, and so I would prefer what I see as the more principled approach. On the other hand, given that we already have the not importing approach implemented, there's little harm in keeping that one and leaving it up to a separate decision whether to at some point switch to the fork-choice filtering approach. I might revert to the original approach in this PR, and open another PR for the fork-choice change only.

[ralexstokes]

great work! I like the simplification and if anything just makes the implementation process a bit more manageable with smaller chunks

[ralexstokes]

going to merge for now to facilitate development targets for peerdas implementers

let's address outstanding concerns in future PRs :)

[nisdas]

I realize the reply is late for this:

In principle I struggle to see why not importing should be simpler than this other solution, and so I would prefer what I see as the more principled approach. On the other hand, given that we already have the not importing approach implemented, there's little harm in keeping that one and leaving it up to a separate decision whether to at some point switch to the fork-choice filtering approach. I might revert to the original approach in this PR, and open another PR for the fork-choice change only.

This is more of an issue on how clients handle invalid blocks. I view unavailable blocks as a class of them. For us if a block is invalid (either invalid consensus validation or execution validation), it would never be inserted into the db or forkchoice. In the event of optimistic sync this block would then be removed from the db. If we do start inserting blocks into the db which we have determined are unavailable and therefore invalid it would require a lot downstream changes ( ex: API,etc) on how to handle these class of blocks.