mirror of
https://github.com/sigp/lighthouse.git
synced 2026-05-30 04:37:13 +00:00
fix: payload_attestation_data when no block received for slot (#9225)
Addresses issue #9220 The `payload_attestation_data` endpoint returns 400 when no block has been received for the requested slot. This causes the VC to log at CRIT level for what is expected behaviour per spec: validators should simply not submit a payload attestation when no block has been seen. - Return 404 (Not Found) instead of 400 from `payload_attestation_data` when no block exists for the slot. This is consistent with other beacon api endpoints. - Downgrade the VC log from `crit` to `debug` when a 503 is received, since this is an expected no-op per spec. - Add `BlockNotFound` rejection type to `warp_utils`. - Add a test asserting the 404 response for an empty slot. Co-Authored-By: Josh King <josh@sigmaprime.io> Co-Authored-By: Eitan Seri-Levi <eserilev@ucsc.edu> Co-Authored-By: Jimmy Chen <jchen.tc@gmail.com>
This commit is contained in:
@@ -333,8 +333,12 @@ pub fn get_validator_payload_attestation_data<T: BeaconChainTypes>(
|
||||
let payload_attestation_data = chain
|
||||
.produce_payload_attestation_data(slot)
|
||||
.map_err(|e| match e {
|
||||
BeaconChainError::InvalidSlot(_)
|
||||
| BeaconChainError::NoBlockForSlot(_) => {
|
||||
BeaconChainError::NoBlockForSlot(_) => {
|
||||
warp_utils::reject::block_not_found(format!(
|
||||
"No block received for slot {slot}"
|
||||
))
|
||||
}
|
||||
BeaconChainError::InvalidSlot(_) => {
|
||||
warp_utils::reject::custom_bad_request(format!(
|
||||
"Unable to produce payload attestation data: {e:?}"
|
||||
))
|
||||
|
||||
@@ -4807,7 +4807,8 @@ impl ApiTester {
|
||||
.client
|
||||
.get_validator_payload_attestation_data(slot)
|
||||
.await
|
||||
.unwrap();
|
||||
.unwrap()
|
||||
.expect("expected payload attestation data for slot with block");
|
||||
|
||||
assert_eq!(response.version(), Some(fork_name));
|
||||
|
||||
@@ -4823,7 +4824,8 @@ impl ApiTester {
|
||||
.client
|
||||
.get_validator_payload_attestation_data_ssz(slot)
|
||||
.await
|
||||
.unwrap();
|
||||
.unwrap()
|
||||
.expect("expected SSZ payload attestation data for slot with block");
|
||||
|
||||
assert_eq!(ssz_result, expected);
|
||||
|
||||
@@ -4894,6 +4896,7 @@ impl ApiTester {
|
||||
.get_validator_payload_attestation_data(slot)
|
||||
.await
|
||||
.unwrap()
|
||||
.expect("expected payload attestation data for slot with block")
|
||||
.into_data();
|
||||
|
||||
assert_eq!(pa_data.beacon_block_root, block_root);
|
||||
@@ -4926,6 +4929,26 @@ impl ApiTester {
|
||||
self
|
||||
}
|
||||
|
||||
pub async fn test_get_validator_payload_attestation_data_no_block(self) -> Self {
|
||||
// Advance the slot clock without producing a block
|
||||
self.harness.advance_slot();
|
||||
let slot = self.chain.slot().unwrap();
|
||||
|
||||
// Should return None when no block exists for the slot
|
||||
let result = self
|
||||
.client
|
||||
.get_validator_payload_attestation_data(slot)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
result.is_none(),
|
||||
"expected None for empty slot, got: {result:?}"
|
||||
);
|
||||
|
||||
self
|
||||
}
|
||||
|
||||
#[allow(clippy::await_holding_lock)] // This is a test, so it should be fine.
|
||||
pub async fn test_get_validator_aggregate_attestation_v1(self) -> Self {
|
||||
let attestation = self
|
||||
@@ -8597,6 +8620,17 @@ async fn get_validator_payload_attestation_data_pre_gloas() {
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn get_validator_payload_attestation_data_no_block() {
|
||||
if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) {
|
||||
return;
|
||||
}
|
||||
ApiTester::new_with_hard_forks()
|
||||
.await
|
||||
.test_get_validator_payload_attestation_data_no_block()
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn payload_attestation_present_after_envelope_publish() {
|
||||
ApiTester::new_with_hard_forks()
|
||||
|
||||
@@ -3030,10 +3030,11 @@ impl BeaconNodeHttpClient {
|
||||
}
|
||||
|
||||
/// `GET validator/payload_attestation_data/{slot}`
|
||||
/// Returns `None` if no block has been received for the requested slot (404).
|
||||
pub async fn get_validator_payload_attestation_data(
|
||||
&self,
|
||||
slot: Slot,
|
||||
) -> Result<BeaconResponse<PayloadAttestationData>, Error> {
|
||||
) -> Result<Option<BeaconResponse<PayloadAttestationData>>, Error> {
|
||||
let mut path = self.eth_path(V1)?;
|
||||
|
||||
path.path_segments_mut()
|
||||
@@ -3042,16 +3043,23 @@ impl BeaconNodeHttpClient {
|
||||
.push("payload_attestation_data")
|
||||
.push(&slot.to_string());
|
||||
|
||||
self.get_with_timeout(path, self.timeouts.payload_attestation)
|
||||
let opt_response = self
|
||||
.get_response(path, |b| b.timeout(self.timeouts.payload_attestation))
|
||||
.await
|
||||
.map(BeaconResponse::ForkVersioned)
|
||||
.optional()?;
|
||||
|
||||
match opt_response {
|
||||
Some(response) => Ok(Some(BeaconResponse::ForkVersioned(response.json().await?))),
|
||||
None => Ok(None),
|
||||
}
|
||||
}
|
||||
|
||||
/// `GET validator/payload_attestation_data/{slot}` in SSZ format
|
||||
/// Returns `None` if no block has been received for the requested slot (404).
|
||||
pub async fn get_validator_payload_attestation_data_ssz(
|
||||
&self,
|
||||
slot: Slot,
|
||||
) -> Result<PayloadAttestationData, Error> {
|
||||
) -> Result<Option<PayloadAttestationData>, Error> {
|
||||
let mut path = self.eth_path(V1)?;
|
||||
|
||||
path.path_segments_mut()
|
||||
@@ -3064,9 +3072,9 @@ impl BeaconNodeHttpClient {
|
||||
.get_bytes_opt_accept_header(path, Accept::Ssz, self.timeouts.payload_attestation)
|
||||
.await?;
|
||||
|
||||
let response_bytes = opt_response.ok_or(Error::StatusCode(StatusCode::NOT_FOUND))?;
|
||||
|
||||
PayloadAttestationData::from_ssz_bytes(&response_bytes).map_err(Error::InvalidSsz)
|
||||
opt_response
|
||||
.map(|bytes| PayloadAttestationData::from_ssz_bytes(&bytes).map_err(Error::InvalidSsz))
|
||||
.transpose()
|
||||
}
|
||||
|
||||
/// `GET v1/validator/aggregate_attestation?slot,attestation_data_root`
|
||||
|
||||
@@ -110,6 +110,17 @@ pub fn not_synced(msg: String) -> warp::reject::Rejection {
|
||||
warp::reject::custom(NotSynced(msg))
|
||||
}
|
||||
|
||||
/// A 404 Not Found response for when no block has been received for the
|
||||
/// requested slot.
|
||||
#[derive(Debug)]
|
||||
pub struct BlockNotFound(pub String);
|
||||
|
||||
impl Reject for BlockNotFound {}
|
||||
|
||||
pub fn block_not_found(msg: String) -> warp::reject::Rejection {
|
||||
warp::reject::custom(BlockNotFound(msg))
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct InvalidAuthorization(pub String);
|
||||
|
||||
@@ -199,6 +210,9 @@ pub async fn handle_rejection(err: warp::Rejection) -> Result<impl warp::Reply,
|
||||
} else if let Some(e) = err.find::<crate::reject::NotSynced>() {
|
||||
code = StatusCode::SERVICE_UNAVAILABLE;
|
||||
message = format!("SERVICE_UNAVAILABLE: beacon node is syncing: {}", e.0);
|
||||
} else if let Some(e) = err.find::<crate::reject::BlockNotFound>() {
|
||||
code = StatusCode::NOT_FOUND;
|
||||
message = format!("NOT_FOUND: {}", e.0);
|
||||
} else if let Some(e) = err.find::<crate::reject::InvalidAuthorization>() {
|
||||
code = StatusCode::FORBIDDEN;
|
||||
message = format!("FORBIDDEN: Invalid auth token: {}", e.0);
|
||||
|
||||
@@ -139,14 +139,22 @@ impl<S: ValidatorStore + 'static, T: SlotClock + 'static> PayloadAttestationServ
|
||||
beacon_node
|
||||
.get_validator_payload_attestation_data(slot)
|
||||
.await
|
||||
.map_err(|e| format!("Failed to get payload attestation data: {e:?}"))
|
||||
.map(|resp| resp.into_data())
|
||||
.map(|opt| opt.map(|resp| resp.into_data()))
|
||||
})
|
||||
.await
|
||||
{
|
||||
Ok(data) => data,
|
||||
Ok(Some(data)) => data,
|
||||
Ok(None) => {
|
||||
// Per the consensus spec, validators should not submit a
|
||||
// payload attestation when no block has been seen for the slot.
|
||||
debug!(
|
||||
%slot,
|
||||
"No block received for slot, skipping payload attestation"
|
||||
);
|
||||
return;
|
||||
}
|
||||
Err(e) => {
|
||||
crit!(
|
||||
error!(
|
||||
error = %e,
|
||||
%slot,
|
||||
"Failed to produce payload attestation data"
|
||||
|
||||
Reference in New Issue
Block a user