Skip to content

Commit b1f3ccd

Browse files
committed
Merge perf/beacon-more-state-fields-on-tree (#650) into tmp/bci-626-63-64-633-636-638-gloas-live
Conflicted files: - crates/blockchain/state_transition/src/beacon/stf/epoch/altair.rs - crates/blockchain/state_transition/src/beacon/stf/mod.rs - crates/common/ssz-tree/tests/model.rs - crates/common/types/src/beacon/containers/mod.rs - crates/common/types/src/beacon/containers/shared.rs Gloas adaptations: - tree_fields! gets a gloas line: validators and balances (progressive trees) plus block_roots, state_roots, historical_roots, eth1_data_votes, randao_mixes, slashings and historical_summaries, which gloas takes from the shared aliases. inactivity_scores is left out on purpose: gloas declares its own flat libssz ProgressiveList for it, which buffers nothing, so there is nothing to flush or rebase. - ssz-tree: ProgressiveList implements Buffered, so the field list can hold the progressive registry beside the bounded fields. - apply_pending_mutations / has_pending_mutations dispatch over all eight beacon forks through buffered()/buffered_mut(); the lean guard on has_pending_mutations is kept (the merge had dropped it with the old registry match). - rebase_on: the validators/balances list-kind match stays (so a gloas state still rebases onto a gloas base only); the other fields are shared types in every fork and rebase unconditionally; inactivity scores rebase only for a Tree/Tree pair. - Inactivity scores are a tree List before gloas and a flat slice-like list in gloas, so no &[u64] can serve both. altair_validator_lists now returns an InactivityScoresRef view (len, get, in-order iter, ptr_eq, has_pending_updates, PartialEq) as its third element, and inactivity_scores_mut (a &mut [u64]) becomes inactivity_score_mut(index), an element write that dispatches over both kinds. - stf/epoch/altair.rs update_inactivity_scores: decides over an in-order walk of the scores (in step with the summary) and writes only changed scores afterwards, which subsumes both #633's "write if different" and #650's no-op skip; the error order (first eligible validator without a score, then checked-add overflow) is unchanged. - stf/mod.rs tests: BEACON_FORKS now includes Gloas, the historical summaries write covers gloas, and the inactivity score case expects no pending write on gloas. process_slot's registry check uses the new BeaconState::registry_has_pending_updates, since the per-slot roots writes stay buffered by design. - storage store.rs test and the epoch-processing fixture runner comments follow the renamed accessors. Other: shared.rs imports both List/Vector and the progressive alias; model.rs keeps both sides' tests (the base side was empty).
2 parents ebc0664 + 3f9aa4f commit b1f3ccd

19 files changed

Lines changed: 1033 additions & 140 deletions

File tree

‎crates/blockchain/state_transition/src/beacon/block_production.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,10 @@ pub fn assemble_block(
539539
&ExecutionEngine::valid(),
540540
&CommitteeCache::default(),
541541
)?;
542+
// Fold the block's writes into their trees first, so the root is computed
543+
// on (and cached in) the state's own nodes rather than a throwaway copy,
544+
// as `state_transition` does before it checks a block's state root.
545+
post.apply_pending_mutations();
542546
block.state_root = post.hash_tree_root();
543547
Ok(block)
544548
}

‎crates/blockchain/state_transition/src/beacon/stf/epoch/altair.rs‎

