Fix custodial peer assumption on lookup custody requests

[dapplion]

Issue Addressed

Current unstable assumes that all peers to which we request columns by root, MUST have the columns in custody

lighthouse/beacon_node/network/src/sync/network_context/custody.rs

Lines 286 to 289 in 06329ec

	// true = enforce max_requests are returned data_columns_by_root. We only issue requests
	// for blocks after we know the block has data, and only request peers after they claim to
	// have imported the block+columns and claim to be custodians
	true,

This statement is incorrect, as we pick a random peer from the global set of peers. The result, is a lot of downscore events like

VerifyError(NotEnoughResponsesReturned { actual: 0 })

for innocent peers.

Proposed Changes

Provide the custody request with an up-to-date list of peers that claim to have imported the lookup block. If peers give us that signal we can expect and enforce that they MUST have the columns in custody. If we don't have any such peers, we draw from the global set, but don't enforce then to have the columns in custody.

Since we need share state between the custody request and the overall lookup request I introduced a Mutex. I don't see another way around it if we want the custody request to have the most up-to-date view of the peer set. With @michaelsproul we discussed using an immutable HashSet and clone it, but that would result in the custody request having a quickly stale view. Only when the overall custody request fails (after 3 retries) the peer set would update.

Consider the following sequence of events, if we don't use a Mutex:

• Receive attestation for block A
• Lookup for block A is created with a single peer
• Custody request is created with a single peer
• Receive attestations for block A from 50 other peers
• (..) wait for custody request to fail 3 times (could take many seconds)
• Re-create custody request with 51 peers

During a significant duration the custody request has a stale view, rendering this feature not that useful.

[jimmygchen]

Nice solution, logic is pretty clean 👍

[jimmygchen]

@mergify queue

[mergify]

queue

🛑 The pull request has been removed from the queue default

Pull request #6815 has been dequeued by a dequeue command.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

[jimmygchen]

@mergify dequeue

[mergify]

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #6815 has been dequeued by a dequeue command

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

[mergify]

dequeue

✅ The pull request has been removed from the queue default

[jimmygchen]

@mergify queue

[mergify]

queue

🛑 The pull request has been removed from the queue default

Pull request #6815 has been dequeued by a dequeue command.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

[jimmygchen]

@mergify requeue

[mergify]

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

[mergify]

queue

🛑 The pull request has been removed from the queue default

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

[mergify]

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

• ☑️ test-suite-success

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

[jimmygchen]

@mergify requeue

[mergify]

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 7a0388e