fix: eliminate reputation update race via append-only event log (#3)

src/reputation/mod.rs

@@ -1,4 +1,5 @@
 pub mod fixed_point;
+pub mod score;
 pub mod score_engine;
 pub mod types;
 

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<NodeId, i64>` 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<NodeId, Vec<ReputationEvent>>,
+    slash_counts: BTreeMap<NodeId, u64>,
+}
+
+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);
+    }
+}

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;

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::<u8>(), 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));
+    }
+}