Force the proposer to reorg unavailable blocks

[potuz]

In the event the proposer has seen the payload timely and its local view of DA passes, the node will have the payload verified. However, if the PTC is signalling that the Payload blob data is not available, the proposer MUST reorg the payload.

This PR implements this mechanism with a few caveats.

• Most importantly, currently it is very aggressive and if there are no PTC attestations the proposer will be forced to reorg the payload. There are many solutions to this problem like simply counting the PTC attestations (which is also a way of counting the NAY votes) or using
  Distinguish absent PTC votes from negative votes #5180. If we go with Distinguish absent PTC votes from negative votes #5180 this PR needs to be accomodated to that and actually count the NAY votes.
• The threshold could be different than 50%.
• The proposer may have all the data available locally and decide to build on full even though the PTC voted no. I think it's better to leave that unspecified.

[mkalinin]

However, if the PTC is signalling that the Payload blob data is not available, the proposer MUST reorg the payload.

Why do we want to make this behavior preferable? Honest proposer will propagate blob data and it will likely reach attesters before the attestation deadline and then attesters will have to vote against the proposed block

[fradamt]

However, if the PTC is signalling that the Payload blob data is not available, the proposer MUST reorg the payload.

Why do we want to make this behavior preferable? Honest proposer will propagate blob data and it will likely reach attesters before the attestation deadline and then attesters will have to vote against the proposed block

If the PTC signals unavailability, attesters will not vote against the proposed block. The PTC basically gives a "reorg pass" to the proposer.

As for why the default behavior should be to reorg, the main reason is that a proposer that samples is not able to fully determine availability by itself, so it makes sense for them to follow the PTC's availability determination. Otherwise, they might just be wrong (their 8 columns are available, but the rest is not), and their proposal would end up being reorged.

However, this does not necessarily need to be the case for proposers that download the whole data. They could ignore the PTC in principle.

[jihoonsong]

After #5186 and #5210, there will be very similar four PTC-related functions. We can at least shrink it into two: timeliness and availability. For instance,

def payload_data_availability(store: Store, root: Root, available: bool) -> bool:
	"""
	Return whether the blob data for the beacon block with root ``root``
    was voted as present by the PTC, and was locally determined to be available or unavailable.
	"""
	# The beacon block root must be known
	assert root in store.payload_data_availability_vote

	# If the payload is not locally available, the blob data
	# is not considered available regardless of the PTC vote
	if not is_payload_verified(store, root):
	  return not available

	votes = store.payload_data_availability_vote[root]
	return sum(vote is available for vote in votes) > DATA_AVAILABILITY_TIMELY_THRESHOLD

[jtraglia]

@jihoonsong I agree. I've fixed this here: ab43f0e

[jtraglia]

For the record, @potuz does not like the shared helper functions.

honestly having payload_timeliness return true or false depending on a boolean flag is a bad antipattern, just to avoid helpers that are named similarly but that have clear meanings, I do not like this next commit

Though Jihoon & I still prefer it for deduplication reasons.

[jihoonsong]

For the record, @potuz does not like the shared helper functions.

honestly having payload_timeliness return true or false depending on a boolean flag is a bad antipattern, just to avoid helpers that are named similarly but that have clear meanings, I do not like this next commit

Sure, and sibling functions with mismatched names is also an anti-pattern: is_payload_data_available and ptc_voted_data_unavailable. It makes it hard to tell they're a pair checking the same dataset. Good names aren't optional; they're part of the code itself.

Having payload_timeliness and payload_data_availability with the convention of using keyword argument for the boolean flag (and assigning the return value to a well-named variable that describes its meaning in our case) is one way. Another approach is adding is_payload_data_available and is_payload_data_unavailable, and making them call payload_data_availability inside.

Whichever we choose, it's an improvement to the original code.