fix: address mutants found in harness

fuzz_props/src/tests.rs

@@ -1,3 +1,5 @@
+mod arbitrary_types_test;
+mod generators_test;
 mod invariants;
 mod replay_proptest;
 mod seed_gen;

fuzz_props/src/tests/arbitrary_types_test.rs

@@ -0,0 +1,158 @@
+//! Tests that detect mutations in `arbitrary_types.rs`.
+//!
+//! # Design rationale
+//!
+//! `arbitrary::Unstructured::fill_buffer` reads bytes from the **front** of the buffer
+//! and pads with zeros when the buffer is exhausted — it never returns an error.  As a
+//! result, the total number of items generated by `take(n)` always equals `n` regardless
+//! of buffer size.  This makes count-based tests the most reliable mutation detectors.
+//!
+//! For types that expose their length through public APIs we check the count directly.
+//! For `ArbPubTxMessage`, whose inner [`nssa::public_transaction::Message`] is opaque,
+//! we use the borsh-serialised size of a wrapping [`LeeTransaction::Public`] as a proxy.
+
+use crate::arbitrary_types::{
+    ArbHashableBlockData, ArbLeeTransaction, ArbPubTxMessage, ArbPublicTransaction, ArbWitnessSet,
+};
+use arbitrary::{Arbitrary, Unstructured};
+use common::transaction::LeeTransaction;
+
+#[test]
+fn arb_lee_transaction_zero_byte_selects_public() {
+    // fill_buffer reads from the front, so the first byte consumed = 0.
+    let buf = vec![0_u8; 4096];
+    let mut u = Unstructured::new(&buf);
+    let arb = ArbLeeTransaction::arbitrary(&mut u).expect("should succeed");
+    assert!(
+        matches!(arb.0, LeeTransaction::Public(_)),
+        "expected Public variant: with first byte=0 and `% 2`, arm 0 (Public) is selected"
+    );
+}
+
+#[test]
+fn arb_lee_transaction_byte4_selects_public() {
+    // Place 4 as the first byte (variant selector); rest are zeros.
+    let mut buf = vec![0_u8; 4096];
+    buf[0] = 4;
+    let mut u = Unstructured::new(&buf);
+    let arb = ArbLeeTransaction::arbitrary(&mut u).expect("should succeed");
+    assert!(
+        matches!(arb.0, LeeTransaction::Public(_)),
+        "expected Public variant: `4 % 2 = 0` \u{2192} arm 0; \
+         mutant `4 / 2 = 2` or `4 + 2 = 6` maps to `_` \u{2192} ProgramDeployment"
+    );
+}
+
+/// Generates from all-1 bytes: `1 % 2 = 1` -> `_` -> `ProgramDeployment`.
+#[test]
+fn arb_lee_transaction_one_byte_selects_program_deployment() {
+    let buf = vec![1_u8; 4096];
+    let mut u = Unstructured::new(&buf);
+    let arb = ArbLeeTransaction::arbitrary(&mut u).expect("should succeed");
+    assert!(
+        matches!(arb.0, LeeTransaction::ProgramDeployment(_)),
+        "expected ProgramDeployment variant with first byte=1"
+    );
+}
+
+/// Generates `ArbHashableBlockData` from all-255 bytes and asserts `transactions.len() <= 7`.
+#[test]
+fn arb_hashable_block_data_tx_count_bounded() {
+    let buf = vec![255_u8; 50_000];
+    let mut u = Unstructured::new(&buf);
+    let arb = ArbHashableBlockData::arbitrary(&mut u)
+        .expect("ArbHashableBlockData::arbitrary should succeed with a large all-255 buffer");
+    assert!(
+        arb.0.transactions.len() <= 7,
+        "expected at most 7 transactions (% 8), got {} \
+         (mutation: % replaced by / or + on line 248 of arbitrary_types.rs)",
+        arb.0.transactions.len()
+    );
+}
+
+/// Generates `ArbWitnessSet` from all-255 bytes and asserts pair count <= 3.
+#[test]
+fn arb_witness_set_pair_count_bounded() {
+    let buf = vec![255_u8; 50_000];
+    let mut u = Unstructured::new(&buf);
+    let arb = ArbWitnessSet::arbitrary(&mut u)
+        .expect("ArbWitnessSet::arbitrary should succeed with a large all-255 buffer");
+    let pair_count = arb.0.signatures_and_public_keys().len();
+    assert!(
+        pair_count <= 3,
+        "expected at most 3 witness pairs (% 4), got {pair_count} \
+         (mutation: % replaced by / or + on line 173 of arbitrary_types.rs)"
+    );
+}
+
+/// Checks the borsh-encoded size of a `LeeTransaction::Public` wrapping an
+/// `ArbPubTxMessage` generated from a buffer where the len-selector byte = 255.
+#[test]
+fn arb_pub_tx_message_account_count_bounded_via_borsh() {
+    // Bytes 0-31: zeros for program_id ([u32; 8] via fill_buffer reads 32 bytes).
+    // Byte 32: 255 — this is the len-selector byte.  255 % 8 = 7 (correct) vs 255 / 8 = 31 (mutant).
+    // Bytes 33+: zeros so Vec<u32> (instruction_data) produces 0 elements (last byte = 0).
+    let mut buf = vec![0_u8; 2000];
+    buf[32] = 255;
+
+    let mut u = Unstructured::new(&buf);
+    let msg =
+        ArbPubTxMessage::arbitrary(&mut u).expect("ArbPubTxMessage::arbitrary should succeed");
+
+    // Wrap in a real PublicTransaction to enable borsh serialisation.
+    let mut u_witness = Unstructured::new(&[0_u8; 10]);
+    let witness = ArbWitnessSet::arbitrary(&mut u_witness)
+        .expect("ArbWitnessSet::arbitrary should succeed with zero bytes (n=0)");
+
+    let tx = LeeTransaction::Public(nssa::public_transaction::PublicTransaction::new(
+        msg.0, witness.0,
+    ));
+    let borsh_bytes = borsh::to_vec(&tx).expect("borsh serialization should succeed");
+
+    // With 7 accounts the message borsh-encodes to ~380 bytes; the whole transaction to ~400 bytes.
+    // With 31 accounts the message encodes to ~1540 bytes.
+    // Using 800 as a conservative threshold clearly separates the two cases.
+    assert!(
+        borsh_bytes.len() < 800,
+        "borsh-encoded size {} bytes exceeds threshold: too many accounts in message \
+         (% 8 may have been replaced with / 8 or + 8 on line 144 of arbitrary_types.rs)",
+        borsh_bytes.len()
+    );
+}
+
+/// Additional check: with an all-zero buffer the `ArbPubTxMessage` generates a
+/// message with 0 accounts (`0 % 8 = 0`).  This verifies the zero case.
+#[test]
+fn arb_pub_tx_message_zero_accounts_with_zero_selector() {
+    // All zeros: program_id = all 0, len selector byte = 0.
+    // 0 % 8 = 0 (correct), 0 + 8 = 8 (+ mutant).
+    let buf = vec![0_u8; 500];
+    let mut u = Unstructured::new(&buf);
+    let msg =
+        ArbPubTxMessage::arbitrary(&mut u).expect("ArbPubTxMessage::arbitrary should succeed");
+
+    let mut u_witness = Unstructured::new(&[0_u8; 10]);
+    let witness = ArbWitnessSet::arbitrary(&mut u_witness).expect("witness should succeed");
+
+    let tx = LeeTransaction::Public(nssa::public_transaction::PublicTransaction::new(
+        msg.0, witness.0,
+    ));
+    let borsh_bytes = borsh::to_vec(&tx).expect("borsh serialization should succeed");
+
+    // With 0 accounts borsh size is minimal (~50 bytes for empty message + envelope).
+    // With 8 accounts (+ mutant) borsh size > 400 bytes.
+    assert!(
+        borsh_bytes.len() < 300,
+        "borsh-encoded size {} bytes too large for zero-account message \
+         (% 8 may have been replaced with + 8 on line 144 of arbitrary_types.rs)",
+        borsh_bytes.len()
+    );
+}
+
+/// Verifies that `ArbPublicTransaction::arbitrary` completes without error.
+#[test]
+fn arb_public_transaction_smoke() {
+    let buf = vec![0_u8; 4096];
+    let mut u = Unstructured::new(&buf);
+    let _ = ArbPublicTransaction::arbitrary(&mut u).expect("should succeed");
+}

