Upgrade regtest to master

[pietro909]

No description provided.

[coderabbitai]

Warning

Rate limit exceeded

@pietro909 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 37 minutes and 20 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 590d252a-389a-4256-9ed6-705c86827f99

📥 Commits

Reviewing files that changed from the base of the PR and between 9e53c73 and 65ffb11.

📒 Files selected for processing (1)

• regtest

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch upgrade-regtest-2

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[arkanaai]

Review: Upgrade regtest to master

Verdict: APPROVE ✅

This PR bumps the regtest submodule from 3ac33b6 → dc23da2 (current master of ArkLabsHQ/arkade-regtest). Single-line submodule pointer change.

What's in the submodule update (9 commits)

1. ARK_CONTAINER variable (PR #16) — centralizes the ark container name. Auto-derives "arkd" (custom image) vs "ark" (nigiri built-in). All docker exec arkd calls replaced with docker exec "$ARK_CONTAINER". This is the biggest change.
2. Built-in nigiri path rewrite — replaces nigiri ark init with direct admin API calls (/v1/admin/wallet/seed, /create, /unlock, /status). Adds wallet sync wait loop (max 60 attempts × 3s). This aligns the built-in path with the custom-image path.
3. Fulmine bump v0.3.21 → v0.3.23
4. lnurl-server pin main → 0.1.0 (good — pinning to a tag instead of floating main)
5. docker volume prune -f when --prune is used
6. Fulmine image default update (PR #23)

Risk assessment

• Not protocol-critical. This only affects the regtest test environment scripts — no VTXO handling, signing, or on-chain logic. No production code paths touched.
• Cross-repo impact: none. Checked ark-go-sdk, ark-rust-sdk, ark-dotnet-sdk — none use this submodule.
• Container name derivation is safe. The ARK_CONTAINER fallback logic in lib/env.sh defaults to "arkd" when ARKD_IMAGE is set, "ark" otherwise. The docker-compose.arkd-override.yml uses ${ARK_CONTAINER:-arkd} — consistent.
• Wallet init path is more robust. The old nigiri ark init was a single CLI call with no retry semantics. The new path polls the admin API with proper sync-wait, which matches what the custom-image path already does.
• Image pins are good hygiene. lnurl-server:main → 0.1.0 prevents surprise breakage from upstream changes.

Minor observations (non-blocking)

1. clean-env.sh:47 — if ARK_CONTAINER is empty (before load_env is called), the grep pattern becomes ^(ark|arkd|)$ which matches empty strings. Not harmful since docker ps won't return empty names, but worth noting.
2. The wallet sync timeout is 60 × 3s = 180s. Generous but fine for regtest.

CI

• build ✅, lint ✅, unit ⏳ (pending at review time)

LGTM. Clean submodule bump with good improvements to the regtest harness.

[arkanaai]

Review: Upgrade regtest to master

Verdict: REQUEST CHANGES — one integration test is failing; needs investigation before merge.

---

What this PR does

Bumps the regtest submodule pointer from 3ac33b66 → dc23da2c (current master of ArkLabsHQ/arkade-regtest). This is a pure test-harness change — no production SDK code is modified.

The submodule update spans 9 commits across 6 files in the regtest repo:

Commit	Summary
15b5f175	Centralize ark container name via ARK_CONTAINER variable
fcef2031	Align built-in nigiri path with admin API wallet setup
179a2394	Add wallet sync wait loop to built-in path (60 × 3s)
ed8bf907	Merge PR #16 (fix/ark-container-variable)
cb874aca	clean.sh: prune dangling docker volumes with --prune
1a10171e	Merge PR #17 (prune_volumes)
6333e4b8	Pin lnurl-server image from main → 0.1.0
0e99ff7f	Update default Fulmine image
dc23da2c	Merge PR #23 (update_fulmine_image)

---

🔴 Blocking issue — integration CI failure

The integration check is failing on this PR:

FAIL  test/e2e/ark.test.ts > Common > Wallet > should finalize pending transactions
AssertionError: expected [] to have a length of 1 but got +0
  at test/e2e/ark.test.ts:1293:43

The test submits an off-chain transaction manually via arkProvider.submitTx(), sets hasPendingTx: true, calls finalizePendingTxs(), and then waits for an incoming-VTXO notification (waitForIncomingFunds). The assertion at line 1293 expects exactly one incoming VTXO but receives zero.

Why this is likely related to the regtest bump: The new start-env.sh rewrites the built-in nigiri wallet-init path to use the admin API directly (/v1/admin/wallet/seed, /create, /unlock, /status) with a 60-attempt sync-wait loop, replacing the single nigiri ark init call. A timing or ordering difference in how the arkd node becomes ready could affect whether the round that includes this manually-submitted transaction is processed in time for the event to fire.

This is not a pre-existing flake. The prior arkanaai review noted unit ⏳ (pending) — but unit passed and integration failed. The failure is deterministic (one test, consistent error), not a timeout-style flake.

Action needed: Confirm whether this test failure is caused by a regression in the new regtest harness or is pre-existing on main. If the failure exists on main too, document it and re-run; if it's new, bisect which regtest commit introduced it.

---

Non-blocking observations

1. ARK_CONTAINER derivation logic (lib/env.sh): The pattern ^(ark|arkd|)$ in clean-env.sh:47 will match an empty string if ARK_CONTAINER is unset/empty at call time (before load_env runs). Not harmful in practice — docker ps won't return empty container names — but a defensive [ -n "$ARK_CONTAINER" ] guard would be cleaner.

2. Wallet sync timeout is 180 s (60 × 3s). Reasonable for regtest, but if CI runners are slow this could still time out. Consider logging progress ("Waiting for wallet sync... attempt N/60") to make CI failures easier to diagnose.

3. Pinning lnurl-server:main → 0.1.0 is good hygiene and prevents silent breakage from upstream changes. No concerns.

4. Fulmine bump (v0.3.21 → v0.3.23) is minor and expected.

---

Protocol-critical assessment

Not protocol-critical. This PR touches only test-harness shell scripts and docker-compose config inside the regtest submodule. No VTXO logic, transaction signing, forfeit paths, round lifecycle, connector tree construction, or exit path code is modified. Standard human sign-off is sufficient.

---

Summary

The changes in the submodule are well-reasoned improvements (container-name centralisation, more robust wallet init, image pinning). However the PR cannot be merged while one integration test is failing. Please investigate the should finalize pending transactions failure and either fix the regression or confirm it is pre-existing before requesting re-review.