-
Notifications
You must be signed in to change notification settings - Fork 745
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.
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
Electra: Add Preset, Constants, & Config #5606
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ pub enum Domain { | |
SyncCommittee, | ||
ContributionAndProof, | ||
SyncCommitteeSelectionProof, | ||
Consolidation, | ||
ApplicationMask(ApplicationDomain), | ||
} | ||
|
||
|
@@ -76,6 +77,7 @@ pub struct ChainSpec { | |
pub genesis_fork_version: [u8; 4], | ||
pub bls_withdrawal_prefix_byte: u8, | ||
pub eth1_address_withdrawal_prefix_byte: u8, | ||
pub compounding_withdrawal_prefix_byte: u8, | ||
|
||
/* | ||
* Time parameters | ||
|
@@ -108,6 +110,7 @@ pub struct ChainSpec { | |
pub(crate) domain_voluntary_exit: u32, | ||
pub(crate) domain_selection_proof: u32, | ||
pub(crate) domain_aggregate_and_proof: u32, | ||
pub(crate) domain_consolidation: u32, | ||
|
||
/* | ||
* Fork choice | ||
|
@@ -177,6 +180,15 @@ pub struct ChainSpec { | |
pub electra_fork_version: [u8; 4], | ||
/// The Electra fork epoch is optional, with `None` representing "Electra never happens". | ||
pub electra_fork_epoch: Option<Epoch>, | ||
pub unset_deposit_receipts_start_index: u64, | ||
pub full_exit_request_amount: u64, | ||
pub min_activation_balance: u64, | ||
pub max_effective_balance_electra: u64, | ||
pub min_slashing_penalty_quotient_electra: u64, | ||
pub whistleblower_reward_quotient_electra: u64, | ||
pub max_pending_partials_per_withdrawals_sweep: u64, | ||
pub min_per_epoch_churn_limit_electra: u64, | ||
pub max_per_epoch_activation_exit_churn_limit: u64, | ||
|
||
/* | ||
* Networking | ||
|
@@ -364,7 +376,9 @@ impl ChainSpec { | |
state: &BeaconState<E>, | ||
) -> u64 { | ||
let fork_name = state.fork_name_unchecked(); | ||
if fork_name >= ForkName::Merge { | ||
if fork_name >= ForkName::Electra { | ||
self.min_slashing_penalty_quotient_electra | ||
} else if fork_name >= ForkName::Merge { | ||
self.min_slashing_penalty_quotient_bellatrix | ||
} else if fork_name >= ForkName::Altair { | ||
self.min_slashing_penalty_quotient_altair | ||
|
@@ -418,6 +432,7 @@ impl ChainSpec { | |
Domain::SyncCommitteeSelectionProof => self.domain_sync_committee_selection_proof, | ||
Domain::ApplicationMask(application_domain) => application_domain.get_domain_constant(), | ||
Domain::BlsToExecutionChange => self.domain_bls_to_execution_change, | ||
Domain::Consolidation => self.domain_consolidation, | ||
} | ||
} | ||
|
||
|
@@ -602,6 +617,7 @@ impl ChainSpec { | |
genesis_fork_version: [0; 4], | ||
bls_withdrawal_prefix_byte: 0x00, | ||
eth1_address_withdrawal_prefix_byte: 0x01, | ||
compounding_withdrawal_prefix_byte: 0x02, | ||
|
||
/* | ||
* Time parameters | ||
|
@@ -635,6 +651,7 @@ impl ChainSpec { | |
domain_voluntary_exit: 4, | ||
domain_selection_proof: 5, | ||
domain_aggregate_and_proof: 6, | ||
domain_consolidation: 0x0B, | ||
|
||
/* | ||
* Fork choice | ||
|
@@ -709,6 +726,30 @@ impl ChainSpec { | |
*/ | ||
electra_fork_version: [0x05, 00, 00, 00], | ||
electra_fork_epoch: None, | ||
unset_deposit_receipts_start_index: u64::MAX, | ||
full_exit_request_amount: 0, | ||
min_activation_balance: option_wrapper(|| { | ||
u64::checked_pow(2, 5)?.checked_mul(u64::checked_pow(10, 9)?) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just write There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mostly consistency with the way the other values are written.. the spec also provides them factored this way so that you can do quick calculations in your head if you want |
||
}) | ||
.expect("calculation does not overflow"), | ||
max_effective_balance_electra: option_wrapper(|| { | ||
u64::checked_pow(2, 11)?.checked_mul(u64::checked_pow(10, 9)?) | ||
}) | ||
.expect("calculation does not overflow"), | ||
min_slashing_penalty_quotient_electra: u64::checked_pow(2, 12) | ||
.expect("pow does not overflow"), | ||
whistleblower_reward_quotient_electra: u64::checked_pow(2, 12) | ||
.expect("pow does not overflow"), | ||
max_pending_partials_per_withdrawals_sweep: u64::checked_pow(2, 3) | ||
.expect("pow does not overflow"), | ||
min_per_epoch_churn_limit_electra: option_wrapper(|| { | ||
u64::checked_pow(2, 7)?.checked_mul(u64::checked_pow(10, 9)?) | ||
}) | ||
.expect("calculation does not overflow"), | ||
max_per_epoch_activation_exit_churn_limit: option_wrapper(|| { | ||
u64::checked_pow(2, 8)?.checked_mul(u64::checked_pow(10, 9)?) | ||
}) | ||
.expect("calculation does not overflow"), | ||
|
||
/* | ||
* Network specific | ||
|
@@ -874,6 +915,7 @@ impl ChainSpec { | |
genesis_fork_version: [0x00, 0x00, 0x00, 0x64], | ||
bls_withdrawal_prefix_byte: 0x00, | ||
eth1_address_withdrawal_prefix_byte: 0x01, | ||
compounding_withdrawal_prefix_byte: 0x02, | ||
|
||
/* | ||
* Time parameters | ||
|
@@ -907,6 +949,7 @@ impl ChainSpec { | |
domain_voluntary_exit: 4, | ||
domain_selection_proof: 5, | ||
domain_aggregate_and_proof: 6, | ||
domain_consolidation: 0x0B, | ||
|
||
/* | ||
* Fork choice | ||
|
@@ -983,6 +1026,30 @@ impl ChainSpec { | |
*/ | ||
electra_fork_version: [0x05, 0x00, 0x00, 0x64], | ||
electra_fork_epoch: None, | ||
unset_deposit_receipts_start_index: u64::MAX, | ||
full_exit_request_amount: 0, | ||
min_activation_balance: option_wrapper(|| { | ||
u64::checked_pow(2, 5)?.checked_mul(u64::checked_pow(10, 9)?) | ||
}) | ||
.expect("calculation does not overflow"), | ||
max_effective_balance_electra: option_wrapper(|| { | ||
u64::checked_pow(2, 11)?.checked_mul(u64::checked_pow(10, 9)?) | ||
}) | ||
.expect("calculation does not overflow"), | ||
min_slashing_penalty_quotient_electra: u64::checked_pow(2, 12) | ||
.expect("pow does not overflow"), | ||
whistleblower_reward_quotient_electra: u64::checked_pow(2, 12) | ||
.expect("pow does not overflow"), | ||
max_pending_partials_per_withdrawals_sweep: u64::checked_pow(2, 3) | ||
.expect("pow does not overflow"), | ||
min_per_epoch_churn_limit_electra: option_wrapper(|| { | ||
u64::checked_pow(2, 7)?.checked_mul(u64::checked_pow(10, 9)?) | ||
}) | ||
.expect("calculation does not overflow"), | ||
max_per_epoch_activation_exit_churn_limit: option_wrapper(|| { | ||
u64::checked_pow(2, 8)?.checked_mul(u64::checked_pow(10, 9)?) | ||
}) | ||
.expect("calculation does not overflow"), | ||
|
||
/* | ||
* Network specific | ||
|
@@ -1206,6 +1273,13 @@ pub struct Config { | |
#[serde(default = "default_blob_sidecar_subnet_count")] | ||
#[serde(with = "serde_utils::quoted_u64")] | ||
blob_sidecar_subnet_count: u64, | ||
|
||
#[serde(default = "default_min_per_epoch_churn_limit_electra")] | ||
#[serde(with = "serde_utils::quoted_u64")] | ||
min_per_epoch_churn_limit_electra: u64, | ||
#[serde(default = "default_max_per_epoch_activation_exit_churn_limit")] | ||
#[serde(with = "serde_utils::quoted_u64")] | ||
max_per_epoch_activation_exit_churn_limit: u64, | ||
} | ||
|
||
fn default_bellatrix_fork_version() -> [u8; 4] { | ||
|
@@ -1320,6 +1394,14 @@ const fn default_blob_sidecar_subnet_count() -> u64 { | |
6 | ||
} | ||
|
||
const fn default_min_per_epoch_churn_limit_electra() -> u64 { | ||
128_000_000_000 | ||
} | ||
|
||
const fn default_max_per_epoch_activation_exit_churn_limit() -> u64 { | ||
256_000_000_000 | ||
} | ||
|
||
const fn default_epochs_per_subnet_subscription() -> u64 { | ||
256 | ||
} | ||
|
@@ -1496,6 +1578,10 @@ impl Config { | |
max_request_blob_sidecars: spec.max_request_blob_sidecars, | ||
min_epochs_for_blob_sidecars_requests: spec.min_epochs_for_blob_sidecars_requests, | ||
blob_sidecar_subnet_count: spec.blob_sidecar_subnet_count, | ||
|
||
min_per_epoch_churn_limit_electra: spec.min_per_epoch_churn_limit_electra, | ||
max_per_epoch_activation_exit_churn_limit: spec | ||
.max_per_epoch_activation_exit_churn_limit, | ||
} | ||
} | ||
|
||
|
@@ -1563,6 +1649,8 @@ impl Config { | |
max_request_blob_sidecars, | ||
min_epochs_for_blob_sidecars_requests, | ||
blob_sidecar_subnet_count, | ||
min_per_epoch_churn_limit_electra, | ||
max_per_epoch_activation_exit_churn_limit, | ||
} = self; | ||
|
||
if preset_base != E::spec_name().to_string().as_str() { | ||
|
@@ -1623,6 +1711,8 @@ impl Config { | |
max_request_blob_sidecars, | ||
min_epochs_for_blob_sidecars_requests, | ||
blob_sidecar_subnet_count, | ||
min_per_epoch_churn_limit_electra, | ||
max_per_epoch_activation_exit_churn_limit, | ||
|
||
// We need to re-derive any values that might have changed in the config. | ||
max_blocks_by_root_request: max_blocks_by_root_request_common(max_request_blocks), | ||
|
@@ -1695,6 +1785,7 @@ mod tests { | |
&spec, | ||
); | ||
test_domain(Domain::SyncCommittee, spec.domain_sync_committee, &spec); | ||
test_domain(Domain::Consolidation, spec.domain_consolidation, &spec); | ||
|
||
// The builder domain index is zero | ||
let builder_domain_pre_mask = [0; 4]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two aren't configurable, so I'm not sure we should add them here. We aren't totally consistent with
ChainSpec
, because the signature domains and prefixes are on this struct. But it should be config + preset from the spec. We have aconsensus/types/consts.rs
file we could move them to.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to issue #5631