packages/rs-dpp/src/address_funds/witness.rs (1)

408-512: Comprehensive bincode test coverage; consider adding serde boundary test.

The tests thoroughly cover the bincode deserialization paths with good boundary testing (exact limit succeeds, limit+1 fails). The intentional asymmetry where encoding succeeds but decoding rejects over-limit signatures is correctly verified.

However, the serde Deserialize path (lines 211-217) is not explicitly tested for limit enforcement. While the existing test_p2sh_serde test covers basic round-trip with a small signature count, adding a boundary test for the serde path would ensure parity with the bincode tests.

🧪 Optional: Add serde boundary test

#[cfg(feature = "state-transition-serde-conversion")]
#[test]
fn test_p2sh_serde_rejects_excessive_signatures() {
    // Manually construct JSON with MAX_P2SH_SIGNATURES + 1 signatures
    let signatures: Vec<Vec<u8>> = (0..MAX_P2SH_SIGNATURES + 1)
        .map(|_| vec![0x30, 0x44, 0x02, 0x20])
        .collect();
    
    let json = serde_json::json!({
        "type": "p2sh",
        "signatures": signatures,
        "redeemScript": [0x52, 0xae]
    });
    
    let result: Result<AddressWitness, _> = serde_json::from_value(json);
    assert!(
        result.is_err(),
        "P2SH witness with {} signatures should be rejected during serde deserialization",
        MAX_P2SH_SIGNATURES + 1,
    );
}

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_top_up_from_addresses/tests.rs (1)

3430-3492: Test may not verify the intended overflow behavior.

The test test_transformer_subtraction_uses_checked_arithmetic creates addresses using create_platform_address but never calls setup_address_with_balance to register them on the platform. The transition will likely be rejected with AddressDoesNotExistError during state validation rather than an overflow error during structure validation.

While the broad assertion !matches!(...SuccessfulExecution...) will pass, it doesn't verify that the overflow path is actually being tested. Consider either:

1. Setting up the addresses with balances matching the inputs to allow the transition to reach overflow validation
2. Adding a more specific assertion that checks for OverflowError

♻️ Suggested improvement to test the actual overflow path

If the intent is to verify overflow detection in structure validation, the addresses need to exist:

             // Two inputs that sum to > u64::MAX
+            let mut signer = TestAddressSigner::new();
+            let addr1 = signer.add_p2pkh([1u8; 32]);
+            let addr2 = signer.add_p2pkh([2u8; 32]);
+            // Store balances at max storable value (i64::MAX fits in sum tree)
+            setup_address_with_balance(&mut platform, addr1, 0, i64::MAX as u64);
+            setup_address_with_balance(&mut platform, addr2, 0, i64::MAX as u64);
+
             let mut inputs = BTreeMap::new();
-            inputs.insert(create_platform_address(1), (0 as AddressNonce, u64::MAX));
-            inputs.insert(create_platform_address(2), (0 as AddressNonce, u64::MAX));
+            inputs.insert(addr1, (1 as AddressNonce, u64::MAX));
+            inputs.insert(addr2, (1 as AddressNonce, u64::MAX));

Then update the assertion to be more specific:

