Skip to content

fix(privacy-channel): remove dead reentrancy guard (audit informative)#41

Open
AquiGorka wants to merge 1 commit into
mainfrom
fix/remove-reentrancy-guard
Open

fix(privacy-channel): remove dead reentrancy guard (audit informative)#41
AquiGorka wants to merge 1 commit into
mainfrom
fix/remove-reentrancy-guard

Conversation

@AquiGorka

@AquiGorka AquiGorka commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

What

RV audit informative finding (2026-07-07 call): the privacy-channel contract carried an explicit reentrancy guard (enter_reentrancy_guard on transact, a transient storage lock), added in the pre-audit pass (#32, MOON-06). But Soroban forbids reentrancy by design — a contract cannot appear twice in the call stack, so a nested call fails first. The guard's lock could never trigger; it is dead code. Gorka confirmed on the call.

This reverts the MOON-06 change in full, restoring the pre-guard state.

Changes

  • contracts/privacy-channel/src/contract.rs: removed enter_reentrancy_guard/exit_reentrancy_guard, the REENTRANCY_GUARD const + its transient storage entry, both call sites in transact, and the now-unused imports (moonlight_errors::Error, panic_with_error, symbol_short, Symbol).
  • modules/errors/src/lib.rs: removed the ReentrantCall = 3_008 variant (introduced solely for the guard).
  • contracts/privacy-channel/src/test/moon06.rs + its pub mod moon06; declaration: deleted (the test existed only to exercise the guard).
  • Unrelated MOON remediations living in the same files — MOON-02 (bump_instance_ttl) and MOON-05 (InvalidExternalAmount = 3_007) — are left untouched.
  • channel-auth carried no such guard (verified).

Coordination

Touches privacy-channel/src/contract.rs, same file as open B1 PR #40 (set_admin). This change does not touch the set_admin code; expect a merge-time overlap — the second of {this, #40} to merge rebases.

No version bump (consistent with the current audit-PR cluster; soroban-core versioning deferred to release).

Verification

  • cargo test --workspace: all 9 suites green, 0 failed (privacy-channel 13/13).
  • cargo fmt --check clean on touched files; cargo clippy introduces no new warnings.
  • stellar contract build (CI gate) green — transact still exported.

Soroban forbids reentrancy at the platform level: a contract cannot
appear twice in the call stack, so a nested call fails first. The
explicit RGUARD storage lock on transact could therefore never trigger
-- it was a preemptive guard added in the pre-audit pass (#32, MOON-06),
flagged as dead code by the RV informative finding (2026-07-07).

Revert the MOON-06 change in full, restoring the pre-guard state: drop
the enter/exit guard fns, the REENTRANCY_GUARD const + transient storage
entry, both transact call sites and now-unused imports, the ReentrantCall
error variant, and the moon06 test. Unrelated MOON remediations in the
same files (MOON-02 instance-TTL bump, MOON-05 InvalidExternalAmount) are
left untouched. channel-auth carried no such guard.
@AquiGorka
AquiGorka force-pushed the fix/remove-reentrancy-guard branch from 16902e5 to 7d24257 Compare July 8, 2026 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants