[BUG] Dead duplicate reentrancy-guard module (core/guards.rs) diverges from the actively-used guards.rs

[N-thnI]

Description

engine-core/src/core/guards.rs implements a second, fully-tested reentrancy guard (ReentrancyGuard, enter_guard, exit_guard, with_guard) that is never called from any #[contract] entry point in the crate. Every real entry point instead uses the separate non_reentrant! macro defined in the top-level engine-core/src/guards.rs. The two implementations use different storage keys and diverge silently.

Location

engine-core/src/core/guards.rs:1-47 (unused module), engine-core/src/guards.rs:46 (non_reentrant!, the one actually used)

Current Behavior

• core/guards.rs defines KEY_REENTRY = "C_REENTR" and exposes ReentrancyGuard::new, with_guard, enter_guard, exit_guard — with 4 passing unit tests inside the module itself.
• guards.rs (top level) defines KEY_GUARD = "RE_GUARD" and the non_reentrant! macro.
• A repo-wide search confirms core::guards is referenced only from its own #[cfg(test)] mod tests block:

  grep -rn "use.*guards\|guards::" engine-core/src --include="*.rs"
  engine-core/src/reentrancy_tests.rs:6: use crate::guards::{enter_reentrancy_guard, exit_reentrancy_guard};
  

  Meanwhile non_reentrant! is invoked from audit.rs, governance.rs, circuit_breaker.rs, emergency_recovery.rs, protocol_fee.rs, treasury.rs, and core/control_plane.rs — i.e. every real state-mutating entry point except the ones in core/proxy.rs (see the companion issue on that file).

Expected Behavior

There should be exactly one reentrancy-guard implementation. Either core/guards.rs is genuinely unused and should be deleted, or it was intended to be wired into core/proxy.rs/core/control_plane.rs and isn't — either way the current split-brain state is a maintenance hazard: a future contributor adding a new entry point under core/ may reach for core::guards (it's right there, in the same module tree) instead of crate::non_reentrant!, producing a guard that uses a different storage key and has never protected a real call path in production.

Repro / Evidence

grep -rn "guards::" engine-core/src --include="*.rs"

returns only the test-only usage above; no #[contractimpl] function calls core::guards::enter_guard, exit_guard, or ReentrancyGuard::new.

Impact

Not an active vulnerability today (the real entry points are guarded via non_reentrant!), but it is dead security-critical code masquerading as tested, load-bearing infrastructure — the 4 passing unit tests in core/guards.rs create false confidence that this path is exercised in production.

Suggested Fix

Delete engine-core/src/core/guards.rs and its pub mod guards; line in core/mod.rs, consolidating on the single non_reentrant! macro — unless there's a concrete plan to wire it into core/proxy.rs, in which case do that instead and delete the duplicate storage key.

Acceptance Criteria

• AC-1: Exactly one reentrancy-guard implementation remains in engine-core.
• AC-2: If core/guards.rs is removed, core/mod.rs no longer declares it and cargo build / cargo test remain green.
• AC-3: If instead it's wired into core/proxy.rs, upgrade()/init() call it and a new test proves double-entry panics.

Definition of Done

• Fix merged with all AC items checked.
• cargo test passes in CI.
• No new cargo clippy warnings introduced.

Labels: bug, engine-core