zebra: 5-MOAD scan; zebra-0001 CWE-407 ZIP-317 dep check O(T^2) Vec scan, 500x at T=1000

defects/zebra-0001/TICKET.md

@@ -0,0 +1,65 @@
+# zebra-0001: ZIP-317 block template `has_direct_dependencies` O(T×S) Vec scan
+
+**MOAD:** MOAD-0001 (CWE-407)
+**Severity:** LOW-MEDIUM
+**Speedup:** ~500x at T=1000 (pathological chain dependency topology)
+**Language:** Rust
+**File:** zebra-rpc/src/methods/types/get_block_template/zip317.rs:228
+
+## Description
+
+`has_direct_dependencies()` is called for each dependent transaction during
+block template construction (ZIP-317). It checks whether all of a candidate
+transaction's dependencies are already in `selected_txs` by iterating the
+entire `Vec<SelectedMempoolTx>` and counting how many entries appear in a
+`HashSet<transaction::Hash>`.
+
+```rust
+fn has_direct_dependencies(
+    candidate_tx_deps: Option<&HashSet<transaction::Hash>>,
+    selected_txs: &Vec<SelectedMempoolTx>,
+) -> bool {
+    // ...
+    let mut num_available_deps = 0;
+    for tx in selected_txs {           // O(S) per call
+        if deps.contains(&tx.transaction.id.mined_id()) {  // O(1) HashSet lookup
+            num_available_deps += 1;
+        }
+        if num_available_deps == deps.len() {
+            return true;
+        }
+    }
+    false
+}
+```
+
+In `checked_add_transaction_weighted_random`, each time a transaction is
+selected and its dependents are evaluated, `has_direct_dependencies` iterates
+all `S` already-selected transactions. As `S` grows with each selected tx,
+the total work across a block template build is O(T×S_avg) = O(T²) for T
+transactions in a deep dependency chain.
+
+A more efficient approach: maintain a `HashSet<transaction::Hash>` of selected
+tx IDs alongside the `Vec`. Then `has_direct_dependencies` reduces to O(D)
+per check (D = number of direct dependencies, typically 1-3), giving O(T×D)
+total.
+
+## Complexity
+
+- Current: O(T²) worst case for T txs in chain dependency topology
+- Fixed: O(T×D) where D = average direct dependency count (typically 1-3)
+- Speedup: ~T/D ≈ 333x at T=1000, D=3
+
+## Context
+
+Block template construction runs once per block (~75 seconds on Zcash mainnet).
+Zcash block size limits bound T to around 1000 transactions in practice.
+This is not a hot path, so impact is LOW in typical operation, but a miner
+constructing blocks from a deep-chained mempool would observe the worst case.
+
+## Fix
+
+Pass a `HashSet<transaction::Hash>` of already-selected tx IDs alongside
+`selected_txs`, built incrementally as transactions are added. Replace the
+Vec scan in `has_direct_dependencies` with a single HashSet intersection
+check: all deps must be present in `selected_tx_ids`.

defects/zebra-0001/patch/zebra-0001-zip317-dep-check-hashset.patch

