feat: add attestion verification negative tests

SECURITY_REVIEW.md

@@ -43,7 +43,7 @@ Out of scope:
 | Metric | Current value | Notes |
 | --- | ---: | --- |
 | Contract implementation size | 1,304 LOC | `contracts/src/contract.rs` |
-| Error enum size | 25 variants | `contracts/src/errors.rs` |
+| Error enum size | 29 variants | `contracts/src/errors.rs` |
 | Type definitions size | 105 LOC | `contracts/src/types.rs` |
 | TypeScript bindings size | 765 LOC | `bindings/src/index.ts` |
 | Test modules | 13 | All modules included by `contracts/src/tests/mod.rs` |
@@ -125,7 +125,8 @@ Soroban-specific risk considerations:
 - **Auth:** state-changing role/user operations call `require_auth()` on the expected `Address`; pause/unpause and windows are admin-gated, resolution is oracle-gated, user actions are user-gated.
 - **Storage:** persistent storage uses indexed per-user keys plus `RoundParticipants(round_id)` to avoid rewriting full maps on each bet, with legacy map fallbacks for migration.
 - **Arithmetic:** critical arithmetic uses checked operations. Payout paths increasingly route through `payout_add` / `payout_mul`, but one Precision indexed path still returns generic `Overflow` rather than `PayoutOverflow`.
