Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions .github/workflows/auto-release-pr-staging.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
name: Auto Promote PR (staging → develop)

on:
push:
branches: [staging]
workflow_dispatch:

permissions:
contents: read
pull-requests: write

concurrency:
group: auto-release-pr-staging
cancel-in-progress: false

jobs:
create-promote-pr:
name: Create Promote PR
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Fetch develop branch
run: git fetch origin develop

- name: Check for existing PR
id: check-pr
run: |
PR_COUNT=$(gh pr list --base develop --head staging --state open --json number --jq 'length')
echo "pr_exists=$([[ $PR_COUNT -gt 0 ]] && echo 'true' || echo 'false')" >> $GITHUB_OUTPUT
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Check for differences
id: check-diff
if: steps.check-pr.outputs.pr_exists == 'false'
run: |
DIFF_COUNT=$(git rev-list --count origin/develop..origin/staging)
echo "has_changes=$([[ $DIFF_COUNT -gt 0 ]] && echo 'true' || echo 'false')" >> $GITHUB_OUTPUT
echo "commit_count=$DIFF_COUNT" >> $GITHUB_OUTPUT

- name: Create Promote PR
if: steps.check-pr.outputs.pr_exists == 'false' && steps.check-diff.outputs.has_changes == 'true'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
COMMIT_COUNT: ${{ steps.check-diff.outputs.commit_count }}
run: |
# Promote PRs intentionally do NOT apply the `ci:full` label.
# The heavy M3 Ultra test + coverage gate stays reserved for
# the develop → main Release PR (auto-release-pr.yaml), which
# remains the authoritative pre-PRD gate. Promote PRs run the
# slim `Lint & Build` job + Analyze / CodeQL, mirroring what
# every ready feature PR sees.
printf '%s\n' \
"## Automatic Promote PR" \
"" \
"**Commits:** ${COMMIT_COUNT} new commit(s)" \
"" \
"- [ ] Review all changes" \
"- [ ] Verify CI passes" \
"- [ ] Merge to promote staging to develop (deploys to DEV)" \
> /tmp/pr-body.md

gh pr create \
--base develop \
--head staging \
--title "Promote: staging -> develop" \
--body-file /tmp/pr-body.md
19 changes: 15 additions & 4 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,16 @@ jobs:
runs-on: [self-hosted, m3-ultra]
timeout-minutes: 120
env:
# Force Esplora broadcasts to fail fast. Some unit tests exercise
# the commit pipeline that ends in a real HTTP broadcast; without
# this, runs against the public Mutinynet API can take >60 s per
# test. Mirrors the pre-push hook.
# All three chain-shaping env vars are required by the node
# bootstrap — no defaults exist (see
# `lib::build_network_config_from_env`). CI uses
# `127.0.0.1:1` endpoints so any test that exercises the commit
# pipeline / scanner WS fails fast instead of reaching a public
# third-party host (a previous Mutinynet-flavoured silent
# fallback used to add >60 s per test).
IS_MAINNET: "false"
ESPLORA_URL: http://127.0.0.1:1/api
ESPLORA_WS_URL: ws://127.0.0.1:1/api/v1/ws
# `USERNAME_DOMAIN` is required by the node bootstrap (no
# default — see node/src/main.rs and issue #95). The test value
# is irrelevant for the `info_returns_*` assertions (they only
Expand Down Expand Up @@ -291,7 +296,13 @@ jobs:
runs-on: [self-hosted, m3-ultra]
timeout-minutes: 90
env:
# All three chain-shaping env vars are required by the node
# bootstrap (see `lib::build_network_config_from_env`). Mirror
# the `node-tests` env block above — `127.0.0.1:1` endpoints
# fail fast for any commit-pipeline / scanner-WS path.
IS_MAINNET: "false"
ESPLORA_URL: http://127.0.0.1:1/api
ESPLORA_WS_URL: ws://127.0.0.1:1/api/v1/ws
USERNAME_DOMAIN: test.zkcoins.local
# `PUBLISHER_KEY` is required on every network (no default — see
# `node/src/lib.rs`); the value mirrors `node-tests` above and is
Expand Down
72 changes: 43 additions & 29 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,36 @@ documents in the order given below.
### No polling — events only

Bitcoin / Esplora signals on the node's hot path are subscribed to,
never polled. The scanner consumes block events from the
mempool.space-compatible WebSocket stream (`scanner_ws.rs`,
`ESPLORA_WS_URL`, default `wss://mutinynet.com/api/v1/ws`); the
never polled. The scanner consumes block events from the Esplora-
compatible WebSocket stream (`scanner_ws.rs`, `ESPLORA_WS_URL` —
required env var, no default; see README §Configuration); the
publisher broadcasts the commit and reveal transactions back-to-back
via REST and never sleeps or polls between them. The previous 30-s
tip-poll gated `/api/mint` and `/api/send` visibility by up to a full
block-time + poll-interval (issue #84); event-driven ingestion brings
that down to the WS round-trip.

Historical note: issue #84 originally replaced a fixed 5 s
`PROPAGATION_WAIT_SECS` sleep with a `{"action":"track-tx","data":"<commit_txid>"}`
WS wait + REST safety-net. After the deployment moved to a
self-hosted `mempool/backend:v3.3.1`, empirical measurement showed
that backend version emits zero frames for `track-tx`, so the WS
wait always timed out and the REST fallback always confirmed the
tx as already on-chain. 16/16 fallbacks in 72 h DEV `request_log`,
0 not-found, 0 errors. The wait was pure latency tax (~30 s/mint
and ~30 s/send+commit) for an in-cluster scenario where bitcoind's
local-mempool accept already orders the two POSTs correctly. The
publisher now runs `client.broadcast(commit) → client.broadcast(reveal)`
sequentially; race-freedom follows from the topology (node, electrs,
bitcoind share the Docker `bitcoin` network), not from a WS
subscription.
`PROPAGATION_WAIT_SECS` sleep with a WS `track-tx` wait + REST
safety-net. PR [#144](https://github.com/zk-coins/node/pull/144)
removed that path and replaced it with direct sequential
`client.broadcast(commit) → client.broadcast(reveal)`. A later
re-analysis (see `MIGRATION_RESEARCH.md` § 7.24) established that
the publisher's subscribe frame had been sent in the wrong wire
format — `{"action":"track-tx","data":"<txid>"}` — whereas the
mempool.js convention and `mempool/backend:v3.3.1`'s
`websocket-handler.ts` both expect `{"track-tx":"<txid>"}` as a
top-level key. The backend silently dropped the malformed frame, so
the WS wait always timed out and the REST safety-net always
confirmed the tx as already on-chain (16/16 fallbacks in the 72 h
DEV `request_log` sample, 0 not-found, 0 errors). PR #144 stands
on independent grounds: in the in-cluster topology (node, electrs,
bitcoind share the Docker `bitcoin` network) bitcoind's
local-mempool accept already orders the two POSTs race-free, and
the closed-test-env model (no external Esplora) means there is no
upstream to subscribe against in the first place. The
architecture is documented here; the wire-format bug is recorded
for the historical record, not as a justification.

Where it applies:

Expand Down Expand Up @@ -376,12 +383,15 @@ node/

| Branch | Purpose | Deploy target |
|---|---|---|
| `develop` | Default branch, active development | DEV node |
| `main` | Production releases | PRD node |
| `staging` | Integration buffer — feature PRs land here first | none |
| `develop` | Active development, promoted from `staging` in batches | DEV node |
| `main` | Production releases, promoted from `develop` | PRD node |

- **Push to `develop` via feature branch + PR** (branch ruleset active)
- **`main` is protected** — changes only via PR
- Never force-push, never amend
- **Open feature PRs against `staging`** (not `develop`) — `staging` is the integration buffer where multiple feature branches accumulate before being batched into a single `develop` promotion. This keeps `develop` clean for DEV-deploy churn and gives reviewers a smaller blast radius per merge.
- **`develop` and `main` are protected** — direct pushes are rejected. `develop` accepts only the auto-PR from `staging`; `main` accepts only the auto-PR from `develop`. Hotfixes still go through `staging` so the same review path applies.
- **`develop` is auto-PR'd from `staging`** by `auto-release-pr-staging.yaml` whenever new commits land on `staging`. Merge that PR to promote the batch to DEV. Promote PRs intentionally skip the `ci:full` label — heavy M3 Ultra tests stay reserved for the develop → main Release PR.
- **`main` is auto-PR'd from `develop`** by `auto-release-pr.yaml` (with `ci:full` applied automatically). Merge to release to PRD.
- Never force-push, never amend.

### Commit Messages

Expand Down Expand Up @@ -513,23 +523,26 @@ on startup if unset — there is no silent fallback.
| `PUBLISHER_KEY` | _(required, no default)_ | 32-byte hex private key for Taproot inscription publishing. **Required on every network — DEV, signet, and mainnet.** No fallback default exists: the previous `1234…` placeholder was a publicly-known test key that drainer bots swept within minutes of any on-chain top-up (4 historical drains confirmed). Node panics on startup if unset. Generate locally via `openssl rand -hex 32`. In any deployed environment, source it from your secret manager — **never commit a real key**. |
| `USERNAME_DOMAIN` | _(required, no default)_ | External hostname returned by `/api/info`; node panics on startup if unset (see PR [#36](https://github.com/zk-coins/node/pull/36) for the regression that introduced the global panic hook). |
| `POSTGRES_PASSWORD` | _(required, no default for the DB container)_ | Read by the Postgres container, not by the node process itself; the node's `DATABASE_URL` already embeds the password. Listed here because it is part of the local-dev bootstrap (see `Local Development with Postgres` below). |
| `ESPLORA_URL` | `https://mutinynet.com/api` | Esplora REST API endpoint (electrs or public). |
| `ESPLORA_WS_URL` | `wss://mutinynet.com/api/v1/ws` | Esplora WebSocket endpoint consumed by `scanner_ws` (issue #84). DEV/PRD override only when the upstream WS path changes. |
| `IS_MAINNET` | `false` | `true` for Bitcoin Mainnet, `false` for Mutinynet/Signet. |
| `NETWORK_NAME` | `Mutinynet` / `Mainnet` | Human-readable name returned by `/api/info`. |
| `IS_MAINNET` | _(required, no default)_ | Exact string `true` or `false`; any other value panics. Truthy values like `1`, `TRUE`, `yes` are rejected to prevent silent misconfiguration. |
| `ESPLORA_URL` | _(required, no default)_ | HTTP Esplora endpoint (electrs or public-compatible) for the chain this stage serves. Empty string is treated as unset and panics. |
| `ESPLORA_WS_URL` | _(required, no default)_ | Esplora-compatible WebSocket endpoint consumed by `scanner_ws` (issue #84). Empty string is treated as unset and panics. Previous Mutinynet default was removed because it coupled the deploy to a public third-party host. |
| `NETWORK_NAME` | `Mutinynet` / `Mainnet` | Human-readable name returned by `/api/info`. Derived from `IS_MAINNET` if unset. Purely cosmetic — no behavioural effect. |
| `PROOFS_DIR` | `./proofs` | Directory for per-proof bincode files (see `Persistent State` below). |
| `SCANNER_INITIAL_SETTLE_TIMEOUT_MS` | (runtime-defined) | Override for the scanner's initial-settle deadline; see `runtime.rs`. |
| `RUST_LOG` | `info` | Log level (`debug`, `info`, `warn`, `error`). |

### Minimal local-dev env

All chain-shaping vars are required — there are no defaults. Set them
explicitly, even for local dev:

```bash
export DATABASE_URL="postgresql://postgres:dev@localhost:5432/postgres"
export PUBLISHER_KEY="$(openssl rand -hex 32)"
export USERNAME_DOMAIN="test.zkcoins.local"
# Optional — defaults are fine for Mutinynet:
# export ESPLORA_URL="https://mutinynet.com/api"
# export IS_MAINNET="false"
export IS_MAINNET="false"
export ESPLORA_URL="http://localhost:3000" # your local electrs
export ESPLORA_WS_URL="ws://localhost:8999/api/v1/ws" # your local mempool/backend, or any Esplora-compatible WS
cargo run -p node
```

Expand Down Expand Up @@ -603,7 +616,8 @@ See [docs.zkcoins.app/infrastructure/backend](https://docs.zkcoins.app/infrastru
| `ci.yaml` (Coverage Gate) | Ready PR → develop with `ci:full` label, push to develop | `cargo llvm-cov nextest` with the 100% line + function gate, MVP scope, on the same runner pool. |
| `deploy-dev.yaml` | Push to develop | Docker build (ARM64) → push `zkcoins/node:beta` → deploy to DEV |
| `deploy-prd.yaml` | Push to main | Docker build (ARM64) → push `zkcoins/node:latest` → deploy to PRD |
| `auto-release-pr.yaml` | Push to develop | Creates Release PR (develop → main) |
| `auto-release-pr-staging.yaml` | Push to staging | Creates Promote PR (staging → develop) |
| `auto-release-pr.yaml` | Push to develop | Creates Release PR (develop → main) with `ci:full` label |

**Draft PRs** skip every `ci.yaml` job — the workflow fires once the
PR is marked ready-for-review.
Expand Down
138 changes: 104 additions & 34 deletions MIGRATION_RESEARCH.md
Original file line number Diff line number Diff line change
Expand Up @@ -1314,40 +1314,110 @@ is a build-time signal, not a runtime-readiness signal.
deploy-dev post-curl-retry fires on every DEV deploy. A regression
that brings back the silent-panic shape fails one or both gates.

### 7.24 Self-hosted `mempool/backend:v3.3.1` does not implement the `track-tx` WS action — **codified**

**Discovered:** DEV operations (May 2026), via combined evidence:
DEV `request_log` showed `/api/mint` p50 ≈ 40 s and
`/api/send` p50 = 11 s + `/api/commit` p50 = 30.7 s — both with the
30 s shape characteristic of the publisher's `track-tx` safety-net;
16/16 REST fallbacks in the 72 h sample succeeded with 0 not-found
and 0 errors; a direct WS probe against
`mempool-api-mutinynet:8999/api/v1/ws` with
`{"action":"track-tx","data":"<txid>"}` returned 0 frames in 15 s,
while `{"action":"want","data":["blocks"]}` answered immediately.

**Root cause:** the self-hosted `mempool/backend:v3.3.1` version
does not emit any frame in response to `track-tx` (the action is
recognised but no event arrives). Issue #84's original design
assumed a public mutinynet endpoint where the action does fire;
self-hosting flipped that assumption silently.

**Fix:** drop the entire WS subscribe + safety-net + REST fallback
path from `publisher::broadcast_inscription_txs`. The publisher now
runs `client.broadcast(commit) → client.broadcast(reveal)` back to
back; race-freedom comes from the deployment topology (node +
electrs + bitcoind in the shared Docker `bitcoin` network,
`bitcoind::sendrawtransaction` returns only after local-mempool
accept), not from a WS subscription. Expected effect:
`/api/mint` p50 ~40 s → ~11 s, `/api/send + /api/commit`
~42 s → ~13 s. See the perf PR for the removed code.

**Generalisation for future migrations:** when porting an
event-driven path that was designed against a public upstream onto
a self-hosted reimplementation of the same protocol, smoke-test
each WS action against the self-hosted endpoint before assuming
parity. Empty-frame-budget probe is cheap; silent latency tax is
expensive.
### 7.24 Wrong WS subscribe wire format on self-hosted `mempool/backend` — empirical correction — **codified**

**Status correction.** An earlier revision of this section claimed
that `mempool/backend:v3.3.1` "does not implement the `track-tx` WS
action". That conclusion was wrong. The backend implements
`track-tx` correctly; the zkCoins publisher had been sending the
subscribe frame in the wrong wire format. Both PR
[#144](https://github.com/zk-coins/node/pull/144) (drop the WS
path) and this codification stand — but for different reasons than
the original write-up gave.

**What the publisher actually sent (pre-PR-#144,
`node/src/scanner_ws.rs:650-655` on `ae78798^`):**

```rust
let subscribe = serde_json::json!({
"action": "track-tx",
"data": txid_str,
});
```

**What `mempool/backend:v3.3.1` parses
(`backend/src/api/websocket-handler.ts`, lines ~165-175):**

```typescript
if (parsedMessage && parsedMessage['track-tx']) {
if (/^[a-fA-F0-9]{64}$/.test(parsedMessage['track-tx'])) {
client['track-tx'] = parsedMessage['track-tx'];
// ... subscribe, will emit txPosition / txConfirmed frames
}
}
```

The backend looks at the top-level `track-tx` key. The publisher's
`{action, data}` envelope has no such key, so the handler falls
through silently — no error frame, no log line, no rejection.

**What mempool.js (the canonical client) actually sends
(`https://raw.githubusercontent.com/mempool/mempool.js/main/src/services/ws/ws-client-node.ts`,
`wsTrackTransaction`):**

```typescript
export const wsTrackTransaction = (ws: WebSocket, txid: string): void => {
wsActionWrapper(ws, { 'track-tx': txid });
}
```

i.e. `{"track-tx": "<txid>"}` as a top-level key — exactly the
shape `websocket-handler.ts` parses. The publisher's frame did
not follow this convention.

**Empirical verification (dfxdev, post-PR-#144 re-probe, May 2026).**
A direct `websocat` probe against
`ws://mempool-api-mutinynet:8999/api/v1/ws` with a live mempool
txid:

- `{"action":"track-tx","data":"<txid>"}` → 0 frames in 6 s
(matches the production observation that motivated PR #144).
- `{"track-tx":"<txid>"}` → immediate `{"txPosition":...}` frame,
followed by `{"txConfirmed":...}` when the next block arrived.

So the backend was working all along; the publisher's frame was
malformed.

**Why PR #144 still stands.** Reverting to a WS path with the
correct wire format is not the right move:

1. **Closed test environment, no external Esplora.** zkCoins runs
against a self-hosted `mempool/backend` colocated with node,
electrs, and bitcoind in the shared Docker `bitcoin` network.
There is no upstream public endpoint to subscribe against.
2. **Topology is race-free without a subscribe.** In that
topology, `bitcoind::sendrawtransaction` returns only after
local-mempool accept, so a sequential
`client.broadcast(commit) → client.broadcast(reveal)` is
already ordered. The WS round-trip the subscribe gave us was a
confirmation of something the REST call already guaranteed.
3. **Simpler code.** The WS subscribe + reconnect-with-backoff +
REST safety-net was three failure modes for a problem the REST
call alone does not have. Removing it shrinks
`publisher.rs`/`scanner_ws.rs` by ~200 lines (see PR #144 diff).

**Empirically measured impact of PR #144 on DEV `request_log`:**
`/api/mint` p50 40 s → 8.7 s (4.6×); `/api/send + /api/commit` p50
42 s → 12.7 s (3.3×). Numbers match the predicted shape — the
removed wait was indeed ~30 s of pure latency tax (15 s WS
timeout + REST fallback round-trip).

**Generalisation for future migrations.** Two distinct lessons,
neither the original one:

1. When a WebSocket subscribe "doesn't work", verify the wire
format against the canonical client's source before concluding
the server is broken. `mempool.js` is the reference; copy its
frame shape verbatim, do not reconstruct it from the action
name.
2. The original investigation (latency probe → REST-fallback hit
rate → empty-frame WS probe with the wrong format) reached a
plausible-but-wrong root cause because every signal was
consistent with both "backend broken" and "client malformed".
When a server silently drops a request, "the server doesn't
support it" and "we asked for it wrong" look identical from
the client side. Always cross-check the request against a
known-good client's wire format before blaming the server.

---

Expand Down
Loading
Loading