@@ -0,0 +1,121 @@
+# UNDF: UNDF-PENDING
+--- a/zebra-rpc/src/methods/types/get_block_template/zip317.rs
++++ b/zebra-rpc/src/methods/types/get_block_template/zip317.rs
+@@ -91,9 +91,11 @@ pub fn select_mempool_transactions(
+     // Setup the transaction lists.
+     let (mut conventional_fee_txs, mut low_fee_txs): (Vec<_>, Vec<_>) = independent_mempool_txs
+         .into_values()
+         .partition(VerifiedUnminedTx::pays_conventional_fee);
+
+     let mut selected_txs = Vec::new();
++    // HashSet of selected tx IDs for O(1) dependency satisfaction checks.
++    let mut selected_tx_ids: HashSet<transaction::Hash> = HashSet::new();
+
+     // Set up limit tracking
+     let mut remaining_block_bytes: usize = MAX_BLOCK_BYTES.try_into().expect("fits in memory");
+@@ -108,7 +110,7 @@ pub fn select_mempool_transactions(
+     while let Some(tx_weights) = conventional_fee_tx_weights {
+         conventional_fee_tx_weights = checked_add_transaction_weighted_random(
+             &mut conventional_fee_txs,
+             &mut dependent_mempool_txs,
+             tx_weights,
+             &mut selected_txs,
++            &mut selected_tx_ids,
+             &mempool_tx_deps,
+             &mut remaining_block_bytes,
+             &mut remaining_block_sigops,
+@@ -122,7 +124,7 @@ pub fn select_mempool_transactions(
+     while let Some(tx_weights) = low_fee_tx_weights {
+         low_fee_tx_weights = checked_add_transaction_weighted_random(
+             &mut low_fee_txs,
+             &mut dependent_mempool_txs,
+             tx_weights,
+             &mut selected_txs,
++            &mut selected_tx_ids,
+             &mempool_tx_deps,
+             &mut remaining_block_bytes,
+             &mut remaining_block_sigops,
+@@ -210,20 +212,16 @@ fn dependencies_depth(
+ /// Checks if every item in `candidate_tx_deps` is present in `selected_txs`.
+ ///
+-/// Requires items in `selected_txs` to be unique to work correctly.
+-fn has_direct_dependencies(
++/// Uses a pre-built `selected_tx_ids` HashSet for O(D) membership checks
++/// instead of scanning the entire Vec<SelectedMempoolTx> in O(S) per call.
++fn has_direct_dependencies(
+     candidate_tx_deps: Option<&HashSet<transaction::Hash>>,
+-    selected_txs: &Vec<SelectedMempoolTx>,
++    selected_tx_ids: &HashSet<transaction::Hash>,
+ ) -> bool {
+     let Some(deps) = candidate_tx_deps else {
+         return true;
+     };
+
+-    if selected_txs.len() < deps.len() {
+-        return false;
+-    }
+-
+-    let mut num_available_deps = 0;
+-    for tx in selected_txs {
+-        #[cfg(test)]
+-        let (_, tx) = tx;
+-        if deps.contains(&tx.transaction.id.mined_id()) {
+-            num_available_deps += 1;
+-        } else {
+-            continue;
+-        }
+-
+-        if num_available_deps == deps.len() {
+-            return true;
+-        }
+-    }
+-
+-    false
++    // O(D) where D = number of direct dependencies (typically 1-3).
++    // Previously O(S) where S = number of already-selected transactions.
++    deps.iter().all(|dep_id| selected_tx_ids.contains(dep_id))
+ }
+
+ /// Chooses a random transaction from `txs` using the weighted index `tx_weights`,
+@@ -275,6 +273,7 @@ fn checked_add_transaction_weighted_random(
+     candidate_txs: &mut Vec<VerifiedUnminedTx>,
+     dependent_txs: &mut HashMap<transaction::Hash, VerifiedUnminedTx>,
+     tx_weights: WeightedIndex<f32>,
+     selected_txs: &mut Vec<SelectedMempoolTx>,
++    selected_tx_ids: &mut HashSet<transaction::Hash>,
+     mempool_tx_deps: &TransactionDependencies,
+     remaining_block_bytes: &mut usize,
+     remaining_block_sigops: &mut u32,
+@@ -298,11 +297,14 @@ fn checked_add_transaction_weighted_random(
+         "all candidate transactions should be independent"
+     );
+
++    selected_tx_ids.insert(*selected_tx_id);
++
+     #[cfg(not(test))]
+     selected_txs.push(candidate_tx);
+
+     #[cfg(test)]
+     selected_txs.push((0, candidate_tx));
+
+     // Try adding any dependent transactions if all of their dependencies have been selected.
+
+     let mut current_level_dependents = mempool_tx_deps.direct_dependents(selected_tx_id);
+@@ -314,7 +316,7 @@ fn checked_add_transaction_weighted_random(
+             // the selected txs, which come from the mempool. If the tx also spends in-chain outputs, it won't
+             // be added. This behavior is not specified by consensus rules and can be changed at any time,
+             // meaning that such txs could be added.
+-            if has_direct_dependencies(tx_dependencies.get(dependent_tx_id), selected_txs) {
++            if has_direct_dependencies(tx_dependencies.get(dependent_tx_id), selected_tx_ids) {
+                 let Some(candidate_tx) = dependent_txs.remove(dependent_tx_id) else {
+                     continue;
+                 };
+@@ -337,6 +339,8 @@ fn checked_add_transaction_weighted_random(
+                     continue;
+                 }
+
++                selected_tx_ids.insert(*dependent_tx_id);
++
+                 #[cfg(not(test))]
+                 selected_txs.push(candidate_tx);
+

defects/zebra-0001/test/test_zip317_dep_check.py

@@ -0,0 +1,93 @@
+"""
+zebra-0001: ZIP-317 has_direct_dependencies O(T*S) -> O(T*D) benchmark.
+
+Simulates the dependency satisfaction check in block template construction.
+Measures the cost of the Vec-scan approach vs the HashSet approach.
+"""
+
+import time
+import random
+import hashlib
+
+def make_tx_id(n):
+    return hashlib.sha256(str(n).encode()).digest()
+
+def simulate_vec_scan(tx_chain_len):
+    """Simulate the buggy Vec scan approach: O(S) per dep check."""
+    selected_txs = []    # list of tx_ids (simulating Vec<SelectedMempoolTx>)
+    ops = 0
+
+    for i in range(tx_chain_len):
+        tx_id = make_tx_id(i)
+        if i > 0:
+            # has_direct_dependencies: iterate ALL selected_txs to find 1 dep
+            dep_id = make_tx_id(i - 1)
+            for sel in selected_txs:  # O(S)
+                ops += 1
+                if sel == dep_id:
+                    break
+        selected_txs.append(tx_id)
+
+    return ops
+
+def simulate_hashset_check(tx_chain_len):
+    """Simulate the fixed HashSet approach: O(D) per dep check."""
+    selected_tx_ids = set()   # HashSet<tx_id>
+    ops = 0
+
+    for i in range(tx_chain_len):
+        tx_id = make_tx_id(i)
+        if i > 0:
+            # has_direct_dependencies: check each dep against HashSet - O(1)
+            dep_id = make_tx_id(i - 1)
+            ops += 1  # single O(1) set lookup
+            _ = dep_id in selected_tx_ids
+        selected_tx_ids.add(tx_id)
+
+    return ops
+
+def benchmark(tx_chain_len, label):
+    t0 = time.perf_counter()
+    vec_ops = simulate_vec_scan(tx_chain_len)
+    t1 = time.perf_counter()
+    set_ops = simulate_hashset_check(tx_chain_len)
+    t2 = time.perf_counter()
+
+    vec_time = t1 - t0
+    set_time = t2 - t1
+    ratio = vec_ops / max(set_ops, 1)
+
+    print(f"T={tx_chain_len:6d} ({label}): "
+          f"vec_ops={vec_ops:9,d}  set_ops={set_ops:6,d}  "
+          f"ratio={ratio:.1f}x  "
+          f"vec_time={vec_time*1000:.2f}ms  set_time={set_time*1000:.2f}ms")
+    return ratio
+
+def test_zip317_dep_check():
+    print("\nzebra-0001: ZIP-317 dep satisfaction check benchmark")
+    print("=" * 70)
+
+    ratios = []
+    for t in [10, 50, 100, 500, 1000]:
+        r = benchmark(t, "chain deps")
+        ratios.append((t, r))
+
+    print()
+    print("Expected O(T²) for Vec scan vs O(T) for HashSet check.")
+    print()
+
+    # Assert ratios scale with T (roughly T/2 ratio in chain topology)
+    t10, r10 = ratios[0]
+    t1000, r1000 = ratios[-1]
+
+    assert r1000 > r10 * 5, (
+        f"Ratio should grow significantly with T: "
+        f"r@T={t10} = {r10:.1f}x, r@T={t1000} = {r1000:.1f}x"
+    )
+    assert r1000 > 100, (
+        f"Expected >100x op-count ratio at T=1000, got {r1000:.1f}x"
+    )
+    print(f"PASS: op-count ratio at T=1000 is {r1000:.1f}x (expected >100x)")
+
+if __name__ == "__main__":
+    test_zip317_dep_check()