Lines changed: 82 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ pub(super) fn weigh_with_totals(
104104
/// epoch's participation and genesis has none.
105105
///
106106
/// Shared by every fork from altair on, gloas included: reading
107-
/// [`crate::beacon::containers::BeaconState::inactivity_scores_mut`] rather than
107+
/// [`crate::beacon::containers::BeaconState::inactivity_score_mut`] rather than
108108
/// [`crate::beacon::containers::BeaconState::altair_validator_lists_mut`] is what
109109
/// makes that so, since this only ever writes a score in place and never
110110
/// grows or replaces the list, and that narrower accessor extends to a gloas
@@ -129,26 +129,40 @@ fn update_inactivity_scores(
129129
) -> Result<()> {
130130
let leaking = is_in_inactivity_leak(state)?;
131131

132-
let inactivity_scores = state.inactivity_scores_mut()?;
133-
let score_count = inactivity_scores.len();
134-
for (index, (flags, _)) in summary.iter().enumerate() {
135-
if !flags.is_eligible() {
136-
continue;
137-
}
138-
let score = inactivity_scores
139-
.get_mut(index)
140-
.ok_or(Error::IndexOutOfBounds {
132+
// Decided over an in-order walk of the scores and written afterwards: a
133+
// tree-backed list is read with an iterator (one descent per element would
134+
// dominate), and only a score that changes is written, since outside a
135+
// leak almost every score stays zero and a write of an equal value would
136+
// still unshare its leaf from the parent state's.
137+
let changes = {
138+
let (_, _, inactivity_scores) = state.altair_validator_lists()?;
139+
let score_count = inactivity_scores.len();
140+
let mut score_iter = inactivity_scores.iter();
141+
let mut changes: Vec<(usize, u64)> = Vec::new();
142+
for (index, (flags, _)) in summary.iter().enumerate() {
143+
// Advanced for ineligible validators too, to stay in step with `index`;
144+
// a score list shorter than the registry is an error only at the first
145+
// eligible validator without one.
146+
let score = score_iter.next().copied();
147+
if !flags.is_eligible() {
148+
continue;
149+
}
150+
let score = score.ok_or(Error::IndexOutOfBounds {
141151
index,
142152
len: score_count,
143153
})?;
144154

145-
let updated = next_inactivity_score(*score, flags, leaking, config)?;
146-
147-
if updated != *score {
148-
*score = updated;
155+
let updated = next_inactivity_score(score, flags, leaking, config)?;
156+
if updated != score {
157+
changes.push((index, updated));
158+
}
149159
}
150-
}
160+
changes
161+
};
151162

163+
for (index, updated) in changes {
164+
*state.inactivity_score_mut(index)? = updated;
165+
}
152166
Ok(())
153167
}
154168

@@ -515,7 +529,7 @@ mod tests {
515529
process_inactivity_updates(&mut state, &Config::mainnet()).unwrap();
516530

517531
let (_, _, scores) = state.altair_validator_lists().unwrap();
518-
assert_eq!(scores, before);
532+
assert_eq!(scores.to_vec(), before);
519533
}
520534

521535
#[test]
@@ -570,6 +584,58 @@ mod tests {
570584
assert_eq!(scores[0], 0);
571585
}
572586

587+
/// Outside a leak a missed epoch adds the bias and recovery takes it back
588+
/// to zero, so every score ends where it started. The pass must then
589+
/// leave the list untouched, not just equal: a write of an equal value
590+
/// would still unshare the tree from the parent state's.
591+
#[test]
592+
fn inactivity_updates_that_change_nothing_leave_the_tree_shared() {
593+
let mut state = altair_state_with_validators(4);
594+
state.apply_pending_mutations();
595+
let before_state = state.clone();
596+
let (_, _, before) = before_state.altair_validator_lists().unwrap();
597+
598+
process_inactivity_updates(&mut state, &Config::mainnet()).unwrap();
599+
600+
let (_, _, scores) = state.altair_validator_lists().unwrap();
601+
assert!(!scores.has_pending_updates(), "no write was buffered");
602+
assert!(scores.ptr_eq(&before));
603+
assert_eq!(scores.to_vec(), vec![0; 4]);
604+
}
605+
606+
#[test]
607+
fn inactivity_updates_buffer_only_the_scores_that_change() {
608+
let mut state = altair_state_with_validators(4);
609+
{
610+
let (previous_epoch_participation, _, scores) =
611+
state.altair_validator_lists_mut().unwrap();
612+
scores[2] = 3;
613+
// Validator 2 participates, so its score falls to zero; the rest
614+
// stay at zero.
615+
previous_epoch_participation[2] = add_flag(0, constants::TIMELY_TARGET_FLAG_INDEX);
616+
}
617+
state.apply_pending_mutations();
618+
619+
process_inactivity_updates(&mut state, &Config::mainnet()).unwrap();
620+
621+
let (_, _, scores) = state.altair_validator_lists().unwrap();
622+
assert!(scores.has_pending_updates());
623+
assert_eq!(scores.to_vec(), vec![0; 4]);
624+
}
625+
626+
#[test]
627+
fn inactivity_updates_still_reject_an_overflowing_score() {
628+
let mut state = altair_state_with_validators(4);
629+
{
630+
let (_, _, scores) = state.altair_validator_lists_mut().unwrap();
631+
scores[1] = u64::MAX;
632+
}
633+
634+
let result = process_inactivity_updates(&mut state, &Config::mainnet());
635+
636+
assert!(matches!(result, Err(Error::ArithmeticOverflow(_))));
637+
}
638+
573639
// -----------------------------------------------------------------------
574640
// process_rewards_and_penalties
575641
// -----------------------------------------------------------------------

‎crates/blockchain/state_transition/src/beacon/stf/epoch/gloas.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
//! [`super::altair::process_rewards_and_penalties`]) reach
3232
//! `previous_epoch_participation`/`current_epoch_participation`/`inactivity_scores`
3333
//! as plain slices, through [`crate::beacon::containers::BeaconState::altair_validator_lists`]
34-
//! and [`crate::beacon::containers::BeaconState::inactivity_scores_mut`]: both
34+
//! and [`crate::beacon::containers::BeaconState::inactivity_score_mut`]: both
3535
//! `SszList` and `ProgressiveList` `Deref` to a plain `[T]`.
3636
//! - **[`super::electra::process_pending_consolidations`]** takes the queue
3737
//! as a `Vec` and writes it back, through

‎crates/blockchain/state_transition/src/beacon/stf/mod.rs‎

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ mod tests {
348348
use super::*;
349349
use crate::beacon::fork::ForkName;
350350
use crate::beacon::helpers::test_state;
351+
use crate::beacon::primitives::Root;
351352

352353
#[test]
353354
fn process_slot_unsets_next_slots_payload_availability_on_gloas() {
@@ -424,7 +425,11 @@ mod tests {
424425

425426
process_slot(&mut state).unwrap();
426427

427-
assert!(!state.has_pending_mutations());
428+
assert!(!state.registry_has_pending_updates());
429+
// Its own two roots writes stay buffered until the next flush, which
430+
// the following slot (or `process_slots`' last step) performs.
431+
assert!(state.state_roots().has_pending_updates());
432+
assert!(state.block_roots().has_pending_updates());
428433
}
429434

430435
/// Epoch processing (`process_rewards_and_penalties` here) writes every
@@ -443,4 +448,112 @@ mod tests {
443448

444449
assert!(!state.has_pending_mutations());
445450
}
451+
452+
#[test]
453+
fn process_slots_flushes_the_roots_it_wrote_in_its_last_slot() {
454+
for fork in BEACON_FORKS {
455+
let mut state = test_state::with_validators_at(fork, 4);
456+
let target_slot = state.slot() + 1;
457+
458+
process_slots(&mut state, target_slot, &Config::mainnet()).unwrap();
459+
460+
assert!(!state.has_pending_mutations(), "{fork:?}");
461+
}
462+
}
463+
464+
const BEACON_FORKS: [ForkName; 8] = [
465+
ForkName::Phase0,
466+
ForkName::Altair,
467+
ForkName::Bellatrix,
468+
ForkName::Capella,
469+
ForkName::Deneb,
470+
ForkName::Electra,
471+
ForkName::Fulu,
472+
ForkName::Gloas,
473+
];
474+
475+
/// Writes one element of the field a case names, or answers `false` if the
476+
/// fork does not have it.
477+
type Write = fn(&mut BeaconState) -> bool;
478+
479+
fn push_historical_summary(state: &mut BeaconState) -> bool {
480+
let summary = containers::HistoricalSummary::default();
481+
match state {
482+
BeaconState::Capella(s) => s.historical_summaries.push(summary).unwrap(),
483+
BeaconState::Deneb(s) => s.historical_summaries.push(summary).unwrap(),
484+
BeaconState::Electra(s) => s.historical_summaries.push(summary).unwrap(),
485+
BeaconState::Fulu(s) => s.historical_summaries.push(summary).unwrap(),
486+
BeaconState::Gloas(s) => s.historical_summaries.push(summary).unwrap(),
487+
_ => return false,
488+
}
489+
true
490+
}
491+
492+
/// One write to every tree-backed field of every fork: the flush and the
493+
/// pending check must both see all of them, since both come from one
494+
/// field list.
495+
#[test]
496+
fn every_tree_field_of_every_fork_is_flushed_and_checked() {
497+
let cases: [(&str, Write); 11] = [
498+
("validators", |s| {
499+
s.validator_mut(0).unwrap().effective_balance -= 1;
500+
true
501+
}),
502+
("balances", |s| {
503+
*s.balance_mut(0).unwrap() += 1;
504+
true
505+
}),
506+
("block_roots", |s| {
507+
s.block_roots_mut()[0] = Root::repeat_byte(1);
508+
true
509+
}),
510+
("state_roots", |s| {
511+
s.state_roots_mut()[0] = Root::repeat_byte(1);
512+
true
513+
}),
514+
("historical_roots", |s| {
515+
s.historical_roots_mut().push(Root::repeat_byte(1)).unwrap();
516+
true
517+
}),
518+
("eth1_data_votes", |s| {
519+
s.eth1_data_votes_mut().push(Default::default()).unwrap();
520+
true
521+
}),
522+
("randao_mixes", |s| {
523+
s.randao_mixes_mut()[0] = Root::repeat_byte(1);
524+
true
525+
}),
526+
("slashings", |s| {
527+
s.slashings_mut()[0] += 1;
528+
true
529+
}),
530+
// Gloas keeps its scores in a flat progressive list that buffers
531+
// nothing, so a write there leaves nothing pending.
532+
("inactivity_scores", |s| {
533+
let buffered = !matches!(s, BeaconState::Gloas(_));
534+
match s.inactivity_score_mut(0) {
535+
Ok(score) => {
536+
*score += 1;
537+
buffered
538+
}
539+
Err(_) => false,
540+
}
541+
}),
542+
("historical_summaries", push_historical_summary),
543+
// A read-only pass writes nothing and must leave nothing pending.
544+
("none", |_| false),
545+
];
546+
547+
for fork in BEACON_FORKS {
548+
let mut state = test_state::with_validators_at(fork, 4);
549+
assert!(!state.has_pending_mutations(), "{fork:?} starts flushed");
550+
551+
for (name, write) in cases {
552+
let wrote = write(&mut state);
553+
assert_eq!(state.has_pending_mutations(), wrote, "{fork:?} {name}");
554+
state.apply_pending_mutations();
555+
assert!(!state.has_pending_mutations(), "{fork:?} {name} flushed");
556+
}
557+
}
558+
}
446559
}

‎crates/blockchain/state_transition/src/beacon/upgrade.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,8 +1411,8 @@ mod tests {
14111411
// place rather than clearing it, since `historical_summaries` is
14121412
// where new history accumulates from here on.
14131413
assert_eq!(
1414-
post.historical_roots.into_inner(),
1415-
bellatrix_state.historical_roots.into_inner()
1414+
post.historical_roots.to_vec(),
1415+
bellatrix_state.historical_roots.to_vec()
14161416
);
14171417
assert_eq!(post.next_withdrawal_index, 0);
14181418
assert_eq!(post.next_withdrawal_validator_index, 0);

‎crates/blockchain/state_transition/tests/beacon_spec/epoch_processing.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,15 +185,15 @@ fn apply(
185185
// module's own doc. `inactivity_updates` reads gloas too, below: its
186186
// participation and inactivity lists are progressive (EIP-7688), a
187187
// different Rust type from the bounded `SszList` altair's own copy
188-
// used to need, but `BeaconState::inactivity_scores_mut` and
189-
// `altair_validator_lists` both now reach either list kind through a
190-
// shared slice type. `participation_flag_updates`, just below, is the
188+
// used to need, but `BeaconState::inactivity_score_mut` and
189+
// `altair_validator_lists` both now reach either list kind (the latter
190+
// through `InactivityScoresRef`). `participation_flag_updates`, just below, is the
191191
// one exception: it *replaces* the whole list rather than writing an
192192
// element, which only the fork's own concrete container type can do,
193193
// so gloas keeps its own copy of that one step; see
194194
// `epoch::gloas`'s own module doc.
195195
"inactivity_updates" => match fork {
196-
// `BeaconState::inactivity_scores_mut` reaches a gloas state's
196+
// `BeaconState::inactivity_score_mut` reaches a gloas state's
197197
// scores too (element writes only, which both list kinds allow
198198
// through `DerefMut`; see that accessor's own doc), so this one
199199
// copy serves gloas as well.

‎crates/common/ssz-tree/src/lib.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,19 @@ pub use vector::Vector;
9191
use libssz::{SszDecode, SszEncode};
9292
use libssz_merkle::HashTreeRoot;
9393

94+
/// The buffered-write surface of a [`List`] or [`Vector`], independent of its
95+
/// element type and update map.
96+
///
97+
/// Object safe, so a container can hold one `&dyn Buffered` per tree field of
98+
/// different element types and flush or check them all from one field list.
99+
pub trait Buffered {
100+
/// Folds every buffered write into the tree.
101+
fn apply_updates(&mut self);
102+
103+
/// Whether any write is buffered and not yet folded into the tree.
104+
fn has_pending_updates(&self) -> bool;
105+
}
106+
94107
/// A 32-byte Merkle node.
95108
pub(crate) type Hash256 = libssz_merkle::Node;
96109

‎crates/common/ssz-tree/src/list.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,16 @@ impl<T: Value, const N: usize, U: UpdateMap<T>> List<T, N, U> {
137137
}
138138
}
139139

140+
impl<T: Value, const N: usize, U: UpdateMap<T>> crate::Buffered for List<T, N, U> {
141+
fn apply_updates(&mut self) {
142+
List::apply_updates(self);
143+
}
144+
145+
fn has_pending_updates(&self) -> bool {
146+
List::has_pending_updates(self)
147+
}
148+
}
149+
140150
impl<T: Value, const N: usize, U: UpdateMap<T>> Default for List<T, N, U> {
141151
fn default() -> Self {
142152
Self::empty()

‎crates/common/ssz-tree/src/progressive_list.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,16 @@ impl<T: Value, U: UpdateMap<T>> ProgressiveIterCow<'_, T, U> {
230230
}
231231
}
232232

233+
impl<T: Value, U: UpdateMap<T>> crate::Buffered for ProgressiveList<T, U> {
234+
fn apply_updates(&mut self) {
235+
ProgressiveList::apply_updates(self);
236+
}
237+
238+
fn has_pending_updates(&self) -> bool {
239+
ProgressiveList::has_pending_updates(self)
240+
}
241+
}
242+
233243
impl<T: Value, U: UpdateMap<T>> Default for ProgressiveList<T, U> {
234244
fn default() -> Self {
235245
Self::empty()

‎crates/common/ssz-tree/src/vector.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,16 @@ impl<T: Value, const N: usize, U: UpdateMap<T>> Vector<T, N, U> {
108108
}
109109
}
110110

111+
impl<T: Value, const N: usize, U: UpdateMap<T>> crate::Buffered for Vector<T, N, U> {
112+
fn apply_updates(&mut self) {
113+
Vector::apply_updates(self);
114+
}
115+
116+
fn has_pending_updates(&self) -> bool {
117+
Vector::has_pending_updates(self)
118+
}
119+
}
120+
111121
impl<T: Value + Default, const N: usize, U: UpdateMap<T>> Default for Vector<T, N, U> {
112122
/// A vector of `N` default-valued elements.
113123
fn default() -> Self {

0 commit comments

Comments
 (0)