Downgrade a CRIT in the VC for builder timeouts (#4366)

## Issue Addressed

NA

## Proposed Changes

Downgrade a `CRIT` to an `ERRO` when there's an `Irrecoverable` error whilst publishing a blinded block.

It's quite common for builders successfully broadcast a block to the network whilst failing to respond to the BN when it publishes a signed, blinded block. The VC is currently raising a `CRIT` when this happens and I think that's excessive.

These changes have the same intent as  #4073. In that PR I only managed to remove the `CRIT`s in the BN but missed this one in the VC.

I've also tidied the log messages to:

- Give them all the same title (*"Error whilst producing block"*) to help with grepping.
- Include the `block_slot` so it's easy to look up the slot in an explorer and see if it was actually skipped.

## Additional Info

This PR should not change any logic beyond logging.
This commit is contained in:
Paul Hauner 2023-06-07 01:50:34 +00:00
parent 299cfe1fe6
commit 5e3fb13cfe

View File

@ -338,35 +338,61 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {
let log = log.clone(); let log = log.clone();
self.inner.context.executor.spawn( self.inner.context.executor.spawn(
async move { async move {
let publish_result = if builder_proposals { if builder_proposals {
let mut result = service.clone() let result = service
.clone()
.publish_block::<BlindedPayload<E>>(slot, validator_pubkey) .publish_block::<BlindedPayload<E>>(slot, validator_pubkey)
.await; .await;
match result.as_ref() { match result {
Err(BlockError::Recoverable(e)) => { Err(BlockError::Recoverable(e)) => {
error!(log, "Error whilst producing a blinded block, attempting to \ error!(
publish full block"; "error" => ?e); log,
result = service "Error whilst producing block";
.publish_block::<FullPayload<E>>(slot, validator_pubkey) "error" => ?e,
.await; "block_slot" => ?slot,
}, "info" => "blinded proposal failed, attempting full block"
Err(BlockError::Irrecoverable(e)) => { );
error!(log, "Error whilst producing a blinded block, cannot fallback \ if let Err(e) = service
because the block was signed"; "error" => ?e);
},
_ => {},
};
result
} else {
service
.publish_block::<FullPayload<E>>(slot, validator_pubkey) .publish_block::<FullPayload<E>>(slot, validator_pubkey)
.await .await
}; {
if let Err(e) = publish_result { // Log a `crit` since a full block
// (non-builder) proposal failed.
crit!( crit!(
log, log,
"Error whilst producing block"; "Error whilst producing block";
"message" => ?e "error" => ?e,
"block_slot" => ?slot,
"info" => "full block attempted after a blinded failure",
);
}
}
Err(BlockError::Irrecoverable(e)) => {
// Only log an `error` since it's common for
// builders to timeout on their response, only
// to publish the block successfully themselves.
error!(
log,
"Error whilst producing block";
"error" => ?e,
"block_slot" => ?slot,
"info" => "this error may or may not result in a missed block",
)
}
Ok(_) => {}
};
} else if let Err(e) = service
.publish_block::<FullPayload<E>>(slot, validator_pubkey)
.await
{
// Log a `crit` since a full block (non-builder)
// proposal failed.
crit!(
log,
"Error whilst producing block";
"message" => ?e,
"block_slot" => ?slot,
"info" => "proposal did not use a builder",
); );
} }
}, },