Handle unknown head during attestation publishing

[michaelsproul]

Issue Addressed

Closes #4699

Proposed Changes

Reprocess attestations received via HTTP which reference unknown blocks. This should reduce false positive logs on BNs used in multi-BN setups, where the VC might send our BN an attestation from another BN before we've finished processing the head block. This applies both to Vouch and the Lighthouse v4.6.0+ VC using --broadcast attestations.

In order to avoid blocking a beacon processor thread waiting for the reprocessing to complete, the implementation does the first round of processing in the beacon processor, then uses a join to wait for the reprocess futures from the Tokio executor. In practice this should be efficient, as Tokio will park the task until all its reprocess futures are ready, and the beacon processor can continue to churn through work.

[dong77]

Waiting for this to be released as I'm seeing a lot of related errors.

[michaelsproul]

@dong77 While this error is annoying, fixing it will only marginally improve performance. If you see this, it means one of your BNs (the one that created the attestation) is ahead of one of your other BNs in terms of blocks imported. If you broadcast the attestation to both of them, then the one that created the attestation (which is "ahead") will process and publish the attestation just fine, while the other one (which is "behind") will log this error. With the change from this PR, both will process and publish and no errors will be logged. So it's just a bit of redundancy during publishing.

[jimmygchen]

The changes looks good to me! Just a few minor questions and an annoying unused beta compiler error to fix!

[michaelsproul]

Ignore previous comment (now deleted). I commented on the wrong issue.

[dong77]

@dong77 While this error is annoying, fixing it will only marginally improve performance. If you see this, it means one of your BNs (the one that created the attestation) is ahead of one of your other BNs in terms of blocks imported. If you broadcast the attestation to both of them, then the one that created the attestation (which is "ahead") will process and publish the attestation just fine, while the other one (which is "behind") will log this error. With the change from this PR, both will process and publish and no errors will be logged. So it's just a bit of redundancy during publishing.

Is there a way to disable/hide this error?

[michaelsproul]

Is there a way to disable/hide this error?

No

[jimmygchen]

LGTM!

[jimmygchen]

@mergify queue

[mergify]

queue

🛑 The pull request has been removed from the queue default

Pull request #5010 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 unqueue

[mergify]

unqueue

✅ The pull request has been removed from the queue default

[jimmygchen]

@michaelsproul did you want to include this in the next release? just saw your comment in the description regarding needing further testing, so I've unqueued this for now

[michaelsproul]

@jimmygchen I'd like to include it. I think the current test demonstrates that it's working, and we'll get a few days to observe on testnets once we've merged

[michaelsproul]

@Mergifyio 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 f17fb29