-- **Oracle inputs:** `OraclePayload` enforces non-zero price, timestamp not in the future, 300-second freshness, and round binding against `Round.start_ledger`.
+- **Oracle inputs:** `OraclePayload` enforces non-zero price, timestamp not in the future, round binding against `Round.start_ledger`, and (optionally) an ed25519 attestation. When an attestation key is configured (`set_attestation_key`), every payload must carry a detached signature over a domain-separated message (`XELMA_ORACLE_ATTESTATION_V1` + XDR of network_id, contract_addr, round_id, price, timestamp, nonce); a missing/bad signature is rejected fail-closed — on-chain a failed `ed25519_verify` traps the transaction before any state change (`settlement.rs` attestation gate; see `contracts/src/tests/attestation.rs`). When no key is configured the gate is a no-op, so behaviour is identical to the pre-attestation ABI (Issue #263).
+- **Attestation threat note:** the attestation (Issue #263) binds the payload to this network, contract, round, price, timestamp, and nonce, and is meant for deployments where the off-chain signer (e.g. HSM) differs from the Soroban account that submits `resolve_round`. It does **not** vouch for price *correctness* — a compromised or faulty signer can still attest wrong prices (see SR-2026-04-004). `confidence` is deliberately **excluded** from the signed message (advisory metadata only), so confidence changes cannot invalidate a valid attestation. The negative test suite in `contracts/src/tests/attestation.rs` asserts invalid/missing/malformed/wrong-key/tampered-field attestations are rejected and leave the round resolvable, and that disabled mode ignores an attacker-supplied signature field entirely.
 - **Resource limits:** resolution is O(n) over participants. Precision rounds include an admin-configurable participant cap; operators still need benchmark evidence before raising it near upper bounds.
 
 ## Findings

contracts/src/errors.rs

@@ -105,4 +105,12 @@ pub enum ContractError {
     ClaimBatchTooLarge = 87,
     /// claim_many batch contains the same address more than once (Issue #277)
     DuplicateClaimAddress = 88,
+    /// Oracle heartbeat is unhealthy and blocks settlement in strict mode (Issue #264)
+    OracleHeartbeatUnhealthy = 89,
+    /// The caller lacks the required role for the requested action
+    AccessDenied = 90,
+    /// The dispute window for the round has expired
+    DisputeWindowExpired = 91,
+    /// Round settlement is locked against the attempted action
+    ClaimLocked = 92,
 }

contracts/src/tests/attestation.rs

@@ -4,7 +4,7 @@
 use crate::contract::{VirtualTokenContract, VirtualTokenContractClient};
 use crate::errors::ContractError;
 use crate::settlement::_build_attestation_message;
-use crate::types::OraclePayload;
+use crate::types::{DataKeyScoped, OraclePayload};
 use ed25519_dalek::{Signer, SigningKey};
 use rand::rngs::OsRng;
 use soroban_sdk::{
@@ -113,7 +113,6 @@ fn test_attestation_valid_signature_resolves_successfully() {
 }
 
 #[test]
-#[should_panic]
 fn test_attestation_wrong_key_signature_rejected() {
     let env = Env::default();
     let (client, contract_id, _admin, _oracle) = setup(&env);
@@ -134,11 +133,15 @@ fn test_attestation_wrong_key_signature_rejected() {
     let signature = sign_payload(&env, &wrong_key, &payload);
     payload.attestation = Some(signature);
 
-    client.resolve_round(&payload);
+    // A bad signature surfaces as a host-level trap (`Err(Err(_))`), not a
+    // contract error, and the round must remain untouched for a legitimate
+    // oracle to settle later.
+    let result = client.try_resolve_round(&payload);
+    assert!(matches!(result, Err(Err(_))));
+    assert!(client.get_active_round().is_some());
 }
 
 #[test]
-#[should_panic]
 fn test_attestation_tampered_price_after_signing_rejected() {
     let env = Env::default();
     let (client, contract_id, _admin, _oracle) = setup(&env);
@@ -159,7 +162,222 @@ fn test_attestation_tampered_price_after_signing_rejected() {
     payload.price = 2_000_0000;
     payload.attestation = Some(signature);
 
+    let result = client.try_resolve_round(&payload);
+    assert!(matches!(result, Err(Err(_))));
+    assert!(client.get_active_round().is_some());
+}
+
+#[test]
+fn test_attestation_malformed_signature_rejected() {
+    let env = Env::default();
+    let (client, contract_id, _admin, _oracle) = setup(&env);
+    let (pubkey, _signing_key) = generate_keypair(&env);
+
+    client.set_attestation_key(&Some(pubkey));
+    client.create_round(&1_000_0000, &None);
+
+    env.ledger().with_mut(|li| {
+        li.sequence_number = 12;
+    });
+
+    let mut payload = base_payload(&env, &contract_id, 0, 1, 1_000_0000);
+    // Well-formed length (64 bytes) but not a valid ed25519 signature.
+    payload.attestation = Some(BytesN::from_array(&env, &[0xABu8; 64]));
+
+    let result = client.try_resolve_round(&payload);
+    assert!(matches!(result, Err(Err(_))));
+    assert!(client.get_active_round().is_some());
+}
+
+#[test]
+fn test_attestation_tampered_timestamp_after_signing_rejected() {
+    let env = Env::default();
+    let (client, contract_id, _admin, _oracle) = setup(&env);
+    let (pubkey, signing_key) = generate_keypair(&env);
+
+    client.set_attestation_key(&Some(pubkey));
+    client.create_round(&1_000_0000, &None);
+
+    env.ledger().with_mut(|li| {
+        li.sequence_number = 12;
+    });
+
+    let mut payload = base_payload(&env, &contract_id, 0, 1, 1_000_0000);
+    let signature = sign_payload(&env, &signing_key, &payload);
+    // Timestamp is part of the signed message, so flipping it invalidates
+    // the signature at the attestation gate (before the window check ever
+    // runs) even though a far-future timestamp would also be rejected later.
+    payload.timestamp = 60_000_000;
+    payload.attestation = Some(signature);
+
+    let result = client.try_resolve_round(&payload);
+    assert!(matches!(result, Err(Err(_))));
+    assert!(client.get_active_round().is_some());
+}
+
+#[test]
+fn test_attestation_tampered_nonce_after_signing_rejected() {
+    let env = Env::default();
+    let (client, contract_id, _admin, _oracle) = setup(&env);
+    let (pubkey, signing_key) = generate_keypair(&env);
+
+    client.set_attestation_key(&Some(pubkey));
+    client.create_round(&1_000_0000, &None);
+
+    env.ledger().with_mut(|li| {
+        li.sequence_number = 12;
+    });
+
+    let mut payload = base_payload(&env, &contract_id, 0, 1, 1_000_0000);
+    let signature = sign_payload(&env, &signing_key, &payload);
+    // Nonce is part of the signed message; tampering with it means the
+    // signature no longer covers this payload.
+    payload.nonce = 99;
+    payload.attestation = Some(signature);
+
+    let result = client.try_resolve_round(&payload);
+    assert!(matches!(result, Err(Err(_))));
+    assert!(client.get_active_round().is_some());
+}
+
+#[test]
+fn test_attestation_disabled_mode_ignores_invalid_signature() {
+    let env = Env::default();
+    let (client, contract_id, _admin, _oracle) = setup(&env);
+    // No attestation key configured — attestation stays a no-op exactly as
+    // before Issue #263, and an attacker-supplied (garbage) signature field
+    // must not change behaviour.
+    assert_eq!(client.get_attestation_key(), None);
+    client.create_round(&1_000_0000, &None);
+
+    env.ledger().with_mut(|li| {
+        li.sequence_number = 12;
+    });
+
+    let mut payload = base_payload(&env, &contract_id, 0, 1, 1_000_0000);
+    payload.attestation = Some(BytesN::from_array(&env, &[0x00u8; 64]));
+
     client.resolve_round(&payload);
+    assert_eq!(client.get_active_round(), None);
+}
+
+#[test]
+fn test_attestation_valid_signature_does_not_bypass_nonce_replay_guard() {
+    let env = Env::default();
+    let (client, contract_id, _admin, _oracle) = setup(&env);
+    let (pubkey, signing_key) = generate_keypair(&env);
+
+    client.set_attestation_key(&Some(pubkey));
+    client.create_round(&1_000_0000, &None);
+
+    env.ledger().with_mut(|li| {
+        li.sequence_number = 12;
+    });
+
+    let mut payload = base_payload(&env, &contract_id, 0, 1, 1_000_0000);
+    let signature = sign_payload(&env, &signing_key, &payload);
+    payload.attestation = Some(signature);
+
+    // Pre-consumed nonce: the first round gets the monotonic
+    // `round.round_id = 1` (distinct from `start_ledger = 0`).
+    env.as_contract(&contract_id, || {
+        env.storage()
+            .persistent()
+            .set(&DataKeyScoped::ConsumedOracleNonce(1, 1), &true);
+    });
+
+    // Even with a perfectly valid signature, an already-used nonce is
+    // rejected before settlement can proceed.
+    let result = client.try_resolve_round(&payload);
+    assert_eq!(result, Err(Ok(ContractError::OracleNonceReused)));
+    assert!(client.get_active_round().is_some());
+
+    // A fresh nonce with the same key and a valid signature still settles —
+    // proving the reject above was nonce-specific, not a round-wide lock.
+    let mut fresh = base_payload(&env, &contract_id, 0, 2, 1_000_0000);
+    let fresh_sig = sign_payload(&env, &signing_key, &fresh);
+    fresh.attestation = Some(fresh_sig);
+
+    client.resolve_round(&fresh);
+    assert_eq!(client.get_active_round(), None);
+}
+
+#[test]
+fn test_attestation_confidence_changes_do_not_invalidate_signature() {
+    let env = Env::default();
+    let (client, contract_id, _admin, _oracle) = setup(&env);
+    let (pubkey, signing_key) = generate_keypair(&env);
+
+    client.set_attestation_key(&Some(pubkey));
+    client.create_round(&1_000_0000, &None);
+
+    env.ledger().with_mut(|li| {
+        li.sequence_number = 12;
+    });
+
+    // Sign the payload before the confidence value is attached. `confidence`
+    // is deliberately excluded from the signed message (advisory metadata),
+    // so attaching or updating it afterwards must not cause a rejection.
+    let mut payload = base_payload(&env, &contract_id, 0, 1, 1_000_0000);
+    let signature = sign_payload(&env, &signing_key, &payload);
+    payload.confidence = Some(5_000);
+    payload.attestation = Some(signature);
+
+    client.resolve_round(&payload);
+    assert_eq!(client.get_active_round(), None);
+}
+
+#[test]
+fn test_attestation_message_is_domain_separated() {
+    let env = Env::default();
+    let (_client, contract_id, _admin, _oracle) = setup(&env);
+
+    let payload = base_payload(&env, &contract_id, 0, 1, 1_000_0000);
+    let message = _build_attestation_message(&env, &payload);
+
+    // Messages carry a fixed domain-separation prefix so signatures can
+    // never be replayed against a different message type with identical
+    // XDR encoding.
+    let slice: std::vec::Vec<u8> = message.iter().collect();
+    assert!(slice.starts_with(b"XELMA_ORACLE_ATTESTATION_V1"));
+
+    // Same payload -> byte-for-byte identical message (deterministic signing
+    // target).
+    assert_eq!(message, _build_attestation_message(&env, &payload));
+
+    // Every bound field must alter the message; `confidence` and
+    // `attestation` must not.
+    let mut field_tamper = payload.clone();
+    field_tamper.price += 1;
+    assert_ne!(message, _build_attestation_message(&env, &field_tamper));
+
+    let mut field_tamper = payload.clone();
+    field_tamper.timestamp += 1;
+    assert_ne!(message, _build_attestation_message(&env, &field_tamper));
+
+    let mut field_tamper = payload.clone();
+    field_tamper.round_id += 1;
+    assert_ne!(message, _build_attestation_message(&env, &field_tamper));
+
+    let mut field_tamper = payload.clone();
+    field_tamper.nonce += 1;
+    assert_ne!(message, _build_attestation_message(&env, &field_tamper));
+
+    let mut field_tamper = payload.clone();
+    field_tamper.network_id = BytesN::from_array(&env, &[0xFFu8; 32]);
+    assert_ne!(message, _build_attestation_message(&env, &field_tamper));
+
+    let mut field_tamper = payload.clone();
+    field_tamper.contract_addr = Address::generate(&env);
+    assert_ne!(message, _build_attestation_message(&env, &field_tamper));
+
+    let mut unsigned_tamper = payload.clone();
+    unsigned_tamper.confidence = Some(5_000);
+    assert_eq!(message, _build_attestation_message(&env, &unsigned_tamper));
+
+    let mut unsigned_tamper = payload.clone();
+    unsigned_tamper.attestation = Some(BytesN::from_array(&env, &[0x01u8; 64]));
+    assert_eq!(message, _build_attestation_message(&env, &unsigned_tamper));
 }
 
 #[test]

docs/ORACLE_OPERATOR_RUNBOOK.md

@@ -53,6 +53,17 @@ field is validated in order; a failure at any step rejects the entire submission
 | `nonce`         | `u64`       | Yes      | Per-round replay protection. Must be unique per round. |
 | `network_id`    | `BytesN<32>`| Yes      | SHA-256 hash of the network passphrase. Prevents cross-network replay. |
 | `contract_addr` | `Address`   | Yes      | The contract this payload targets. Prevents cross-contract replay. |
+| `attestation`   | `BytesN<64>` | Only when a key is configured | Detached ed25519 signature over the domain-separated message (see attestation threat note below). Rejected if missing or invalid when an attestation key is set. |
+| `confidence`    | `Option<u32>` | No      | Advisory belief strength in basis points. **Not** covered by the attestation signature. |
+
+> **Attestation threat note (Issue #263):** when `set_attestation_key` is configured, the
+> oracle signs a domain-separated message (`XELMA_ORACLE_ATTESTATION_V1` + the XDR encoding
+> of network_id, contract_addr, round_id, price, timestamp, nonce) with a key that may differ
+> from the submitting Soroban account. Verification is fail-closed: an invalid signature
+> aborts the transaction before any state change, and the round is left resolvable. When no
+> key is configured this gate is a no-op, preserving pre-#263 behaviour. The signature vouches
+> for *binding* (correct network/contract/round/price/time/nonce), not for the correctness of
+> the price itself — the oracle remains a trusted signer for price accuracy (§1).
 
 ### Validation order (`settlement.rs`)