-
Notifications
You must be signed in to change notification settings - Fork 782
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 epoch processing #5761
Electra epoch processing #5761
Changes from all commits
3b7132b
e6c7f14
3a41e13
9b98f4e
7c6526d
19a9479
9f6de8e
38382a3
2c2e44c
e2e82ff
e0abede
f1f9f92
3c68841
1d5f755
32357d8
c40bec9
31955c2
75ab913
5728f78
8517236
90179d4
f30246b
7c0a8f8
43c3f63
721e73f
7abb762
7cb7653
e32dfcd
3ea3d22
07229b7
36a559e
cb8c8f5
c30f709
ab9e58a
ca09671
b807d39
19f8333
411fcee
e448557
aa83e8b
437e851
16265ef
72548cb
08e0458
7926afe
89e4de9
f60eac6
677a94d
e1dcfb6
b819d2d
40c4c00
a97e86c
261551e
a75257f
f4907ef
75f22ee
3b1fb0a
af7ba6f
aaf8e50
c900a88
97e88dd
217fa9f
793764f
c53d4ac
fc15736
d8941d7
0c29896
210ad2f
79a5f25
a8088f1
bafb5f0
f9c50bc
987abe0
82858bc
154b7a7
bb734af
469296b
3f169ef
3e10e68
9440c36
57b6a9a
75432e1
e340998
b61d244
49de63f
29ed1c5
a647a36
4013944
f9d3545
77c630b
f25531d
772ab53
b21b108
8dc9f38
c43d1c2
35e07eb
d7f3c95
d5aa2d8
c4f2284
3ac3ddb
9a01b6b
d87541c
dd0d5e2
3ec21a2
795eff9
960f8c5
1d0e3f4
5acc052
4f08f6e
f049285
5070ab2
45d007a
9e84779
7af3f2e
2634a1f
dec7cff
6a4d842
6f0b784
444cd62
d264736
7521f97
4d3edfe
7fce143
4d4c268
370d511
cbb7c5d
70a2d4d
9e6e76f
a8d8989
d67270f
3977b92
6e44832
afb9122
381bbab
0e2add2
f85a124
efb8a01
dd0aa8e
536c9f8
27ed90e
b6913ae
ebbb17b
13b1b05
339d1b8
70a80d5
7509cf6
8715589
09141ec
8fc5333
5517c78
cf030d0
68fd7a7
d137881
87fde51
a8d84d6
51a8c80
f60c1a0
bf4b92f
d16faea
14af712
3f2f222
0b1c45e
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 |
---|---|---|
|
@@ -9,6 +9,7 @@ use types::{ActivationQueue, BeaconState, ChainSpec, EthSpec, ForkName, Hash256} | |
pub struct PreEpochCache { | ||
epoch_key: EpochCacheKey, | ||
effective_balances: Vec<u64>, | ||
total_active_balance: u64, | ||
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 do we need to track total_active_balance here? 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. EDIT: Because now we call |
||
} | ||
|
||
impl PreEpochCache { | ||
|
@@ -36,27 +37,59 @@ impl PreEpochCache { | |
Ok(Self { | ||
epoch_key, | ||
effective_balances: Vec::with_capacity(state.validators().len()), | ||
total_active_balance: 0, | ||
}) | ||
} | ||
|
||
pub fn push_effective_balance(&mut self, effective_balance: u64) { | ||
self.effective_balances.push(effective_balance); | ||
pub fn update_effective_balance( | ||
&mut self, | ||
validator_index: usize, | ||
effective_balance: u64, | ||
is_active_next_epoch: bool, | ||
) -> Result<(), EpochCacheError> { | ||
if validator_index == self.effective_balances.len() { | ||
self.effective_balances.push(effective_balance); | ||
if is_active_next_epoch { | ||
self.total_active_balance | ||
.safe_add_assign(effective_balance)?; | ||
} | ||
|
||
Ok(()) | ||
} else if let Some(existing_balance) = self.effective_balances.get_mut(validator_index) { | ||
// Update total active balance for a late change in effective balance. This happens when | ||
// processing consolidations. | ||
if is_active_next_epoch { | ||
self.total_active_balance | ||
.safe_add_assign(effective_balance)?; | ||
self.total_active_balance | ||
.safe_sub_assign(*existing_balance)?; | ||
} | ||
*existing_balance = effective_balance; | ||
Ok(()) | ||
} else { | ||
Err(EpochCacheError::ValidatorIndexOutOfBounds { validator_index }) | ||
} | ||
} | ||
|
||
pub fn get_total_active_balance(&self) -> u64 { | ||
self.total_active_balance | ||
} | ||
|
||
pub fn into_epoch_cache( | ||
self, | ||
total_active_balance: u64, | ||
activation_queue: ActivationQueue, | ||
spec: &ChainSpec, | ||
) -> Result<EpochCache, EpochCacheError> { | ||
let epoch = self.epoch_key.epoch; | ||
let total_active_balance = self.total_active_balance; | ||
let sqrt_total_active_balance = SqrtTotalActiveBalance::new(total_active_balance); | ||
let base_reward_per_increment = BaseRewardPerIncrement::new(total_active_balance, spec)?; | ||
|
||
let effective_balance_increment = spec.effective_balance_increment; | ||
let max_effective_balance_eth = spec | ||
.max_effective_balance | ||
.safe_div(effective_balance_increment)?; | ||
let max_effective_balance = | ||
spec.max_effective_balance_for_fork(spec.fork_name_at_epoch(epoch)); | ||
let max_effective_balance_eth = | ||
max_effective_balance.safe_div(effective_balance_increment)?; | ||
|
||
let mut base_rewards = Vec::with_capacity(max_effective_balance_eth.safe_add(1)? as usize); | ||
|
||
|
@@ -131,9 +164,9 @@ pub fn initialize_epoch_cache<E: EthSpec>( | |
decision_block_root, | ||
}, | ||
effective_balances, | ||
total_active_balance, | ||
}; | ||
*state.epoch_cache_mut() = | ||
pre_epoch_cache.into_epoch_cache(total_active_balance, activation_queue, spec)?; | ||
*state.epoch_cache_mut() = pre_epoch_cache.into_epoch_cache(activation_queue, spec)?; | ||
|
||
Ok(()) | ||
} |
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.
Why is it necessary to delay computing the exit cache now?
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.
I don't know. @realbigsean deleted it in this commit: f4907ef
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.
I needed to do this to fix some EF tests. In Electra we are mutating the state when calculating
exit_queue_epoch
because we callcompute_exit_epoch_and_update_churn
. So we need to actually do the check for whether the validator exited before calculatingexit_queue_epoch
.I figured it wasn't worth trying to maintain the pre-fork optimization at the expense of clarity. Is it worth adding back or trying to find a similar post-fork optimization?