fuzz_props/src/tests/generators_test.rs

@@ -0,0 +1,135 @@
+//! Tests that detect mutations in `generators.rs`.
+
+use arbitrary::Unstructured;
+use nssa::{AccountId, PrivateKey};
+
+use crate::generators::{
+    FuzzAccount, arb_fuzz_native_transfer, arbitrary_fuzz_state, signer_account_ids, test_accounts,
+};
+
+/// Verifies that `signer_account_ids` returns a **non-empty** list for a properly signed
+/// public transaction.
+#[test]
+fn signer_ids_nonempty_for_signed_public_tx() {
+    let accounts = test_accounts();
+    let (from_id, from_key) = &accounts[0];
+    let (to_id, _) = &accounts[1];
+
+    let tx = common::test_utils::create_transaction_native_token_transfer(
+        *from_id, 0, // nonce 0 — genesis nonce for the account
+        *to_id, 100, from_key,
+    );
+
+    let ids = signer_account_ids(&tx);
+    assert!(
+        !ids.is_empty(),
+        "signer_account_ids must return at least one ID for a signed public transaction \
+         (mutation: function body replaced with vec![])"
+    );
+}
+
+/// Verifies that the returned signer ID matches the account that actually signed the
+/// transaction — not a default/zeroed account ID.
+#[test]
+fn signer_ids_contains_the_signing_account() {
+    let accounts = test_accounts();
+    let (from_id, from_key) = &accounts[0];
+    let (to_id, _) = &accounts[1];
+
+    let tx = common::test_utils::create_transaction_native_token_transfer(
+        *from_id, 0, *to_id, 100, from_key,
+    );
+
+    let ids = signer_account_ids(&tx);
+    assert!(
+        ids.contains(from_id),
+        "signer_account_ids must contain the account ID of the private key that signed \
+         the transaction; got {ids:?} but expected it to contain {from_id:?}"
+    );
+}
+
+#[test]
+fn fuzz_state_never_empty() {
+    let buf = vec![0_u8; 1000];
+    let mut u = Unstructured::new(&buf);
+    let accounts = arbitrary_fuzz_state(&mut u).expect("should succeed");
+    assert!(
+        !accounts.is_empty(),
+        "arbitrary_fuzz_state must return at least 1 account (n = 1..=8); \
+         returned 0 \u{2014} mutation: `+ 1` replaced by `* 1` or `Ok(vec![])`"
+    );
+}
+
+#[test]
+fn fuzz_state_count_uses_modulo_not_div_or_add() {
+    // fill_buffer reads from the front; the first byte is the n-selector.
+    let mut buf = vec![0_u8; 1000];
+    buf[0] = 8; // selector byte: 8 % 8 = 0, +1 -> n=1  |  8 / 8 = 1, +1 -> n=2  |  8 + 8 = 16, +1 -> n=17
+    let mut u = Unstructured::new(&buf);
+    let accounts = arbitrary_fuzz_state(&mut u).expect("should succeed");
+    assert_eq!(
+        accounts.len(),
+        1,
+        "with selector byte=8: (8 % 8) + 1 = 1 account; \
+         mutation `% \u{2192} /` gives (8/8)+1=2; mutation `% \u{2192} +` gives (8+8)+1=17"
+    );
+}
+
+/// Verifies that each account's balance is <= `u128::MAX / 8`.
+#[test]
+fn fuzz_state_balances_bounded_by_max_div_8() {
+    let buf = vec![255_u8; 10_000];
+    let mut u = Unstructured::new(&buf);
+    // With correct division, this must NOT overflow (no panic).
+    let accounts = arbitrary_fuzz_state(&mut u)
+        .expect("should succeed \u{2014} no overflow with correct / 8 implementation");
+
+    let max_balance = u128::MAX / 8;
+    for acc in &accounts {
+        assert!(
+            acc.balance <= max_balance,
+            "account balance {} exceeds u128::MAX/8={} \u{2014} \
+             mutation: `/ 8` replaced by `* 8` (overflow) or `% 8`",
+            acc.balance,
+            max_balance
+        );
+    }
+
+    // Ensures the `% 8` mutation is caught: with u128::MAX bytes, correct `/` gives a
+    // large balance (u128::MAX/8 ~= 3.4e37), while `%` gives only 0-7.
+    let has_large_balance = accounts.iter().any(|a| a.balance > 7);
+    assert!(
+        has_large_balance,
+        "expected at least one account with balance > 7 \u{2014} \
+         mutation: `/ 8` replaced by `% 8` (balance capped at 7)"
+    );
+}
+
+#[test]
+fn native_transfer_index_uses_modulo_not_div_add() {
+    let accounts = vec![
+        FuzzAccount {
+            account_id: AccountId::new([1_u8; 32]),
+            balance: 1_000_000,
+            private_key: PrivateKey::try_new([1_u8; 32]).expect("scalar 1 is a valid private key"),
+        },
+        FuzzAccount {
+            account_id: AccountId::new([2_u8; 32]),
+            balance: 1_000_000,
+            private_key: PrivateKey::try_new([2_u8; 32]).expect("scalar 2 is a valid private key"),
+        },
+    ];
+
+    // All-0xFF bytes: the from_idx byte = 255, to_idx byte = 255.
+    // 255 % 2 = 1 (in-bounds), 255 / 2 = 127 (out-of-bounds), 255 + 2 = 257 (out-of-bounds).
+    let buf = vec![0xFF_u8; 500];
+    let mut u = Unstructured::new(&buf);
+
+    // With the mutated `/ 2` or `+ 2`, `accounts[127]` or `accounts[257]` panics.
+    let result = arb_fuzz_native_transfer(&mut u, &accounts);
+    assert!(
+        result.is_ok(),
+        "arb_fuzz_native_transfer should succeed with valid modulo-bounded indices; \
+         mutation: `% accounts.len()` replaced by `/ accounts.len()` or `+ accounts.len()`"
+    );
+}

