Repository navigation
feat: run arkd v0.9.25 (standalone signer, postgres + redis) - #82
Conversation
arkd v0.9.25 split the signer into arkd-signer and dropped SQLite, Badger and the in-memory live store. Add arkd-signer and redis, point arkd at the shared postgres, and rotate signers by recreating arkd-signer. Block-typed delays count a block as a second, so the 300s default expiry margin rejects boarding inputs; set it to 10, with the session duration.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Reviewed head e34f9c0. Stack migration from arkd-wallet-embedded signer to standalone arkd-signer is correctly threaded through compose, the CLI rotation helper, and the env defaults. The readiness-poll approach (waiting for signer health before restarting arkd) is an improvement over the old 3-second sleep. One protocol-adjacent issue requires a fix before merge; several cross-repo docs issues are worth cleaning up.
🔴 Must fix
docker/compose.ark.yml — depends_on: arkd-signer: condition: service_started (line ~88)
The PR uses service_started for the signer dependency, but the upstream arkd/docker-compose.regtest.yml uses condition: service_healthy. The signer image ships an explicit HEALTHCHECK (wget /healthz every 5s, confirmed in arkdsigner.Dockerfile), so service_healthy is available and meaningful.
With service_started, on initial stack boot arkd races the signer's initialization window. If arkd's connection to the signer fails during its startup signer-set fetch, it could cache an empty set and start serving without a valid signer — funds would be unserviceable until arkd restarts and reconnects. restart: unless-stopped self-heals this in practice, but it creates a startup path where arkd briefly starts with no signer. The rotation code (applyAndRestart in lib/setup/signer.mjs) already correctly waits for healthy via docker inspect; initial boot should use the same gate.
Fix: change condition: service_started to condition: service_healthy for the arkd-signer entry in arkd's depends_on block, matching upstream.
🟡 Should fix (cross-repo stale documentation)
The ts-sdk has not been updated to reflect the rename from arkd-wallet-based signing to standalone arkd-signer. Functional paths work (ts-sdk delegates to regtest.mjs set-signers, which this PR correctly updates), but two files have stale copy that will mislead developers debugging rotation timeouts:
ts-sdk/packages/sdk/test/e2e/utils.ts:477,488–489— doc comment still namesARKD_WALLET_DEPRECATED_SIGNER_KEYSandARKD_WALLET_SIGNER_KEY.ts-sdk/packages/sdk/test/e2e/utils.ts:564— timeout error message:"The pinned arkd-wallet image may not support ARKD_WALLET_DEPRECATED_SIGNER_KEYS (signer rotation)."— a developer hitting this error will search for the wrong env var and the wrong service.ts-sdk/packages/sdk/test/e2e/deprecatedSignerMigration.test.ts:29,37,55— test-block doc comment still says "rotateArkdSignerrecreatesarkd-wallet" and referencesARKD_WALLET_DEPRECATED_SIGNER_KEYS.
These are in a separate repo; a follow-up PR is fine, but it should land before the next ts-sdk e2e run targets this stack version.
🟢 Notes (no action needed)
Signer healthcheck source — arkd-signer has no explicit healthcheck: stanza in compose.ark.yml, which means Docker uses the image's built-in one. The Dockerfile has HEALTHCHECK --interval=5s --timeout=3s --start-period=5s --retries=5 CMD wget -q --spider http://127.0.0.1:${ARKD_SIGNER_PORT:-6061}/healthz. Once the service_started → service_healthy fix is in, this is fine. Consider adding an explicit compose healthcheck for discoverability (the redis service sets a good precedent in the same file).
process.env.ARKD_SIGNER_SECRET_KEY in signer.mjs (lib/setup/signer.mjs:117–118 post-patch) — the rotation helper sets the private key in process.env before spawning compose. This key is visible in /proc/self/environ for the duration of the spawn. Pre-existing pattern (same as the old ARKD_WALLET_SIGNER_KEY path), acceptable for a regtest tool with known keys; documented here for the human reviewer given the protocol-critical flag.
ARKD_UNROLLED_VTXO_MIN_EXPIRY_MARGIN=10 / ARKD_SESSION_DURATION=10 — rationale (block = 1s in regtest, default 300s margin would reject boarding inputs under 300-block delays) is sound. Margin equals session duration (the documented minimum).
arkd-wallet image default (compose.ark.yml:21) — fallback tag is still v0.9.15; incompatible with the new stack if compose is run without .env.defaults. .env.defaults always wins in normal use, but aligning the fallback tags to v0.9.25-rc.2 would remove a silent foot-gun for direct docker compose users.
Redis persistence — no volume or --appendonly yes for the redis service; live-store data is intentionally ephemeral. Worth a one-line comment in the service definition so the next reader doesn't wonder if the absence is accidental.
CI diagnostic loop (ci.yml diff, Dump diagnostics on failure step) — arkd-signer and redis are correctly added to the loop. ✓
clean / clearSignerState — the regtest.mjs comment is updated to remove the stale "arkd-wallet's volume" reference. ✓
Cross-repo alignment confirmed — arkd/docker-compose.regtest.yml already has ARKD_SIGNER_SECRET_KEY, ARKD_SIGNER_ADDR=arkd-signer:6061, and the same ARKD_UNROLLED_VTXO_MIN_EXPIRY_MARGIN=10 / ARKD_SESSION_DURATION=10 defaults. This PR's env names and values are consistent with upstream.
Address review feedback on #82: use service_healthy for the signer dependency (matches upstream and the rotation helper), bump the arkd/arkd-wallet fallback tags to v0.9.25-rc.2, and note that redis is intentionally ephemeral.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Follow-up pass on commit 3d24baa. One prior must-fix resolved; two 🟡 notes remain open (one cross-repo, one minor doc inconsistency just introduced). No new blocking findings.
Prior findings — close-out
🔴 Must fix → Fixed (commit 3d24baa)
docker/compose.ark.yml — service_started → service_healthy for arkd-signer
Resolved. The file at HEAD now has:
arkd-signer:
condition: service_healthyThis matches the upstream arkd/docker-compose.regtest.yml and the rotation helper's own healthcheck gate. The startup race window is closed.
🟡 Should fix → Still open (different repo, not changed)
Cross-repo stale ts-sdk documentation — ts-sdk/.../utils.ts:477,488–489,564 and deprecatedSignerMigration.test.ts:29,37,55 still reference ARKD_WALLET_DEPRECATED_SIGNER_KEYS, ARKD_WALLET_SIGNER_KEY, and "recreates arkd-wallet". No change here (different repo); a follow-up PR is still needed before the next e2e run against this stack version.
🟢 Notes — status
- arkd-wallet / arkd fallback image tags (
v0.9.15foot-gun) — Fixed (commit 3d24baa). Both nowv0.9.25-rc.2. - Redis ephemeral comment — Fixed (commit 3d24baa). The
# Intentionally ephemeralline is in place. process.env.ARKD_SIGNER_SECRET_KEYkey-in-env note — unchanged; pre-existing pattern, still the same note for the human reviewer.- Explicit compose healthcheck for discoverability — unchanged;
arkd-signerstill relies on the image HEALTHCHECK with no explicit compose stanza. Was flagged as "no action needed" last pass; still the case. - CI diagnostic loop,
clean/clearSignerStatecomment, cross-repo env alignment — all confirmed correct; no change.
New finding (commit 3d24baa) — 🟡 Minor doc inconsistency
README.md examples show v0.9.25; compose and .env.defaults ship v0.9.25-rc.2
The README's "Custom arkd version" section (changed in an earlier commit) shows:
The defaults are `v0.9.25`.
…
ARKD_IMAGE=ghcr.io/arkade-os/arkd:v0.9.25
ARKD_WALLET_IMAGE=ghcr.io/arkade-os/arkd-wallet:v0.9.25
ARKD_SIGNER_IMAGE=ghcr.io/arkade-os/arkd-signer:v0.9.25
But the actual fallback tags now in docker/compose.ark.yml and .env.defaults are v0.9.25-rc.2. If v0.9.25 (without -rc) hasn't been published yet, a developer following the README example verbatim would get a failed image pull. The README's example should either match the composed fallback (v0.9.25-rc.2) or the section should frame the examples as the target stable version with a note that the defaults trail it. Worth fixing before merge or in a fast follow.
Summary
The one blocking issue from the prior pass is resolved. The PR is in a mergeable state from this repository's perspective. The cross-repo ts-sdk docs issue (#1 in "Should fix" above) and the README/compose version skew are the only remaining items; both are non-blocking for regtest correctness, and the former requires a separate ts-sdk PR.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Third pass on PR #82 — 2 version-bump commits since 3d24baa; 1 prior finding fixed, 1 still open in a separate repo.
Prior findings — close-out
🔴 Must fix → Fixed (3d24baa, prior pass, unchanged)
docker/compose.ark.yml — service_started → service_healthy for arkd-signer. Remains correct at HEAD.
🟡 README/compose version skew (v0.9.25-rc.2 vs v0.9.25) → Fixed (commit 88a0c35)
All three files (.env.defaults, docker/compose.ark.yml fallback tags, README.md examples) now consistently read v0.9.25. The foot-gun I flagged is gone.
🟡 Cross-repo stale ts-sdk documentation → Still open (different repo, not in this diff)
ts-sdk/packages/sdk/test/e2e/utils.ts:477,488–489,564 and deprecatedSignerMigration.test.ts:29,37,55 still reference ARKD_WALLET_DEPRECATED_SIGNER_KEYS, ARKD_WALLET_SIGNER_KEY, and "recreates arkd-wallet". Needs a separate ts-sdk PR before the next e2e run against this stack version.
Incremental diff (3d24baa → 7ac50b8)
commit 88a0c35 — "chore: bump arkd images to v0.9.25"
.env.defaults:67–69:v0.9.25-rc.2→v0.9.25forARKD_IMAGE,ARKD_WALLET_IMAGE,ARKD_SIGNER_IMAGE. ✓docker/compose.ark.yml:21,48,75: fallback tags updated to match. ✓README.md:148,151–153: prose and example blocks updated to match. ✓
All three files are consistent. No new concerns.
commit 7ac50b8 — "chore: bump emulator image to v0.0.9"
.env.defaults:116:v0.0.8→v0.0.9forEMULATOR_IMAGE. ✓docker/compose.ark.yml:237: fallback tag updated to match. ✓
Consistent across both files. The emulator is an optional last-started service with no protocol role; a minor version bump here carries no cross-repo signing or VTXO impact.
Summary
No new findings. The only remaining open item is the cross-repo ts-sdk docs issue, which requires a separate PR in that repository. This PR is mergeable from this repo's perspective once the human protocol reviewer signs off.
|
@arkana-ai-bot ts-sdk part is being handled in arkade-os/ts-sdk#1018 you can mark it off |
|
Thanks @pietro909 — noted. The cross-repo stale ts-sdk documentation finding (the last open 🟡 from my earlier passes) is tracked in arkade-os/ts-sdk#1018. With that in flight, PR #82 has no remaining open findings from my side. All prior items are resolved:
Mergeable once the human protocol reviewer signs off and arkade-os/ts-sdk#1018 lands. |
Moves the stack to arkd
v0.9.25-rc.2, which split the signer intoarkd-signerand dropped SQLite, Badger and the in-memory live store.arkd-signerandredis; arkd uses the shared postgres (projectionandeventDBs, autocreated)arkd-signer(ARKD_SIGNER_SECRET_KEY/ARKD_SIGNER_DEPRECATED_KEYS) instead of arkd-walletARKD_UNROLLED_VTXO_MIN_EXPIRY_MARGIN=10and session duration 10: block-typed delays count a block as a second, so the 300s default rejects boarding inputsBreaking: the compose file no longer runs arkd
v0.9.16or older.Verified with ts-sdk e2e (onchain cosign, deprecated-signer migration) and a
swap-rfqstack bring-up.