Skip to content

Migrate regtest usage off nigiri (in-house Node CLI)#94

Draft
Kukks wants to merge 4 commits into
masterfrom
migrate-regtest-denigiri
Draft

Migrate regtest usage off nigiri (in-house Node CLI)#94
Kukks wants to merge 4 commits into
masterfrom
migrate-regtest-denigiri

Conversation

@Kukks

@Kukks Kukks commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

arkade-regtest was rewritten to remove nigiri / chopsticks / esplora and instead run an in-house Docker Compose stack (Bitcoin Core + Fulcrum + mempool + NBXplorer + arkd + …) driven by a zero-dependency Node CLI (regtest.mjs). This PR migrates this repo's own standalone integration-test environment off nigiri to match.

The emulator does not vendor arkade-regtest as a git submodule and does not use its start-env.sh/regtest.sh wrappers — it consumes nigiri directly (external nigiri Docker network in docker-compose.regtest.yml, the vulpemventures/nigiri-github-action in CI, and runCommand("nigiri", …) calls in the Go integration tests). The changes below reflect that consumption pattern.

Changes

  • Esplora REST endpoint moved to mempool's /api:
    • Host side: http://localhost:3000http://localhost:3000/api — every mempoolexplorer.NewExplorer(...) call across test/*.go (19 occurrences).
    • In-network: http://chopsticks:3000http://mempool_web/apiARKD_ESPLORA_URL in docker-compose.regtest.yml and the ark init --explorer call (runs inside the arkd container).
    • Electrum/Fulcrum endpoints are unchanged (this repo does not use them).
  • Removed direct nigiri CLI use in Go integration tests, replaced with the documented container interface docker exec bitcoin bitcoin-cli … via three helpers in test/tx_test.go:
    • onchainFaucet(addr, amountBtc)sendtoaddress + mine 1 (replaces nigiri faucet), returns the funding txid.
    • mineBlocks(n)bitcoin-cli -generate n (replaces nigiri rpc -generate/--generate).
    • bitcoinCli(args…) — generic passthrough (replaces nigiri rpc …, e.g. getrawtransaction).
  • CI (.github/workflows/test.yaml): replaced the vulpemventures/nigiri-github-action@v1 step with actions/setup-node + cloning arkade-regtest and starting its base profile via the Node CLI (REGTEST_PROFILES=base node regtest/regtest.mjs start); added a teardown that runs regtest.mjs clean.
  • README: updated Prerequisites (dropped Nigiri, added Node.js) and the Testing steps to use the arkade-regtest Node CLI.
  • docker-compose.regtest.yml: pointed the shared external network from nigiri to arkade-regtest_default.

⚠ Behavioral change — mining

  • Old nigiri auto-mined, and nigiri faucet confirmed immediately.
  • The new stack's faucet spends from the Bitcoin Core node wallet and does not mine by default; a background auto-miner mines 1 block every AUTOMINE_INTERVAL seconds (default 600; 0 disables). Any test that faucets and then waits for a confirmation must mine.
  • To preserve the old confirm-on-faucet semantics, onchainFaucet mines 1 block after each send. Existing explicit mine steps after waitForUtxo were kept (converted to mineBlocks) and remain valid.

Verify after ArkLabsHQ/arkade-regtest#27 merges

Integration tests cannot run green until #27 is merged to arkade-regtest master. After it merges:

  • Confirm the CI clone (git clone --branch master …/arkade-regtest.git regtest) resolves to the post-SubmitTx: Reject invalid arkade scripts instead of skipping them #27 code, then run the integration job end-to-end.
  • Verify the network wiring: this repo's docker-compose.regtest.yml joins the external arkade-regtest_default network so its arkd/arkd-wallet reach the stack's bitcoin, nbxplorer, and mempool_web containers. Confirm the project/network name matches what regtest.mjs start actually creates (see open questions).
  • Re-run make integrationtest locally against a freshly started arkade-regtest stack and confirm faucet/mine timing holds (no flakiness from auto-mine interactions).

Open questions / manual follow-ups

