From 5c3b4b7a511e3e904b1f43222b5f357a08a43a2f Mon Sep 17 00:00:00 2001 From: real-venus Date: Wed, 24 Jun 2026 06:58:28 -0700 Subject: [PATCH] fix: eliminate reputation update race via append-only event log (#3) Reputation was a single scalar updated read-compute-write, so a successful attestation (+10) and a slashing event (-500) processed together raced: the last writer won and a +10 could clobber the slash (final 760 instead of 260), dropping the authoritative slash. Fix stores reputation as a per-event log (blueprint step 3): each adjustment is appended as a ReputationEvent and the score is the clamped sum, so two updates each record their event and the result is independent of write order. A monotonic slash_count + try_slash (step 4) make slashing idempotent against duplicate replays. - reputation/score.rs: ReputationLedger (set_initial_score, update_reputation, try_slash, score, slash_count), ReputationSource, ReputationEvent, range/adjustment constants - tests/score_tests.rs: concurrent attestation+slash -> 260 (never 760/250), slash-always-(-500), duplicate-slash skip, clamp, order-independence property test --- src/reputation/mod.rs | 1 + src/reputation/score.rs | 161 ++++++++++++++++++++++++++++++++ tests/reputation.rs | 3 + tests/reputation/score_tests.rs | 111 ++++++++++++++++++++++ 4 files changed, 276 insertions(+) create mode 100644 src/reputation/score.rs create mode 100644 tests/reputation/score_tests.rs diff --git a/src/reputation/mod.rs b/src/reputation/mod.rs index 1cdcd84..fc98c04 100644 --- a/src/reputation/mod.rs +++ b/src/reputation/mod.rs @@ -1,4 +1,5 @@ pub mod fixed_point; +pub mod score; pub mod score_engine; pub mod types; diff --git a/src/reputation/score.rs b/src/reputation/score.rs new file mode 100644 index 0000000..4647595 --- /dev/null +++ b/src/reputation/score.rs @@ -0,0 +1,161 @@ +//! Race-free node reputation scoring via an append-only event log (#3). +//! +//! ## The bug +//! +//! Reputation was a single `Map` scalar, and `update_reputation()` +//! did a read-compute-write: read the current score, add the adjustment +//! (`+10` reward, `-50` failure, `-500` slash), write it back. When a +//! successful attestation and a slashing event for the same node were processed +//! together, both read the same prior score and the last writer won — so a +//! `+10` write could clobber a concurrent `-500` slash, losing the slash +//! entirely (final `760` instead of `260`). +//! +//! ## The fix +//! +//! Store reputation as a **per-event log** (blueprint step 3): every adjustment +//! is *appended* as a [`ReputationEvent`] rather than folded into a shared +//! scalar, and the score is the (clamped) sum of all adjustments. Appends do +//! not read-modify-write a shared value, so two concurrent updates each record +//! their event and the score reflects both — independent of write order. +//! +//! A monotonic [`slash_count`](ReputationLedger::slash_count) plus +//! [`try_slash`](ReputationLedger::try_slash) (blueprint step 4) additionally +//! makes slashing idempotent: a duplicate slash carrying an already-consumed +//! sequence number is skipped rather than double-applied. + +extern crate alloc; +use alloc::collections::BTreeMap; +use alloc::vec::Vec; + +/// Node identifier. +pub type NodeId = u32; + +/// Inclusive reputation bounds. +pub const REPUTATION_MIN: i64 = -1000; +pub const REPUTATION_MAX: i64 = 1000; + +/// Adjustment for a successful attestation. +pub const ATTESTATION_REWARD: i64 = 10; +/// Adjustment for a failed attestation. +pub const ATTESTATION_FAILURE_PENALTY: i64 = -50; +/// Adjustment for a slashing event. +pub const SLASHING_PENALTY: i64 = -500; + +/// The origin of a reputation adjustment. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ReputationSource { + /// A baseline / starting score (carries an explicit amount). + Initial, + /// A successful attestation (`+10`). + AttestationReward, + /// A failed attestation (`-50`). + AttestationFailure, + /// A slashing event (`-500`). + Slashing, +} + +impl ReputationSource { + /// The fixed adjustment this source applies. `Initial` is `0` here — its + /// amount is supplied explicitly via + /// [`set_initial_score`](ReputationLedger::set_initial_score). + pub const fn adjustment(self) -> i64 { + match self { + ReputationSource::Initial => 0, + ReputationSource::AttestationReward => ATTESTATION_REWARD, + ReputationSource::AttestationFailure => ATTESTATION_FAILURE_PENALTY, + ReputationSource::Slashing => SLASHING_PENALTY, + } + } +} + +/// A single recorded reputation adjustment. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct ReputationEvent { + pub source: ReputationSource, + pub adjustment: i64, +} + +/// Append-only reputation ledger. +#[derive(Clone, Debug, Default)] +pub struct ReputationLedger { + events: BTreeMap>, + slash_counts: BTreeMap, +} + +impl ReputationLedger { + /// Create an empty ledger. + pub fn new() -> Self { + Self { + events: BTreeMap::new(), + slash_counts: BTreeMap::new(), + } + } + + /// Seed a node's starting score as an initial event, so the summed score + /// includes it. + pub fn set_initial_score(&mut self, node: NodeId, score: i64) { + self.push_event( + node, + ReputationEvent { + source: ReputationSource::Initial, + adjustment: score, + }, + ); + } + + /// Record a reputation adjustment for `node` from `source`, returning the + /// new (clamped) score. Slashing additionally bumps the node's monotonic + /// slash counter. + pub fn update_reputation(&mut self, node: NodeId, source: ReputationSource) -> i64 { + if source == ReputationSource::Slashing { + *self.slash_counts.entry(node).or_insert(0) += 1; + } + self.push_event( + node, + ReputationEvent { + source, + adjustment: source.adjustment(), + }, + ); + self.score(node) + } + + /// Apply a slashing event only if it is the expected next one. + /// + /// `expected_new_count` is the sequence number the caller believes this + /// slash produces. If another slash already advanced the counter (so this + /// is a duplicate), the call is skipped and returns `false`; otherwise the + /// slash is applied and returns `true`. + pub fn try_slash(&mut self, node: NodeId, expected_new_count: u64) -> bool { + if expected_new_count != self.slash_count(node) + 1 { + return false; + } + self.update_reputation(node, ReputationSource::Slashing); + true + } + + /// The node's current score: the sum of all recorded adjustments, clamped + /// to `[REPUTATION_MIN, REPUTATION_MAX]`. + pub fn score(&self, node: NodeId) -> i64 { + let sum: i64 = self + .events + .get(&node) + .map(|events| events.iter().map(|event| event.adjustment).sum()) + .unwrap_or(0); + sum.clamp(REPUTATION_MIN, REPUTATION_MAX) + } + + /// Number of slashing events recorded for `node`. + pub fn slash_count(&self, node: NodeId) -> u64 { + self.slash_counts.get(&node).copied().unwrap_or(0) + } + + /// Number of recorded events for `node` (including the initial seed). + pub fn event_count(&self, node: NodeId) -> usize { + self.events.get(&node).map(|events| events.len()).unwrap_or(0) + } + + fn push_event(&mut self, node: NodeId, event: ReputationEvent) { + self.events.entry(node).or_insert_with(Vec::new).push(event); + } +} diff --git a/tests/reputation.rs b/tests/reputation.rs index 5cf5ed5..ee25698 100644 --- a/tests/reputation.rs +++ b/tests/reputation.rs @@ -5,3 +5,6 @@ mod ema_divergence_test; #[path = "reputation/decay_accuracy_test.rs"] mod decay_accuracy_test; + +#[path = "reputation/score_tests.rs"] +mod score_tests; diff --git a/tests/reputation/score_tests.rs b/tests/reputation/score_tests.rs new file mode 100644 index 0000000..bb589ce --- /dev/null +++ b/tests/reputation/score_tests.rs @@ -0,0 +1,111 @@ +//! Concurrent reputation-update race tests (#3). + +use proptest::prelude::*; +use sorosusu_contracts::reputation::score::{ + ReputationLedger, ReputationSource, REPUTATION_MAX, +}; + +const NODE: u32 = 1; + +/// Blueprint step 5: a successful attestation and a slashing event for the same +/// node both apply, regardless of order — final score is `750 + 10 - 500 = 260`, +/// never the lost-update `760` or the dropped-reward `250`. +#[test] +fn concurrent_attestation_and_slash_apply_both() { + // Order A: reward then slash. + let mut a = ReputationLedger::new(); + a.set_initial_score(NODE, 750); + a.update_reputation(NODE, ReputationSource::AttestationReward); + a.update_reputation(NODE, ReputationSource::Slashing); + + // Order B: slash then reward. + let mut b = ReputationLedger::new(); + b.set_initial_score(NODE, 750); + b.update_reputation(NODE, ReputationSource::Slashing); + b.update_reputation(NODE, ReputationSource::AttestationReward); + + assert_eq!(a.score(NODE), 260); + assert_eq!(b.score(NODE), 260); + assert_ne!(a.score(NODE), 760); // reward did not clobber the slash + assert_ne!(a.score(NODE), 250); // slash did not drop the reward +} + +/// The slashing invariant: a slash always reduces the score by exactly 500. +#[test] +fn slash_always_reduces_by_500() { + let mut ledger = ReputationLedger::new(); + ledger.set_initial_score(NODE, 750); + let prior = ledger.score(NODE); + + ledger.update_reputation(NODE, ReputationSource::Slashing); + + assert_eq!(ledger.score(NODE), prior - 500); + assert_eq!(ledger.slash_count(NODE), 1); +} + +/// A duplicate slash carrying an already-consumed sequence number is skipped; +/// the genuine next slash still applies. +#[test] +fn duplicate_slash_is_skipped() { + let mut ledger = ReputationLedger::new(); + ledger.set_initial_score(NODE, 750); + + assert!(ledger.try_slash(NODE, 1)); + assert_eq!(ledger.slash_count(NODE), 1); + assert_eq!(ledger.score(NODE), 250); + + // Replay of the same slash (expected count 1 again) is rejected. + assert!(!ledger.try_slash(NODE, 1)); + assert_eq!(ledger.slash_count(NODE), 1); + assert_eq!(ledger.score(NODE), 250); + + // The real next slash applies. + assert!(ledger.try_slash(NODE, 2)); + assert_eq!(ledger.score(NODE), -250); + assert_eq!(ledger.slash_count(NODE), 2); +} + +/// The summed score is clamped to the reputation range. +#[test] +fn score_is_clamped_to_range() { + let mut ledger = ReputationLedger::new(); + ledger.set_initial_score(NODE, 900); + for _ in 0..50 { + ledger.update_reputation(NODE, ReputationSource::AttestationReward); // +500 total + } + assert_eq!(ledger.score(NODE), REPUTATION_MAX); // 1400 clamped to 1000 +} + +fn source_of(byte: u8) -> ReputationSource { + match byte % 3 { + 0 => ReputationSource::AttestationReward, + 1 => ReputationSource::AttestationFailure, + _ => ReputationSource::Slashing, + } +} + +proptest! { + /// The append-only log is order-independent: applying the same multiset of + /// adjustments in any order yields the same score (the race that dropped + /// updates cannot occur). + #[test] + fn prop_score_is_order_independent( + seq in prop::collection::vec(any::(), 0..40), + initial in -1000_i64..=1000, + ) { + let mut forward = ReputationLedger::new(); + forward.set_initial_score(NODE, initial); + for &b in &seq { + forward.update_reputation(NODE, source_of(b)); + } + + let mut reversed = ReputationLedger::new(); + reversed.set_initial_score(NODE, initial); + for &b in seq.iter().rev() { + reversed.update_reputation(NODE, source_of(b)); + } + + prop_assert_eq!(forward.score(NODE), reversed.score(NODE)); + prop_assert_eq!(forward.slash_count(NODE), reversed.slash_count(NODE)); + } +}