lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Validator registrations on non-builder BNs

Open paulhauner opened this issue 2 years ago • 3 comments

Description

There might exist a setup where a VC has two BNs:

  • BN A: Uses a --builder.
  • BN B: Does not use a --builder.

If BN A goes down, the VC will switch to BN_B and then start to publish validator registrations. However, that will fail and we'll get the following log:

ERRO Unable to publish validator registrations to the builder network, error: All endpoints failed https://BN_B => RequestFailed(ServerMessage(ErrorMessage { code: 500, message: "UNHANDLED_ERROR: BuilderMissing", stacktraces: [] })), https://XXXX/ => Unavailable(Offline), [omitted]

This will cause the VC to set the BN as "offline". It might be nice to try and handle this differently, it seems like a very sane setup. Suggestions:

  1. Don't let the VC mark the BN as offline if the registrations fail, downgrade the severity of the logs.
  2. Define some other handling in the BN (e.g., return a specific HTTP error code) and then handle that in the VC.

paulhauner avatar Aug 03 '22 00:08 paulhauner

Agree we definitely shouldn't be marking the BN as offline.. seems like a bug (my bad). For a log level I think we would want at least a WARN because this could be misconfiguration of the backup node.

I'm guessing this WARN would most frequently be annoying with Infura as a backup, but with the merge an Infura shouldn't be used as a backup anymore anyways.

I'm not sure this case should warrant a specific error code because the BN can't serve what's requested. It'd probably be better for the VC to just request what it really wants. So the VC could have logic "if I'm requesting a backup, don't use the builder API" if this type of setup is preferred

realbigsean avatar Aug 10 '22 15:08 realbigsean

I'm guessing this WARN would most frequently be annoying with Infura as a backup, but with the merge an Infura shouldn't be used as a backup anymore anyways.

I think Infura are planning to continue supporting hosted BNs after the merge, so this may be quite common. Although I agree a WARN is fine.

michaelsproul avatar Aug 10 '22 23:08 michaelsproul

Mentioned in the linked PR, but I think adding a --builder-beacon-nodes flag might be the best way to fully support VC fallback from builder BN's to non-builder BN's.. what do you guys think of that type of thing?

I'm not sure if we'd be teetering on too many config options to where it's not worth the complexity

realbigsean avatar Aug 19 '22 19:08 realbigsean