These were left as-is because the right answer depends on how the emulator is meant to run against the new stack, and the brief covers submodule consumers (which this repo is not):

  1. Stack orchestration / duplication. arkade-regtest's own compose already builds and runs both arkd and an emulator service (profile emulator), while this repo's docker-compose.regtest.yml also spins up its own arkd + arkd-wallet + nbxplorer and relied on nigiri only for bitcoin/esplora. After SubmitTx: Reject invalid arkade scripts instead of skipping them #27 these overlap. Should the emulator (a) keep its own partial compose and join the arkade-regtest network for just bitcoin/mempool (current approach here), or (b) drop its compose entirely and test against arkade-regtest's arkd/emulator services? This needs a maintainer decision.
  2. External network name. arkade-regtest_default is the Compose default project network name (project arkade-regtest). If regtest.mjs sets a different project name or defines an explicit network, this value must be updated. Not verified against a running stack.
  3. CI bootstrap of arkade-regtest. The workflow currently git clones arkade-regtest at master. If maintainers prefer pinning a tag/SHA or vendoring it as a submodule, the checkout step should change accordingly.
  4. Bitcoin Core wallet assumption. onchainFaucet/mineBlocks call wallet RPCs (sendtoaddress, -generate) without an explicit -rpcwallet, matching arkade-regtest's lib/chain.mjs which ensures exactly one loaded wallet. Core 31 has no default "" wallet — if the stack ever loads more than one wallet, these calls would need an explicit -rpcwallet.

Notes

  • Verified locally where cheap: go vet ./test/... passes (the test package compiles with the new helpers); edited files are gofmt-clean (the only gofmt noise is CRLF from a Windows checkout, not a formatting change). Full integration tests cannot run until SubmitTx: Reject invalid arkade scripts instead of skipping them #27 merges and a stack is available — not faked here.
  • This repo has no NIGIRI_*, BITCOIN_LOW_FEE, or ARK_CONTAINER env vars to remove. The arkd container is already named arkd and docker exec -t arkd ark … was already correct (left unchanged).

Blocked by ArkLabsHQ/arkade-regtest#27

arkade-regtest replaced nigiri/chopsticks/esplora with an in-house Docker
Compose stack (Bitcoin Core + Fulcrum + mempool + ...) driven by a Node CLI.
This updates the emulator's standalone integration-test environment to match.

- Esplora REST endpoints moved to mempool's /api:
  - host side: http://localhost:3000 -> http://localhost:3000/api
    (all mempoolexplorer.NewExplorer(...) calls in test/)
  - in-network: http://chopsticks:3000 -> http://mempool_web/api
    (ARKD_ESPLORA_URL in docker-compose.regtest.yml; `ark init --explorer`)
- Replaced direct `nigiri` CLI use in Go integration tests with the documented
  container interface: `docker exec bitcoin bitcoin-cli ...` helpers
  (onchainFaucet / mineBlocks / bitcoinCli). nigiri auto-mined; the new stack
  does not, so onchainFaucet mines 1 block to preserve confirm-on-faucet.
- CI: replaced the nigiri GitHub Action with setup-node + the arkade-regtest
  Node CLI (REGTEST_PROFILES=base ... start / clean).
- README: updated prerequisites and testing steps off nigiri.
- docker-compose.regtest.yml: point the shared external network at the
  arkade-regtest stack's default network.

Blocked by ArkLabsHQ/arkade-regtest#27

@arkanaai arkanaai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Arkana Code Review — #94

Verdict: APPROVE (with notes — no protocol-critical concerns)

This PR is a clean infrastructure migration: nigiri → arkade-regtest Node CLI. No protocol logic (VTXOs, signing, forfeit paths, round lifecycle, exit paths) is touched. All changes are in test scaffolding, CI, Docker Compose networking, and README docs.


What I verified

  1. All 19 mempoolexplorer.NewExplorer calls updated from http://localhost:3000http://localhost:3000/api
  2. In-network URL http://chopsticks:3000http://mempool_web/api in both docker-compose.regtest.yml (ARKD_ESPLORA_URL) and ark init --explorer call in setupServerWalletAndCLI()
  3. onchainFaucet helper correctly mines 1 block after sendtoaddress to preserve the old confirm-on-faucet semantics ✅
  4. mineBlocks(n) consolidates the old loop-of-single-mines pattern in onchain_test.go:339-342 into a single -generate n call — cleaner and functionally identical ✅
  5. bitcoinCli helper safely copies the args slice: append(append([]string{}, bitcoinCliArgs...), arg...) — no slice header mutation ✅
  6. RPC credentials (-rpcuser=admin1 -rpcpassword=123) are hardcoded in bitcoinCliArgs — these match the existing NBXPLORER_BTCRPCUSER/PASSWORD already in docker-compose.regtest.yml and are consistent with arkade-regtest defaults ✅
  7. CI workflow adds actions/setup-node@v4, clones arkade-regtest at master, and tears down with regtest.mjs clean || true on if: always()
  8. Network name arkade-regtest_default follows Docker Compose's <project>_default naming convention ✅
  9. generateBlock() function now delegates to mineBlocks(1) — single source of truth ✅
  10. No remaining nigiri references in any changed file ✅

