# Tolk security findings Date: 2026-05-14 (original review) Merge: 2026-05-14 (second review, TON\_TOLK\_SECURITY\_REVIEW\.md, now consolidated here) Fourth-review update: 2026-05-15 (NoBounce architectural shift + refund refactor + dead-code purge, see closeout section) Fifth-review update: 2026-05-15 (current-tree verification + coverage/doc drift review + live testnet e2e, see fifth-review section) Scope reviewed: * `apps/contracts/contracts/**/*.tolk` * Adjacent contract wrappers, tests, scripts, and security docs. This is an internal security review, not a substitute for a professional TON/ZK audit. ## Finding Index | ID | Severity | Title | Status | | --- | ------------------------: | --------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | | C-1 | Critical | Native pool can be drained via unverified `newRoot` | **Resolved** | | C-2 | Critical / Deploy blocker | `pool_jetton.tolk` is not production-valid | **Resolved** (deleted) | | H-1 | High | Owner recovery can drain native TVL | **Resolved** | | H-2 | High | Jetton deposit path is gas-prohibitive and can lock funds | **Resolved** (deleted) | | H-3 | High | Jetton withdraw public input packing exceeds cell limit | **Resolved** (deleted) | | H-4 | High | Admin recovery can seize bounced withdrawal liabilities | **Resolved (by elimination, 2026-05-15)** ★ | | H-5 | High | Withdraw refund formula leaves denomination stranded as admin-recoverable excess | **Resolved (sixth-review, 2026-05-15)** ★★ | | H-6 | Medium | Empty-body / bounce: false accidental TON loss (footgun under burn-key mainnet) | **Resolved (Phase-1 hardening, 2026-05-15)** ★★★ | | M-1 | High\* | Native pool capacity overstated vs. real TON state limits | **Resolved** *(severity raised, capacity capped)* | | M-2 | Medium | Bounce recovery is incomplete for receiver-side failures | **Resolved (by elimination, 2026-05-15)** ★ | | M-3 | Medium | Jetton `forward_payload` parser only accepts ref payloads | **Resolved** (deleted) | | M-4 | Medium | Gas budgets are not benchmarked against action paths | **Resolved** | | M-5 | Medium | `ReclaimStuck` can turn recoverable stuck funds into final loss | **Resolved (by elimination, 2026-05-15)** ★ | | M-6 | Medium | `onBouncedMessage` parses every bounce as RichBounce; mixed modes | **Resolved (by elimination, 2026-05-15)** ★ | | M-7 | Medium | Worst-case stuck-payout growth can still exceed state-cell cap | **Resolved (by elimination, 2026-05-15)** ★ | | M-8 | Medium | Withdraw logs are specified and indexed but never emitted | **Resolved (sixth-review, 2026-05-15)**, see WithdrawLog emit + shared decoder | | L-1 | Low | Jetton pool uses manual slice storage/parsing | **Resolved** (deleted) | | L-2 | Low | Jetton tests are empty and excluded from Vitest | **Resolved** (deleted) | | L-3 | Low | Excess deposit gas/value is not returned to the sender | **Resolved** *(extended to all handlers 2026-05-15, see refund refactor)* | | L-4 | Low | Direct chain commitment scan can stop before older deposits | **Resolved (sixth-review, 2026-05-15)**, early-break removed + leafIndex sort + 4 unit tests | | L-5 | Low | Documentation and public SDK/CLI surfaces still describe removed jetton / on-chain-merkle contracts | **Resolved (Phase-1 hardening, 2026-05-15)**, README + CLI `tonado deposit jetton` updated; v1.1 markers added | | I-1 | Informational | Static analyzers and Acton toolchain unavailable locally | **Resolved** | | I-2 | Informational | Direct `deploy-pool.ts` call bypasses the `mainnet.sh` ceremony gate | **Resolved** | | I-3 | Informational | Placeholder `*.spec.ts` tests are excluded from Vitest coverage | **Resolved (sixth-review, 2026-05-15)**, `merkle_tree.spec.ts` and `pool_native.spec.ts` deleted; `circuit.spec.ts` stripped of empty stub function | | P-1 | Privacy | `RichBounce` recipient payout silently breaks delivery to uninitialized recipients (canonical privacy use case) | **Resolved (2026-05-15)** ★ | ★ See the fourth-review closeout section for mechanism details. ★★ See the H-5 section; surfaced by post-fix testnet e2e accounting on 2026-05-15. ★★★ See the H-6 section at the end of this document; surfaced during burn-key mainnet readiness review. \*M-1's severity was raised to **High** by the second review: pool capacity overrun is not just a docs-mismatch but a liveness risk for already-paid deposits if state-cell limits are hit before withdraws finish. ## Resolved findings (closeout) ### C-1: unverified `newRoot` exploit (Resolved) Deposit now carries a `merkleUpdate` Groth16 proof that `newRoot = insert(prevRoot, commitment, leafIndex)`. The pool checks `prevRoot == storage.currentRoot` and `leafIndex == storage.nextIndex` on-chain, eliminating the parallel-tree attack. Artifacts: * [`apps/contracts/circuits/merkleUpdate.circom`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/circuits/merkleUpdate.circom) * [`apps/contracts/contracts/verifier_merkle_update.tolk`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/contracts/verifier_merkle_update.tolk) (auto-generated, symbols prefixed with `mu`) * Four sandbox regression tests: fake-multi-leaf root, commitment rebinding, leafIndex skip, stale prevRoot reuse. See [`off-chain-root.md`](/protocol-reference/off-chain-root.md) for the architecture. ### C-2 / H-2 / H-3 / M-3 / L-1 / L-2: jetton cluster (Resolved by deletion) `pool_jetton.tolk` and its dependencies (`merkle_tree.tolk`, `poseidon.tolk`, `poseidon_constants.tolk`) have been deleted. The empty `pool_jetton.spec.ts` test file also deleted. `deploy-pool.ts` now hard-errors on `--asset jetton:...` requests. Jetton support will be reintroduced with the same Tolk 1.0 + off-chain-root + merge-proof architecture used by the native pool. ### H-1: admin recovery cap (Resolved, but see H-4 follow-up) `OP_ADMIN_RECOVER_STUCK` now caps to: ``` excess = balance - (nextIndex - nullifiersCount) * denomination - attached_gas ``` `nullifiersCount: uint32` added to storage. The cap is enforced even against the owner. New error code `ERR_RECOVER_EXCEEDS_EXCESS = 105`. Three new admin sandbox tests cover overshoot, excess-recovery, and stranger-rejection. > **Follow-up:** the second review identified that this cap excludes liabilities sitting in `stuckWithdrawals`. See **H-4** below. ### M-2: bounce recovery (Resolved, but see M-5/M-6 follow-ups) Recipient payouts now use `BounceMode.RichBounce` and carry `(nullifierHash, recipient)` inline. `onBouncedMessage` parses the bounced body and stashes `(nullifierHash → amount, recipient)` in a new `stuckWithdrawals: cell?` storage dict. New `OP_RECLAIM_STUCK` and `isStuck()` get-method. > **Follow-ups:** see **M-5** (ReclaimStuck's `NoBounce` retry can lose funds with no record) and **M-6** (`onBouncedMessage` parses every bounce as RichBounce even though relayer/admin sends use a different mode). ### M-4: gas budgets benchmarked (Resolved) [`tests/pool.gas.test.ts`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.gas.test.ts) measures every handler's gas in sandbox and asserts against committed budgets. Current measurements: deposit ≈ 106K, withdraw ≈ 115K, all others under 5K. CI runs this test on every PR so regressions trip immediately. ### I-1: CI scaffolding + mainnet gate (Resolved, but see I-2 follow-up) [`.github/workflows/ci.yml`](https://github.com/tonadocash/monorepo/blob/main/.github/workflows/ci.yml) runs build, typecheck, the full test suite, Poseidon parity, `acton fmt --check`, `acton build`, and Misti on every PR. [`shell/mainnet.sh`](https://github.com/tonadocash/monorepo/blob/main/shell/mainnet.sh) is the new mainnet deploy gate. > **Follow-up:** the second review noted that direct `pnpm deploy:pool --network mainnet` calls bypass `mainnet.sh`. See **I-2** below. *** ## Second-review closeout (2026-05-14, same-day) All findings from the second review (H-4, M-1 elevated, M-5, M-6, L-3, I-2) are now resolved. Summary: * **H-4:** `PoolStorage.stuckWithdrawalsTotal: coins` added. Bumped in `onBouncedMessage` when the dict grows; decremented in `ReclaimStuck`. `AdminRecoverStuck`'s cap now includes it: `liability = outstanding * denomination + stuckWithdrawalsTotal`. New regression test in `pool.bounce.test.ts` injects a stuck entry via `preStuck` and asserts admin recovery is blocked for the stuck amount. * **M-1:** `MAX_LEAVES` reduced from 2²⁰ → 2¹³ (8,192) on-chain after empirical measurement in `tests/pool.capacity.test.ts`. The off-chain merkle tree + circuits still operate at the full 2²⁰; the cap is purely an on-chain state-pressure guard. Measured numbers (2026-05-14): * per-deposit: \~3.8 cells (commitments + roots dicts) * per-withdraw: \~1.7 cells (nullifiers dict) * full lifecycle: \~5.5 cells * at 8,192 deposits + 8,192 withdraws → \~45,000 cells used (\~30% headroom under TON's 65,536 max\_acc\_state\_cells). The earlier 2¹⁴ guess overshot: 16,384 × 5.5 ≈ 90K cells, past the TON cap. Past the new cap, deposits revert with `ERR_TREE_FULL`; existing withdraws continue. The capacity test now ASSERTS the projection (perDeposit ≤ 6, perWithdraw ≤ 3, headroom ≥ 20%) instead of just logging. A storage-layout regression that bloats cells will trip CI. * **M-5:** `ReclaimStuck` now requires `in.senderAddress == storedRecipient`. Old `stuckTake` (peek+delete in one shot) replaced with `stuckPeek` + `stuckDelete` so the entry survives an authorization failure. New error code `ERR_RECLAIM_NOT_RECIPIENT = 107`. Two regression tests in `pool.bounce.test.ts` (non-recipient rejected, recipient succeeds). * **M-6:** `onBouncedMessage` now inspects the 32-bit prefix before parsing. `0xfffffffe` → RichBounce (handle WithdrawPayout). `0xffffffff` → legacy bounce (silently absorbed). Mixed-mode bounces no longer cause parser-level failures. * **L-3:** Deposit handler now refunds excess attached value back to `in.senderAddress` (`BounceMode.NoBounce`) after committing the deposit. Regression test in `pool.deposit.test.ts` asserts net spent is well below `denomination + overpay`. * **I-2:** `deploy-pool.ts` enforces the ceremony-attestation gate itself when invoked with `--network mainnet`. Same check as `shell/mainnet.sh` GATE 1. Emergency override via `TONADO_ALLOW_UNATTESTED_MAINNET=1` is logged loudly. Test count went from 99 → 116 (60 unit + 56 sandbox), all passing. *** ## New findings (from second review): full details ### H-4: Admin recovery can seize bounced withdrawal liabilities Severity: High Status: **Resolved** Files: * `apps/contracts/contracts/pool_native.tolk`: `AdminRecoverStuck` handler * `apps/contracts/contracts/pool_native.tolk`: `onBouncedMessage` * `apps/contracts/contracts/pool_native.tolk`: `PoolStorage.nullifiersCount` Evidence: On withdraw the pool increments `nullifiersCount` *before* the recipient send. If that send bounces, `onBouncedMessage` records the payout in `storage.stuckWithdrawals` but `nullifiersCount` stays incremented. `AdminRecoverStuck` computes liability as: ``` outstanding = nextIndex - nullifiersCount liability = outstanding * denomination excess = balance - liability - attached_gas ``` This excludes the funds in `stuckWithdrawals`. They're no longer "outstanding" by this count, even though they're owed to the original recipient. The H-1 cap therefore treats them as excess. Impact: A compromised or malicious owner can drain stuck-payout backing funds. Equivalent to the H-1 risk we just closed, only routed through the bounce path. Recommendation: * Track `stuckWithdrawalsTotal: coins` in storage, summed across the `stuckWithdrawals` dict. * Maintain it in `onBouncedMessage` (`+=`) and `ReclaimStuck` (`-=`). * Include it in the H-1 cap: `excess = balance - liability - stuckWithdrawalsTotal - attached_gas`. * Add a sandbox test that creates a stuck withdrawal and asserts admin recovery cannot withdraw the stuck amount. ### M-1 (severity raised): Pool capacity overrun is a liveness risk Severity: High (raised from Medium) Status: **Resolved** `MAX_LEAVES = 2²⁰ = 1,048,576` advertises a capacity that TON's `max_acc_state_cells = 65536` cannot support. Worse, when the pool approaches the state-cell limit: * Future deposits revert from cell-overflow (acceptable). * *Pending withdraws* also revert because they need to write the `nullifiers` and (M-2) `stuckWithdrawals` dicts. That last case is the new severity driver. Once we cross the state threshold, already-paid depositors cannot withdraw. Funds are effectively frozen until an admin sweep that itself depends on storage writing. Recommendation: * Set `MAX_LEAVES` empirically from a measured cell-growth test. * Add an emergency-pause trip when the account's cell count crosses a conservative threshold (e.g. 90% of `max_acc_state_cells`). * Long term: epoch-pool sharding so each pool has a bounded lifetime. ### M-5: `ReclaimStuck` can turn recoverable stuck funds into final loss Severity: Medium Status: **Resolved** Files: * `apps/contracts/contracts/pool_native.tolk`: `ReclaimStuck` handler Evidence: `ReclaimStuck` removes the stuck entry from storage and then sends the payout with `BounceMode.NoBounce`. If the recipient still rejects the payment, the value disappears with no on-chain record (no second bounce, no re-stash). The dict entry that previously bounded the liability is gone. Additionally, `ReclaimStuck` is permissionless: *any* caller can trigger the retry. A griefer can front-run the recipient's wallet-deploy tx, trigger the retry while delivery is still impossible, lose the funds. Impact: User-owned stuck funds can be permanently lost by a third party. The existing `OP_ADMIN_RECOVER_STUCK` cap then converts this into an admin-mediated sweep (potentially compounded by H-4). Recommendation (one of): 1. **Recipient-only retry.** Require `in.senderAddress == stored recipient`. Strictly safest, but breaks the "anyone can help" UX. 2. **Preserve on failure.** Use `BounceMode.RichBounce` on the retry too, delete the entry only inside `onBouncedMessage` for the successful path. Requires bounded retry counter to avoid infinite re-stashing. 3. **Two-step reclaim.** First call re-bounces and re-stashes; second call after `N` blocks deletes. Adds friction. Option 1 is recommended for the first iteration; we can relax later if operational pain shows up. ### M-6: `onBouncedMessage` parses every bounce as RichBounce; mixed modes Severity: Medium Status: **Resolved** Files: * `apps/contracts/contracts/pool_native.tolk`: `onBouncedMessage` * `apps/contracts/contracts/pool_native.tolk`: relayer-fee send (`BounceMode.Only256BitsOfBody`) * `apps/contracts/contracts/pool_native.tolk`: admin recover send (`BounceMode.Only256BitsOfBody`) Evidence: The recipient payout uses `BounceMode.RichBounce` but the relayer-fee and admin-recover sends use `BounceMode.Only256BitsOfBody`. The bounce handler unconditionally does `lazy RichBounceBody.fromSlice(in.bouncedBody)`, which expects the `0xfffffffe` rich-bounce prefix. A legacy bounce with the `0xffffffff` prefix will not parse cleanly. Tolk docs explicitly warn against mixing bounce modes within a single contract for this reason. Impact: * Observability/accounting: bounced relayer-fee payments may throw inside `onBouncedMessage` instead of being silently ignored. * Not a direct theft surface (bounced messages can't be bounced again), but failure modes become harder to reason about. Recommendation: * Inspect the prefix first: if `0xffffffff`, handle as legacy bounce (just log/return). If `0xfffffffe`, parse as `RichBounceBody`. * Or migrate every bounceable outgoing send to `RichBounce` so there's only one parser path. * Add a sandbox test for a bounced `Only256BitsOfBody` message (e.g., by sending the admin-recover to an invalid destination). ### L-3: Excess deposit gas/value is not returned to the sender Severity: Low Status: **Resolved** Files: * `apps/contracts/contracts/pool_native.tolk`: `DepositNative` handler * `apps/contracts/tests/pool.deposit.test.ts`: explicit "over-attached value is fine" test Evidence: The deposit handler accepts any `valueCoins >= denomination + MIN_DEPOSIT_GAS`. Surplus stays in the pool (becomes "excess" by the H-1 cap, eventually admin-recoverable). The existing test even asserts that over-attaching 5 TON succeeds. Impact: * User overpayment accrues at the pool address. Not directly theft (H-1 cap), but adds an unnecessary intermediate state. * Privacy/UX: variable-fee deposits create a side channel. A user with a chronically over-attached wallet is potentially fingerprintable. * Admin-recoverable excess (H-1) means user money is reachable by the owner. Even if the cap is correct, this is centralization smell. Recommendation: * Emit a "refund excess" send back to `in.senderAddress` after computing `excess = valueCoins - denomination - GAS_RESERVE`. `BounceMode.NoBounce` so a misbehaving wallet can't grief the deposit. * Or document the policy clearly in the CLI and pin a fixed attached-value in the SDK so users don't overshoot. ### I-2: Direct `deploy-pool.ts` call bypasses the mainnet ceremony gate Severity: Informational Status: **Resolved** Files: * `apps/contracts/scripts/deploy-pool.ts` * `shell/mainnet.sh` Evidence: `shell/mainnet.sh` enforces ceremony attestation, no SKIP\_\* flags, audit closeout, etc. But the script just calls `pnpm deploy:pool --network mainnet --asset … --amount …`. A user with the deployer mnemonic can run that command directly and skip every gate. Impact: A well-intentioned operator under time pressure (or a malicious one with deployer access) can ship a mainnet pool that used the dev zkey, bypassing the gate. Then the C-1 fix is moot because the merkle-update verifier was built from a known-toxic-waste zkey. Recommendation: * `deploy-pool.ts` itself should refuse `--network mainnet` unless a hash-pinned attestation file is present and verifies. * Same check as `mainnet.sh`'s GATE 1, run again at the lowest layer. Defense in depth: gate at every entrypoint, not only the orchestrator. *** ## Third-review findings (2026-05-15): open The following issues came from a follow-up working-tree review after the H-4 / M-5 / M-6 / L-3 closeout. No critical on-chain drain was found in the native TON pool path, but these are still production-relevant. ### M-7: Worst-case stuck-payout growth can still exceed state-cell cap Severity: Medium Status: **Open** Files: * `apps/contracts/contracts/pool_native.tolk`: `MAX_LEAVES` cap and stuck-withdrawal accounting * `apps/contracts/tests/pool.capacity.test.ts`: capacity projection Evidence: `MAX_LEAVES` is now capped at 8192, and the normal deposit+withdraw projection is below TON's `max_acc_state_cells = 65536`. However, the contract's own source comment notes that the pathological lifecycle cost is about 8.5 cells per deposit when every recipient payout bounces: ``` 8192 * 8.5 ~= 70K cells ``` That exceeds the 65,536-cell account-state cap. The capacity test's assertion path projects only deposit + successful withdraw growth and uses a 20% headroom factor for `stuckWithdrawals`, rather than measuring or bounding the adversarial "many recipients bounce" case. Impact: An attacker can pay for deposits and withdraw to bounce-prone recipient addresses, forcing `stuckWithdrawals` entries to accumulate. If the pool approaches the state-cell cap, future withdrawals that need to write `nullifiers` or `stuckWithdrawals` can fail, creating a liveness issue for honest depositors. This is not a direct theft path, but it can freeze already-paid deposits until operational intervention. Recommendation: * Lower `MAX_LEAVES` to fit the measured full worst case, including `stuckWithdrawals`, with explicit headroom. * Add a capacity test that injects or triggers stuck-payout entries and projects against that measured cost. * Longer term: shard pools by epoch or move stuck-payout liabilities into bounded helper contracts so one pool account has a strict state ceiling. ### M-8: Withdraw logs are specified and indexed but never emitted Severity: Medium Status: **Open** Files: * `apps/contracts/contracts/pool_native.tolk`: `Withdraw` handler * `apps/relayer/src/libs/Indexer.ts`: `OP_CODES.LOG_WITHDRAW` parser * `docs/contracts-spec.md`: withdraw effects specification Evidence: The withdraw handler marks the nullifier, saves storage, and sends the recipient/relayer payments. It does not emit any external-out `OP_LOG_WITHDRAW` message. Off-chain code expects this event: the relayer indexer has an `OP_CODES.LOG_WITHDRAW` branch that inserts rows into the `withdrawals` table. The contract spec also lists "Emit `OP_LOG_WITHDRAW`" as the final withdraw effect. Impact: On-chain replay protection is still enforced by `storage.nullifiers`, so this is not a double-spend bug. The practical failure is off-chain: * relayer `/note/check` and duplicate-withdraw gas-saving checks can miss already-spent notes unless they call `isSpent` directly; * withdrawal history is incomplete; * clients and operators see behavior that contradicts the published contract spec. Recommendation: * Add a `WithdrawLog` external-out body carrying at least `(nullifierHash, recipientField, relayerField, fee, timestamp)` and send it after the successful payout actions are staged. * Add sandbox coverage asserting a successful withdraw emits the log and that `apps/relayer/src/libs/Indexer.ts` decodes the exact layout. * If withdraw logs are intentionally omitted for privacy, delete the indexer branch/spec entry and make relayers query `isSpent` for status. ### L-4: Direct chain commitment scan can stop before older deposits Severity: Low Status: **Open** Files: * `packages/core/src/depositFlow.ts`: `loadAllCommitments` * `packages/core/src/indexer.ts`: `scanPoolTransactions` Evidence: `loadAllCommitments` paginates backward through pool transactions and breaks when a page contains zero decoded deposit events: ``` if (events.length === 0) break ``` A page can legitimately contain only withdraws, admin ops, top-ups, or failed messages while older pages still contain deposits. Stopping on "no events in this page" can therefore build an incomplete Merkle tree. Impact: The pool fails closed: an incomplete tree produces stale `prevRoot` / `leafIndex` values and the deposit handler rejects with `ERR_PROOF_INVALID`. That protects funds, but can make direct SDK/CLI deposit and withdraw flows unusable after enough non-deposit pool activity. Relayer DB-backed tree endpoints are less exposed because they index deposits incrementally, but the direct chain path remains brittle. Recommendation: * Have `scanPoolTransactions` return whether the transaction page itself was empty/exhausted, not just whether decoded deposit events were found. * Continue paginating through non-empty transaction pages even when a page has zero deposit logs. * Add a unit test with a mock transaction sequence where a middle page has no deposit logs but an older page does. ### L-5: Documentation and public SDK/CLI surfaces still describe removed contracts Severity: Low Status: **Open** Files: * `README.md` * `docs/contracts-spec.md` * `docs/threat-model.md` * `docs/audit-scope.md` * `docs/security-review.md` * `shell/testnet.sh` * `packages/core/src/depositFlow.ts` * `packages/core/src/jettons.ts` * `apps/cli/src/commands/deposit/jetton.ts` Evidence: The active Acton manifest builds exactly one Tolk contract: `PoolNative`. `PoolJetton` is explicitly removed pending a rewrite, and `deploy-pool.ts` hard-errors on `--asset jetton:...`. Several public surfaces still describe or expose the removed design: * README sections still refer to `pool_jetton.boc`, `pool_jetton.tolk`, `merkle_tree.tolk`, and `poseidon.tolk`. * `docs/contracts-spec.md` still documents an old storage layout with `merkle_state`, `filled_subtrees`, a 30-root ring buffer, and `get_current_root` / `get_root_history` methods. The current storage is `nextIndex`, `nullifiersCount`, `currentRoot`, and three dictionaries. * Threat/audit-scope docs still describe a deployable jetton pool and counterfeit-jetton fuzz scope, even though that contract is absent. * `shell/testnet.sh` still reports `pool_native.boc + pool_jetton.boc` ready after Step 6, even though the current Acton build produces only the native pool. * The SDK and CLI still export jetton-note/deposit helpers. They should be marked experimental/disabled or hidden until a replacement jetton pool exists. Impact: This is not an on-chain exploit because the deploy path blocks jetton pools and the active contract is native-only. The risk is operational and audit-process drift: users, auditors, and relayer operators can follow documentation or SDK paths that do not correspond to deployable contracts. Recommendation: * Update `docs/contracts-spec.md` to match the current `PoolStorage`, deposit body, withdraw body, get-methods, and known-root set design. * Move the jetton pool description into a clearly labelled future-work section, or remove it from user-facing docs until the rewrite lands. * Gate or hide `tonado deposit jetton` in the CLI unless a real jetton deployment exists and the contract is in Acton scope. * Delete references to on-chain `merkle_tree.tolk` / `poseidon.tolk`; the current design uses off-chain Poseidon plus a deposit root-update SNARK. ### I-3: Placeholder `*.spec.ts` tests are excluded from Vitest coverage Severity: Informational Status: **Open** Files: * `apps/contracts/vitest.config.ts` * `apps/contracts/tests/merkle_tree.spec.ts` * `apps/contracts/tests/pool_native.spec.ts` * `apps/contracts/tests/circuit.spec.ts` Evidence: `apps/contracts/vitest.config.ts` includes only `tests/**/*.test.ts`. The three `*.spec.ts` files therefore do not run under `pnpm --filter @tonado/contracts run test`. Two of them are placeholders: `merkle_tree.spec.ts` contains empty test bodies for on-chain/off-chain Merkle parity even though the on-chain Merkle library no longer exists, and `pool_native.spec.ts` is an old Acton-sandbox sketch. Impact: The runnable sandbox suite is meaningful and green for the active native pool, but the repo can give a false impression that additional Acton or circuit coverage exists. This matters because the user-facing question "all contracts covered?" currently has a narrow answer: **yes for the active native pool, no for removed/future jetton contracts and no for the placeholder spec files.** Recommendation: * Delete or convert the placeholder specs. If kept, make them `.test.ts` and implement the assertions. * Replace `merkle_tree.spec.ts` with tests for the current architecture: root-update public-signal order, deposit proof rejection for stale `prevRoot`, and SDK chain-scan pagination. * Keep the coverage statement in this file and README tied to the runnable count: 60 core unit tests + 40 contract sandbox tests. *** ## Fourth-review closeout (2026-05-15): NoBounce shift + refund refactor + dead-code purge A live testnet end-to-end run on 2026-05-15 surfaced that the M-2 / M-5 / M-6 / H-4 mitigation choices broke the canonical Tornado privacy use case: withdrawing to a never-seen, uninitialized recipient address. This section closes that out, documents the broader refund refactor, and lists the dead code that was purged. ### P-1: `RichBounce` recipient payout breaks delivery to uninitialized recipients Severity: Privacy / UX Status: **Resolved** Files (pre-fix): * `apps/contracts/contracts/pool_native.tolk`: recipient send in `Withdraw` handler Evidence: The recipient payout used `BounceMode.RichBounce` with an inline `WithdrawPayout { nullifierHash, recipient }` body so `onBouncedMessage` could re-stash the entry in `stuckWithdrawals` for later retry. In TVM, an internal message sent with the bounce flag set to an `uninitialized` account is guaranteed to bounce back (no code = no handler = bounce). The testnet e2e exercised exactly this scenario: a freshly-funded but never-deployed `kQAM5iKMd0JJhSNV0DmTrD1aZYLIX3_t86SPbAe1NAfxJo1Z` as `TEST_RECIPIENT_ADDRESS`. Result: the proof verified on-chain, the nullifier was marked spent, and the 0.1 TON payout bounced into `stuckWithdrawals`. The recipient received nothing until they (a) deployed a wallet at that address with the matching mnemonic and (b) called `ReclaimStuck` from that wallet. For a privacy primitive whose user-facing promise is "withdraw to a fresh, unlinked address" this is a silent failure of the canonical use case. The git log on `shell/testnet.sh` shows the project had discovered this earlier (line 116–121, "Switched to `BounceMode.NoBounce`") and at some point swung back to `RichBounce` for the H-4/M-2 stuck- tracking benefit. The fourth review reversed that decision. Trade-off matrix: | Trade-off | `RichBounce` + body | `NoBounce` + empty body | | ------------------------------------ | ----------------------------------------- | ------------------------------- | | Uninit recipient | ✗ bounces, parks in `stuckWithdrawals` | ✓ delivered, account auto-inits | | Contract recipient that rejects | ✓ tracked, recoverable via `ReclaimStuck` | ✗ funds lost, no record | | Recipient = canonical privacy target | Broken | Works | | Recipient-bound in SNARK | Yes (user CHOSE this address) | Yes (user CHOSE this address) | For a privacy pool, NoBounce is the right pick. The lost case (contract recipient that rejects) is pathological relative to the gained case (fresh privacy-protecting recipient), and the SNARK binding means the recipient is always user-chosen, so there is no "wrong address sent the funds by mistake" recovery story to lose. Recommendation applied: * Recipient send → `BounceMode.NoBounce` + empty body. * Relayer-fee send retained at `BounceMode.Only256BitsOfBody` (relayers are always deployed wallets, bounce-on-action-fail still useful). * All bounce-tracking infrastructure (`stuckWithdrawals` dict, `stuckWithdrawalsTotal` counter, `WithdrawPayout` body struct, `ReclaimStuck` op + handler, `stuckPut/Peek/Delete` helpers, the whole `onBouncedMessage` function, `isStuck` get method) deleted as unreachable surface. * `AdminRecoverStuck`'s liability calc simplified from `outstanding * denomination + stuckWithdrawalsTotal` to `outstanding * denomination`. ### Refund refactor (L-3 extended scope) The original L-3 fix refunded `valueCoins − denomination − MIN_DEPOSIT_GAS` to the depositor. `MIN_DEPOSIT_GAS = 0.15 TON` was a gas floor masquerading as a reserve, so the pool ended up retaining 0.15 TON of "surplus" per deposit even though actual compute + send fees come in well under 0.02 TON. A new `REFUND_RESERVE: int = 10_000_000 // 0.01 TON` constant decouples the two concerns: * `MIN_DEPOSIT_GAS` / `MIN_WITHDRAW_GAS` are **floor checks**: tx fails fast if attached value is below them (guarantees enough budget for compute + sends). * `REFUND_RESERVE` is what the pool **retains** per tx for ongoing storage rent. Anything above `(committed + REFUND_RESERVE)` is refunded to `in.senderAddress` with `BounceMode.NoBounce`. Applied to every state-changing handler that accepts an internal message: | Handler | `committed` (subtracted from incoming) | Refund | | ----------------------------- | ------------------------------------------------- | ----------------------------------- | | `DepositNative` | `denomination` (retained as liability) | `attached − denomination − RESERVE` | | `Withdraw` | `denomination` (split: `recipient + relayer fee`) | `attached − denomination − RESERVE` | | `AdminPause` / `AdminUnpause` | 0 | `attached − RESERVE` | | `AdminRecoverStuck` | `msg.amount` (validated against on-chain excess) | `attached − RESERVE` | `ReclaimStuck` had a refund block in the interim refactor, but the whole handler was subsequently deleted alongside the bounce-tracking infrastructure (see P-1 above). Per-cycle accounting (testnet measurement, 2026-05-15): * User attaches `0.30 TON` to deposit. Pool refunds `0.19 TON`. Net per-leg cost = `0.01 TON + ~0.005 TON real fees`. * User attaches `0.30 TON` to withdraw. Pool refunds `0.19 TON`. Net per-leg cost = `0.01 TON + ~0.005 TON real fees`. * Full deposit + withdraw cycle: **\~0.02 TON net user cost**, vs \~0.20 TON before the refactor. **10× friction reduction per cycle.** * Pool retains `REFUND_RESERVE × 2 ≈ 0.02 TON gross / ~0.01 TON net` per cycle for storage rent. Sustainable at any realistic activity level. ### Dead code purged Deleted in one sweep with the NoBounce switch: **Contracts** ([pool\_native.tolk](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/contracts/pool_native.tolk)): * `PoolStorage.stuckWithdrawals: cell?` field * `PoolStorage.stuckWithdrawalsTotal: coins` field * `struct WithdrawPayout` (op-code 0xded057b1) * `struct ReclaimStuck` (op-code 0xded05703) * `ReclaimStuck` arm of the `AllowedMessage` union and its handler * `fun stuckPut`, `fun stuckPeek`, `fun stuckDelete` * `fun onBouncedMessage` (TVM absorbs bounces by default with no handler) * `get fun isStuck` * `stuckWithdrawalsTotal` term in `AdminRecoverStuck`'s liability calc **Errors** ([lib/errors.tolk](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/contracts/lib/errors.tolk)): * `ERR_NO_STUCK_PAYOUT` (was 106) * `ERR_RECLAIM_NOT_RECIPIENT` (was 107) **TS surface** ([packages/core/src/pool.ts](https://github.com/tonadocash/monorepo/blob/main/packages/core/src/pool.ts), [packages/core/src/constants.ts](https://github.com/tonadocash/monorepo/blob/main/packages/core/src/constants.ts)): * `TonadoPool.isStuck()` get-method wrapper * `TonadoPool.sendReclaimStuck()` + `buildReclaimStuckBody()` * The `stuckWithdrawals` + `stuckWithdrawalsTotal` bytes in `buildInitialData` * `OP_CODES.RECLAIM_STUCK` (0xded05703) * `OP_CODES.WITHDRAW_PAYOUT` (0xded057b1) **Tests** ([tests/](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/README.md)): * `pool.stuck-injected.test.ts`: deleted (only exercised the stuck flow) * `pool.bounce.test.ts`: deleted (only exercised RichBounce / `onBouncedMessage`) * `helpers.ts`: dropped `preStuck` parameter, `buildInitialDataWithStuck`, `setHashmapEntry256`, `cellValueCodec`. `deployPool` now uses `TonadoPool.buildInitialData` directly. * `pool.gas.test.ts`: dropped `reclaim_stuck` budget + test case * `pool.capacity.test.ts`: dropped the "every recipient bounces" pathological projection; the linear deposit+withdraw lifecycle is now the only path to MAX\_LEAVES (M-7 resolved by deletion). **Deploy script** ([scripts/deploy-pool.ts](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/scripts/deploy-pool.ts)): * Two trailing fields removed from the initial storage cell. **Shell** ([shell/testnet.sh](https://github.com/tonadocash/monorepo/blob/main/shell/testnet.sh)): * `h.isSpent` → `h.getIsSpent` (was a latent typo that would have failed Step 14 of the e2e once it reached that step). ### Status changes from this closeout | ID | Prior status | New status | Mechanism | | --- | ---------------------------------------------------- | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | | H-4 | Resolved (via `stuckWithdrawalsTotal` cap) | **Resolved (by elimination)** | No bounced-withdrawal liabilities exist; the `stuckWithdrawalsTotal` term is gone from the H-1 cap because there is no stuck dict | | M-2 | Resolved (via RichBounce + stuck-stash) | **Resolved (by elimination)** | No bounce-tracking surface to defend; the original M-2 risk (recipient rejects payout) is now an explicit accepted trade-off documented in P-1 | | M-5 | Resolved (recipient-only `ReclaimStuck`) | **Resolved (by elimination)** | `ReclaimStuck` doesn't exist | | M-6 | Resolved (mixed-prefix bounce parser) | **Resolved (by elimination)** | `onBouncedMessage` doesn't exist; TVM absorbs all bounces by default | | M-7 | **Open** (8.5 cells/deposit pathological worst case) | **Resolved (by elimination)** | `stuckWithdrawals` is gone; lifecycle is flat at 5.5 cells/deposit with no adversarial branch | | L-3 | Resolved (deposit-only refund) | **Resolved** (scope extended) | Refund block on every state-changing handler with `REFUND_RESERVE = 0.01 TON` | | P-1 | (new) | **Resolved** | Recipient send switched to `NoBounce` + empty body | ### Testnet evidence (run summary) * **Pool v4**: `EQBvg02q5F6Ui3h6V024FjQVOFJWwj560AT31WNVwCbKNaJY` * **Recipient** (`state: uninitialized` per toncenter `getAddressInformation`): `kQAM5iKMd0JJhSNV0DmTrD1aZYLIX3_t86SPbAe1NAfxJo1Z` * **Recipient delta**: +99,999,998 nanoTON (≈ exactly 0.1 TON, \~2 nanoTON dust from forward fees) * **Nullifier `getIsSpent`**: `true` * **`isStuck` get-method**: removed; no parking occurred * **Refunds to relayer**: 2 inbound txs from pool, 190,000,000 nanoTON each (deposit-leg + withdraw-leg) * **Manifest**: `apps/contracts/build/e2e-testnet-20260515T123232Z.json` ### Test count after this closeout * 60 off-chain unit tests (in `packages/core/`), unchanged. * 40 sandbox tests (in `apps/contracts/tests/`), down from 56 after deleting `pool.stuck-injected.test.ts` (10 specs) and `pool.bounce.test.ts` (5 specs) plus the one `reclaim_stuck` budget case in `pool.gas.test.ts`. * **Total: 100 tests, all passing.** *** ## Fifth-review update (2026-05-15): current-tree verification + coverage/doc drift + live testnet e2e Scope rechecked: * Active Tolk contract: `apps/contracts/contracts/pool_native.tolk` * Generated Tolk verifier modules: `apps/contracts/contracts/verifier.tolk` and `apps/contracts/contracts/verifier_merkle_update.tolk` * Adjacent SDK wrappers, relayer indexer paths, CLI/deploy scripts, docs, and runnable test suites. ### Current active-contract status The repo currently has one deployable Acton/Tolk pool contract: `PoolNative`. `PoolJetton` is not in `Acton.toml`; the deploy script rejects `--asset jetton:...`. Therefore "all contracts covered" means: * **Covered:** native TON pool (`PoolNative`) deposit, withdraw, admin, capacity, gas, root-update proof path, and withdraw proof path. * **Generated dependency:** withdraw verifier and merkle-update verifier are covered through real sandbox proof-verification tests, but their internals remain generated-code / TVM-BLS-opcode trust assumptions. * **Not covered / not deployable:** jetton pool behavior. Jetton support was deliberately deleted and should be treated as future work. ### Security review result No new critical or high-severity native-pool issues were found in the current working tree. The native contract enforces the important Tornado-like invariants: * deposit extends the single canonical root chain with `prevRoot == currentRoot` and `leafIndex == nextIndex`; * deposit root changes are verified by a dedicated merkle-update Groth16 proof before state changes; * withdraw proof public inputs are range-checked to canonical BLS12-381 scalar representatives before dict lookup / verifier calls; * nullifiers are marked before payout actions are staged; * recipient / relayer / fee / refund are bound to the withdraw proof; * native `refund` is rejected; * owner-only admin operations check `senderAddress == owner`; * owner recovery is capped to balance above outstanding depositor liability. Open items remain M-8, L-4, L-5, and I-3. M-8 and L-4 were already in this file; L-5 and I-3 were added by this review. ### Crypto / ZK assessment The current cryptographic plumbing is internally consistent under local tests: * `packages/core/src/poseidon.ts` uses the circom-compiled Poseidon probe wasm as the SDK hash source, avoiding a separate hand-rolled Poseidon implementation. * `scripts/check-poseidon-parity.ts` confirms sampled 2-input Poseidon values against circom compiled with `--prime bls12381`. * `withdraw.circom` public-signal order is `[root, nullifierHash, recipient, relayer, fee, refund]`, and `pool_native.tolk` builds the verifier payload in that order. * `merkleUpdate.circom` public-signal order is `[prevRoot, newRoot, commitment, leafIndex]`, and `pool_native.tolk` builds the merkle-update verifier payload in that order. * Proof points are serialized through `export-ton-verifier`'s `groth16CompressProof`, matching the generated Tolk verifier and TVM BLS opcode expectations. Limits of this confirmation: * It does not independently audit the Groth16 trusted setup; production still requires the ceremony process and attestation gate. * It does not independently verify TVM's BLS opcodes or `export-ton-verifier`; those remain chain/toolchain trust assumptions. * It does not prove Poseidon security parameters; it verifies implementation parity for the chosen circomlib/BLS12-381 setup. ### Verification performed Commands run locally on 2026-05-15: ```sh pnpm --filter @tonado/core run typecheck pnpm --filter @tonado/contracts run typecheck pnpm --filter @tonado/core run test acton build pnpm --filter @tonado/contracts run test pnpm --filter @tonado/contracts run check-poseidon-parity pnpm --filter @tonado/contracts run format:tolk:check pnpm --filter @tonado/core build pnpm -r typecheck pnpm e2e:testnet pnpm --filter @tonado/contracts exec tsx scripts/topup.ts --to EQBnt2DRzNmLkpvSyGQPOToGiijTXjIGqraSMcuvzArC1WiG --amount 0.25 --network testnet REUSE_DEPLOYMENT=1 SKIP_PTAU=1 SKIP_CIRCUITS=1 SKIP_VERIFIER_EXPORT=1 SKIP_POSEIDON_PARITY=1 SKIP_ACTON_BUILD=1 SKIP_SANDBOX_TESTS=1 pnpm e2e:testnet ``` Results: * Core unit tests: 60 passed. * Contract sandbox tests: 40 passed before and after a fresh `acton build`. * Poseidon parity: all sampled circom/core pairs matched. * Tolk formatting: passed. * Full workspace TypeScript typecheck: passed. Live testnet e2e result: * A full `shell/testnet.sh` run regenerated circuits/verifiers, passed Poseidon parity, built the Tolk contract, passed 40 sandbox tests, deployed a fresh 0.1 TON native pool, and verified it on-chain. * The first deploy attempt was blocked by a transient Toncenter `LITE_SERVER_UNKNOWN` backend timeout before broadcast. The next run deployed and verified the pool successfully. * The deployed pool was `EQD5cHZDuvBNgahnrpZQkk7xUb29grcaYLRm0d_aUkE-Ur1c`. * After deployment, Step 10 correctly failed because the configured user wallet had only `0.592080889 TON`, below the script's `0.7 TON` smoke-test threshold. The deployer then topped that wallet up by `0.25 TON`; re-check showed `0.8420444 TON`. * The smoke path was resumed with `REUSE_DEPLOYMENT=1` against the same verified pool. Deposit generated a merkle-update proof in `8.8s`, landed at `leafIndex=0`, and advanced `get_next_index` to `1`. * Withdraw generated a proof in `5.0s`, broadcast with `nullifierHash = 0x5259e2e63f1a9110e7c686a4f705f3785f9debd3c5370053715f6ab2d7a15e82`, credited the recipient by `99,999,995 nanoTON`, and `get_is_spent(nullifierHash)` returned true. * Manifest: `apps/contracts/build/e2e-testnet-20260515T130741Z.json`. Static analyzers: * `misti` and `tsa` were not installed locally, and the current CI file explicitly documents Misti as disabled until useful Tolk support exists. Keep professional TON + ZK audit as a required pre-mainnet gate. ## Acton check warnings noted by second review `acton check` reported the following advisories. None are direct vulnerabilities; cleaning them reduces audit-noise for the next pass: * `verifier_merkle_update.tolk` symbol style (autogenerated; the `mu`-prefix mangler in `export-verifier.ts` doesn't follow Tolk's preferred casing). * Write-only-variable warnings around `mutate payload` use in `pool_native.tolk`. * Error-code style: prefer enum-based symbolic errors over loose `const` values in `lib/errors.tolk`. ## Positive Observations * Native pool has no external-message handler (no external replay surface). * Uses Tolk struct/union message dispatch and typed `address`. * Range-checks Groth16 public inputs against the BLS12-381 scalar field before any dict lookup or proof verification. * Native value-bearing sends use `SEND_MODE_BOUNCE_ON_ACTION_FAIL` for payout legs whose destinations are deployed wallets (relayer fee, admin recover). The recipient payout uses `NoBounce` deliberately to match the canonical Tornado privacy use case (fresh, uninitialized recipient). See P-1 in the fourth-review closeout. * Circuit/core Poseidon parity passes on sampled values (now a CI gate). * Every state-changing handler refunds excess attached value to the sender, keeping only `REFUND_RESERVE = 0.01 TON` per tx for storage rent. Pool balance ≈ outstanding deposits × denomination, with no admin-recoverable excess accumulating (L-3 closeout, extended). * Enforces canonical deposit-chain extension: `prevRoot == currentRoot` and `leafIndex == nextIndex` (C-1 closeout). * 100 tests pass (60 off-chain unit + 40 sandbox). Down from 116 after the dead-code purge (see fourth-review closeout for the deleted M-2/M-5/M-6 surfaces). ## Verification Performed ```sh pnpm -r test pnpm --filter @tonado/contracts run check-poseidon-parity pnpm --filter @tonado/contracts exec acton build pnpm --filter @tonado/contracts run format:tolk:check ``` Results (post-fourth-review closeout, rechecked in the fifth review): all green. 100 runnable tests cover deposit/withdraw happy paths, the C-1 attack variants, the H-1 cap (excess/overshoot, simplified after H-4 elimination), the M-4 gas budgets, the L-3/refund-refactor coverage on every entry point, the empty-body deploy/top-up path, and the address↔field encoding. Additionally, a real testnet end-to-end run (`shell/testnet.sh`) on 2026-05-15 against pool `EQBvg02q5F6Ui3h6V024FjQVOFJWwj560AT31WNVwCbKNaJY` verified the NoBounce delivery path against a `state: uninitialized` recipient address. Recipient credited 99,999,998 nanoTON (\~0.1 TON), nullifier marked spent, two refund txs (deposit + withdraw legs) of 0.19 TON each delivered back to the relayer wallet. Not run: * Misti / TSA static analysis on the maintainer's machine. Fifth-review recheck found neither command installed locally, and current CI documents Misti as disabled until a useful Tolk-aware analyzer path exists. Treat external TON + ZK audit as mandatory before mainnet. * The bounce-end-to-end sandbox test (rejecting recipient contract) is no longer needed: with the recipient send at `NoBounce`, there is no contract-driven bounce path. The pool also no longer defines `onBouncedMessage`; TVM absorbs bounces by default. The previously-cited `pool.stuck-injected.test.ts` and `pool.bounce.test.ts` were deleted as part of the dead-code purge (see fourth-review closeout). ## Closeout summary (per new finding) Quick reference for what landed for each open finding from the second review. Code locations are linked from the per-finding sections above. | ID | Resolution | | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | **H-4** | `stuckWithdrawalsTotal: coins` in `PoolStorage`; bumped in `onBouncedMessage`, drained in `ReclaimStuck`; subtracted from the H-1 cap in `AdminRecoverStuck`. Three new sandbox tests in `pool.stuck-injected.test.ts`. | | **M-1** | `MAX_LEAVES` capped from 2²⁰ → 2¹³ (8,192), backed by a cell-growth measurement in `tests/pool.capacity.test.ts`. Projection: \~45K cells for the measured deposit+withdraw lifecycle, leaving \~30% headroom under the 65,536 cell account limit. CI runs the measurement to catch storage-bloat regressions. See **M-7** for the still-open all-recipients-bounce worst case. | | **M-5** | `ReclaimStuck` now requires `in.senderAddress == storedRecipient`. New error code `ERR_RECLAIM_NOT_RECIPIENT = 107`. The dict-take was split into `stuckPeek` + `stuckDelete` so a failed auth check no longer consumes the entry. Three sandbox tests cover bystander-reject, recipient-accept, and not-found. | | **M-6** | `onBouncedMessage` inspects the bounce prefix (`0xFFFFFFFE` rich vs. `0xFFFFFFFF` legacy) before parsing. Legacy bounces from relayer-fee / admin-recover sends are silently absorbed; rich bounces route to the M-2 stuck-stash logic. | | **L-3** | Deposit handler refunds `valueCoins − denomination − MIN_DEPOSIT_GAS` to `in.senderAddress` with `BounceMode.NoBounce` after the LOG\_DEPOSIT emit. Eliminates the admin-recoverable excess and reduces fee-fingerprint risk. | | **I-2** | `deploy-pool.ts` calls `enforceMainnetCeremonyGate()` whenever `--network mainnet`. The gate reads `deploy/ceremony/MAINNET_ATTESTATION.txt`, verifies both `withdraw_final.zkey` and `merkleUpdate_final.zkey` hashes match on-disk artifacts byte-for-byte, and requires ≥ 3 `signed-by:` entries. Mirrors `shell/mainnet.sh` GATE 1 at the lowest layer. Emergency override `TONADO_ALLOW_UNATTESTED_MAINNET=1` logs loudly. | | **L-5** | Open fifth-review item: docs and public SDK/CLI surfaces still describe removed jetton / on-chain-merkle contracts. No on-chain exploit because deploy hard-errors for jetton, but audit/user-facing materials should be updated. | | **I-3** | **Resolved (sixth-review)**: `merkle_tree.spec.ts` and `pool_native.spec.ts` deleted; `circuit.spec.ts` stripped of its empty stub function. `apps/contracts/README.md` and `docs/circuit-spec.md` updated. | **Current runnable test count after the NoBounce/dead-code closeout and fifth-review recheck:** 60 off-chain unit + 40 sandbox = **100 tests**, all green. *** ## Sixth-review closeout (2026-05-15): priority-fix pass after the consolidated review Following the consolidated audit report, the top-10 priority items were worked through in order. Status of each: | # | Item | Status | | -- | ------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | 1 | CI guard that catches a circuit edit landing without a re-snapshot | **Resolved (revised approach)**: originally a `git diff --exit-code` on regenerated `verifier.tolk`, but that gate was unsatisfiable because snarkjs's `getRandomRng` mixes `crypto.randomBytes(64)` into every contribution regardless of the `-e` flag, drifting the per-circuit `delta` on every run. Replaced with a structural-shape snapshot at [`apps/contracts/circuits/circuit-shape.json`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/circuits/circuit-shape.json) compared by [`apps/contracts/scripts/check-circuit-shape.ts`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/scripts/check-circuit-shape.ts) and wired into [`.github/workflows/ci.yml`](https://github.com/tonadocash/monorepo/blob/main/.github/workflows/ci.yml). Catches constraint-count, public/private-input-count, protocol, and curve drift (the entire class of meaningful circuit edits) while staying stable across snarkjs's deliberate non-determinism. | | 2 | Delete the three stub spec files | **Resolved**: `merkle_tree.spec.ts` and `pool_native.spec.ts` deleted; `circuit.spec.ts`'s empty stub function removed | | 3 | Emit `OP_LOG_WITHDRAW` from the Withdraw handler (M-8) | **Resolved**: see M-8 section below | | 4 | Stale-but-known-root withdraw test (Tornado privacy property) | **Resolved**: [`pool.withdraw.test.ts`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.withdraw.test.ts) `stale-but-known root` describe block | | 5 | Bounce-on-action-fail behavioural coverage | **Documented limitation**: added two `it.todo` placeholders in [`pool.withdraw.test.ts`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.withdraw.test.ts) explaining why `@ton/sandbox` cannot reliably force action-phase failure; static review of the `+16` flag at [pool\_native.tolk:449,:457,:525](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/contracts/pool_native.tolk#L449) remains the load-bearing guarantee | | 6 | Complete field-range negative test matrix | **Documented reachability**: only `commitment`, `newRoot`, `root`, `nullifierHash` are reachable from external inputs. The other four asserts (`prevRoot`, `fee`, `refund`, `recipientField`, `relayerField`) are unreachable by type/state invariants; kept as defense-in-depth with explanatory comments at [pool\_native.tolk:319-330, :393-405, :407-413](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/contracts/pool_native.tolk#L319) and matching docstrings in the test files | | 7 | Wrong-vkey cross-test (merge proof in withdraw slot) | **Resolved**: [`pool.withdraw.test.ts`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.withdraw.test.ts) `verifier namespacing` describe block; merge proof in the withdraw slot is rejected at the pairing check | | 8 | `leafIndex == nextIndex` isolation test | **Resolved**: new `rejects leafIndex != nextIndex even when prevRoot matches (isolated check)` test in [`pool.deposit.test.ts`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.deposit.test.ts) | | 9 | Fix `loadAllCommitments` early-break (L-4) | **Resolved**: see L-4 closeout below | | 10 | Reconcile `ZERO_VALUE` doc comments | **Resolved**: comments in [`constants.ts:27-42`](https://github.com/tonadocash/monorepo/blob/main/packages/core/src/constants.ts#L27), [`pool_native.tolk:73`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/contracts/pool_native.tolk#L73), [`merkleUpdate.circom:70`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/circuits/merkleUpdate.circom#L70), [`docs/architecture.md`](https://github.com/tonadocash/monorepo/blob/main/docs/security/docs/architecture.md), [`docs/ton-vs-evm.md`](https://github.com/tonadocash/monorepo/blob/main/docs/security/docs/ton-vs-evm.md) all now consistently document the seed as `keccak256("tornado") mod r`; value itself was already consistent | ### M-8: `WithdrawLog` emission (Resolved) The Withdraw handler now emits a `WithdrawLog` external-out message right after `storage.save()`, before the value-bearing sends. Layout: ``` struct (0xded057a2) WithdrawLog { nullifierHash: uint256 recipientField: uint256 // addressToField(core.recipient) relayerField: uint256 // addressToField(core.relayer) fee: coins timestamp: uint64 } ``` This is the exact layout the relayer indexer already decoded at [apps/relayer/src/libs/Indexer.ts:68-87](https://github.com/tonadocash/monorepo/blob/main/apps/relayer/src/libs/Indexer.ts#L68), so no indexer change was needed. A new sandbox test (`emits a LOG_WITHDRAW external-out matching the relayer indexer layout (M-8)` in [`pool.withdraw.test.ts`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.withdraw.test.ts)) parses the emitted body and asserts each field byte-for-byte, including the `addressToFieldElement(...)` round-trip on both recipient and relayer. Atomicity: the log is emitted in the same compute phase as the nullifier write and the value sends. The `+16` flag on the recipient and relayer sends causes any action-phase failure to roll back the compute phase atomically. Log, nullifier, and storage all revert together. Gas: withdraw measured at \~120K (was \~115K). Still well under the budget of 170K in [`pool.gas.test.ts`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.gas.test.ts). ### L-4: `loadAllCommitments` pagination fix (Resolved) [`packages/core/src/depositFlow.ts:140-167`](https://github.com/tonadocash/monorepo/blob/main/packages/core/src/depositFlow.ts#L140) no longer early-breaks when a particular transaction page carries zero deposit events; it only stops when the indexer reports the scan is genuinely exhausted (`lastLt` undefined or unchanged from the cursor). Additionally, the returned commitments are now explicitly sorted by `leafIndex` so the merkle-tree builder receives them in chronological order regardless of toncenter's page-traversal direction. The function is now exported and covered by four unit tests in [`packages/core/src/depositFlow.test.ts`](https://github.com/tonadocash/monorepo/blob/main/packages/core/src/depositFlow.test.ts): pagination past empty pages, leafIndex sort, empty pool, and the cursor-stall defensive break. ### Verification ```sh pnpm format:tolk:check # pass pnpm -r typecheck # pass pnpm --filter @tonado/contracts exec acton build # pass pnpm --filter @tonado/contracts run check-poseidon-parity # pass (all samples) pnpm --filter @tonado/core test # 64 passing (was 60) pnpm --filter @tonado/contracts test # 44 passing + 2 todo ``` **Current runnable test count after the sixth-review closeout:** **108 tests** (64 off-chain unit + 44 sandbox), all green, plus 2 documented `it.todo` placeholders for the bounce-on-action-fail behavioural gap. Net change vs. fifth review: +4 off-chain (loadAllCommitments) and +4 sandbox (leafIndex isolation, stale-root, wrong-vkey, LOG\_WITHDRAW emit). *** ## H-5: L-3 refund refactor leaves denomination stranded as admin-recoverable excess Severity: **High** (centralisation / fund-stranding, not direct theft) Status: **Resolved (2026-05-15, sixth-review)**, fix verified on-chain testnet ### Files * `apps/contracts/contracts/pool_native.tolk`: Withdraw handler refund block * `apps/contracts/tests/pool.withdraw.test.ts`: new regression test ### Discovery context Surfaced during the post-fix sixth-review testnet e2e. After a full deposit → withdraw cycle on pool `EQDRMwnqPevwSQPF3tIYW7lNzTOhH4EQzOgWhqfi2GEO60X7`, the on-chain balance was `0.6045 TON` instead of the expected `~0.505 TON` (deploy reserve + 2 × REFUND\_RESERVE − fees). The difference was **exactly one denomination** (0.1 TON) per cycle. ### Evidence (pre-fix on-chain accounting) | Tx | Inbound | Outbound | Pool delta | | -------- | ------------------------------------- | ---------------------------------------- | ---------------------------------------------------------- | | Deploy | +0.5 TON | n/a | +0.4999 | | Deposit | +0.3 TON (denomination 0.1 + gas 0.2) | 0.19 refund + log | +0.1027 (= denomination 0.1 + RESERVE 0.01 − fees 0.007 ✓) | | Withdraw | +0.3 TON (relayer gas) | **0.1 to recipient + 0.19 refund + log** | **+0.0020** ← should be `−0.098` | After full cycle: `outstanding = 0`, `liability = 0`, so by the H-1 cap `excess = balance = 0.6045 TON`, all admin-recoverable, including the depositor's 0.1 TON denomination. ### Root cause The pre-fix Withdraw refund formula at [pool\_native.tolk:475](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/contracts/pool_native.tolk) was: ```tolk val refundExcess = in.valueCoins - storage.denomination - REFUND_RESERVE; ``` The accompanying comment claimed *"Already-committed outbound value equals denomination (toRecipient + core.fee)"*, true literally but misleading about the funds source. **The denomination outflow at withdraw time is funded from the pool's balance**, not from the relayer's attached value. The depositor's denomination was credited to the pool's balance at deposit time and held as liability. The relayer's attached value at withdraw time covers gas + REFUND\_RESERVE only. So the pre-fix formula short-refunded the relayer by exactly `denomination` per withdraw, leaving that amount stranded as admin-recoverable excess. ### Impact * Centralisation / governance: every complete cycle accrues `denomination` worth of admin-recoverable excess. After N cycles on a 0.1 TON pool, \~0.1 × N TON is reachable by the pool owner via `OP_ADMIN_RECOVER_STUCK`. The H-1 cap does not prevent this because `liability` (computed as `outstanding × denomination`) zeroes out the moment `nullifiersCount` catches up to `nextIndex`. * Relayer economics: relayers consistently overpay by \~denomination per withdraw they submit. For a 1-TON pool with many cycles, this could be meaningful operational cost. * Not a SNARK-level theft: the recipient is still paid the correct `denomination − fee`; the nullifier still marks spent. The withdraw succeeds. The funds are stuck in the pool rather than lost outright. ### Fix ```tolk val refundExcess = in.valueCoins - REFUND_RESERVE; ``` The `- storage.denomination` term was removed. The deposit-side refund formula stays as-is (correct, because deposit attached value DOES include denomination). ### Verification **Sandbox regression test:** `H-5: per-cycle pool balance growth is bounded by REFUND_RESERVE (not denomination)` in [`pool.withdraw.test.ts`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.withdraw.test.ts). Deploys a pool, captures balance, runs a complete cycle, asserts pool growth < `2 × REFUND_RESERVE`. Catches the regression if the formula reverts. **Testnet e2e (post-fix), pool `EQDSA92w1kYxhBoJOjPjDIAiVsvfPqOkmjJyMYnAbGuVIYOv`:** | Tx | Inbound | Outbound | Pool delta | | -------- | -------- | ---------------------------------------- | ------------------------------------------------ | | Deploy | +0.5 TON | n/a | +0.4999 | | Deposit | +0.3 TON | 0.19 refund + log | +0.1027 (same as before; deposit side unchanged) | | Withdraw | +0.3 TON | **0.1 to recipient + 0.29 refund + log** | **−0.0980** ✓ | Final balance: **0.5046 TON** (vs 0.6045 TON pre-fix; exactly 0.1 TON recovered per cycle). Excess relative to liability is now ≈ deploy reserve * minor REFUND\_RESERVE accrual, as intended. Recipient delta: 99,999,998 nanoTON (only 2 nanoTON forward fee). Manifest: [`apps/contracts/build/e2e-testnet-20260515T140851Z.json`](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/build/e2e-testnet-20260515T140851Z.json). ### Status changes | ID | Prior status | New status | | --- | --------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | L-3 | Resolved (deposit + withdraw refund refactor) | **Partially resolved**: deposit side correct; withdraw side had the H-5 accounting bug, now fixed. The L-3 closeout's "10× friction reduction per cycle" measurement was correct on the relayer's perceived cost but missed the pool's accumulating excess | | H-5 | (new) | **Resolved** | ### Test count after H-5 closeout * 64 off-chain unit (unchanged) * 45 sandbox (was 44, +1 H-5 regression) * 2 documented `it.todo` * **Total: 109 tests, all green** ## TON References Used * TON sending modes: * TON internal messages and bounce formats: * Tolk message handling and `BounceMode`: * TON external message replay guidance: * TON blockchain limits: * Jetton TEP-74 layout overview (deferred; jetton pool deleted): *** ## Phase 1 pre-audit hardening closeout (2026-05-15) Following the consolidated `MAINNET_LAUNCH_PLAN.md` Phase 1 work plan, the following items landed as a single hardening pass before audit handoff. ### Items completed | Phase 1 item | Resolution | | ------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | 1. Bounce-on-action-fail behavioural tests | Sandbox state manipulation (`blockchain.getContract(addr).balance = X`) used to force recipient-send and relayer-fee-send action-phase failures. Both confirm the `+16 SEND_MODE_BOUNCE_ON_ACTION_FAIL` flag rolls back compute (nullifier remains unspent). Two new tests in [pool.withdraw.test.ts](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.withdraw.test.ts) replace the prior `it.todo` placeholders. | | 2. Shared LOG\_WITHDRAW decoder | `WithdrawEvent` interface added to [packages/core/src/types.ts](https://github.com/tonadocash/monorepo/blob/main/packages/core/src/types.ts); `decodeLogWithdrawOutMessage` exported from [packages/core/src/indexer.ts](https://github.com/tonadocash/monorepo/blob/main/packages/core/src/indexer.ts) mirroring the existing deposit decoder; relayer's [apps/relayer/src/libs/Indexer.ts](https://github.com/tonadocash/monorepo/blob/main/apps/relayer/src/libs/Indexer.ts) refactored to use the shared decoder; 6 new unit tests in [indexer.test.ts](https://github.com/tonadocash/monorepo/blob/main/packages/core/src/indexer.test.ts); sandbox emit test now flows through the shared decoder for byte-for-byte parity. | | 3. L-5 user-facing doc cleanup | README updated to describe native-only v1 with v1.1 jetton roadmap markers; `apps/cli/src/commands/deposit/jetton.ts` rewritten to hard-fail with a clear message; deploy script's CLI help no longer offers `--asset jetton:...` as a current option. `circuit-spec.md` and `architecture.md` updated in prior closeout. | | 4. MIN\_WITHDRAW\_GAS tuning | Lowered from `200_000_000` (0.20 TON) to `50_000_000` (0.05 TON), 3× headroom over measured \~0.015 TON real cost, 5× over REFUND\_RESERVE. New `insufficient gas` test in [pool.withdraw.test.ts](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.withdraw.test.ts) exercises the floor (previously untested). | | 5. N-2 / N-3 / N-4 defense-in-depth | **N-2 / N-3:** Tolk's typed `address` deserializer already rejects addr\_none / addr\_extern before either handler runs. Added audit-clarifying comments at [pool\_native.tolk:419, :525](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/contracts/pool_native.tolk) explaining the type-system guarantee. **N-4:** Added `assert(msg.amount > 0) throw ERR_RECOVER_EXCEEDS_EXCESS` to `AdminRecoverStuck` + regression test. | | 6. Burn-key owner model + deploy mode | `docs/threat-model.md` extended with "Owner / governance model" section detailing the mainnet burn-key stance, its trade-offs (no kill-switch, no recover-stuck) and storage-rent sustainability. `deploy-pool.ts` accepts `--burn-owner` (and `TONADO_BURN_OWNER=1`) which writes `TonadoPool.BURN_ADDRESS` (workchain=0, hash=0x00…00) as the owner. The Tolk `address` type rejects addr\_none at parse time (exit 9), so the burn pattern uses a sentinel addr\_std with no recoverable private key instead, same operational effect, no storage-struct change required. New `burn-key owner (mainnet stance)` describe block in [pool.admin.test.ts](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.admin.test.ts) with 4 tests: clean deploy, AdminPause / AdminUnpause / AdminRecoverStuck all rejected from the deploying wallet, and a regular deposit cycle still works. | | 7. Contract freeze | Commit ready for tagging once these closeouts are reviewed. | ### Implementation details: burn-key sentinel The naïve burn-key approach (`storeAddress(null)` → addr\_none in the data cell) was the first attempt and failed at runtime: every get-method call returned exit code 9 (cell underflow) because Tolk's typed `PoolStorage.owner: address` deserializer expects addr\_std (prefix `10`) and rejects addr\_none (prefix `00`). Two options were considered: 1. Change `owner: address` to `owner: any_address` in the storage struct. This would allow true addr\_none but **changes the storage layout's binary format**, expanding audit scope. 2. Use a canonical "burned" addr\_std (workchain=0, hash=0x00…00), a valid addr\_std with no recoverable private key. Same operational effect, storage layout unchanged. Option 2 was chosen. The constant is exposed as `TonadoPool.BURN_ADDRESS` in `@tonado/core` and used by `buildInitialData({ owner: null })` and the deploy script's `--burn-owner` flag. ### Test count after Phase 1 closeout * 70 off-chain unit tests in `packages/core/` (was 64; +6 indexer decoder tests) * 54 sandbox tests in `apps/contracts/tests/` (was 45; +9 new): * 2 bounce-on-action-fail (replacing `it.todo` placeholders) * 1 ERR\_INSUFFICIENT\_GAS withdraw * 1 N-4 zero-amount admin recover * 4 burn-key owner (deploy, AdminPause reject, AdminUnpause reject, AdminRecoverStuck reject, deposit cycle still works) * 1 H-5 pool-balance growth bound (from prior closeout) * 0 `it.todo` placeholders remaining * **Total: 124 tests, all green** ### Verification ```sh pnpm format:tolk:check # pass pnpm -r typecheck # pass (excl. apps/dapp; preexisting, missing pnpm install) pnpm --filter @tonado/contracts exec acton build # pass pnpm --filter @tonado/contracts run check-poseidon-parity # pass (all samples) pnpm --filter @tonado/core test # 70 passing pnpm --filter @tonado/contracts test # 54 passing ``` ### Mainnet-readiness gates closed by Phase 1 * ✅ All known findings resolved (no Open Critical / High / Medium) * ✅ No `it.todo` test placeholders * ✅ Burn-key deploy path implemented + sandbox-verified * ✅ Shared decoder for contract↔relayer log-format parity * ✅ Field-range reachability matrix documented in contract source ### Mainnet-readiness gates STILL OPEN * ❌ External professional audit (Phase 2) * ❌ Multi-party trusted setup ceremony (Phase 3) * ❌ `MAINNET_ATTESTATION.txt` from real contributors (not the dev-entropy zkey) * ❌ Relayer production infrastructure (Phase 4) * ❌ Bug bounty program funded + announced (Phase 5) *** ## Refund-correctness regression suite (Phase-1 hardening follow-up, 2026-05-15) After Phase-1 wrap-up, an in-depth audit of the refund mechanism surfaced a gap: deposit had an over-attach regression test (the original L-3 test, with a loose `< denomination + 0.5 TON` upper bound) but withdraw had NO over-attach test at all. Given the relayer flow is exactly "attach arbitrary gas, get refunded what wasn't used", this was the highest-impact missing coverage for mainnet readiness. ### Refund-formula audit (every state-changing handler) | Handler | Formula | What user attaches | What pool keeps | What pool sends out | | --------------------- | ------------------------------------------ | ------------------ | ------------------------------------------ | -------------------------------------------------------------- | | `DepositNative` | `attached − denomination − REFUND_RESERVE` | gas + denomination | denomination (liability) + REFUND\_RESERVE | refund to depositor | | `Withdraw` (post-H-5) | `attached − REFUND_RESERVE` | gas only | REFUND\_RESERVE only | denomination to recipient + fee to relayer + refund to relayer | | `AdminPause` | `attached − REFUND_RESERVE` | gas only | REFUND\_RESERVE only | refund to admin | | `AdminUnpause` | `attached − REFUND_RESERVE` | gas only | REFUND\_RESERVE only | refund to admin | | `AdminRecoverStuck` | `attached − REFUND_RESERVE` | gas only | REFUND\_RESERVE only | recovered amount to `msg.to` + refund to admin | **Invariant the regression suite locks in:** per-tx pool balance growth ≈ `REFUND_RESERVE − action_fees ≈ 0.005 TON net`. Nothing more. Any future formula drift that leaks attached value to admin-recoverable excess (the H-5 bug pattern) fails the regression. ### New tests added (5) 1. **`relayer gets refunded ~all attached value minus REFUND_RESERVE on a huge overpay`** ([pool.withdraw.test.ts](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.withdraw.test.ts)): attaches **10 TON** to a withdraw, asserts relayer's net cost is `< REFUND_RESERVE + 0.05 TON` (≈ 0.06 TON, not 10 TON). The single most important regression test for H-5-style accounting bugs. 2. **`relayer gets refunded correctly on a tiny overpay just above MIN_WITHDRAW_GAS`**: attaches **0.06 TON** (0.01 above the new floor) and asserts refund correctness at the boundary. 3. **`recipient receives exactly denomination − fee regardless of overpay`**: privacy regression test. Proves attached value never leaks to recipient (which would create a "size of overpay" side-channel on the recipient delta). 4. **`refunds even huge overpays (100 TON) to the depositor`** ([pool.deposit.test.ts](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.deposit.test.ts)): deposit-side stress test at 100 TON; proves no upper bound on the refund formula. 5. **`refunds overpay on AdminPause / AdminUnpause / AdminRecoverStuck`** ([pool.admin.test.ts](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.admin.test.ts)): admin paths verified to use the same refund property. Plus the **existing L-3 deposit overpay test was tightened** from `netSpent < denomination + 0.5 TON` (loose, 500m nanoTON of slack) to `< denomination + REFUND_RESERVE + 0.05 TON` (precise, ≈ 60m nanoTON of slack). Any future drift in the deposit formula will trip this test. ### Test count after refund-correctness closeout * 70 off-chain unit tests * 59 sandbox tests (was 54; +5 new refund tests) * **129 tests, all green** ## H-6: Empty-body / bounce: false accidental TON loss Severity: **Medium** (user-fund safety on mainnet under burn-key; recoverable on testnet via admin paths that still work there) Status: **Resolved (Phase-1 hardening, 2026-05-15)** ### Files * `apps/contracts/contracts/pool_native.tolk`: `onInternalMessage` empty-body branch * `apps/contracts/tests/pool.deposit.test.ts`: 3 new sandbox tests under `empty body (state-fresh heuristic)` * `apps/contracts/scripts/seed-pool.ts`: **new** operator-driven seed-deposit script * `apps/contracts/package.json`: new `pnpm seed-pool` script entry * `shell/mainnet.sh`: **new GATE 7** (seed deposit, state-fresh window close-out) * `docs/threat-model.md`: "Owner / governance model" section covers the operational pattern ### Background Under `--burn-owner` mainnet (the chosen owner model, see [docs/threat-model.md](https://github.com/tonadocash/monorepo/blob/main/docs/security/docs/threat-model.md)), the admin handlers (`AdminPause`, `AdminUnpause`, `AdminRecoverStuck`) are permanently uncallable because `senderAddress == BURN_ADDRESS` can never hold for any inbound message. This means any TON that ends up "stranded" in the pool via the empty-body short-circuit cannot be recovered by anyone, ever. The original empty-body handler was a silent `return`, retaining attached value for two intended cases (deploy state-init + intentional top-ups). On burn-key mainnet there is no operator who would intentionally top up (the pool is self-sustaining via REFUND\_RESERVE accrual at \~0.005 TON net per tx), but a third party who accidentally sends TON to the pool with empty body would lose the entire attached value permanently. **TVM bounce semantics asymmetry:** * `bounce: true` (default for EQ-prefix friendly addresses): if compute throws, TVM auto-bounces the inbound message, returning attached value minus bounce fees. **Self-recovering.** * `bounce: false` (UQ-prefix non-bounceable addresses, explicit `{ bounce: false }` in code): TON is credited to the contract during the credit phase (which runs **before** compute). Whatever compute does after that (return, throw, refund), the TON is already in the pool's balance. **The only way to refund is for compute to emit an explicit outbound message during the action phase.** The original silent-return retained TON for BOTH bounce: true and bounce: false sends. The pre-H-6 design therefore had two distinct loss paths: * bounce: true accident → silent return succeeds → TON retained (no bounce triggered) * bounce: false accident → silent return succeeds → TON retained (bounce never possible) ### Resolution A two-layer fix: a contract-level state-fresh heuristic, plus an operational seed-deposit step that closes the brief pre-first-deposit window. **Layer 1: State-fresh heuristic in `onInternalMessage`** (`pool_native.tolk:319-348`): ```tolk var storage = PoolStorage.load(); if (in.body.remainingBitsCount() == 0 && in.body.remainingRefsCount() == 0) { if (storage.nextIndex == 0 && storage.nullifiersCount == 0) { // Fresh state → deploy message → retain attached value. return; } // Post-fresh: refund attached − REFUND_RESERVE to sender, NoBounce // so the refund lands on uninitialised destinations and the // sender's own wallet alike. val refundExcess = in.valueCoins - REFUND_RESERVE; if (refundExcess > 0) { val refundMsg = createMessage({ bounce: BounceMode.NoBounce, dest: in.senderAddress, value: refundExcess, }); refundMsg.send(SEND_MODE_PAY_FEES_SEPARATELY); } return; } ``` The contract distinguishes: * **Deploy-time empty-body** message → sees fresh state (nextIndex=0, nullifiersCount=0) → retains. * **Post-deploy empty-body** message → sees at least one deposit landed → refunds attached − REFUND\_RESERVE via `BounceMode.NoBounce` so the refund lands on uninitialised destinations too. Cost of the heuristic: one storage load on every empty-body message (\~500 gas). Empty-body messages are rare; this is noise. **Layer 2: Operator seed deposit at deploy time** (`apps/contracts/scripts/seed-pool.ts`, gated by `shell/mainnet.sh` GATE 7): Immediately after deploy + verify, the operator funds a single deposit from `DEPLOYER_MNEMONIC`. The note is printed ONCE to the operator's terminal and discarded (never saved), so the seed deposit is permanently locked in the pool. This: 1. Flips `getNextIndex()` from 0 → 1. 2. Closes the fresh-state window in the contract's heuristic. 3. Permanently locks one denomination into the pool's anonymity set (incidentally improving the first real depositor's anonymity). After GATE 7 the pool is safe to advertise publicly. The mainnet wrapper **refuses to exit** without completing GATE 7: post-seed `getNextIndex == 1` is asserted, and on failure the script demands manual completion via `pnpm seed-pool` before announcement. **Cost to operator:** 1 × denomination + \~0.01 TON one-time. For a 1-TON pool, \~1.01 TON; for a 10-TON pool, \~10.01 TON. ### Verification **Sandbox tests** in [pool.deposit.test.ts](https://github.com/tonadocash/monorepo/blob/main/apps/contracts/tests/pool.deposit.test.ts) under `empty body (state-fresh heuristic)`: 1. **`retains attached value on a fresh pool (deploy/state-init path)`**: verifies the deploy path. Pool balance grows by approximately the full attached value (asserted via balance-delta, not absolute value, to be sandbox-deploy-amount-agnostic). 2. **`refunds attached value on a post-first-deposit pool (accident-recovery path)`**: a "stranger" sends 3 TON with `bounce: false` after a real deposit has happened; net loss is bounded by `REFUND_RESERVE + 0.05 TON ≈ 0.06 TON`, NOT the full 3 TON. 3. **`retains accidental sends in the brief pre-first-deposit window (documented limitation)`**: pins the operational limitation that GATE 7 exists to mitigate. Pre-seed pool is not safe to advertise; this test exists so future contributors understand WHY the seed step is required. ### Test count after H-6 closeout * 70 off-chain unit tests * 61 sandbox tests (was 59; +2 for the heuristic + 1 for the documented limitation) * **131 tests, all green** ### Trade-offs accepted * **Pre-first-deposit window** (between deploy tx landing and seed tx landing, typically <60 seconds) is operationally controlled by not advertising the pool address until GATE 7 completes. The shell script enforces this ordering with the post-seed `getNextIndex == 1` assertion. * **Burn-key pools cannot accept legitimate top-ups** post-launch. Any third party who sends TON to "keep the pool alive" will be refunded. Under burn-key this is correct behavior. REFUND\_RESERVE accrual covers storage rent indefinitely; there's no operator who should be receiving externally-funded top-ups. * **Seed deposit's denomination is permanently locked.** For high-denomination pools (10 TON, 100 TON) this is a non-trivial operator cost, but it's a one-time launch expense and the locked TON contributes positively to the first real depositor's anonymity set. ### Operational note on testnet vs. mainnet behavior Testnet pools that were deployed during this session **before** the H-6 heuristic landed retained TON on empty-body sends and were recoverable via `AdminRecoverStuck` (deployer-owned pools). All five such pools were drained back to the deployer wallet in the "test contracts to withdraw orphaned TON from" cleanup (\~2.10 TON recovered). Future testnet deploys with the H-6 heuristic in place will refund accidental sends just like mainnet, except that the deployer wallet is still the owner on testnet (not burned) so admin recovery remains available as a backstop during iteration. ### Final verification ```sh pnpm format:tolk:check # pass pnpm --filter @tonado/contracts exec acton build # pass pnpm --filter @tonado/contracts run check-poseidon-parity # pass (all samples) pnpm --filter @tonado/core test # 70 passing pnpm --filter @tonado/contracts test # 61 passing ``` **Total Phase-1 closeout: 131 tests, zero open Critical/High/Medium findings, zero `it.todo` placeholders, burn-key + state-fresh + refund mechanism all sandbox-verified, ready for external audit handoff.** --- # Agent Instructions: Querying This Documentation If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question. Perform an HTTP GET request on the current page URL with the `ask` query parameter: ``` GET https://docs.tonadocash.com/security/tolk-security-findings.md?ask= ``` The question should be specific, self-contained, and written in natural language. The response will contain a direct answer to the question and relevant excerpts and sources from the documentation. Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.