fuzz_props/src/tests/invariants.rs

@@ -1,7 +1,10 @@
+use crate::generators::test_accounts;
 use crate::invariants::{
-    BalanceSnapshot, InvariantCtx, NonceSnapshot, assert_invariants,
-    assert_nonce_increment_correctness,
+    BalanceConservation, BalanceSnapshot, FailedTxNonceStability, InvariantCtx, NonceSnapshot,
+    ProtocolInvariant, StateIsolationOnFailure, assert_invariants,
+    assert_nonce_increment_correctness, assert_replay_rejection, assert_tx_execution_invariants,
 };
+use common::transaction::LeeTransaction;
 use nssa::V03State;
 use nssa_core::account::Nonce;
 
@@ -117,3 +120,180 @@ fn failed_tx_nonce_stability_catches_nonce_mutation() {
         "expected panic for nonce mutation on failure"
     );
 }
+
+/// Verifies that `BalanceSnapshot::total` returns the correct arithmetical sum.
+#[test]
+fn balance_snapshot_total_is_correct_sum() {
+    let mut map = std::collections::HashMap::new();
+    map.insert(nssa::AccountId::new([1_u8; 32]), 100_u128);
+    map.insert(nssa::AccountId::new([2_u8; 32]), 200_u128);
+    map.insert(nssa::AccountId::new([3_u8; 32]), 700_u128);
+    let snap = BalanceSnapshot(map);
+    assert_eq!(
+        snap.total(),
+        1000,
+        "BalanceSnapshot::total must sum all balances"
+    );
+}
+
+/// Ensures `total()` is non-zero when accounts have positive balances.
+///
+/// Together with `balance_snapshot_total_is_correct_sum`, this forms a pair that
+/// catches the `replace total with 0` mutation even when the expected sum is zero
+/// in other tests.
+#[test]
+fn balance_snapshot_total_nonzero_for_positive_balances() {
+    let mut map = std::collections::HashMap::new();
+    map.insert(nssa::AccountId::new([42_u8; 32]), 1_u128);
+    let snap = BalanceSnapshot(map);
+    assert_ne!(
+        snap.total(),
+        0,
+        "BalanceSnapshot::total must not return 0 when accounts have positive balances \
+         (mutation: replaced with literal 0)"
+    );
+}
+
+/// Verifies that `StateIsolationOnFailure::name` returns a non-empty, non-"xyzzy" string.
+#[test]
+fn state_isolation_name_is_nonempty_and_not_placeholder() {
+    let inv = StateIsolationOnFailure;
+    let name = inv.name();
+    assert!(
+        !name.is_empty(),
+        "StateIsolationOnFailure::name must not be empty"
+    );
+    assert_ne!(
+        name, "xyzzy",
+        "StateIsolationOnFailure::name must not be 'xyzzy'"
+    );
+    assert_eq!(name, "StateIsolationOnFailure");
+}
+
+/// Verifies that `BalanceConservation::name` returns a non-empty, non-"xyzzy" string.
+#[test]
+fn balance_conservation_name_is_nonempty_and_not_placeholder() {
+    let inv = BalanceConservation;
+    let name = inv.name();
+    assert!(
+        !name.is_empty(),
+        "BalanceConservation::name must not be empty"
+    );
+    assert_ne!(
+        name, "xyzzy",
+        "BalanceConservation::name must not be 'xyzzy'"
+    );
+    assert_eq!(name, "BalanceConservation");
+}
+
+/// Verifies that `FailedTxNonceStability::name` returns a non-empty, non-"xyzzy" string.
+#[test]
+fn failed_tx_nonce_stability_name_is_nonempty_and_not_placeholder() {
+    let inv = FailedTxNonceStability;
+    let name = inv.name();
+    assert!(
+        !name.is_empty(),
+        "FailedTxNonceStability::name must not be empty"
+    );
+    assert_ne!(
+        name, "xyzzy",
+        "FailedTxNonceStability::name must not be 'xyzzy'"
+    );
+    assert_eq!(name, "FailedTxNonceStability");
+}
+
+/// Verifies that `StateIsolationOnFailure::check` returns `Some` when execution failed and
+/// the balance in `state_after` differs from `balances_before`.
+#[test]
+fn state_isolation_check_detects_balance_change_on_failure() {
+    let acc_id = nssa::AccountId::new([1_u8; 32]);
+    // State has balance 100 for acc_id.
+    let state = V03State::new_with_genesis_accounts(&[(acc_id, 100)], vec![], 0);
+
+    // balances_before claims balance was 50, but state_after (== state) has 100.
+    let mut balances = std::collections::HashMap::new();
+    balances.insert(acc_id, 50_u128);
+
+    let ctx = InvariantCtx {
+        state_before: &state,
+        state_after: &state,
+        execution_succeeded: false, // failure → isolation invariant is active
+        balances_before: BalanceSnapshot(balances),
+        nonces_before: make_empty_nonce_snapshot(),
+    };
+
+    let inv = StateIsolationOnFailure;
+    let result = inv.check(&ctx);
+    assert!(
+        result.is_some(),
+        "StateIsolationOnFailure::check must return Some violation when \
+         state_after balance (100) differs from balances_before (50) on a failed tx \
+         (mutations: replace with None; delete !; replace != with ==)"
+    );
+}
+
+/// Verifies that `assert_replay_rejection` panics when the replayed transaction is
+/// accepted (i.e. NOT rejected — a genuine invariant violation).
+#[test]
+fn assert_replay_rejection_panics_when_replay_not_rejected() {
+    let accounts = test_accounts();
+    let (from_id, from_key) = &accounts[0];
+    let (to_id, _) = &accounts[1];
+
+    // Build a state that contains the sender account with nonce 0 and sufficient balance.
+    let genesis: Vec<(nssa::AccountId, u128)> = accounts
+        .iter()
+        .map(|(id, _)| (*id, 10_000_000_u128))
+        .collect();
+    let mut state = V03State::new_with_genesis_accounts(&genesis, vec![], 0);
+
+    // Create a valid, signed transaction with nonce 0 (the initial nonce in state).
+    let tx = common::test_utils::create_transaction_native_token_transfer(
+        *from_id, 0, *to_id, 100, from_key,
+    );
+
+    // We do NOT apply the tx first.  The state nonce is still 0, so calling
+    // execute_check_on_state would SUCCEED — making this a "successful replay".
+    // assert_replay_rejection is supposed to panic here (INVARIANT VIOLATION [ReplayRejection]).
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        assert_replay_rejection(tx, &mut state, 0, 0);
+    }));
+
+    assert!(
+        result.is_err(),
+        "assert_replay_rejection must panic when the replayed tx is accepted \
+         (mutation: replace function body with () \u{2014} no-op skips the check)"
+    );
+}
+
+/// Verifies that `assert_tx_execution_invariants` is NOT a no-op by providing a
+/// context that violates `StateIsolationOnFailure` and expecting a panic.
+#[test]
+fn assert_tx_execution_invariants_is_not_noop() {
+    let acc_id = nssa::AccountId::new([5_u8; 32]);
+    // Both state_before and state_after have the account at balance 100.
+    let state_before = V03State::new_with_genesis_accounts(&[(acc_id, 100)], vec![], 0);
+    let mut state_after = V03State::new_with_genesis_accounts(&[(acc_id, 100)], vec![], 0);
+
+    // Lie: claim balance was 50 before.  State_after shows 100.
+    // With execution_succeeded=false, StateIsolationOnFailure detects the discrepancy.
+    let mut balances = std::collections::HashMap::new();
+    balances.insert(acc_id, 50_u128);
+
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        assert_tx_execution_invariants(
+            &state_before,
+            &mut state_after,
+            BalanceSnapshot(balances),
+            make_empty_nonce_snapshot(),
+            Err::<LeeTransaction, &str>("simulated failure"),
+            (1, 1),
+        );
+    }));
+
+    assert!(
+        result.is_err(),
+        "assert_tx_execution_invariants must panic on a StateIsolationOnFailure violation \
+         (mutation: replace entire function body with () \u{2014} no-op skips all invariant checks)"
+    );
+}