Request: VC option to submit messages to all connected beacon nodes

[xrchz]

Description

Sometimes to maximise the chance of timely inclusion a node operator may want to have their validator client submit duties (attestations and proposals) to all connected beacon nodes, for redundancy. (This should be safe since only one validator client is operating so all submitted attestations should be the same and compatible, even if they are seen by different parts of the network initially.)

Version

All versions so far (up to 4.3.0).

Present Behaviour

Currently Lighthouse does not support broadcasting to all connected beacon nodes, and instead seems to only submit to the first/best node it knows about.

This seems to be the behaviour despite the existence of the --disable-run-on-all option.

Expected Behaviour

When provided with an appropriate option (e.g. --broadcast-duties), submit duties to all connected beacon nodes.

[xrchz]

Seems related to #3001

[xrchz]

I'm guessing the place to edit for this would be validator_client/src/beacon_node_fallback.rs, in particular the first_success function could be changed (or another version added) that runs on all successes instead of stopping as soon as it finds one.

[michaelsproul]

@xrchz There are functions in there called run, and run_on_all that have that behaviour. They don't support gathering data from all endpoints (e.g. forming attestations), but could be used for broadcasts

[michaelsproul]

How it works at the moment is that subscriptions for duties are broadcast, but the messages themselves (blocks + attestations) are not. We could add a new flag to replace --disable-run-on-all called --broadcast-subscriptions (taking a bool) and a new flag/flags to control the broadcast of messages, e.g. --broadcast-attestations, --broadcast-blocks, etc

[xrchz]

Yes I like those flag suggestions.

[uvizhe]

Hi, I'm ready to make a PR for this issue.

I've a few questions, however:

1. Am I correct that broadcasting is only required for these API calls: publish_signed_block_contents and post_beacon_pool_attestations?
2. Currently --disable-run-on-all flag affects these calls: post_validator_beacon_committee_subscriptions, post_validator_sync_committee_subscriptions, and post_validator_prepare_beacon_proposer. Is it right to attribute all three to --broadcast-subscriptions flag? I'm not sure about the latter one.

[michaelsproul]

Hey @uvizhe thanks for working on this

1. Those 2 are the main ones, but there's also post_beacon_pool_sync_committee_signatures
2. Good point about post_validator_prepare_beacon_proposer. I think grouping it with the subscriptions is fine for now, we can always add a distinct flag for it later if that proves useful.

[uvizhe]

Thanks for a quick response, @michaelsproul!

So, to which of new flags post_beacon_pool_sync_committee_signatures is related? :)

Also, I'd like to have some integration tests for this, but I'm not familiar with your code base. Can you tell me if it's possible to quickly add such tests and where should I look for a reference, or should I abandon this idea (I don't really want to create new test harnesses, looks out of scope of this issue).

[michaelsproul]

So, to which of new flags post_beacon_pool_sync_committee_signatures is related?

I think it would be best as its own flag: --broadcast-sync-committee-messages.

Also, I'd like to have some integration tests for this, but I'm not familiar with your code base.

The validator client tests are a bit weird at the moment. You could try adding something in testing/simulator, or we can leave it for a follow-up issue.

There are also CLI tests in lighthouse/tests that just confirm the CLI plumbing

[uvizhe]

Got it, thanks.

So we got:

• --broadcast-attestations for post_beacon_pool_attestations
• --broadcast-blocks for publish_signed_block_contents
• --broadcast-sync-committee-messages for post_beacon_pool_sync_committee_signatures
• --broadcast-subscriptions for post_validator_beacon_committee_subscriptions, post_validator_sync_committee_subscriptions, and post_validator_prepare_beacon_proposer

Maybe it would be more convenient to have a single --broadcast flag, which accepts multiple keywords like blocks, subscriptions? (This makes a code more complex however)

[michaelsproul]

I like the single broadcast flag option actually! If you use strum on the enum it should be OK from a complexity PoV. Just splitting the commas might be a bit annoying.

[michaelsproul]

Closed by #4920.