# Subtensor Security Audit — Final Report

> **Date:** 2026-02-21
> **Total unique findings:** 35 (36 filed; issue-025 is a duplicate of issue-005)
> **Confirmed:** 30 | **Challenged / False-positive:** 5

---

## Table of Contents

- [Summary](#summary)
- [Critical](#critical)
  - [ISSUE-029 — clear_identity has no authorization check](#issue-029)
- [High](#high)
  - [ISSUE-001 — CurrentLiquidity and token amounts wrong on auto-remove in modify_position](#issue-001)
  - [ISSUE-005 — AEAD decrypt failure silently drops submissions without on-chain notification](#issue-005)
  - [ISSUE-010 — Dispatch precompile bypasses PrecompileEnable admin check](#issue-010)
  - [ISSUE-030 — Deposit released to caller instead of original depositor, permanently locking funds](#issue-030)
  - [ISSUE-031 — Identity update by new owner inherits wrong deposit, causing accounting mismatch](#issue-031)
  - [ISSUE-032 — Preimage never dropped after successful finalize — permanent deposit lock](#issue-032)
  - [ISSUE-033 — MaxContributors check incorrectly rejects existing contributors trying to top up](#issue-033)
- [Medium](#medium)
  - [ISSUE-002 — modify_position missing reserve tracking](#issue-002)
  - [ISSUE-003 — disable_lp hardcodes subnet range 1..=128, missing subnets > 128](#issue-003)
  - [ISSUE-006 — Unbounded Submissions::iter() in on_initialize enables DoS via submission spam](#issue-006)
  - [ISSUE-007 — Silent U256-to-u64 saturation truncates staking amounts without error](#issue-007)
  - [ISSUE-008 — Unsafe `as i16` cast of u16 steepness parameter wraps large values to negative](#issue-008)
  - [ISSUE-009 — BN128Add precompile at non-standard address 9; Dispatch occupies standard address 6](#issue-009)
  - [ISSUE-012 — sudo_set_min_difficulty restricted to root despite documented owner access](#issue-012)
  - [ISSUE-015 — sudo_set_ema_price_halving_period missing subnet existence and admin window checks](#issue-015)
  - [ISSUE-016 — sudo_set_liquid_alpha_enabled missing subnet existence check](#issue-016)
  - [ISSUE-017 — sudo_set_min_non_immune_uids missing subnet existence and admin window checks](#issue-017)
  - [ISSUE-018 — Randomness field not validated against BLS signature](#issue-018)
  - [ISSUE-019 — Pruning pulses can permanently lock commitments with user deposits](#issue-019)
  - [ISSUE-021 — Deposit not unreserved when commitment removed after timelock reveal](#issue-021)
  - [ISSUE-023 — Timelocked commitment fields silently dropped on decryption failure](#issue-023)
  - [ISSUE-024 — Unbounded TimelockedIndex StorageValue read/written every block](#issue-024)
  - [ISSUE-027 — Revealer processes all imported blocks causing wrong-key decryption attempts](#issue-027)
  - [ISSUE-034 — inc_providers on funds_account never balanced by dec_providers after finalize](#issue-034)
- [Low](#low)
  - [ISSUE-004 — liquidity_delta.abs() panics on i64::MIN](#issue-004)
  - [ISSUE-013 — sudo_set_network_registration_allowed missing subnet existence check](#issue-013)
  - [ISSUE-014 — sudo_set_network_pow_registration_allowed missing subnet existence check](#issue-014)
  - [ISSUE-022 — Deposit not unreserved in purge_netuid during network dissolution](#issue-022)
  - [ISSUE-036 — Stale crowdloan state written back after dispatch](#issue-036)
- [Challenged / False-Positive](#challenged--false-positive)
  - [ISSUE-011 — Transaction extension prepare() result discarded](#issue-011)
  - [ISSUE-020 — Pruning assumes consecutive rounds, wastes weight on gaps](#issue-020)
  - [ISSUE-026 — Commitment hash never verified after decryption](#issue-026)
  - [ISSUE-028 — WrapperBuffer upsert overwrites existing entries with same ID without warning](#issue-028)
  - [ISSUE-035 — refund callable during active contribution period](#issue-035)

---

## Summary

| Severity | Confirmed | Challenged | Total |
|----------|-----------|------------|-------|
| Critical | 1 | 0 | 1 |
| High | 7 | 0 | 7 |
| Medium | 17 | 0 | 17 |
| Low | 5 | 5 | 10 |
| **Total** | **30** | **5** | **35** |

**Components affected:**

| Component | Confirmed Findings |
|-----------|--------------------|
| `pallets/registry` | 3 (1 critical, 2 high) |
| `pallets/crowdloan` | 4 (2 high, 1 medium, 1 low) |
| `pallets/admin-utils` | 6 (4 medium, 2 low) |
| `pallets/commitments` | 4 (3 medium, 1 low) |
| `node/mev_shield` | 3 (1 high, 2 medium) |
| `pallets/swap` | 4 (1 high, 2 medium, 1 low) |
| `precompiles` | 4 (1 high, 3 medium) |
| `pallets/drand` | 2 (2 medium) |
| `pallets/shield` | 1 (1 medium) |

---

## Critical

### ISSUE-029

**clear_identity has no authorization check — anyone can delete any identity**

- **File:** `pallets/registry/src/lib.rs` (lines 185–205)
- **Severity:** Critical
- **Status:** Confirmed — reproduced via Rust unit tests and TypeScript integration tests on a live node
- **Age:** ~21 months (introduced May 17, 2024; on mainnet since ~Feb 2025)
- **GitHub tracking:** No issue or PR exists for this bug

#### Root cause

Commit `751f834ce` ("update more errors", 2024-05-17, `open-junius`) silently removed the `ensure!(T::CanRegister::can_register(&who, &identified), Error::<T>::CannotRegister)` guard from `clear_identity` during what appeared to be a routine error-renaming refactor (`TooManyFields` → `TooManyFieldsInIdentityInfo`). The guard was present in the original commit `ade302819` ("Identity system for Subnets and Root-validators", 2023-11-15) and was never restored.

#### Vulnerable code

```rust
pub fn clear_identity(origin: OriginFor<T>, identified: T::AccountId) -> DispatchResultWithPostInfo {
    let who = ensure_signed(origin)?;
    // MISSING: ensure!(T::CanRegister::can_register(&who, &identified), Error::<T>::CannotRegister)
    let id = <IdentityOf<T>>::take(&identified).ok_or(Error::<T>::NotRegistered)?;
    let deposit = id.total_deposit();
    let release_res = T::Currency::release(
        &HoldReason::RegistryIdentity.into(),
        &who,          // ← releases from caller (attacker holds 0), NOT from original depositor
        deposit,
        Precision::BestEffort,  // ← silently releases 0 if caller holds nothing
    );
    debug_assert!(release_res.is_ok_and(|released| released == deposit)); // fires in test, no-op in production WASM
    Self::deposit_event(Event::IdentityDissolved { who: identified });
    Ok(().into())
}
```

#### Impact

1. **Authorization bypass** — any signed account can delete any other account's registered identity with a single transaction.
2. **Permanent deposit lock** — `Currency::release` is called on `who` (the attacker or wrong caller). Attacker holds 0 → `BestEffort` releases 0 silently. The original depositor's **0.1 TAO** (`InitialDeposit = 100_000_000`) remains on hold forever with no extrinsic available to recover it.
3. **Attack cost** — one transaction fee (dust-level TAO).

#### Proof of concept — Rust unit tests

File: `pallets/registry/src/lib.poc.test.rs`

All three tests **fail on the current codebase** (`cargo test -p pallet-registry` → `3 passed; 4 failed`):

| Test | Assertion | Failure |
|------|-----------|---------|
| `poc_029_case1_third_party_deletes_any_identity` | `assert_noop!(clear_identity(attacker, victim), CannotRegister)` | Call returns `Ok` instead of `Err` |
| `poc_029_case2_victim_deposit_permanently_locked_after_unauthorized_clear` | `assert_eq!(victim_held_after, 0)` | `victim_held_after = 10` — deposit never released |
| `poc_029_case3_admin_deposit_permanently_locked_after_hotkey_self_clear` | `assert_eq!(coldkey_held_after, 0)` | `coldkey_held_after = 10` — admin deposit lost even on legitimate self-clear |

#### Proof of concept — TypeScript integration tests (live node)

File: `contract-tests/test/registry.issue-029.test.ts`

Tests were executed against a two-node local devnet (`cargo build -p node-subtensor --features fast-runtime`). All three cases **fail**:

| Case | Expected | Actual |
|------|----------|--------|
| Case 1 — attacker deletes victim's identity | `ok=false, error=CannotRegister` | `ok=true` — call succeeded for unauthorized account |
| Case 2 — victim's 0.1 TAO deposit locked after attack | `frozenAfter = 0` | `frozenAfter = 100_000_000` — 0.1 TAO permanently held |
| Case 3 — deposit locked even when attacker has no prior holds | `frozenAfterAttack = 0` | `frozenAfterAttack = 100_000_000` — victim's hold never cleared |

#### Interaction with ISSUE-030 / ISSUE-031

ISSUE-030 (deposit released to wrong account) and ISSUE-031 (deposit accounting mismatch on update) share the same root class: `clear_identity` uses `who` (caller) instead of the stored depositor address when releasing holds. These three issues compound: ISSUE-031 corrupts the depositor accounting, ISSUE-029 allows a third party to trigger the corrupted release, and ISSUE-030 formalizes the general pattern of misattributed releases.

---

## High

### ISSUE-001

**CurrentLiquidity and token amounts wrong on auto-remove in modify_position**

- **File:** `pallets/swap/src/pallet/impls.rs`
- **Severity:** High
- **Status:** Confirmed

When removing liquidity that would leave a position below `MinimumLiquidity`, `delta_liquidity_abs` is corrected to `position.liquidity` at line 644 — but `CurrentLiquidity` and token return amounts were already computed using the original (smaller) value. Tick-level liquidity uses the corrected value, creating an inconsistency between the global pool state and per-tick accounting.

---

### ISSUE-005

**AEAD decrypt failure silently drops submissions without on-chain notification**

- **File:** `node/src/mev_shield/proposer.rs`
- **Severity:** High
- **Status:** Confirmed

Line 649 `failed_calls.push(...)` is commented out. AEAD decryption failures skip the on-chain `DecryptionFailed` notification. All other ~18 failure paths correctly push to `failed_calls`. Users see `EncryptedSubmitted` but never `DecryptionFailed`, making it impossible to distinguish silent key mismatch from successful decryption.

---

### ISSUE-010

**Dispatch precompile bypasses PrecompileEnable admin check**

- **File:** `precompiles/src/lib.rs`
- **Severity:** High
- **Status:** Confirmed

The Dispatch precompile at address 6 calls `execute()` directly, bypassing the `PrecompileEnable` toggle used by all other Subtensor precompiles. Additionally, `PrecompileEnum` has no `Dispatch` variant, so it cannot be disabled via `sudo_toggle_evm_precompile`. `StorageQueryPrecompile` has the same issue. These precompiles are permanently active regardless of admin configuration.

---

### ISSUE-030

**Deposit released to caller instead of original depositor, permanently locking funds**

- **File:** `pallets/registry/src/lib.rs`
- **Severity:** High
- **Status:** Confirmed

`Registration` stores the deposit amount but not the depositor's `AccountId`. Both `set_identity` and `clear_identity` release funds to `who` (current caller), not the original depositor. In the production flow, coldkey A can register an identity for hotkey B — A pays the deposit — but after a coldkey swap, the deposit can never be returned to A. Compounded by ISSUE-029 (unauthorized deletion) and ISSUE-031 (wrong delta accounting).

---

### ISSUE-031

**Identity update by new owner inherits wrong deposit, causing accounting mismatch**

- **File:** `pallets/registry/src/lib.rs`
- **Severity:** High
- **Status:** Confirmed

When `set_identity` is called for an account that already has a registration, `old_deposit` is read from storage and the delta is charged against the new caller who never made the original deposit. The new caller is charged only the increment (subsidized by the original depositor's locked funds) or is erroneously refunded. `BestEffort` masks the accounting error silently.

---

### ISSUE-032

**Preimage never dropped after successful finalize — permanent deposit lock**

- **File:** `pallets/crowdloan/src/lib.rs`
- **Severity:** High
- **Status:** Confirmed

`create()` calls `T::Preimages::bound()`, reserving a deposit for calls larger than 128 bytes. `finalize()` uses `T::Preimages::peek()` (non-consuming) on the success path. `T::Preimages::drop()` appears only in the error branch. `dissolve()` is permanently blocked post-finalization by `ensure!(!crowdloan.finalized)`. The preimage deposit is permanently locked after every successful finalization of a non-inline call.

---

### ISSUE-033

**MaxContributors check incorrectly rejects existing contributors trying to top up**

- **File:** `pallets/crowdloan/src/lib.rs`
- **Severity:** High
- **Status:** Confirmed

The `ensure!(contributors_count < MaxContributors)` guard fires for ALL callers before the existing-vs-new contributor distinction at lines 461–473. Once the cap is full, existing contributors cannot increase their contributions even though doing so would not increment `contributors_count`. The existing test at line 727 encodes this broken behavior as expected output.

---

## Medium

### ISSUE-002

**modify_position missing reserve tracking (increase_provided/decrease_provided)**

- **File:** `pallets/swap/src/pallet/mod.rs`
- **Severity:** Medium
- **Status:** Confirmed

`modify_position` does not call `TaoReserve::increase_provided` / `decrease_provided`, unlike `remove_liquidity` and the (commented-out) `add_liquidity`. This causes incorrect reserve accounting whenever liquidity is modified via `modify_position`.

---

### ISSUE-003

**disable_lp hardcodes subnet range 1..=128, missing subnets > 128**

- **File:** `pallets/swap/src/pallet/mod.rs`
- **Severity:** Medium
- **Status:** Confirmed

The emergency `disable_lp` extrinsic iterates `for netuid in 1..=128`, missing subnets with IDs above 128 even though `NetUid` is `u16` and `SubnetLimit` is configurable up to 65535. During an emergency shutdown, subnets with higher IDs would remain active.

---

### ISSUE-006

**Unbounded Submissions::iter() in on_initialize enables DoS via submission spam**

- **File:** `pallets/shield/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

`on_initialize` iterates ALL `Submissions` entries every block with no cap. No `MaxSubmissions` limit exists; each entry can be up to 8192 bytes. Fee barriers and TTL windows partially mitigate the issue, but unbounded iteration in `on_initialize` is a known Substrate anti-pattern that can be exploited with sustained submission spam.

---

### ISSUE-007

**Silent U256-to-u64 saturation truncates staking amounts without error**

- **File:** `precompiles/src/staking.rs`
- **Severity:** Medium
- **Status:** Confirmed

Nine locations across seven staking functions use `unique_saturated_into()` to convert EVM `U256` to `u64`, silently clamping values > `u64::MAX`. A safe `try_u64_from_u256()` helper exists and is used in exactly one place (`remove_stake_full_limit`) but is not consistently applied. The silent truncation is especially dangerous for `limit_price` (slippage protection), where a clamped value produces a wrong slippage guard without any error.

---

### ISSUE-008

**Unsafe `as i16` cast of u16 steepness parameter wraps large values to negative**

- **File:** `precompiles/src/subnet.rs`
- **Severity:** Medium
- **Status:** Confirmed

`steepness as i16` in `set_alpha_sigmoid_steepness` wraps values 32768–65535 to negative `i16`. Non-root callers receive a confusing `NegativeSigmoidSteepness` error; root callers silently store a corrupted value. Additionally, the Solidity ABI uses `uint16`/`int16` types that generate different 4-byte selectors, causing ABI compatibility issues.

---

### ISSUE-009

**BN128Add precompile at non-standard address 9; Dispatch occupies standard address 6**

- **File:** `precompiles/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

EIP-196 assigns BN128Add to address 6; Subtensor maps Dispatch there instead, pushing BN128Add to address 9. Ported Solidity contracts using the standard BN128Add address will hit the Dispatch precompile (which safely fails, but breaks compatibility). The deviation was acknowledged but not resolved in PR #1896.

---

### ISSUE-012

**sudo_set_min_difficulty restricted to root despite documented owner access**

- **File:** `pallets/admin-utils/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

Docs say "root or subnet owner" but the implementation uses `ensure_root`. The symmetric counterpart `sudo_set_max_difficulty` correctly uses `ensure_sn_owner_or_root_with_limits`. The same broken pattern affects `sudo_set_kappa`, `sudo_set_network_registration_allowed`, and `sudo_set_difficulty` — all missing `record_owner_rl` as well.

---

### ISSUE-015

**sudo_set_ema_price_halving_period missing subnet existence and admin window checks**

- **File:** `pallets/admin-utils/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

Call index 65 lacks both `if_subnet_exist()` and `ensure_admin_window_open()`. `EMAPriceHalvingBlocks` directly affects subnet token pricing. This extrinsic was missed by the PR #2235 systematic admin-window migration.

---

### ISSUE-016

**sudo_set_liquid_alpha_enabled missing subnet existence check**

- **File:** `pallets/admin-utils/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

Call index 50 lacks `if_subnet_exist()`, while adjacent `sudo_set_commit_reveal_weights_enabled` (call 49) has the check. Orphaned `LiquidAlphaOn` storage for a non-existent netuid could affect consensus if a subnet is later created with that same ID.

---

### ISSUE-017

**sudo_set_min_non_immune_uids missing subnet existence and admin window checks**

- **File:** `pallets/admin-utils/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

Call index 84 lacks both `if_subnet_exist()` and `ensure_admin_window_open()`. The direct peer `sudo_set_min_allowed_uids` (call 79) has both checks. `MinNonImmuneUids` controls the neuron pruning safety floor; writing this for a non-existent subnet and later creating that subnet could alter deregistrations during consensus.

---

### ISSUE-018

**Randomness field not validated against BLS signature**

- **File:** `pallets/drand/src/verifier.rs`
- **Severity:** Medium
- **Status:** Confirmed

`QuicknetVerifier::verify()` checks the BLS pairing but never verifies `randomness == SHA256(signature)`. A node operator with a valid OCW key could submit an arbitrary `randomness` value alongside a valid signature. Primary consumers (timelock/commit-reveal) use `pulse.signature` directly and are unaffected; `random_at()` and the `Randomness` trait are vulnerable.

---

### ISSUE-019

**Pruning pulses can permanently lock commitments with user deposits**

- **File:** `pallets/drand/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

`prune_old_pulses()` removes pulses older than ~1 week. Commitments referencing pruned rounds silently persist in `TimelockedIndex` forever; no `unreserve` is called when such a commitment is eventually removed, permanently locking the deposit. Currently zero-deposit configuration mitigates financial impact, but this is a latent fund-loss issue if deposits are ever enabled.

---

### ISSUE-021

**Deposit not unreserved when commitment removed after timelock reveal**

- **File:** `pallets/commitments/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

`reveal_timelocked_commitments()` calls `CommitmentOf::remove()` but never `T::Currency::unreserve()`. The deposit is permanently locked. The same issue exists in `purge_netuid()`. Currently mitigated by zero deposits in production; latent fund-loss if deposits are enabled.

---

### ISSUE-023

**Timelocked commitment fields silently dropped on decryption failure**

- **File:** `pallets/commitments/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

On any decryption failure (signature parse, ciphertext parse, or decrypt error), the field is silently discarded — not added to `remain_fields` or `revealed_fields`. Introduced intentionally in commit `0c2cd89ad7`. No on-chain event is emitted. If a drand pulse has a corrupted stored signature, all commitments for that round are irreversibly destroyed with no on-chain trace.

---

### ISSUE-024

**Unbounded TimelockedIndex StorageValue read/written every block**

- **File:** `pallets/commitments/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

`TimelockedIndex` is a single `StorageValue<BTreeSet<(NetUid, AccountId)>>` deserialized in full every block in `on_initialize`. No per-block batch limit or pagination exists. The theoretical maximum is ~524K entries. No benchmark covers `reveal_timelocked_commitments`. `#[pallet::without_storage_info]` suppresses Substrate's own warnings about this pattern.

---

### ISSUE-027

**Revealer processes all imported blocks causing wrong-key decryption attempts and silent wrapper loss**

- **File:** `node/src/mev_shield/proposer.rs`
- **Severity:** Medium
- **Status:** Confirmed

The revealer at line 168 processes ALL imported blocks with no `BlockOrigin::Own` filter, while the author side correctly filters. In a multi-validator Aura network each validator wastes CPU on ML-KEM decapsulation for other validators' blocks. Combined with the commented-out `failed_calls.push` (ISSUE-005), failures are entirely silent. No data loss occurs in practice since each validator has the correct key only for its own blocks.

---

### ISSUE-034

**inc_providers on funds_account never balanced by dec_providers after successful finalize — zombie account**

- **File:** `pallets/crowdloan/src/lib.rs`
- **Severity:** Medium
- **Status:** Confirmed

`create()` calls `inc_providers` on the crowdloan funds account. The only `dec_providers` call is in `dissolve()`, which is permanently blocked post-finalization by `ensure!(!crowdloan.finalized)`. Every successfully finalized crowdloan leaves a zombie `frame_system::Account` entry with `providers = 1` and zero balance, accumulating indefinitely on chain.

---

## Low

### ISSUE-004

**liquidity_delta.abs() panics on i64::MIN**

- **File:** `pallets/swap/src/pallet/impls.rs`
- **Severity:** Low
- **Status:** Confirmed

`liquidity_delta.abs()` on a user-supplied `i64` panics in debug / overflow-check builds when the value is `i64::MIN`. In production WASM release builds, wrapping semantics cause the value to fail the `ensure` check harmlessly. No production impact, but the code is undefined behavior-adjacent in debug builds.

---

### ISSUE-013

**sudo_set_network_registration_allowed missing subnet existence check**

- **File:** `pallets/admin-utils/src/lib.rs`
- **Severity:** Low
- **Status:** Confirmed

No `if_subnet_exist()` or `ensure_admin_window_open()` check. Root can write storage for non-existent subnets. Root-only, so practical impact is limited to orphaned storage entries; however, it is consistent with a class of ~12–15 similarly missing checks across admin-utils extrinsics.

---

### ISSUE-014

**sudo_set_network_pow_registration_allowed missing subnet existence check**

- **File:** `pallets/admin-utils/src/lib.rs`
- **Severity:** Low
- **Status:** Confirmed

Same class as ISSUE-013. Root can write `NetworkPowRegistrationAllowed` storage for non-existent subnets. Root-only; creates orphaned storage entries.

---

### ISSUE-022

**Deposit not unreserved in purge_netuid during network dissolution**

- **File:** `pallets/commitments/src/lib.rs`
- **Severity:** Low
- **Status:** Confirmed

`purge_netuid()` uses `clear_prefix()` without iterating to call `unreserve` for each entry's deposit. Currently mitigated by zero deposits in production. Latent fund-loss on subnet dissolution if deposits are ever enabled.

---

### ISSUE-036

**Stale crowdloan state written back after dispatch — inner call's storage mutations silently overwritten**

- **File:** `pallets/crowdloan/src/lib.rs`
- **Severity:** Low
- **Status:** Confirmed

In `finalize()`, a pre-dispatch snapshot of the crowdloan struct is written back after the inner call executes. If the inner call mutates crowdloan storage fields, those mutations are overwritten. No financial or security impact in practice since `finalized = true` is persisted and all subsequent operations check `!finalized` first; the stale fields are therefore unreachable. Cosmetic storage inconsistency only.

---

## Challenged / False-Positive

The following issues were filed and investigated but assessed as false positives or intentional design decisions after further review.

---

### ISSUE-011

**Transaction extension prepare() result discarded; post_dispatch receives unit instead**

- **File:** `precompiles/src/extensions.rs`
- **Severity:** Low
- **Status:** Challenged

The `prepare()` result is discarded and `()` is hardcoded in `post_dispatch` calls. Currently not a bug because `SubtensorTransactionExtension::Pre = ()`. Rust's type system would catch this if `Pre` were ever changed to a non-unit type. Assessed as a code style issue only; no runtime impact.

---

### ISSUE-020

**Pruning assumes consecutive rounds, wastes weight on gaps**

- **File:** `pallets/drand/src/lib.rs`
- **Severity:** Low
- **Status:** Challenged

`prune_old_pulses()` iterates sequentially and wastes budget on gaps. However, `write_pulse` enforces strict ordering and the OCW fetches sequentially — gaps cannot form under normal operation. The POC test constructs an artificially gapped storage state that cannot arise in production. Assessed as a false positive; fragile code at most.

---

### ISSUE-026

**Commitment hash never verified after decryption, bypassing integrity check**

- **File:** `node/src/mev_shield/proposer.rs`
- **Severity:** Low
- **Status:** Challenged

The `commitment` field in `Submission` is never verified as `hash(decrypted_plaintext)`. However, XChaCha20-Poly1305 (AEAD) already guarantees plaintext integrity — garbage output on key mismatch is cryptographically impossible. Assessed as a false positive; AEAD authentication subsumes the hash check.

---

### ISSUE-028

**WrapperBuffer upsert overwrites existing entries with same ID without warning**

- **File:** `node/src/mev_shield/proposer.rs`
- **Severity:** Low
- **Status:** Challenged

`HashMap::insert` silently overwrites entries. During a reorg, re-processing the wrapper is actually correct behavior since the first processing is undone with the reverted block. Transaction-pool nonce-based dedup prevents true double-spend. The POC scenario (different ciphertext, same ID) is impossible in production because the ID is derived from the ciphertext. Assessed as a false positive.

---

### ISSUE-035

**refund callable during active contribution period — creator can grief contributors**

- **File:** `pallets/crowdloan/src/lib.rs`
- **Severity:** Low
- **Status:** Challenged

The `ensure!(now >= crowdloan.end)` guard was deliberately removed in PR #2115 ("Allow crowdloan finalize/refund before end"). The POC tests against a dead error variant `ContributionPeriodNotEnded`. Assessed as a false positive — intentional design change, not a bug.