fix(zebrad): handle EvictionList re-insertion gracefully instead of asserting

[natalieesk]

Summary

Closes #10690.

• zebrad/src/components/mempool/storage/eviction_list.rs: EvictionList::insert no longer assert_eq!s that the key is absent; a still-present key refreshes its timestamp in place and only a genuinely new key appends an ordered_entries slot. The doc comment now states the real invariant (the old # Panics note is gone).
• Tests: two unit tests in eviction_list.rs, and the existing eviction_list_refresh prop test (storage/tests/prop.rs) is converted from #[should_panic] to asserting the graceful path.

Approach & Key Decisions

The assert enforced "key never already present," but that only held via three undocumented properties acting together: prune_old runs before the insert, entries expire front-to-back in insertion order, and a live entry keeps the transaction rejected via contains_key so it can't re-enter the mempool to be re-evicted. A new caller, a reordered prune, or a re-inserted still-rejected key would turn it into a node panic.

The graceful path guards the one invariant that actually matters for this structure — that unique_entries and ordered_entries have identical membership. push_back runs only when old_value.is_none() (key absent from unique_entries, therefore absent from ordered_entries), so no phantom duplicate is ever pushed and pop_front's assert!(removed.is_some()) cannot be tripped. The refresh deliberately does not move the key to the back of ordered_entries: on this not-expected path that can briefly leave ordered_entries out of timestamp order, whose only effect is that entries behind a refreshed key are pruned no earlier than the refreshed key — harmless, and cheaper than an O(n) reorder for a path the mempool doesn't reach. The comment at eviction_list.rs:58 documents this. The assert!/panic! in pop_front/prune_old are left as-is: they guard the membership invariant that this change preserves, not attacker/caller input.

Testing & Verification

• reinsert_live_key_is_graceful: inserts the same key twice while live; asserts no panic, contains_key, and unique_entries.len() == ordered_entries.len() == 1.
• reinsert_then_fill_past_max_size_does_not_panic: re-inserts a live key then evicts past max_size, so pop_front runs over the re-inserted key; asserts the two collections stay equal-length.
• Both fail before the fix (the second insert panics on the old assert_eq!) and pass after.

cargo test -p zebrad --lib -- components::mempool::storage
test result: ok. 34 passed; 0 failed

cargo fmt and cargo clippy -p zebrad --lib --all-features are clean.

Risk & Impact

Low, and confined to the mempool eviction list. No consensus, network, RPC, state-format, or config surface changes; no DB format bump. The only behavior change is that a re-insert of an already-present key updates the timestamp instead of panicking — a path not reached through the normal mempool flow today.

Changelog

No changelog entry: the assert is not currently triggerable through the mempool, so there is no operator-visible behavior change (latent-robustness fix). Per the changelog guidelines, changes with no operator-visible effect are excluded from the zebrad changelog.

AI Disclosure

Claude (Claude Code) wrote the fix and the tests.

PR Checklist

• CHANGELOG.md intentionally not updated (not operator-visible; rationale above)
• DB format version unchanged (no state-format change)
• Linked issue exists and was acknowledged by the team before work started

[v12-auditor]

Note

Complete: Audit complete. V12 found one issue worth reviewing.

Open the full results here.

Finding	Severity	Details
F-205373	🔵 Low	Duplicate refresh evicts unrelated live rejection insert now documents that a present TXID is refreshed in place, but capacity eviction still runs before key novelty is known. With max_size = 2 and a long expiry, inserting A, then B, then live duplicate B makes the third call observe a full list. That call removes front key A before updating B. Replacing B returns an old map value, so the conditional branch appends no key. The list therefore shrinks to one entry and forgets unexpired A, despite the operation requiring no new slot. Both new regression tests refresh at length one below capacity, so neither covers this sequence.

And two more auto-invalidated findings.

Analyzed one file, diff 05d129b...26e20a6.