Skip to content

Commit 7684d1f

Browse files
authored
ContextDeserialize and Beacon API Improvements (#7372)
* #7286 * BeaconAPI is not returning a versioned response when it should for some V1 endpoints * these [strange functions with vX in the name that still accept `endpoint_version` arguments](https://github.com/sigp/lighthouse/blob/stable/beacon_node/http_api/src/produce_block.rs#L192) This refactor is a prerequisite to get the fulu EF tests running.
1 parent fcfcbf9 commit 7684d1f

File tree

92 files changed

+1862
-826
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

92 files changed

+1862
-826
lines changed

Cargo.lock

Lines changed: 22 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ members = [
5353
"common/warp_utils",
5454
"common/workspace_members",
5555

56+
"consensus/context_deserialize",
57+
"consensus/context_deserialize_derive",
5658
"consensus/fixed_bytes",
5759
"consensus/fork_choice",
5860
"consensus/int_to_bytes",
@@ -127,6 +129,8 @@ clap = { version = "4.5.4", features = ["derive", "cargo", "wrap_help"] }
127129
# feature ourselves when desired.
128130
c-kzg = { version = "1", default-features = false }
129131
compare_fields_derive = { path = "common/compare_fields_derive" }
132+
context_deserialize = { path = "consensus/context_deserialize" }
133+
context_deserialize_derive = { path = "consensus/context_deserialize_derive" }
130134
criterion = "0.5"
131135
delay_map = "0.4"
132136
derivative = "2"

beacon_node/beacon_chain/src/beacon_chain.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6115,7 +6115,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
61156115
payload_attributes: payload_attributes.into(),
61166116
},
61176117
metadata: Default::default(),
6118-
version: Some(self.spec.fork_name_at_slot::<T::EthSpec>(prepare_slot)),
6118+
version: self.spec.fork_name_at_slot::<T::EthSpec>(prepare_slot),
61196119
}));
61206120
}
61216121
}

beacon_node/builder_client/src/lib.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1+
use eth2::types::beacon_response::EmptyMetadata;
12
use eth2::types::builder_bid::SignedBuilderBid;
2-
use eth2::types::fork_versioned_response::EmptyMetadata;
33
use eth2::types::{
4-
ContentType, EthSpec, ExecutionBlockHash, ForkName, ForkVersionDecode, ForkVersionDeserialize,
4+
ContentType, ContextDeserialize, EthSpec, ExecutionBlockHash, ForkName, ForkVersionDecode,
55
ForkVersionedResponse, PublicKeyBytes, SignedValidatorRegistrationData, Slot,
66
};
77
use eth2::types::{FullPayloadContents, SignedBlindedBeaconBlock};
@@ -119,7 +119,7 @@ impl BuilderHttpClient {
119119
}
120120

121121
async fn get_with_header<
122-
T: DeserializeOwned + ForkVersionDecode + ForkVersionDeserialize,
122+
T: DeserializeOwned + ForkVersionDecode + for<'de> ContextDeserialize<'de, ForkName>,
123123
U: IntoUrl,
124124
>(
125125
&self,
@@ -147,15 +147,23 @@ impl BuilderHttpClient {
147147
self.ssz_available.store(true, Ordering::SeqCst);
148148
T::from_ssz_bytes_by_fork(&response_bytes, fork_name)
149149
.map(|data| ForkVersionedResponse {
150-
version: Some(fork_name),
150+
version: fork_name,
151151
metadata: EmptyMetadata {},
152152
data,
153153
})
154154
.map_err(Error::InvalidSsz)
155155
}
156156
ContentType::Json => {
157157
self.ssz_available.store(false, Ordering::SeqCst);
158-
serde_json::from_slice(&response_bytes).map_err(Error::InvalidJson)
158+
let mut de = serde_json::Deserializer::from_slice(&response_bytes);
159+
let data =
160+
T::context_deserialize(&mut de, fork_name).map_err(Error::InvalidJson)?;
161+
162+
Ok(ForkVersionedResponse {
163+
version: fork_name,
164+
metadata: EmptyMetadata {},
165+
data,
166+
})
159167
}
160168
}
161169
}

