Skip to content

Commit 5d64e8d

Browse files
committed
feat(iota-core): skip scoreboard.update_scores on no-report commits (#11562)
# Description of change Skip the per-commit `Scoreboard::update_scores(&report_aggregator)` recompute when no `MisbehaviorReport` was processed in the commit. Stacks on top of #11561 (which introduces `ConsensusCommitOutput.report_state_snapshots` — the signal we gate on). ## How the gate works - `ConsensusCommitOutput::has_report_state_changes()` returns `true` iff at least one `process_report` ran during this commit (i.e. `report_state_snapshots` is non-empty). - In `process_consensus_transactions_and_commit_boundary`, the existing call becomes: ```rust if self.protocol_config().calculate_validator_scores() && output.has_report_state_changes() { self.scoreboard.update_scores(&self.report_aggregator); } ``` ## Why this is safe (cross-validator determinism) - `Scoreboard::update_scores` is a pure function of `(voting_power, aggregator.reporters_with_voting_power())`. Same inputs → same output across validators. - All validators see the same `report_state_snapshots` emptiness (consensus is deterministic), so all skip / run in lock-step. - When all skip, `current_scores` retains its prior `Arc<Vec<u64>>` — same across validators because the previous `update_scores` ran on the same aggregator state. - `invalid_reports_count` bumps don't affect scores (`reporters_with_voting_power` reads only `received_metrics`), so verify-time bumps that don't mark dirty are also score-irrelevant. ## Startup bootstrap A freshly constructed `Scoreboard` starts at `[MAX_SCORE; committee_size]`, but a never-restarted peer's reflects accumulated updates. Without a fix, the next no-report commit would skip on both, diverging the published vector. So `AuthorityPerEpochStore::new` now calls `scoreboard.update_scores(&report_aggregator)` once, immediately after `restore_from_tables`. If the restored aggregator is empty (fresh epoch), `update_scores` returns `None` and leaves the default — correct for the fresh-epoch path too. ## How the change has been tested - [x] Basic tests (linting, compilation, formatting, unit/integration tests) - [x] Patch-specific tests (correctness, functionality coverage) - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have checked that new and existing unit tests pass locally with my changes Local verification: - `cargo ci-clippy` — clean. - `IOTA_SKIP_SIMTESTS=1 cargo nextest run -p iota-core --lib` — 564 tests pass. - New tests in `scorer.rs`: - `test_skipped_update_leaves_current_scores_unchanged` — proves the skip path preserves `current_scores` bit-for-bit. - `test_bootstrap_from_restored_aggregator_matches_live_path` — proves a restored-and-bootstrapped scoreboard publishes the same vector as a never-restarted peer's, given the same aggregator state. ## Links to any relevant issues fixes iotaledger/iota-private#408
1 parent d385ff5 commit 5d64e8d

2 files changed

Lines changed: 25 additions & 1 deletion

File tree

crates/iota-core/src/authority/authority_per_epoch_store.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,16 @@ impl AuthorityPerEpochStore {
11181118
.expect("AuthorityEpochTables should contain valid ReportAggregator state");
11191119
let voting_power = committee.members().map(|(_, v)| *v).collect::<Vec<u64>>();
11201120
let scoreboard = Scoreboard::new(voting_power, &protocol_config);
1121+
// Prime the published score vector from any restored aggregator state.
1122+
// Without this, a freshly constructed Scoreboard starts at [MAX_SCORE;
1123+
// committee_size], so the next no-report commit (which both restored
1124+
// and never-restarted validators skip) would publish different vectors
1125+
// across the network. `update_scores` is a no-op when the aggregator
1126+
// is empty (no `received_metrics` rows restored), so this is also
1127+
// correct for the fresh-epoch path.
1128+
if protocol_config.calculate_validator_scores() {
1129+
scoreboard.update_scores(&report_aggregator);
1130+
}
11211131

11221132
let s = Arc::new(Self {
11231133
name,
@@ -3247,7 +3257,14 @@ impl AuthorityPerEpochStore {
32473257
// reports and snapshotting. This avoids cross-thread reads of the
32483258
// aggregator — the checkpoint service only reads the published score
32493259
// snapshot (`ArcSwap<Vec<u64>>`).
3250-
if self.protocol_config().calculate_validator_scores() {
3260+
//
3261+
// Skip the recompute entirely when no `process_report` ran this commit:
3262+
// the aggregator state didn't change, so the score vector can't either.
3263+
// All validators see the same `report_state_snapshots` emptiness, so
3264+
// they all skip / run in lock-step — `current_scores` stays consistent
3265+
// across the network.
3266+
if self.protocol_config().calculate_validator_scores() && output.has_report_state_changes()
3267+
{
32513268
self.scoreboard.update_scores(&self.report_aggregator);
32523269
}
32533270
self.process_user_signatures(

crates/iota-core/src/authority/consensus_quarantine.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,13 @@ impl ConsensusCommitOutput {
152152
.insert(authority_index as u8, snapshot);
153153
}
154154

155+
/// Returns `true` if at least one `process_report` ran during this commit.
156+
/// Used by the consensus handler to skip `Scoreboard::update_scores` on
157+
/// commits that can't change the score vector.
158+
pub(crate) fn has_report_state_changes(&self) -> bool {
159+
!self.report_state_snapshots.is_empty()
160+
}
161+
155162
pub fn set_next_shared_object_versions(
156163
&mut self,
157164
next_versions: HashMap<ObjectID, SequenceNumber>,

0 commit comments

Comments
 (0)