Notes (non-blocking)

1. CI: no health-check gate between regtest.mjs start and make docker-run (test.yaml:38-41)
regtest.mjs start launches Docker Compose in the background. If the bitcoin/mempool containers aren't fully healthy before docker-run starts, the emulator compose may fail to connect. The old nigiri-github-action presumably blocked until healthy. Consider adding a wait-for-healthy step or verifying that regtest.mjs start blocks until services are ready.

2. CI: cloning arkade-regtest at master with no SHA pin (test.yaml:37)
A force-push or breaking change to arkade-regtest master will silently break this repo's CI. Consider pinning to a tag or SHA once arkade-regtest#27 stabilizes. The PR body already flags this as an open question — just reinforcing it.

3. onchainFaucet default amount changed from nigiri's default
nigiri's default faucet was 1 BTC, and the new helper also defaults to "1" BTC — consistent ✅. But nigiri returned "txId: <txid>" while bitcoinCli("sendtoaddress", ...) returns just the raw txid. The old TestBoarding code had strings.TrimPrefix(faucetOutput, "txId:") — the PR correctly removes that parsing. Clean.

4. Bitcoin Core wallet assumption (bitcoinCli uses no -rpcwallet)
The PR body flags this (open question #4). Acceptable for now since arkade-regtest ensures a single loaded wallet, but if the stack ever changes, all bitcoinCli calls will break. A future hardening pass could add -rpcwallet=<name>.

5. Cross-repo impact: zero
This PR changes no public APIs, types, protos, or interfaces. The emulator/pkg/client and emulator/pkg/arkade packages (consumed by solver and bancod via Go module) are untouched. Only test files, CI, Docker Compose, and README are modified.

6. Other repos still on nigiri
solver, bancod, coinflip, banco, introspector-review, layerzero-usdt0-arkade-demo, and ts-sdk-review-427 all still reference nigiri and chopsticks:3000. This PR is correctly scoped to emulator only, but the broader migration is incomplete. Track separately.

Protocol safety

No protocol-critical code paths are modified. All changes are confined to test infrastructure. No human sign-off required beyond normal review.


🤖 Reviewed by Arkana (opus) · aggressive mode

@arkanaai arkanaai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Arkana Code Review — #94

Verdict: APPROVE (with notes — no protocol-critical concerns)

This PR is a clean infrastructure migration: nigiri → arkade-regtest Node CLI. No protocol logic (VTXOs, signing, forfeit paths, round lifecycle, exit paths) is touched. All changes are in test scaffolding, CI, Docker Compose networking, and README docs.


What I verified

  1. All 19 mempoolexplorer.NewExplorer calls updated from http://localhost:3000http://localhost:3000/api
  2. In-network URL http://chopsticks:3000http://mempool_web/api in both docker-compose.regtest.yml (ARKD_ESPLORA_URL) and ark init --explorer call in setupServerWalletAndCLI()
  3. onchainFaucet helper correctly mines 1 block after sendtoaddress to preserve the old confirm-on-faucet semantics ✅
  4. mineBlocks(n) consolidates the old loop-of-single-mines pattern in onchain_test.go:339-342 into a single -generate n call — cleaner and functionally identical ✅
  5. bitcoinCli helper safely copies the args slice: append(append([]string{}, bitcoinCliArgs...), arg...) — no slice header mutation ✅
  6. RPC credentials (-rpcuser=admin1 -rpcpassword=123) are hardcoded in bitcoinCliArgs — these match the existing NBXPLORER_BTCRPCUSER/PASSWORD already in docker-compose.regtest.yml and are consistent with arkade-regtest defaults ✅
  7. CI workflow adds actions/setup-node@v4, clones arkade-regtest at master, and tears down with regtest.mjs clean || true on if: always()
  8. Network name arkade-regtest_default follows Docker Compose's <project>_default naming convention ✅
  9. generateBlock() function now delegates to mineBlocks(1) — single source of truth ✅
  10. No remaining nigiri references in any changed file ✅

Notes (non-blocking)

1. CI: no health-check gate between regtest.mjs start and make docker-run (test.yaml:38-41)
regtest.mjs start launches Docker Compose in the background. If the bitcoin/mempool containers aren't fully healthy before docker-run starts, the emulator compose may fail to connect. The old nigiri-github-action presumably blocked until healthy. Consider adding a wait-for-healthy step or verifying that regtest.mjs start blocks until services are ready.

2. CI: cloning arkade-regtest at master with no SHA pin (test.yaml:37)
A force-push or breaking change to arkade-regtest master will silently break this repo's CI. Consider pinning to a tag or SHA once arkade-regtest#27 stabilizes. The PR body already flags this as an open question — just reinforcing it.

3. onchainFaucet default amount changed from nigiri's default
nigiri's default faucet was 1 BTC, and the new helper also defaults to "1" BTC — consistent ✅. But nigiri returned "txId: <txid>" while bitcoinCli("sendtoaddress", ...) returns just the raw txid. The old TestBoarding code had strings.TrimPrefix(faucetOutput, "txId:") — the PR correctly removes that parsing. Clean.

4. Bitcoin Core wallet assumption (bitcoinCli uses no -rpcwallet)
The PR body flags this (open question #4). Acceptable for now since arkade-regtest ensures a single loaded wallet, but if the stack ever changes, all bitcoinCli calls will break. A future hardening pass could add -rpcwallet=<name>.

5. Cross-repo impact: zero
This PR changes no public APIs, types, protos, or interfaces. The emulator/pkg/client and emulator/pkg/arkade packages (consumed by solver and bancod via Go module) are untouched. Only test files, CI, Docker Compose, and README are modified.

6. Other repos still on nigiri
solver, bancod, coinflip, banco, introspector-review, layerzero-usdt0-arkade-demo, and ts-sdk-review-427 all still reference nigiri and chopsticks:3000. This PR is correctly scoped to emulator only, but the broader migration is incomplete. Track separately.

Protocol safety

No protocol-critical code paths are modified. All changes are confined to test infrastructure. No human sign-off required beyond normal review.


🤖 Reviewed by Arkana (opus) · aggressive mode

The base-stack step cloned master, which predates regtest.mjs (#27 unmerged), so CI failed with 'Cannot find module regtest.mjs'. Track the denigiri-regtest branch until #27 merges, then switch back to master.

@arkanaai arkanaai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Arkana Code Review — #94 (incremental)

New commit: 10b30e6 — CI: clone denigiri-regtest branch instead of master

Verdict: Still APPROVE — this is the correct fix.

What changed

Single file: .github/workflows/test.yaml:37-39. The git clone now targets --branch denigiri-regtest instead of --branch master, since regtest.mjs only exists on the unmerged #27 branch. Comment clearly marks it as temporary.

Verified

  1. Branch denigiri-regtest exists on ArkLabsHQ/arkade-regtest
  2. arkade-regtest#27 is still open — this branch pin is necessary ✅
  3. No other files changed ✅
  4. Comment documents the intent to switch back to master post-merge ✅

One note (non-blocking, carried from prior review)

Once arkade-regtest#27 merges, remember to push a follow-up commit switching back to master (or better, pin to a tag/SHA). The TODO comment is good — just flagging it so it doesn't get lost.

No protocol-critical code touched. No new concerns.


🤖 Reviewed by Arkana (opus) · aggressive mode

On the shared arkade-regtest network, this repo's own `nbxplorer` container/port (32838) collided with the base stack's nbxplorer ('container name /nbxplorer is already in use'). Rename to `emulator-nbxplorer`, drop its host port (only arkd-wallet uses it in-network), and update arkd-wallet's URL/depends_on.

@arkanaai arkanaai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Arkana Code Review — #94 (incremental)

🤖 Reviewed by Arkana (opus) · aggressive mode

New commit: 32505a1 — Rename emulator's nbxplorer to avoid base-stack collision

Verdict: Still APPROVE — correct fix for a real container name collision.

What changed

Single file: docker-compose.regtest.yml. The emulator's nbxplorer service/container is renamed to emulator-nbxplorer to avoid colliding with the arkade-regtest base stack's own nbxplorer container (at compose.base.yml:99) on the shared arkade-regtest_default network. Host port 32838 is dropped since only arkd-wallet talks to it in-network.

Verified

  1. Service name nbxploreremulator-nbxplorer
  2. Container name nbxploreremulator-nbxplorer
  3. depends_on in arkd-wallet updated to emulator-nbxplorer
  4. ARKD_WALLET_NBXPLORER_URL updated to http://emulator-nbxplorer:32838
  5. pgnbxplorer (postgres) correctly left unchanged — the base stack has no pgnbxplorer container, so no collision ✅
  6. Port 32838 removal is safe — no Go test code or CI references localhost:32838; only arkd-wallet consumes it in-network on the container port ✅
  7. Comment clearly explains the rename rationale ✅

No protocol-critical code touched. No new concerns.

@arkanaai arkanaai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Arkana Code Review — #94 (incremental)

🤖 Reviewed by Arkana (opus) · aggressive mode

New commit: 32505a1 — Rename emulator's nbxplorer to avoid base-stack collision

Verdict: Still APPROVE — correct fix for a real container name collision.

What changed

Single file: docker-compose.regtest.yml. The emulator's nbxplorer service/container is renamed to emulator-nbxplorer to avoid colliding with the arkade-regtest base stack's own nbxplorer container (at compose.base.yml:99) on the shared arkade-regtest_default network. Host port 32838 is dropped since only arkd-wallet talks to it in-network.

Verified

  1. Service name nbxploreremulator-nbxplorer
  2. Container name nbxploreremulator-nbxplorer
  3. depends_on in arkd-wallet updated to emulator-nbxplorer
  4. ARKD_WALLET_NBXPLORER_URL updated to http://emulator-nbxplorer:32838
  5. pgnbxplorer (postgres) correctly left unchanged — the base stack has no pgnbxplorer container, so no collision ✅
  6. Port 32838 removal is safe — no Go test code or CI references localhost:32838; only arkd-wallet consumes it in-network on the container port ✅
  7. Comment clearly explains the rename rationale ✅

No protocol-critical code touched. No new concerns.

Old nbxplorer 2.5.30 crash-loops against the base stack's Bitcoin Core 31 (indexer dies -> arkd-wallet can't init -> arkd fatal: failed to connect to wallet). Align with arkade-regtest's working base (arkd/arkd-wallet v0.9.6, nbxplorer 2.6.7).

@arkanaai arkanaai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Arkana Code Review — #94 (incremental)

🤖 Reviewed by Arkana (opus) · aggressive mode

Scope: Only commit 2d03e934 (pushed after my last review at 10:40 UTC).

Changes reviewed

Version bumps in docker-compose.regtest.yml:

  • nicolasdorier/nbxplorer: 2.5.30 → 2.6.7
  • ghcr.io/arkade-os/arkd-wallet: v0.9.3 → v0.9.6
  • arkade-os/arkd build context: v0.9.3 → v0.9.6

Verification

  • ✅ All three versions exist (arkd v0.9.6 tag, arkd-wallet v0.9.6 container image, nbxplorer 2.6.7)
  • ✅ nbxplorer 2.6.7 matches arkd's own upgrade (arkade-os/arkd#1077)
  • ✅ New config fields in arkd v0.9.6 (PG_DB_MAX_OPEN_CONN, UNROLLED_VTXO_MIN_EXPIRY_MARGIN, StreamConnPoolSize) are all optional with defaults — not needed for this compose's sqlite/badger/inmemory setup
  • ✅ No breaking env var changes between v0.9.3 and v0.9.6
  • ✅ arkd-wallet and arkd versions are kept in sync (both v0.9.6)

Clean version bump. LGTM.

@arkanaai arkanaai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Arkana Code Review — #94 (incremental)

🤖 Reviewed by Arkana (opus) · aggressive mode

Scope: Only commit 2d03e934 (pushed after my last review at 10:40 UTC).

Changes reviewed

Version bumps in docker-compose.regtest.yml:

  • nicolasdorier/nbxplorer: 2.5.30 → 2.6.7
  • ghcr.io/arkade-os/arkd-wallet: v0.9.3 → v0.9.6
  • arkade-os/arkd build context: v0.9.3 → v0.9.6

Verification

  • ✅ All three versions exist (arkd v0.9.6 tag, arkd-wallet v0.9.6 container image, nbxplorer 2.6.7)
  • ✅ nbxplorer 2.6.7 matches arkd's own upgrade (arkade-os/arkd#1077)
  • ✅ New config fields in arkd v0.9.6 (PG_DB_MAX_OPEN_CONN, UNROLLED_VTXO_MIN_EXPIRY_MARGIN, StreamConnPoolSize) are all optional with defaults — not needed for this compose's sqlite/badger/inmemory setup
  • ✅ No breaking env var changes between v0.9.3 and v0.9.6
  • ✅ arkd-wallet and arkd versions are kept in sync (both v0.9.6)

Clean version bump. LGTM.

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.

1 participant