fix(F-9): add reentrancy locks to the v3 successor pools (Option B)

[unixwhisperer]

Summary

Implements Flashstack-ajv.4.9 per your Option B decision (email, 2026-10-05).

• Lock: flashstack-stx-pool-v3 and flashstack-sbtc-pool-v3 get pool-v3's pv3-F1 guard as one reentrancy-locked bool per pool, shared by deposit, withdraw and flash-loan. Each entry point checks and sets it first and clears it just before its (ok ...). A blocked call returns the new ERR-REENTRANT (u411 STX / u712 sBTC), which is unique within each contract.
• Deposit order: deposit now records shares before the token transfer, matching pool-v3. This is behaviour-neutral: new-shares and current-shares are bound in the let before either order, so the values written are identical. A failed transfer makes the public function return err, which rolls back every write in the call, the lock included. The lock blocks reentry either way.
• Unchanged: the flash-loan repayment check (reserve-after >= reserve-before + fee), fee logic, pause gating, admin and receiver approval are all untouched. The only removed lines in the diff are deposit's two moved statements.
• Test copies: the contracts/test/ copies were regenerated from the canonical sources and differ only in principal rewrites.
• Test receivers: two new test-only receivers, test-{stx,sbtc}-pool-v3-receiver-reentrant, with five callback modes: honest repay, reenter deposit, reenter withdraw, under-repay, nested flash-loan.
• Docs: the F-9 entry in FINDINGS_REGISTER.md is updated.

Verification

All of this ran in a clean git worktree at origin/main e0367e006c45 (fresh npm ci, clarinet 3.23.2 as in CI). The commit's tree hash (1c9db567) is identical to the verified tree.

Check	Baseline origin/main	This PR
clarinet check	exit 0, 213 contracts	exit 0, 215 contracts (+2 receivers), 0 errors; all 6 touched/new files also check clean individually
Full suite (CLARINET_BIN=… npx vitest run)	27 files, 265 passed + 1 expected fail	28 files, 279 passed + 1 expected fail. The +14 is exactly the new file (7 tests × 2 pools)
tests/v3-pools-reentrancy-lock.test.ts	—	14/14
Same file against the pre-fix pool sources	—	8 fail / 6 pass. All lock-dependent tests fail (deposit-reentry, zero-mutation, withdraw-reentry, lock release; 4 per pool). Happy path, under-repay and the nested-loan case pass, as they should
mainnet-plan-guard (ran, not skipped)	2 passed + 1 expected fail	2 passed + 1 expected fail
Test-copy equivalence	—	Regenerated output is byte-identical to the committed copies. A separate normalizer finds canonical == copy, and the substituted lines are the same set as on main (1 STX, 13 sBTC)

Per pool, the suite covers:

• happy path (flash-loan, deposit, withdraw in sequence, with stats)
• deposit-reentry rejected
• the same attempt leaves pool balance, stats, total-shares and both positions unchanged
• withdraw-reentry rejected with the receiver's seeded shares untouched
• lock released after a blocked reentry
• lock released after an under-repay revert (then withdraw, deposit and loan all work)

The negative cases assert the exact ERR-REENTRANT code, which nothing else on that path produces. The withdraw case first asserts that the receiver really holds shares, so it can't pass on a zero-amount error.

One finding from writing the tests: a nested flash-loan on the same pool never reaches the lock. The Clarity VM aborts it as RuntimeCheck(CircularReference), with or without this fix. The test pins that behaviour rather than crediting it to the lock.

Security notes

• F-9 is fixed in the repository only. Both successor pools remain undeployed. This PR does not remediate any deployed contract; the live v2 pools are immutable and unchanged by it.
• No deployment of any kind was performed.
• Deploying the successors needs the separately tracked process:
  • receiver-vetting checklist before any add-approved-receiver: Flashstack-ajv.6.8
  • a new testnet deploy plan with dedicated receivers: Flashstack-ajv.6.9
  • your mainnet bar (independent reproduction, dry-run-by-default script with key-match check, on-chain read-back)
  • the v2 disposition: Flashstack-ajv.4.4
• D6 disclosure: registering the two receivers adds them to the plan clarinet deployments apply --mainnet would compute. That takes the known contracts/test/ publishes from 28 to 30, the same precedent as CRITICAL FINDING (F-9): deposit-as-repayment reentrancy on live v2 pools #83 and fix(F-9): independently prove the sbtc-pool-v2 deposit-reentrancy variant #86. The guard pins the exact set, and the deployments/README.md "never apply --mainnet from the repo" rule still applies. (That README still says 26; it was already stale at 28 before this PR.)
• Merge note: this conflicts textually with docs+fix(security): resolve web/ decode-uri-component chain, document braces/elliptic as accepted risk #87 in FINDINGS_REGISTER.md. This PR edits the F-9 row, and docs+fix(security): resolve web/ decode-uri-component chain, document braces/elliptic as accepted risk #87 inserts DEP-1/DEP-2 directly after it. The changes don't contradict each other; resolve by keeping both.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
web	Ready	Preview	Oct 5, 2026 7:07am UTC

[mattglory]

Independently verified rather than taken on the writeup:

• Checked out the PR fresh, ran the full suite: 279 passed + 1 expected fail (280), matching exactly. clarinet check exit 0, 215 contracts.
• Read the actual lock placement in both pools: checked first, set true, before any other assert in deposit/withdraw/flash-loan; cleared right before each (ok ...). Matches pool-v3's pv3-F1 pattern, and Clarity's all-or-nothing revert semantics mean a failed assert anywhere in the call undoes the lock-set too -- no stuck-lock path.
• Confirmed the regression claim directly, not just read it: reverted both pool contracts (canonical + test copies) to pre-fix main in a worktree and re-ran tests/v3-pools-reentrancy-lock.test.ts -- 8 failed / 6 passed, matching the PR's own table exactly (deposit-reentry, zero-mutation, withdraw-reentry, and lock-release cases are the ones that fail; happy path, under-repay, and the nested-loan CircularReference case pass either way).
• Deposit's effects-before-interaction reorder is genuinely behaviour-neutral here, as described -- new-shares/current-shares are bound once in the let, so write order doesn't change the values written, and a failed transfer still rolls back the whole call.
• Withdraw locked too (Option B), matching what Matt decided.

Approving.