feat: use arkade-regtest for regtest environment#106
Conversation
Replace bespoke test.docker-compose.yml + cors.nginx.conf with shared arkade-regtest submodule. Pins arkd to v0.8.11 via .env.regtest. Simplify CI by removing nigiri GH Action and docker build steps.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughExternalized regtest into a git submodule, added Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions"
participant Repo as "Repository (with regtest submodule)"
participant Submod as "regtest submodule\n(./regtest/*-env.sh)"
participant Docker as "Docker Engine"
participant Tests as "Integration Tests"
GH->>Repo: checkout (with submodules)
GH->>GH: setup Node/Go, restore caches
GH->>Submod: run start script
Submod->>Docker: bring up regtest containers
Docker->>Submod: report readiness
GH->>Repo: run wallet setup script
GH->>Tests: run pnpm test:integration-docker
Tests->>Docker: exercise services
alt test failure
GH->>Docker: collect logs for named containers
end
GH->>Submod: run clean/stop script (always)
Submod->>Docker: teardown containers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 PR Review —
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 66-69: Update the package.json scripts regtest:start,
regtest:stop, regtest:clean and the composite regtest to perform a submodule
preflight before invoking ./regtest/*.sh; specifically, prepend each script
invocation with a guard that ensures the regtest submodule is populated (e.g.,
run a check and call git submodule update --init --recursive if needed) so the
shell scripts exist before running them, and keep the same script names
regtest:start, regtest:stop, regtest:clean, regtest.
In `@regtest`:
- Line 1: The README currently lacks instructions to initialize git submodules
required for regtest; update README.md to add a brief "Local setup" note
instructing contributors to run git submodule update --init --recursive after
cloning (or clone with --recurse-submodules), and/or add an automated step in
package.json by creating a postinstall script (e.g., "postinstall": "git
submodule update --init --recursive") so npm install will initialize submodules
before running npm run regtest:start; reference the regtest:start script name
and ensure the documentation and package.json change mention the need for
submodules to avoid missing script files.
In `@test/e2e/setup.mjs`:
- Around line 53-54: The Ark readiness probe can wrongly succeed on HTTP error
pages because waitForArkServer() shells out to curl -s which exits 0 on 404/500;
update waitForArkServer() so the curl invocation uses the fail flag (e.g.,
--fail or -f with -sS) or otherwise checks the HTTP status and returns non-zero
on non-2xx responses, so the setup() readiness gate (await waitForArkServer())
will fail on HTTP errors instead of treating an error page as healthy.
- Around line 30-35: The readiness probe is checking Boltz on port 9069 while
the SDK's BoltzSwapProvider defaults to port 9001, causing mismatch; fix by
updating the BoltzSwapProvider constructors in swap-manager.test.ts and
arkade-swaps.test.ts to include apiUrl: "http://localhost:9001" (e.g., new
BoltzSwapProvider({ network: "regtest", apiUrl: "http://localhost:9001" }));
alternatively, if you prefer changing the probe, update the URL used in
setup.mjs's execSync curl call to http://localhost:9001/v2/swap/submarine so the
readiness check matches the SDK default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c73245a5-c77d-4c8b-b7b8-9eed8e5793db
📒 Files selected for processing (8)
.env.regtest.github/workflows/ci.yml.gitmodulescors.nginx.confpackage.jsonregtesttest.docker-compose.ymltest/e2e/setup.mjs
💤 Files with no reviewable changes (2)
- cors.nginx.conf
- test.docker-compose.yml
| "regtest:start": "./regtest/start-env.sh", | ||
| "regtest:stop": "./regtest/stop-env.sh", | ||
| "regtest:clean": "./regtest/clean-env.sh", | ||
| "regtest": "pnpm regtest:clean && pnpm regtest:start && pnpm test:setup-docker" |
There was a problem hiding this comment.
Add a submodule preflight before invoking these scripts.
These commands assume ./regtest/*.sh already exists, but a normal clone leaves the regtest gitlink empty until git submodule update --init --recursive is run. That makes pnpm regtest* fail immediately for contributors and any CI checkout that does not fetch submodules.
🔧 Minimal guard
- "regtest:start": "./regtest/start-env.sh",
+ "regtest:start": "[ -x ./regtest/start-env.sh ] || { echo 'Missing regtest submodule. Run: git submodule update --init --recursive' >&2; exit 1; }; ./regtest/start-env.sh",Apply the same guard to regtest:stop and regtest:clean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 66 - 69, Update the package.json scripts
regtest:start, regtest:stop, regtest:clean and the composite regtest to perform
a submodule preflight before invoking ./regtest/*.sh; specifically, prepend each
script invocation with a guard that ensures the regtest submodule is populated
(e.g., run a check and call git submodule update --init --recursive if needed)
so the shell scripts exist before running them, and keep the same script names
regtest:start, regtest:stop, regtest:clean, regtest.
regtest
Outdated
| @@ -0,0 +1 @@ | |||
| Subproject commit e33f8e417a19c18f7413f772521e9646d5f2e198 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check CI workflow for recursive submodule checkout =="
rg -n -C3 'actions/checkout|submodules:\s*(true|recursive)' .github/workflows/ci.yml || true
echo
echo "== Check docs/scripts for local submodule init/update guidance =="
rg -n -C2 'submodule (update|init)|--recurse-submodules|git clone .*--recurse-submodules|regtest/start-env.sh' README.md . github || true
echo
echo "== Verify .gitmodules includes regtest submodule =="
cat .gitmodulesRepository: arkade-os/boltz-swap
Length of output: 1626
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search README for setup/installation/development instructions =="
fd -t f 'README' . | head -5 | while read f; do
echo "File: $f"
rg -n -C5 'setup|install|development|clone.*submodule|git submodule' "$f" || echo "(no matches)"
done
echo
echo "== Check package.json for setup/postinstall/prepare hooks =="
rg -n '"scripts"|postinstall|prepare' package.json -A 20
echo
echo "== Check if lint/unit/build jobs reference regtest =="
rg -n 'regtest' .github/workflows/ci.yml || echo "(no regtest references outside regtest job)"Repository: arkade-os/boltz-swap
Length of output: 3446
Document submodule initialization for local development.
CI's regtest job correctly uses submodules: true, but local setup documentation lacks guidance on initializing submodules. Contributors cloning without --recurse-submodules and running npm run regtest:start will encounter missing script files. Add submodule initialization steps to README.md or create a setup hook in package.json (e.g., postinstall script).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@regtest` at line 1, The README currently lacks instructions to initialize git
submodules required for regtest; update README.md to add a brief "Local setup"
note instructing contributors to run git submodule update --init --recursive
after cloning (or clone with --recurse-submodules), and/or add an automated step
in package.json by creating a postinstall script (e.g., "postinstall": "git
submodule update --init --recursive") so npm install will initialize submodules
before running npm run regtest:start; reference the regtest:start script name
and ensure the documentation and package.json change mention the need for
submodules to avoid missing script files.
| const response = execSync( | ||
| "curl -s http://localhost:9069/v2/swap/submarine", | ||
| { encoding: "utf8", stdio: "pipe" } | ||
| ); | ||
| const newCount = JSON.parse(newCountResponse).chain_stats.tx_count; | ||
| if (newCount > initialCount) { | ||
| console.log(" ✔ Confirmed"); | ||
| return txid; | ||
| if (response.includes('"ARK"')) { | ||
| console.log(" ✔ Boltz pairs ready"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'new\s+BoltzSwapProvider\s*\(' -C2
rg -n 'apiUrl\s*:' -C2 test srcRepository: arkade-os/boltz-swap
Length of output: 6145
🌐 Web query:
What default API URL does new BoltzSwapProvider({ network: "regtest" })use in@arkade-os/sdk version 0.4.6?
💡 Result:
The default API URL for new BoltzSwapProvider({ network: "regtest" }) in @arkade-os/sdk version 0.4.6 is http://127.0.0.1:9001 or http://localhost:9001. This is based on Boltz Backend regtest configuration where the API server binds to host "127.0.0.1" port 9001, as shown in official Boltz documentation for regtest Docker Compose setup used in development. Arkade SDK's BoltzSwapProvider, when configured with network "regtest", uses this standard local Boltz regtest endpoint since no explicit apiUrl is provided in the constructor call, and examples show explicit local URLs for regtest-like environments (mutinynet uses custom subdomain, but regtest defaults to localhost). The SDK integrates with Boltz for swaps, and regtest assumes local Boltz backend running.
Citations:
- 1: https://api.docs.boltz.exchange/backend-development.html
- 2: https://github.com/arkade-os/boltz-swap
- 3: https://docs.boltz.exchange/api/regtest
- 4: https://api.docs.boltz.exchange/api-v2.html
Boltz readiness check URL port mismatch: setup checks 9069, SDK defaults to 9001.
The setup readiness probe at http://localhost:9069/v2/swap/submarine will pass independently of whether the SDK can reach Boltz. The SDK default for network: "regtest" is http://localhost:9001—not 9069. Both test/e2e/swap-manager.test.ts:39 and test/e2e/arkade-swaps.test.ts:252 create new BoltzSwapProvider({ network: "regtest" }) without specifying apiUrl, so they use the SDK's default port 9001. Either add apiUrl: "http://localhost:9001" to those BoltzSwapProvider constructors, or change the setup probe to check port 9001.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/setup.mjs` around lines 30 - 35, The readiness probe is checking
Boltz on port 9069 while the SDK's BoltzSwapProvider defaults to port 9001,
causing mismatch; fix by updating the BoltzSwapProvider constructors in
swap-manager.test.ts and arkade-swaps.test.ts to include apiUrl:
"http://localhost:9001" (e.g., new BoltzSwapProvider({ network: "regtest",
apiUrl: "http://localhost:9001" })); alternatively, if you prefer changing the
probe, update the URL used in setup.mjs's execSync curl call to
http://localhost:9001/v2/swap/submarine so the readiness check matches the SDK
default.
| await waitForArkServer(); | ||
| await waitForBoltzPairs(); |
There was a problem hiding this comment.
Make the Ark probe fail on HTTP errors before using it as the only gate.
waitForArkServer() still shells out to curl -s, which returns exit code 0 on HTTP 404/500 responses. Now that setup() only waits on readiness probes, a transient error page on :7070 can mark the environment as ready and move the flake into the tests.
🔧 Minimal hardening
- execSync("curl -s http://localhost:7070/v1/info", {
+ execSync("curl -fsS http://localhost:7070/v1/info", {
stdio: "pipe",
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/setup.mjs` around lines 53 - 54, The Ark readiness probe can wrongly
succeed on HTTP error pages because waitForArkServer() shells out to curl -s
which exits 0 on 404/500; update waitForArkServer() so the curl invocation uses
the fail flag (e.g., --fail or -f with -sS) or otherwise checks the HTTP status
and returns non-zero on non-2xx responses, so the setup() readiness gate (await
waitForArkServer()) will fail on HTTP errors instead of treating an error page
as healthy.
Git submodules on Windows lose execute permissions. Ensure scripts are executable before running in CI.
🔍 PR Review —
|
- Change docker exec container from 'arkd' to 'ark' (arkade-regtest name) - Add nigiri build dir to GITHUB_PATH so tests can find the binary
🔍 Review: arkade-regtest submodule migrationMajor cleanup — removes 748 lines of bespoke docker-compose, nginx CORS config, and setup scripting in favor of the shared Observations:
Cross-repo: Coordinated with arkade-os/ts-sdk#386, arkade-os/dotnet-sdk#56, and ArkLabsHQ/arkade-regtest#1. Clean migration. Main question is the v0.8.11 vs v0.9.0 divergence with ts-sdk. |
Iterative Review — New CommitsNew commits since last review:
Changes
Both are CI plumbing fixes discovered during the first CI run. The Cross-repoSibling PRs: arkade-os/ts-sdk#386, arkade-os/dotnet-sdk#56 — all three share the same LGTM — straightforward CI fixes. |
🔍 Arkana Review — boltz-swap#106Same pattern as ts-sdk#386 — clean migration to arkade-regtest submodule. Net -749 lines of removed infra is significant. Highlights
Observations1. Same submodule branch concern as ts-sdk#386 2. -const arkdExec = "docker exec -t arkd";
+const arkdExec = "docker exec -t ark";Good catch — consistent with the nigiri naming convention. Verify no other files reference the old 3. Boltz pairs readiness check in Cross-Repo |
🔍 Arkana Review — Updated CommitsScope: Replace What Changed (vs. previous review)Submodule pointer updated to Key Points
Cross-RepoDepends on No Issues FoundThis is the cleanest of the three SDK migrations — boltz-swap had the least custom infrastructure to begin with. The version pin to v0.8.11 is the only meaningful SDK-specific config. |
|
Followup review (commit |
|
Iterative review (new commits since last review) New commit adds ✅ Correct approach — the override compose in arkade-regtest plumbs these via No issues with this change. Pairs with the |
|
Iterative review (commit Bumps arkade-regtest submodule to always recreate arkd containers when env vars change. This pairs with the previous All four coordinated PRs (arkade-regtest#1, ts-sdk#386, boltz-swap#106, dotnet-sdk#56) are aligned. LGTM ✅ |
🔍 Arkana PR Review — boltz-swap #106PR: feat: use arkade-regtest for regtest environment OverviewClean migration from a custom ✅ What looks good
|
🔍 Arkana PR ReviewPR: feat: use arkade-regtest for regtest environment OverviewReplaces custom ✅ What looks good
📝 Minor notes
🔒 Security
Cross-repo notePart of coordinated migration with wallet#492 and ts-sdk#386. |
🔍 Arkana PR Review —
|
🔍 Arkana PR ReviewOverall: Clean parallel migration to shared regtest. Same pattern as ts-sdk#386, well-executed. -749 lines of duplicated infra. ✅ What looks good
|
Set BITCOIN_LOW_FEE=false to avoid restarting bitcoind, which breaks nbxplorer's connection and causes arkd to have zero balance.
|
Review (followup commit Sets Solid fix — the root cause (nbxplorer losing its zmq subscription when bitcoind restarts) is a known footgun in regtest setups. Disabling the low-fee config avoids the restart entirely rather than trying to reconnect, which is the right call for CI. No security concerns. ✅ |
🔍 Review — arkade-regtest migration (boltz-swap)Net: +115 / −749 — massive simplification, removing the custom ✅ What works well
|
|
Iterative review — new commit This is the key fix for boltz-swap: Combined with |
🔍 Arkana PR Review — boltz-swap#106Scope: Replace custom test infrastructure with Changes — Follows Established Pattern ✅Same migration pattern as ts-sdk#386. Removes Key Observations1. Pins arkd v0.8.11 (older than ts-sdk's v0.9.0 and wallet's v0.9.1) This is likely intentional — boltz-swap may depend on v0.8.x API/behavior. Worth confirming this is deliberate and not just a stale pin. If boltz-swap works with v0.9.x, upgrading would reduce version sprawl. 2. Zero fees + block scheduler Comment: "boltz-swap tests don't account for intent fees". Correct approach — explicit about why fees are zeroed. 3. 4. CI 5. Test file - "test:integration-docker": "vitest run test/e2e/**",
+ "test:integration-docker": "vitest run test/e2e/**",The actual test change in VerdictClean migration following the same pattern. The v0.8.11 pin is the main thing to double-check for staleness. 👍 |
- ARKD_VTXO_TREE_EXPIRY 20 -> 512 (20 blocks expired during 30+ block setup, causing VTXO_RECOVERABLE errors in reverse/chain swaps) - Update regtest submodule to eecb40e (passes ARK fee env vars to arkd)
|
Follow-up review (new commits: Two submodule bumps tracking arkade-regtest fixes:
The last fix is critical for boltz-swap since tests assume zero fees. Without it, the |
ARKD_ALLOW_CSV_BLOCK_TYPE=true is required when SCHEDULER_TYPE=block, otherwise VTXO_TREE_EXPIRY=512 is interpreted as seconds and arkd refuses to start.
🔍 Arkana PR Review —
|
arkd interprets values >= 512 as seconds (BIP 112 threshold). Set to 200 blocks which survives the ~30 block setup while staying in the block-interpretation range. Also enable ALLOW_CSV_BLOCK_TYPE=true.
|
🔍 Iterative review — changes since New commit Good catch on the BIP 112 threshold — values ≥ 512 are interpreted as seconds by the CSV opcode, which conflicts with
Cross-repo note: wallet#492 uses |
🔍 Arkana PR Review — boltz-swap#106Summary: Replaces custom ✅ Looks Good
|
|
Iterative review (commit Single submodule bump: picks up conditional nbxplorer restart from arkade-regtest. Same pattern as the other SDK PRs in this batch. No concerns. |
|
Follow-up review (new commits since last review) Latest commits point the regtest submodule to master after the arkade-regtest PR merged. Key state since last review: Config (
CI — Clean split into lint/unit/build/integration jobs. Integration gates on the other three. Failure log capture covers all relevant containers. Cross-repo alignment — Same submodule commit ( No security concerns. The Looks ready pending CI green on integration. |
|
Tried out, it's missing some plumbing: |
2 participants
Summary
Test plan
./regtest/start-env.shstarts full environmentpnpm test:unitpassespnpm test:integration-dockerpassesSummary by CodeRabbit