-            assert!(
-                !matches!(
-                    processing_result.execution_results().as_slice(),
-                    [StateTransitionExecutionResult::SuccessfulExecution { .. }]
-                ),
+            // Expect either OverflowError (structure) or AddressNotEnoughFundsError (state)
+            // depending on validation order
+            assert_matches!(
+                processing_result.execution_results().as_slice(),
+                [StateTransitionExecutionResult::UnpaidConsensusError(_)]

packages/rs-dpp/src/util/is_non_zero_fibonacci_number.rs (1)

42-51: Consider updating the test documentation comment.

The comment describes the previous floating-point implementation ("casts u64 to f64 and checks sqrt().fract() == 0.0") that was vulnerable to precision issues. Since the implementation now uses integer Newton's method, this documentation could confuse future readers into thinking the current code has this vulnerability.

Consider updating to clarify this documents the fixed behavior and why the test should pass.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_funding_from_asset_lock/tests.rs (1)

9276-9292: Use the actual fee result instead of max_reasonable_fee.

The fixed 0.01‑Dash threshold can go stale if fee schedules change. Use the execution fee_result.total_base_fee() to assert exact conservation. Apply the same pattern in both conservation tests.

♻️ Example (M5 test) using fee_result

-            assert_matches!(
-                processing_result.execution_results().as_slice(),
-                [StateTransitionExecutionResult::SuccessfulExecution { .. }],
-                "Pure asset lock funding should succeed"
-            );
+            let fee_result = assert_matches!(
+                processing_result.execution_results().as_slice(),
+                [StateTransitionExecutionResult::SuccessfulExecution { fee_result, .. }] => fee_result.clone(),
+                "Pure asset lock funding should succeed"
+            );
@@
-            let max_reasonable_fee = dash_to_credits!(0.01);
-
-            assert!(
-                total_in_addresses >= asset_lock_credits - max_reasonable_fee,
-                "AUDIT M5: Credits not conserved. Asset lock provided {} credits, \
-                but output addresses only received {} credits total. \
-                Expected at least {} (asset_lock - max_fee {}). \
-                There is no explicit conservation-of-credits assertion in the processing code.",
-                asset_lock_credits,
-                total_in_addresses,
-                asset_lock_credits - max_reasonable_fee,
-                max_reasonable_fee,
-            );
-
-            assert!(
-                total_in_addresses <= asset_lock_credits,
-                "AUDIT M5: Output total {} exceeds asset lock credits {}. \
-                Credits were created from nothing!",
-                total_in_addresses,
-                asset_lock_credits,
-            );
+            let total_fee = fee_result.total_base_fee();
+            assert_eq!(
+                total_in_addresses + total_fee,
+                asset_lock_credits,
+                "Credits should conserve: outputs + fees must equal asset lock credits"
+            );

Also applies to: 9611-9624

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create_from_addresses/tests.rs (2)

8842-8848: Consider asserting the overflow guard here.

The test sets a near‑max balance but never executes the transition, so regressions in overflow handling wouldn’t be caught. Suggest running the transition and asserting the expected overflow/consensus error (or mark as TODO/ignored).

---

11377-11385: Make the fee‑drain condition explicit.

This assumes fees exceed 1000 credits so the first input is fully drained; if fees drop below that, the test won’t exercise the index‑shift bug. Consider computing the fee (or asserting the first entry is removed) before checking second_final.

packages/rs-dpp/src/util/is_non_zero_fibonacci_number.rs (1)

42-51: Test comments describe old floating-point implementation in present tense.

The comments at lines 44-46 state that is_perfect_square "casts u64 to f64 and checks sqrt().fract() == 0.0", but the actual implementation now uses integer Newton's method. This could confuse future maintainers.

Consider updating the comments to past tense to clarify these describe the previous implementation that was fixed:

📝 Suggested documentation update

     /// AUDIT M6: Floating-point imprecision in is_perfect_square for large values.
     ///
-    /// `is_perfect_square` casts u64 to f64 and checks `sqrt().fract() == 0.0`.
-    /// f64 has only 52 bits of mantissa, so for values above 2^52, precision is lost.
-    /// This means `is_perfect_square` can return incorrect results for large numbers.
+    /// The previous `is_perfect_square` implementation cast u64 to f64 and checked
+    /// `sqrt().fract() == 0.0`. f64 has only 52 bits of mantissa, so for values
+    /// above 2^52, precision was lost, causing incorrect results for large numbers.
+    /// This was fixed by replacing the floating-point approach with integer Newton's method.
     ///
-    /// Currently not exploitable for `core_fee_per_byte` (u32), but the function
-    /// accepts u64 and is technically unsound for large inputs.
-    ///
-    /// Location: rs-dpp/src/util/is_non_zero_fibonacci_number.rs:1-3
+    /// This test verifies the integer implementation doesn't have false positives.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_credit_withdrawal/tests.rs (1)

3923-3988: Derive large_balance from max_withdrawal_amount to keep the test future-proof.

large_balance is hard-coded to 600 DASH while the withdrawal amount is sourced from platform_version.system_limits.max_withdrawal_amount. If the max changes, this test can fail due to insufficient balance even though the intent is to validate the max withdrawal boundary. Consider tying the balance to the max plus a small fee buffer.

Suggested update

-            // Balance large enough to cover max withdrawal + fees
-            let large_balance = dash_to_credits!(600.0);
+            let max_withdrawal = platform_version.system_limits.max_withdrawal_amount;
+            // Balance large enough to cover max withdrawal + fees
+            let large_balance = max_withdrawal.saturating_add(dash_to_credits!(10.0));
             setup_address_with_balance(&mut platform, input_address, 0, large_balance);
@@
-            // Withdraw the max allowed amount (500 Dash = 50_000_000_000_000 credits)
-            let max_withdrawal = platform_version.system_limits.max_withdrawal_amount;
+            // Withdraw the max allowed amount
             inputs.insert(input_address, (1 as AddressNonce, max_withdrawal));