Skip to content

Commit 134039d

Browse files
michaelsproulmichaelsproul
authored
Simplify ConfigAndPreset (#7777)
I noticed that we are serving preset values for Fulu on mainnet nodes prior to the fork. This has already gone live in v7.1.0, but should hopefully be handled in a graceful way by API consumers. This PR _reverts_ the serving of Fulu data prior to Fulu, by serving Fulu data only if Fulu is scheduled.
1 parent 09065a8 commit 134039d

File tree

7 files changed

+41
-46
lines changed

7 files changed

+41
-46
lines changed

‎beacon_node/http_api/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2763,7 +2763,7 @@ pub fn serve<T: BeaconChainTypes>(
27632763
move |task_spawner: TaskSpawner<T::EthSpec>, chain: Arc<BeaconChain<T>>| {
27642764
task_spawner.blocking_json_task(Priority::P0, move || {
27652765
let config_and_preset =
2766-
ConfigAndPreset::from_chain_spec::<T::EthSpec>(&chain.spec, None);
2766+
ConfigAndPreset::from_chain_spec::<T::EthSpec>(&chain.spec);
27672767
Ok(api_types::GenericResponse::from(config_and_preset))
27682768
})
27692769
},

‎beacon_node/http_api/tests/tests.rs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2616,13 +2616,19 @@ impl ApiTester {
26162616
}
26172617

26182618
pub async fn test_get_config_spec(self) -> Self {
2619-
let result = self
2620-
.client
2621-
.get_config_spec::<ConfigAndPresetFulu>()
2622-
.await
2623-
.map(|res| ConfigAndPreset::Fulu(res.data))
2624-
.unwrap();
2625-
let expected = ConfigAndPreset::from_chain_spec::<E>(&self.chain.spec, None);
2619+
let result = if self.chain.spec.is_fulu_scheduled() {
2620+
self.client
2621+
.get_config_spec::<ConfigAndPresetFulu>()
2622+
.await
2623+
.map(|res| ConfigAndPreset::Fulu(res.data))
2624+
} else {
2625+
self.client
2626+
.get_config_spec::<ConfigAndPresetElectra>()
2627+
.await
2628+
.map(|res| ConfigAndPreset::Electra(res.data))
2629+
}
2630+
.unwrap();
2631+
let expected = ConfigAndPreset::from_chain_spec::<E>(&self.chain.spec);
26262632

26272633
assert_eq!(result, expected);
26282634

‎consensus/types/src/config_and_preset.rs‎

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{
22
consts::altair, consts::deneb, AltairPreset, BasePreset, BellatrixPreset, CapellaPreset,
3-
ChainSpec, Config, DenebPreset, ElectraPreset, EthSpec, ForkName, FuluPreset,
3+
ChainSpec, Config, DenebPreset, ElectraPreset, EthSpec, FuluPreset,
44
};
55
use maplit::hashmap;
66
use serde::{Deserialize, Serialize};
@@ -43,7 +43,7 @@ pub struct ConfigAndPreset {
4343
}
4444

4545
impl ConfigAndPreset {
46-
pub fn from_chain_spec<E: EthSpec>(spec: &ChainSpec, fork_name: Option<ForkName>) -> Self {
46+
pub fn from_chain_spec<E: EthSpec>(spec: &ChainSpec) -> Self {
4747
let mut config = Config::from_chain_spec::<E>(spec);
4848
let base_preset = BasePreset::from_chain_spec::<E>(spec);
4949
let altair_preset = AltairPreset::from_chain_spec::<E>(spec);
@@ -52,15 +52,7 @@ impl ConfigAndPreset {
5252
let deneb_preset = DenebPreset::from_chain_spec::<E>(spec);
5353
let extra_fields = get_extra_fields(spec);
5454

55-
// Remove blob schedule for backwards-compatibility.
56-
if spec.fulu_fork_epoch.is_none() {
57-
config.blob_schedule.set_skip_serializing();
58-
}
59-
60-
if spec.fulu_fork_epoch.is_some()
61-
|| fork_name.is_none()
62-
|| fork_name == Some(ForkName::Fulu)
63-
{
55+
if spec.is_fulu_scheduled() {
6456
let electra_preset = ElectraPreset::from_chain_spec::<E>(spec);
6557
let fulu_preset = FuluPreset::from_chain_spec::<E>(spec);
6658

@@ -75,10 +67,10 @@ impl ConfigAndPreset {
7567
fulu_preset,
7668
extra_fields,
7769
})
78-
} else if spec.electra_fork_epoch.is_some()
79-
|| fork_name.is_none()
80-
|| fork_name == Some(ForkName::Electra)
81-
{
70+
} else {
71+
// Remove blob schedule for backwards-compatibility.
72+
config.blob_schedule.set_skip_serializing();
73+
8274
let electra_preset = ElectraPreset::from_chain_spec::<E>(spec);
8375

8476
ConfigAndPreset::Electra(ConfigAndPresetElectra {
@@ -91,16 +83,6 @@ impl ConfigAndPreset {
9183
electra_preset,
9284
extra_fields,
9385
})
94-
} else {
95-
ConfigAndPreset::Deneb(ConfigAndPresetDeneb {
96-
config,
97-
base_preset,
98-
altair_preset,
99-
bellatrix_preset,
100-
capella_preset,
101-
deneb_preset,
102-
extra_fields,
103-
})
10486
}
10587
}
10688
}
@@ -159,8 +141,7 @@ mod test {
159141
let mut mainnet_spec = ChainSpec::mainnet();
160142
// setting fulu_fork_epoch because we are roundtripping a fulu config
161143
mainnet_spec.fulu_fork_epoch = Some(Epoch::new(42));
162-
let mut yamlconfig =
163-
ConfigAndPreset::from_chain_spec::<MainnetEthSpec>(&mainnet_spec, None);
144+
let mut yamlconfig = ConfigAndPreset::from_chain_spec::<MainnetEthSpec>(&mainnet_spec);
164145
let (k1, v1) = ("SAMPLE_HARDFORK_KEY1", "123456789");
165146
let (k2, v2) = ("SAMPLE_HARDFORK_KEY2", "987654321");
166147
let (k3, v3) = ("SAMPLE_HARDFORK_KEY3", 32);

‎testing/validator_test_rig/src/mock_beacon_node.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ impl<E: EthSpec> MockBeaconNode<E> {
4141

4242
pub fn mock_config_spec(&mut self, spec: &ChainSpec) {
4343
let path_pattern = Regex::new(r"^/eth/v1/config/spec$").unwrap();
44-
let config_and_preset = ConfigAndPreset::from_chain_spec::<E>(spec, None);
44+
let config_and_preset = ConfigAndPreset::from_chain_spec::<E>(spec);
4545
let data = GenericResponse::from(config_and_preset);
4646
self.server
4747
.mock("GET", Matcher::Regex(path_pattern.to_string()))

‎validator_client/http_api/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
315315
.and(spec_filter.clone())
316316
.then(|spec: Arc<_>| {
317317
blocking_json_task(move || {
318-
let config = ConfigAndPreset::from_chain_spec::<E>(&spec, None);
318+
let config = ConfigAndPreset::from_chain_spec::<E>(&spec);
319319
Ok(api_types::GenericResponse::from(config))
320320
})
321321
});

‎validator_client/http_api/src/test_utils.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ impl ApiTester {
260260
.await
261261
.map(|res| ConfigAndPreset::Fulu(res.data))
262262
.unwrap();
263-
let expected = ConfigAndPreset::from_chain_spec::<E>(&E::default_spec(), None);
263+
let expected = ConfigAndPreset::from_chain_spec::<E>(&E::default_spec());
264264

265265
assert_eq!(result, expected);
266266

‎validator_client/http_api/src/tests.rs‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ struct ApiTester {
4545
validator_store: Arc<LighthouseValidatorStore<TestingSlotClock, E>>,
4646
url: SensitiveUrl,
4747
slot_clock: TestingSlotClock,
48+
spec: Arc<ChainSpec>,
4849
_validator_dir: TempDir,
4950
_secrets_dir: TempDir,
5051
_test_runtime: TestRuntime,
@@ -117,7 +118,7 @@ impl ApiTester {
117118
validator_store: Some(validator_store.clone()),
118119
graffiti_file: None,
119120
graffiti_flag: Some(Graffiti::default()),
120-
spec: E::default_spec().into(),
121+
spec: spec.clone(),
121122
config: HttpConfig {
122123
enabled: true,
123124
listen_addr: IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)),
@@ -152,6 +153,7 @@ impl ApiTester {
152153
validator_store,
153154
url,
154155
slot_clock,
156+
spec,
155157
_validator_dir: validator_dir,
156158
_secrets_dir: secrets_dir,
157159
_test_runtime: test_runtime,
@@ -206,13 +208,19 @@ impl ApiTester {
206208
}
207209

208210
pub async fn test_get_lighthouse_spec(self) -> Self {
209-
let result = self
210-
.client
211-
.get_lighthouse_spec::<ConfigAndPresetFulu>()
212-
.await
213-
.map(|res| ConfigAndPreset::Fulu(res.data))
214-
.unwrap();
215-
let expected = ConfigAndPreset::from_chain_spec::<E>(&E::default_spec(), None);
211+
let result = if self.spec.is_fulu_scheduled() {
212+
self.client
213+
.get_lighthouse_spec::<ConfigAndPresetFulu>()
214+
.await
215+
.map(|res| ConfigAndPreset::Fulu(res.data))
216+
} else {
217+
self.client
218+
.get_lighthouse_spec::<ConfigAndPresetElectra>()
219+
.await
220+
.map(|res| ConfigAndPreset::Electra(res.data))
221+
}
222+
.unwrap();
223+
let expected = ConfigAndPreset::from_chain_spec::<E>(&self.spec);
216224

217225
assert_eq!(result, expected);
218226

0 commit comments

Comments
 (0)