Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure GET v2/validator/aggregate_attestation is backwards compatible #6984

Merged
merged 5 commits into from
Feb 12, 2025
Merged

Ensure GET v2/validator/aggregate_attestation is backwards compatible #6984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d6e0f96
Ensure aggregate attestation endpoint v2 works pre electra and add so…
eserilev Feb 11, 2025
a57f64c
refactor
eserilev Feb 11, 2025
09e0072
fix tests
eserilev Feb 11, 2025
f6071e9
fix test
eserilev Feb 11, 2025
638a233
Remove await while holding lock
michaelsproul Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions beacon_node/http_api/src/aggregate_attestation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ pub fn get_aggregate_attestation<T: BeaconChainTypes>(
endpoint_version: EndpointVersion,
chain: Arc<BeaconChain<T>>,
) -> Result<Response<Body>, warp::reject::Rejection> {
if endpoint_version == V2 {
let fork_name = chain.spec.fork_name_at_slot::<T::EthSpec>(slot);
pawanjay176 marked this conversation as resolved.
Show resolved Hide resolved
let aggregate_attestation = if fork_name.electra_enabled() {
let Some(committee_index) = committee_index else {
return Err(warp_utils::reject::custom_bad_request(
"missing committee index".to_string(),
));
};
let aggregate_attestation = chain
chain
.get_aggregated_attestation_electra(slot, attestation_data_root, committee_index)
.map_err(|e| {
warp_utils::reject::custom_bad_request(format!(
Expand All @@ -34,8 +35,22 @@ pub fn get_aggregate_attestation<T: BeaconChainTypes>(
})?
.ok_or_else(|| {
warp_utils::reject::custom_not_found("no matching aggregate found".to_string())
})?;
let fork_name = chain.spec.fork_name_at_slot::<T::EthSpec>(slot);
})?
} else {
chain
.get_pre_electra_aggregated_attestation_by_slot_and_root(slot, attestation_data_root)
.map_err(|e| {
warp_utils::reject::custom_bad_request(format!(
"unable to fetch aggregate: {:?}",
e
))
})?
.ok_or_else(|| {
warp_utils::reject::custom_not_found("no matching aggregate found".to_string())
})?
};

if endpoint_version == V2 {
let fork_versioned_response = ForkVersionedResponse {
version: Some(fork_name),
metadata: EmptyMetadata {},
Expand All @@ -46,19 +61,7 @@ pub fn get_aggregate_attestation<T: BeaconChainTypes>(
fork_name,
))
} else if endpoint_version == V1 {
let aggregate_attestation = chain
.get_pre_electra_aggregated_attestation_by_slot_and_root(slot, attestation_data_root)
.map_err(|e| {
warp_utils::reject::custom_bad_request(format!(
"unable to fetch aggregate: {:?}",
e
))
})?
.map(GenericResponse::from)
.ok_or_else(|| {
warp_utils::reject::custom_not_found("no matching aggregate found".to_string())
})?;
Ok(warp::reply::json(&aggregate_attestation).into_response())
Ok(warp::reply::json(&GenericResponse::from(aggregate_attestation)).into_response())
} else {
return Err(unsupported_version_rejection(endpoint_version));
}
Expand Down
94 changes: 57 additions & 37 deletions beacon_node/http_api/tests/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3555,44 +3555,48 @@ impl ApiTester {
}

#[allow(clippy::await_holding_lock)] // This is a test, so it should be fine.
pub async fn test_get_validator_aggregate_attestation(self) -> Self {
if self
pub async fn test_get_validator_aggregate_attestation_v1(self) -> Self {
let attestation = self
.chain
.spec
.fork_name_at_slot::<E>(self.chain.slot().unwrap())
.electra_enabled()
{
for attestation in self.chain.naive_aggregation_pool.read().iter() {
let result = self
.client
.get_validator_aggregate_attestation_v2(
attestation.data().slot,
attestation.data().tree_hash_root(),
attestation.committee_index().expect("committee index"),
)
.await
.unwrap()
.unwrap()
.data;
let expected = attestation;
.head_beacon_block()
.message()
.body()
.attestations()
.next()
.unwrap()
.clone_as_attestation();
let result = self
.client
.get_validator_aggregate_attestation_v1(
attestation.data().slot,
attestation.data().tree_hash_root(),
)
.await
.unwrap()
.unwrap()
.data;
let expected = attestation;

assert_eq!(&result, expected);
}
} else {
let attestation = self
.chain
.head_beacon_block()
.message()
.body()
.attestations()
.next()
.unwrap()
.clone_as_attestation();
assert_eq!(result, expected);

self
}

pub async fn test_get_validator_aggregate_attestation_v2(self) -> Self {
let attestations = self
.chain
.naive_aggregation_pool
.read()
.iter()
.cloned()
.collect::<Vec<_>>();
for attestation in attestations {
let result = self
.client
.get_validator_aggregate_attestation_v1(
.get_validator_aggregate_attestation_v2(
attestation.data().slot,
attestation.data().tree_hash_root(),
attestation.committee_index().expect("committee index"),
)
.await
.unwrap()
Expand All @@ -3602,7 +3606,6 @@ impl ApiTester {

assert_eq!(result, expected);
}

self
}

Expand Down Expand Up @@ -6775,19 +6778,36 @@ async fn get_validator_attestation_data_with_skip_slots() {
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn get_validator_aggregate_attestation() {
async fn get_validator_aggregate_attestation_v1() {
ApiTester::new()
.await
.test_get_validator_aggregate_attestation()
.test_get_validator_aggregate_attestation_v1()
.await;
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn get_validator_aggregate_attestation_v2() {
ApiTester::new()
.await
.test_get_validator_aggregate_attestation_v2()
.await;
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn get_validator_aggregate_attestation_with_skip_slots_v1() {
ApiTester::new()
.await
.skip_slots(E::slots_per_epoch() * 2)
.test_get_validator_aggregate_attestation_v1()
.await;
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn get_validator_aggregate_attestation_with_skip_slots() {
async fn get_validator_aggregate_attestation_with_skip_slots_v2() {
ApiTester::new()
.await
.skip_slots(E::slots_per_epoch() * 2)
.test_get_validator_aggregate_attestation()
.test_get_validator_aggregate_attestation_v2()
.await;
}

Expand Down