fix(circuit_breaker): remove anonymous failure reporting and restore Sybil resistance (closes #313)

src/circuit_breaker.rs

@@ -228,27 +228,6 @@ pub fn record_failure(env: &Env, reporter: Address) -> Result<(), ContractError>
     Ok(())
 }
 
-/// Records an anonymous (reporter-less) failure increment for use by the
-/// batch-dispatch path, where the batch transaction itself is authenticated
-/// but no individual reporter address is carried in the `RecordFailure` unit
-/// variant.  Per-reporter accounting (cooldown, quota, quorum) is skipped;
-/// the global failure counter and the auto-pause check still apply.
-pub fn record_failure_anonymous(env: &Env) -> Result<(), ContractError> {
-    let count = failure_count(env).saturating_add(1);
-    env.storage().instance().set(&DataKey::FailureCount, &count);
-
-    // Quorum check uses the distinct-reporter count from prior attributed
-    // reports; an anonymous increment alone cannot satisfy the quorum.
-    let distinct_reporters = failure_reporters(env).len();
-    if count > FAILURE_THRESHOLD && distinct_reporters >= MIN_DISTINCT_REPORTERS && !is_paused(env)
-    {
-        env.storage().instance().set(&DataKey::Paused, &true);
-        events::emit_circuit_breaker_triggered(env, count);
-    }
-
-    Ok(())
-}
-
 /// Resets the failure counter, clears all per-reporter accounting and unpauses
 /// the contract. Authorization is enforced by the calling entry point;
 /// address validation is not repeated here.

src/contracts/proxy_entry/mod.rs

@@ -184,7 +184,9 @@ impl VeroContract {
                 BatchCall::TogglePause(admin) => Self::toggle_pause(env.clone(), admin)?,
                 BatchCall::Pause(admin) => Self::pause(env.clone(), admin)?,
                 BatchCall::Unpause(admin) => Self::unpause(env.clone(), admin)?,
-                BatchCall::RecordFailure => crate::circuit_breaker::record_failure_anonymous(&env)?,
+                BatchCall::RecordFailure(reporter) => {
+                    Self::record_failure(env.clone(), reporter)?
+                }
                 BatchCall::ResetCircuitBreaker(admin) => {
                     Self::reset_circuit_breaker(env.clone(), admin)?;
                 }

src/lib.rs

@@ -6,8 +6,8 @@
 
 #![no_std]
 #![warn(missing_docs)]
-
-mod circuit_breaker;
+/// Circuit breaker protection against cascading failures.
+pub mod circuit_breaker;
 // The Soroban contract surface (entrypoints, shared logic, RBAC, storage
 // layout and upgrades) lives under `contracts/`; see `contracts/mod.rs` for
 // the documented boundary. Domain primitives composed by that surface stay at

src/types.rs

@@ -172,7 +172,7 @@ pub enum BatchCall {
     TogglePause(Address),
     Pause(Address),
     Unpause(Address),
-    RecordFailure,
+    RecordFailure(Address),
     ResetCircuitBreaker(Address),
     EmergencyRecover(Address, Address, i128),
     /// Set multi-sig upgrade signers and threshold.
@@ -215,7 +215,7 @@ impl BatchCall {
             BatchCall::TogglePause(..) => Operation::TogglePause,
             BatchCall::Pause(..) => Operation::Pause,
             BatchCall::Unpause(..) => Operation::Unpause,
-            BatchCall::RecordFailure => Operation::RecordFailure,
+            BatchCall::RecordFailure(..) => Operation::RecordFailure,
             BatchCall::ResetCircuitBreaker(..) => Operation::ResetCircuitBreaker,
             BatchCall::EmergencyRecover(..) => Operation::EmergencyRecover,
             BatchCall::SetUpgradeSigners(..) => Operation::SetUpgradeSigners,

tests/circuit_breaker_dos.rs

@@ -393,20 +393,56 @@ fn test_zero_address_cannot_report() {
     assert_eq!(client.get_failure_count(), 0);
 }
 
-/// `BatchCall::RecordFailure` is a unit variant — it carries no address payload
-/// and increments the global failure counter via the anonymous batch path.
+/// `BatchCall::RecordFailure` carries an attributable reporter address payload
+/// and increments the global failure counter via the authenticated path.
 #[test]
 fn test_batch_execute_with_record_failure_variant() {
     let (env, _admin, client) = setup();
+    let reporter = Address::generate(&env);
 
-    let calls = soroban_sdk::vec![&env, vero_core_contracts::BatchCall::RecordFailure];
+    let calls = soroban_sdk::vec![
+        &env,
+        vero_core_contracts::BatchCall::RecordFailure(reporter.clone())
+    ];
 
+    env.mock_all_auths();
     let result = client.try_batch_execute(&calls);
     assert!(result.is_ok());
 
     assert_eq!(client.get_failure_count(), 1);
 }
 
+/// The invariant MAX_REPORTS_PER_REPORTER * (MIN_DISTINCT_REPORTERS - 1) <= FAILURE_THRESHOLD
+/// must strictly hold so that fewer than MIN_DISTINCT_REPORTERS cannot trip the circuit breaker.
+#[test]
+fn test_sybil_barrier_arithmetic_invariant() {
+    use vero_core_contracts::circuit_breaker::{
+        FAILURE_THRESHOLD, MAX_REPORTS_PER_REPORTER, MIN_DISTINCT_REPORTERS,
+    };
+    assert!(
+        MAX_REPORTS_PER_REPORTER * (MIN_DISTINCT_REPORTERS - 1) <= FAILURE_THRESHOLD,
+        "Invariant violated: sub-quorum coalition must not be able to reach FAILURE_THRESHOLD"
+    );
+}
+
+/// An unauthenticated caller on the batch path cannot increment FailureCount.
+#[test]
+fn test_batch_execute_record_failure_requires_auth() {
+    let (env, _admin, client) = setup();
+    let reporter = Address::generate(&env);
+
+    let calls = soroban_sdk::vec![
+        &env,
+        vero_core_contracts::BatchCall::RecordFailure(reporter.clone())
+    ];
+
+    // Without mocked auth the call must fail the signature check
+    env.set_auths(&[]);
+    let result = client.try_batch_execute(&calls);
+    assert!(result.is_err());
+    assert_eq!(client.get_failure_count(), 0);
+}
+
 /// When the contract is paused, `set_weight_threshold` is rejected with `ContractPaused`.
 #[test]
 fn test_set_weight_threshold_rejected_while_paused() {