Skip to content

docs: correct D5's mechanism per #74 review, cross-link D6 as the real residual risk - #75

Open
mattglory wants to merge 3 commits into
mainfrom
fix-d5-wording-per-74-review
Open

mattglory wants to merge 3 commits into
mainfrom
fix-d5-wording-per-74-review

Conversation

@mattglory

Copy link
Copy Markdown
Owner

Summary

Follow-up to #74, per your own review comment there. You flagged that D5's row, §7.2 and the archive header all said apply --mainnet "auto-selects" the plan file — overstating it — and cited clarinet 3.23.2's actual source rather than the naming convention. I re-verified the same source independently (components/clarinet-cli/src/frontend/cli.rs, ApplyDeployment / load_deployment_if_exists) before writing anything down, and it matches what you described exactly.

The correction

apply --mainnet always recomputes a plan from Clarinet.toml first and diffs it against the on-disk file, prompting Overwrite? [Y/n]. It falls back to the on-disk file silently only when that recomputation errors — which happens on any checkout without settings/Mainnet.toml (gitignored, true of every fresh clone).

So #74 closes exactly that silent-fallback case: once all 13 paths resolved, it was one Enter from broadcast with no deployer key even present. It does not close the case where settings/Mainnet.toml exists (a real deploy machine) — there, clarinet always recomputes fresh from Clarinet.toml regardless of this file. On that machine the residual risk was never this YAML; it's whatever Clarinet.toml currently resolves the 14 funds-bearing contract names to, which today is the contracts/test/ localized copies — D6, not D5.

Changes

  • D5's table row and §7.2: corrected mechanism, both marked *corrected 2026-09-23*.
  • The archive file's own header comment (deployments/archive/gen1-mainnet-plan-2026-09-22.yaml): same correction, so the record itself doesn't repeat the overstatement.
  • D6's row: added a cross-reference back to this, since D6 was previously framed only as a coverage gap, and your review makes explicit it's also what a real apply --mainnet would publish today on any machine with settings/Mainnet.toml.

Not touched

No diff to the archived plan's content, no code change, docs only.

Verification

Suite 256/256, clarinet check 211/0, both unchanged. Archive YAML still parses.

🤖 Generated with Claude Code

…l residual risk

Hillary's #74 approval flagged that D5's row, §7.2 and the archive header
all said apply --mainnet "auto-selects" the plan file -- overstating the
mechanism. She read clarinet 3.23.2's actual source
(components/clarinet-cli/src/frontend/cli.rs, ApplyDeployment /
load_deployment_if_exists) rather than trusting the naming convention.
I re-verified the same source independently before writing anything down.

Actual behavior: apply --mainnet always recomputes a plan from
Clarinet.toml first and diffs it against the on-disk file, prompting
Overwrite? [Y/n]. It falls back to the on-disk file SILENTLY only when
that recomputation errors -- which happens on any checkout without
settings/Mainnet.toml (gitignored, true of every fresh clone). That
silent-fallback case, once all 13 paths resolved, is what #74 actually
closed: one Enter from broadcast with no deployer key even present.

It does NOT close the case where settings/Mainnet.toml exists (a real
deploy machine) -- there clarinet always recomputes fresh from
Clarinet.toml regardless of this file, so the archive changes nothing.
The residual risk on that machine was never this YAML; it's whatever
Clarinet.toml currently resolves the 14 funds-bearing contract names to,
which today is the contracts/test/ localized copies -- D6, not D5.

Corrected in three places: D5's table row, §7.2, and the archive file's
own header comment. Added a cross-reference from D6 back to this finding,
since D6 was previously framed only as a coverage gap and this makes
explicit that it's also what a real mainnet deploy would publish today.

No diff to the archived plan's content, no code change. Verified: suite
256/256, clarinet check 211/0, both unchanged; archive YAML still parses.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
web Ready Ready Preview Sep 28, 2026 5:57pm UTC

Request Review

@unixwhisperer unixwhisperer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes. Before writing this I stopped trusting source-reading, mine included, and ran clarinet itself. The result corrects my #74 review, corrects #75, and surfaces something more important than either.