beacon_node/client/src/builder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -463,12 +463,12 @@ where
463463
let blobs = if block.message().body().has_blobs() {
464464
debug!("Downloading finalized blobs");
465465
if let Some(response) = remote
466-
.get_blobs::<E>(BlockId::Root(block_root), None)
466+
.get_blobs::<E>(BlockId::Root(block_root), None, &spec)
467467
.await
468468
.map_err(|e| format!("Error fetching finalized blobs from remote: {e:?}"))?
469469
{
470470
debug!("Downloaded finalized blobs");
471-
Some(response.data)
471+
Some(response.into_data())
472472
} else {
473473
warn!(
474474
block_root = %block_root,

beacon_node/execution_layer/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1980,7 +1980,7 @@ enum InvalidBuilderPayload {
19801980
expected: Option<u64>,
19811981
},
19821982
Fork {
1983-
payload: Option<ForkName>,
1983+
payload: ForkName,
19841984
expected: ForkName,
19851985
},
19861986
Signature {
@@ -2013,7 +2013,7 @@ impl fmt::Display for InvalidBuilderPayload {
20132013
write!(f, "payload block number was {} not {:?}", payload, expected)
20142014
}
20152015
InvalidBuilderPayload::Fork { payload, expected } => {
2016-
write!(f, "payload fork was {:?} not {}", payload, expected)
2016+
write!(f, "payload fork was {} not {}", payload, expected)
20172017
}
20182018
InvalidBuilderPayload::Signature { signature, pubkey } => write!(
20192019
f,
@@ -2116,7 +2116,7 @@ fn verify_builder_bid<E: EthSpec>(
21162116
payload: header.block_number(),
21172117
expected: block_number,
21182118
}))
2119-
} else if bid.version != Some(current_fork) {
2119+
} else if bid.version != current_fork {
21202120
Err(Box::new(InvalidBuilderPayload::Fork {
21212121
payload: bid.version,
21222122
expected: current_fork,

beacon_node/execution_layer/src/test_utils/mock_builder.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,7 @@ impl<E: EthSpec> MockBuilder<E> {
743743
.await
744744
.map_err(|_| "couldn't get head".to_string())?
745745
.ok_or_else(|| "missing head block".to_string())?
746-
.data;
746+
.into_data();
747747

748748
let head_block_root = head_block_root.unwrap_or(head.canonical_root());
749749

@@ -761,7 +761,7 @@ impl<E: EthSpec> MockBuilder<E> {
761761
.await
762762
.map_err(|_| "couldn't get finalized block".to_string())?
763763
.ok_or_else(|| "missing finalized block".to_string())?
764-
.data
764+
.data()
765765
.message()
766766
.body()
767767
.execution_payload()
@@ -774,7 +774,7 @@ impl<E: EthSpec> MockBuilder<E> {
774774
.await
775775
.map_err(|_| "couldn't get justified block".to_string())?
776776
.ok_or_else(|| "missing justified block".to_string())?
777-
.data
777+
.data()
778778
.message()
779779
.body()
780780
.execution_payload()
@@ -815,7 +815,7 @@ impl<E: EthSpec> MockBuilder<E> {
815815
.await
816816
.map_err(|_| "couldn't get state".to_string())?
817817
.ok_or_else(|| "missing state".to_string())?
818-
.data;
818+
.into_data();
819819

820820
let prev_randao = head_state
821821
.get_randao_mix(head_state.current_epoch())
@@ -980,7 +980,7 @@ pub fn serve<E: EthSpec>(
980980
.await
981981
.map_err(|e| warp::reject::custom(Custom(e)))?;
982982
let resp: ForkVersionedResponse<_> = ForkVersionedResponse {
983-
version: Some(fork_name),
983+
version: fork_name,
984984
metadata: Default::default(),
985985
data: payload,
986986
};
@@ -1040,7 +1040,7 @@ pub fn serve<E: EthSpec>(
10401040
),
10411041
eth2::types::Accept::Json | eth2::types::Accept::Any => {
10421042
let resp: ForkVersionedResponse<_> = ForkVersionedResponse {
1043-
version: Some(fork_name),
1043+
version: fork_name,
10441044
metadata: Default::default(),
10451045
data: signed_bid,
10461046
};

beacon_node/http_api/src/aggregate_attestation.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::version::{add_consensus_version_header, V1, V2};
44
use beacon_chain::{BeaconChain, BeaconChainTypes};
55
use eth2::types::{self, EndpointVersion, Hash256, Slot};
66
use std::sync::Arc;
7-
use types::fork_versioned_response::EmptyMetadata;
7+
use types::beacon_response::EmptyMetadata;
88
use types::{CommitteeIndex, ForkVersionedResponse};
99
use warp::{
1010
hyper::{Body, Response},
@@ -52,7 +52,7 @@ pub fn get_aggregate_attestation<T: BeaconChainTypes>(
5252

5353
if endpoint_version == V2 {
5454
let fork_versioned_response = ForkVersionedResponse {
55-
version: Some(fork_name),
55+
version: fork_name,
5656
metadata: EmptyMetadata {},
5757
data: aggregate_attestation,
5858
};

0 commit comments

Comments
 (0)