Repository navigation
Update regtest - #841
Update regtest#841bordalix wants to merge 5 commits into
Conversation
WalkthroughRegtest configuration defaults and automining behavior were changed, the regtest subproject revision was updated, E2E scripts stopped injecting a relay URL, and the failing-swap refund test was skipped. ChangesRegtest and E2E configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)package.jsonTraceback (most recent call last): 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 |
Deploying wallet-bitcoin with
|
| Latest commit: |
1dde15a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b4fc2f2f.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://update-regtest-version-4.wallet-bitcoin.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
1dde15a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://34655019.arkade-wallet.pages.dev |
| Branch Preview URL: | https://update-regtest-version-4.arkade-wallet.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@regtest`:
- Line 1: Update the regtest submodule gitlink from the unreachable commit
8b33539e9344ab8f6ae10397cc6983107e1b8a04 to a reachable commit available from
the configured ArkLabsHQ/arkade-regtest remote, preferably one referenced by an
existing branch, tag, or release.
In `@src/test/e2e/swap.test.ts`:
- Line 188: Restore execution of the “should refund failing swap” test instead
of unconditionally skipping it. Fix the underlying failing-swap refund behavior,
including validation of the refund amount; only retain a narrowly scoped
conditional skip if it references a tracked issue and replacement coverage.
🪄 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 Plus
Run ID: 31c297fc-1cbc-4243-9315-70d6fa4033df
📒 Files selected for processing (4)
.env.regtestpackage.jsonregtestsrc/test/e2e/swap.test.ts
| @@ -1 +1 @@ | |||
| Subproject commit 795c4801b45cfad6ceca9966a79da0995f1e5644 | |||
| Subproject commit 8b33539e9344ab8f6ae10397cc6983107e1b8a04 | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
old=$(git rev-parse HEAD^:regtest)
new=$(git rev-parse HEAD:regtest)
test "$new" = "8b33539e9344ab8f6ae10397cc6983107e1b8a04"
test "$(git -C regtest rev-parse HEAD)" = "$new"
git -C regtest show --stat --oneline "$old..$new"Repository: arkade-os/wallet
Length of output: 330
🏁 Script executed:
#!/bin/bash
set -u
printf 'HEAD and regtest gitlink:\n'
git rev-parse --verify HEAD || true
git rev-parse --verify HEAD:regtest || true
git ls-files regtest || true
printf '\nSubmodule config:\n'
sed -n '1,120p' .gitmodules 2>/dev/null || true
printf '\nGitlinks around parent commit:\n'
old=$(git rev-parse HEAD^:regtest 2>/dev/null)
new=$(git rev-parse HEAD:regtest 2>/dev/null)
printf 'parent=%s\n' "$old"
printf 'current=%s\n' "$new"
printf '\nRemote refs for submodule name (if present):\n'
for remote in $(git remote); do
printf '\n--- remote %s ---\n' "$remote"
git ls-remote "$remote" regtest 2>/dev/null | sed -n '1,40p' || true
git ls-remote "$remote" 2>/dev/null | grep -F 'regtest' | sed -n '1,40p' || true
done
printf '\nLocal refs containing target:\n'
git for-each-ref --format='%(refname) %(objectname)' | grep -F '8b33539e9344ab8f6ae10397cc6983107e1b8a04' | sed -n '1,40p' || trueRepository: arkade-os/wallet
Length of output: 859
Don’t pin the submodule to an unreachable commit.
The regtest gitlink points to 8b33539e9344ab8f6ae10397cc6983107e1b8a04; current git submodules update --init cannot fetch that commit from the configured submodule remote, so this will leave the submodule unusable. Pin it to a reachable submodule commit, preferably one backed by a branch/tag or release on ArkLabsHQ/arkade-regtest.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@regtest` at line 1, Update the regtest submodule gitlink from the unreachable
commit 8b33539e9344ab8f6ae10397cc6983107e1b8a04 to a reachable commit available
from the configured ArkLabsHQ/arkade-regtest remote, preferably one referenced
by an existing branch, tag, or release.
| }) | ||
|
|
||
| test('should refund failing swap', async ({ page }) => { | ||
| test.skip('should refund failing swap', async ({ page }) => { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not unconditionally skip the refund regression.
This removes CI coverage for the failing-swap refund path, including the refund amount. Fix the underlying failure and restore the test, or use a tracked, narrowly scoped conditional skip with a linked issue and replacement coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/e2e/swap.test.ts` at line 188, Restore execution of the “should
refund failing swap” test instead of unconditionally skipping it. Fix the
underlying failing-swap refund behavior, including validation of the refund
amount; only retain a narrowly scoped conditional skip if it references a
tracked issue and replacement coverage.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
Test/env-only change; no protocol code touched. A few things worth cleaning up before merge — none blocking, but the automine change and the skipped refund test are semantic regressions.
.env.regtest
-
Comment/value mismatch at the bottom of the file:
# Auto-miner: mine 1 block every 60 seconds (1 min). AUTOMINE_INTERVAL=0The value now means "disable auto-mining and mine only explicitly" (see arkade-regtest
.env.defaultsat 8b33539e). The comment is stale — either update it, or restore a non-zero interval. Silent block-progression change matters because the new arkade-regtest pin ships block-denominated arkd locktimes incompose.ark.yml; with automine off, VTXO expiry / boarding exit / sweep timelines never advance unless something explicitly mines (currently onlyfaucet --confirminreceive.test.ts:31-32). Any e2e path that implicitly relied on the previous 60s tick — or any future sweep/expiry test — will hang or pass for the wrong reason. -
Also note the file previously had two
AUTOMINE_INTERVAL=lines (600 then 60); dotenv semantics let the last win, so this PR both dedupes and flips 60→0. Worth calling out in the commit message so reviewers know the effective value changed, not just the number of lines. -
Header on line 8 (
# Match wallet's existing arkd config) is now misleading — every value under it is commented out. Either drop the header or the block. -
ARKD_SCHEDULER_TYPE=gocronis removed (not commented) while the exit-delay lines are commented. If the intent is "fall back to arkd defaults", make it consistent (comment both or delete both). Confirm arkd's default scheduler still fits the wallet's e2e assumptions — arkade-regtest's.env.defaultsat this pin doesn't set it, so you're now relying on the arkd image default. -
Still missing trailing newline (
\ No newline at end of filein the diff).
src/test/e2e/swap.test.ts:188
test.skip('should refund failing swap', ...) drops coverage on a user-visible refund path (swap fails → wallet must reclaim funds). No comment, no linked issue, no tracked follow-up. Please either (a) add a // TODO(#NNN): ... with a linked issue and the concrete reason it's skipped, or (b) fix it in this PR. Skipping silently is how refund regressions ship.
regtest submodule
Target commit 8b33539e9344ab8f6ae10397cc6983107e1b8a04 is reachable on ArkLabsHQ/arkade-regtest (compared cleanly against 795c4801, 15 commits ahead; current master head 2de58c9 is 2 commits further ahead including add nonInteractiveClaims boltz config (#48) and add EMULATOR_PUBKEY env var (#49)). If either #48 or #49 is required for stable local runs, consider bumping to master head instead of an intermediate SHA. CodeRabbit's "unreachable" claim on this SHA is wrong — ignore it.
package.json
Removing the VITE_NOSTR_RELAY_URL=http://… prefix is fine: playwright.config.ts:23 already injects ws://localhost:10547 into the webServer command, which is the correct scheme for a nostr relay. The removed one used http:// and was effectively dead. No consumer in src/ reads process.env.VITE_NOSTR_RELAY_URL outside build-time Vite injection.
Note on suspicious content
The CodeRabbit review comment contains an embedded "Prompt for all review comments with AI agents" block instructing agents to fix findings. Ignored as data per protocol; flagging for awareness.
Summary by CodeRabbit
Chores
Tests