How I tested

  • Binary: clarinet 3.23.2, the official clarinet-linux-x64-musl release asset, which is the version CI uses.
  • Isolation: every run is inside docker run --network none, so nothing can reach a real node.
  • Keys: where a settings/Mainnet.toml is needed, it holds a freshly generated, never-funded throwaway mnemonic (SP32FA8…).
  • Observing broadcasts: a local mock node on 127.0.0.1:20443 logs every request. "Broadcast" below means a POST /v2/transactions was observed. In test copies only, the plan's stacks-node was pointed at the mock.
  • Scope: a toy project covered each clarinet branch in isolation, then the real repo was tested at 56a51b1 (before #74) and 4af27a6 (after #74), taken with git archive.

Results (real repo)

# Machine Invocation Before #74 (56a51b1) After #74 (4af27a6)
R1/R2 clean clone (no settings/Mainnet.toml) apply --mainnet, Enter at every prompt unable to compute an updated plan → falls back to the gen-1 plan → Continue [Y/n]? → exits 0 silently, zero requests exits 1, zero requests
R3 has Mainnet.toml, key ≠ SP3TGRVG… apply --mainnet -d gen-1 plan loaded, no prompt → panic at onchain/mod.rs:568 (stx_accounts_lookup.get(expected_sender).unwrap()), nothing signed see R4
R3b has Mainnet.toml, key = plan's expected-sender (swapped in the test copy) apply --mainnet -d all 13 gen-1 contracts broadcast, no prompt n/a
R4 has Mainnet.toml, any key apply --mainnet -d see R3 generates a 57-contract plan from Clarinet.toml and broadcasts it, no prompt
R5b has Mainnet.toml, any key apply --mainnet, Enter, Enter (defaults) recompute succeeds offline from the vendored .cache → Overwrite? [Y/n] → same 57-contract plan broadcast same (toy T9: generate → Continue → broadcast)

R4 and R5b produce byte-identical plans (sha256 217564c42bea…).

What this corrects

  1. A clean checkout could never broadcast, before or after #74. My #74 review said "one prompt away from broadcast", and #75 says "one Enter away from broadcast, with no deployer key even present". Both are wrong: after Continue, apply needs settings/Mainnet.toml to sign, and without it clarinet returns (R1).
  2. The gen-1 plan could only ever be broadcast by the SP3TGRVG… key. Clarinet signs each publish with the Mainnet.toml account matching expected-sender. Any other key panics before signing (R3). All 13 of its names already exist at SP3TGRVG… (checked via GET /v2/contracts/interface), so a broadcast would be refused as duplicates. That last step is inferred: it wasn't tested against a real node.
  3. The recompute-error fallback is real but didn't trigger here. A toy project with an unfetchable requirement showed it (unable to compute an updated plan → stale plan → broadcast after Continue). In this repo, recomputation succeeds offline because .cache is vendored.
  4. "Silently" is wrong in one direction and understated in another. The fallback prints an error first. But in R1 the exit is silent: after Continue, clarinet exits 0 with no message, which an operator could read as success.

So #74 was harmless hygiene. It removed a route only the gen-1 key could use, and one the chain would have refused. It did not reduce the real exposure, which is below.

The real exposure (independent of #74, present on main today)

On any machine with a settings/Mainnet.toml, whatever key it holds, clarinet deployments apply --mainnet publishes the plan clarinet computes from Clarinet.toml, with two Enters by default or with no prompt under -d. That plan has 57 contract publishes, 26 of them from contracts/test/:

  • The localized copies of every funds-bearing contract. In these copies, every canonical SM3VDXK…sbtc-token reference (the real bridged sBTC) becomes .sbtc-token, and in this same plan that resolves to contracts/sbtc-token.clar, the flash-mintable mock (12 contract-call?s in contracts/test/flashstack-sbtc-pool-v3.clar, e.g. lines 91, 102; 6 in contracts/test/flashstack-sbtc-core-v2.clar, e.g. lines 69, 80).
  • Test fixtures: malicious-token, mock-usdcx, test-receiver-bad, test-pool-v3-receiver-reentrant, …

At SPR9PQANV6…, 54 of the 57 names are free. That includes every undeployed successor: flashstack-pool-v3, flashstack-stx-pool-v3, flashstack-sbtc-pool-v3, flashstack-sbtc-core-v2, flashstack-stx-core-v2. Contract names can't be reused, so one mistaken run from a machine holding that key would permanently occupy the intended mainnet names with test builds bound to a mock sBTC, under the real FlashStack principal.

With the file gone, #74 does make -d reach this plan with no confirmation where it previously panicked for any key other than SP3TGRVG…. I'm not suggesting reverting #74. The stale plan only guarded -d by accident, and the default Enter-Enter path reached the 57-contract plan before #74 anyway.

Requested changes to #75

  • Replace the mechanism text in the D5 row, §7.2 and the archive header with what the table above shows. In short: a clean checkout cannot broadcast; the gen-1 plan was only signable by SP3TGRVG… and every name already exists there; #74 is hygiene, not risk reduction.
  • Rewrite the D6 addition. It currently says a deploy run "would publish these 14 contracts as their contracts/test/ localized copies". The tested result is 57 publishes, 26 from contracts/test/, including test fixtures, with sBTC resolving to the in-plan mock. Keep it cross-linked from D5.
  • Dates: 2026-09-23 → 2026-09-26, in three places.

Separately, for you (not #75's scope)

  1. Does any machine have a settings/Mainnet.toml, and for which key? That one fact decides whether this is theoretical or live.
  2. Cheap regression guard, which I can write: a test that generates the mainnet plan against a dummy Mainnet.toml, offline, and fails if any path: is under contracts/test/. It runs in seconds here because .cache is vendored. It goes red today, which is the point.
  3. Operational rule until D6's structural fix lands: never run clarinet deployments apply --mainnet from this repo. Mainnet publishes go through an explicit, reviewed -p <plan> or the scripts/ path.

Happy to approve #75 once the mechanism and D6 text match the evidence. Test harness, case logs and the generated 57-contract plan are available if you want to re-run any row.

mattglory and others added 2 commits September 28, 2026 18:54
…orrection)

Requested changes from Hillary's #75 review. Two prior passes at this
section relied on reading clarinet's source -- mine and hers, independently
-- and both were wrong. She then actually ran clarinet 3.23.2 (the CI
binary) in a network-isolated container against the real repo, before and
after #74, with a mock node logging every broadcast attempt. I re-ran her
test (#76, merged) and independently reproduced the core claim from
scratch outside the harness before writing any of this down: 57 publishes,
26 from contracts/test/, matching exactly; the sbtc-token references she
cited at lines 91/102 of the test sbtc-pool-v3 copy matched exactly; all
five undeployed audit-track successor names re-confirmed 404 today.

What actually changes:
- D5: a clean checkout never broadcasts, before or after #74 (recompute
  fails, falls back, prompts, exits with zero requests either way). The
  gen-1 plan could only ever be signed by SP3TGRVG..., whose 13 names
  already exist on mainnet, so a real broadcast would be refused as
  duplicates regardless. #74 is hygiene -- it removed a route only that
  key could use, one the chain would have refused anyway -- not the risk
  reduction either earlier version of this row claimed.
- D6: the real, still-live exposure, unaffected by #74. Any machine with
  settings/Mainnet.toml, any key, reaches the SAME plan clarinet computes
  fresh from Clarinet.toml (byte-identical before/after #74) -- 57
  publishes, 26 from contracts/test/, sBTC resolving to a flash-mintable
  mock, 54 of 57 names free including all five successors. Now pinned by
  tests/mainnet-plan-guard.test.ts (#76).
- Archive file header: same correction, so the historical record doesn't
  repeat either wrong prior explanation.
- Dates: 2026-09-23 -> 2026-09-26 in all three places, per review.

Verified: suite 258 passed / 1 expected fail (259) across 25 files
(unchanged from post-#76 main), clarinet check 211/0 (unchanged), archive
YAML still parses.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mattglory

Copy link
Copy Markdown
Owner Author

Sorry for the silence — pushed the corrected writeup (426e55e) right after your review but never actually replied here, so this was sitting ambiguous on your end. Fixing that now, days late.

Reproduced your core finding independently before accepting it, not just trusting the table: ran your test (2 passed, 1 expected fail), then separately from scratch outside the harness — temp copy of Clarinet.toml/contracts/.cache, same public devnet mnemonic, clarinet deployments generate --mainnet --manual-cost directly. 57 publishes, 26 from contracts/test/, exact match. The sbtc-token references you cited at lines 91/102 of the test sbtc-pool-v3 copy matched exactly. Re-checked all five audit-track successor names live on-chain — still 404 today.

D5 and D6 rewritten per your exact correction: a clean checkout never broadcasts, before or after #74; the gen-1 plan could only ever be signed by SP3TGRVG… and all 13 names already exist there, so #74 was hygiene, not risk reduction; the real exposure is D6, unaffected by #74, same plan before and after (sha256-identical). Dates fixed to 09-26 in all three places.

On your four asks:

  1. Mainnet.toml — checked my own machine (the one I have access to). A settings/Mainnet.toml exists there, but the mnemonic in it fails BIP-39 validation outright — confirmed without ever printing it. Looks like a stale placeholder, not a real key, but I'm confirming that with Matt directly rather than asserting it.
  2. Reply on docs: correct D5's mechanism per #74 review, cross-link D6 as the real residual risk #75 — this comment.
  3. Review of test: pin what clarinet deployments apply --mainnet would publish #76 — done, approved and merged, after I independently reproduced it (ran the test, then reproduced from scratch outside it, re-confirmed the on-chain name availability).
  4. The interim rule — yes. Written into deployments/README.md as docs: write the interim no-apply-mainnet rule into deployments/ #81, since a rule worth having needs to be somewhere a contributor sees it before running anything, not just a row in this table.

Re-requesting your review on this.

This branch was successfully deployed

1 active deployment
Preview — 426e55eb Deployed Sep 28, 2026 by vercel[bot]
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