Skip to content

chore: send regtest auth from env - #1284

Merged
ovitrif merged 3 commits into
masterfrom
feat/regtest-auth
Nov 11, 2026
Merged

ovitrif merged 3 commits into
masterfrom
feat/regtest-auth

Conversation

@ovitrif

@ovitrif ovitrif commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1283

Twin: synonymdev/bitkit-android#1867
Dependency: synonymdev/bitkit-core#202 (released as https://github.com/synonymdev/bitkit-core/releases/tag/v0.8.0-rc14)

Staging Blocktank protects its /regtest/* routes with HTTP basic auth, so the Settings > Blocktank Regtest actions failed with 401. This PR sends the credentials from a local variable in debug builds.

Description

  • Bump bitkit-core from 0.8.0-rc13 to 0.8.0-rc14, whose regtest calls take an optional auth (user:password). The only other change since rc13 is new API the app does not call yet (pubky swaps as a Paykit route, feat: pubky swaps as paykit route bitkit-core#182).
  • Env.blocktankRegtestAuth reads BLOCKTANK_REGTEST_AUTH from the launch environment or config, only in Debug and E2E builds; release builds get nil. CoreService passes it as auth to regtest_mine, regtest_deposit, regtest_pay and regtest_close_channel. Set BLOCKTANK_REGTEST_AUTH=user:password in the scheme's environment, or launch with SIMCTL_CHILD_BLOCKTANK_REGTEST_AUTH. The value is never committed.
  • A 401 from the regtest routes now fails at once with "Blocktank regtest auth is missing or was rejected. Launch the app with BLOCKTANK_REGTEST_AUTH=user:password." instead of a generic error after the retries.
  • journeys/README.md documents the variable.

Design

N/A — no UI changes.

Preview

Settings > Blocktank Regtest > Deposit against staging, without and with BLOCKTANK_REGTEST_AUTH:

Without auth With auth

QA Notes

Manual Tests

  1. Without BLOCKTANK_REGTEST_AUTH, Settings > Blocktank Regtest > Deposit shows the 401 auth error.
  2. With it set, Deposit succeeds and shows the txid.

Automated Checks

  • xcodebuild build for the simulator: passed with rc14 resolved from the release.
  • swiftformat --lint on the changed files: passed.
  • Unit tests run in CI.

Evidence

Cases 1 and 2 passed on a3a35ae (this branch before the rebase onto master, with the same rc14 and app changes), against staging regtest on an iPhone 17 Pro simulator (iOS 27). Without the variable, Deposit failed at once with the auth message; with it, Deposit succeeded (txid 2b7f4926…).

@ovitrif ovitrif added this to the 3.2.0 milestone Nov 11, 2026
@ovitrif ovitrif self-assigned this Nov 11, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-11T13:03:01.543553Z a3a35ae PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ovitrif ovitrif removed this from the 3.2.0 milestone Nov 11, 2026
@ovitrif
ovitrif requested review from a team, ben-kaufman and pwltr and removed request for a team November 11, 2026 13:02
@greptile-apps

greptile-apps Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable issues were found.

Summary

Adds environment-supplied credentials to Blocktank regtest calls.

  • Blocktank Regtest actions can send launch-provided credentials.
  • A rejected Blocktank Regtest login now shows an auth-specific message.

No actionable issues were found. Review was based on source inspection; no build or runtime checks were run.

Reviews (1) · Last reviewed commit: "chore: bump bitkit-core to 0.8.0-rc14" · Reviewed by Greptile

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No findings. Bumps bitkit-core to 0.8.0-rc14 and passes Env.blocktankRegtestAuth (Debug and E2E only) as basic auth to the four regtest calls, failing fast with a clear message on a 401. Reviewed a3a35ae, full tier. Static review: the app was not built for this review, so it is reasoned from the code, the rc14 bindings and CI.

What I checked, and 4 candidates I ruled out

Read in full: Bitkit/Constants/Env.swift (configValue, isDebug, isE2E), BlocktankService.executeWithRetry and the four regtest wrappers in CoreService.swift, ServiceQueue.background / queueError, AppError.init(error:).
Dependency: v0.8.0-rc14 resolves to 8fbf1d7, matching Package.resolved. In bindings/ios/bitkitcore.swift the only removed public signatures between rc11 and rc14 are the five regtest functions, each replaced by one with a trailing auth: String? = nil; no public enum case or init was removed, so existing call sites still compile. regtestGetPayment is not called anywhere in the app.

Ruled out

  • 401 string match never fires after ServiceQueue wraps the error: the wrap keeps the core error's localizedDescription in debugMessage, and BlocktankError.errorDescription is String(reflecting: self), which carries error_details. rc14's regtest_request returns ERR_REGTEST_UNAUTHORIZED (contains 401 Unauthorized) on a 401, so "\(error)" matches. The author's without-auth screenshot shows the message.
  • Credential reaching release builds: blocktankRegtestAuth returns nil unless isDebug || isE2E, and isE2E is false in a non-DEBUG, non-E2E build. The value comes only from the environment, launch arguments or Info.plist, none of which the diff commits.
  • Malformed value without a colon: rc14 splits on the first : and sends an empty password; Blocktank answers 401 and the new message is shown. No crash path.
  • Short-circuiting the retry loop on 401 hides transient failures: a 401 is not transient, and every other error still goes through the six retries.

CI: SwiftFormat, validate and Greptile green; Run Tests, Run Integration Tests and build-local were still pending at review time.

Merge confidence: 4/5, no test exercises the auth path and the build and unit jobs had not finished when this was read.

@pwltr pwltr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed a3a35ae.

The regtest auth change is sound: release builds never read BLOCKTANK_REGTEST_AUTH, nothing is committed, and a 401 fails at once with a clear message.

Medium candidate — The bitkit-core bump brings untested behaviour changes the description calls additive. Bitkit.xcodeproj/project.pbxproj:1244 moves bitkit-core from 0.8.0-rc11 to 0.8.0-rc14. Besides the additive modules, that range changes paths the app already calls:

  • duplicate BIP21 parameter keys are now rejected by the scanner (bitkit-core #151), which is what #909 is meant to adopt;
  • onchain_compose_transaction, used by hardware-wallet sends, now reuses a synced wallet and times out connections holding the wallet lock;
  • trezor-connect-rs is upgraded to 10.0.1;
  • RBF replacement fee rates are preserved;
  • unknown stored fee rates are filled on sync.

The QA notes cover only the regtest screens. Running the hardware-wallet send journey, a boost and the BIP21 scan cases on rc14, or splitting the bump from the regtest change, would settle it.

Review details

General review
Model: Claude Opus 5.5
Reasoning effort: high

@ovitrif

ovitrif commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

@pwltr You're right, that range isn't additive. I corrected the description to name the five changed paths: synonymdev/bitkit-core#151, synonymdev/bitkit-core#166, synonymdev/bitkit-core#188, synonymdev/bitkit-core#192 and synonymdev/bitkit-core#187. Splitting the bump wouldn't help, because the regtest auth needs rc14. So I'll run the hardware-wallet send, a boost and the BIP21 duplicate-key scan cases on rc14 and add the results to QA Notes here.

@jvsena42 jvsena42 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan at a3a35aee: no findings.

  • Debug and E2E only: Env.swift:240-243 returns nil unless isDebug || isE2E, and the environment and launch-argument lookup in configValue is itself inside #if DEBUG || E2E_BUILD. No Info.plist or xcconfig carries the key.
  • Regtest only: all four callers in CoreService.swift (:2307-2365) guard Env.network == .regtest before passing it.
  • Not committed, not logged: git grep finds only the variable name and the workflow secret reference. The new 401 branch throws the core error text, which in rc14 is a fixed constant with the URL redacted (bitkit-core blocktank/api.rs).

The bitkit-core bump from rc11 to rc14 also reaches release paths this PR's QA doesn't cover: duplicate BIP21 keys refused (core#151), the hardware onchain_compose_transaction cache (core#188), the trezor-connect-rs 10.0.1 THP check (core#192), and RBF fee rates (core#166). Those came in with rc12/rc13, which #909 adopts; I gated that today.

rc13 → rc14 adds three things:

  • the regtest auth: parameter, and regtest requests sent from core (core#201, #203);
  • the Pubky Swaps module (core#182);
  • two new PaykitRouteRequest fields (providers, fee_rate_sat_vb) and a PaykitRoute.Swap case.

Nothing on iOS master calls the route API, which is why this builds. #1191 does call it, so it will need the new fields and the .swap case once it is on rc14.

The PR body's Twin: synonymdev/bitkit-android#TWIN is a placeholder.

Device gate (partial): a3a35aee — not run yet; a regtest deposit and mine against staging, with and without BLOCKTANK_REGTEST_AUTH, is next.

Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto master after #909 merged (force push to 4b872da). The bump is now 0.8.0-rc13 to 0.8.0-rc14, and the BIP21 duplicate-key handling comes from master. The simulator build passes.

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

@pwltr #909 has since moved master to 0.8.0-rc13. That brings the BIP21, compose, Trezor, RBF and fee-rate changes you listed, so this PR's bump is now rc13 to rc14: the regtest auth plus new swaps API the app doesn't call yet. I updated the description. I re-ran the regtest cases on rc14 and will post them with the journey runs.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Device gate at 4b872daa6: all three cases pass. The rebase onto master (#909) is clean: git range-diff shows 42842f89a and 0def4fea0 unchanged, and 4b872daa6 only moves the base of the core pin from rc11 to rc13, with rc14 at 8fbf1d75 as before. Approving.

Case Result
No variable Deposit and Mine fail at once with no retries: "Regtest action failed: Blocktank regtest auth is missing or was rejected. Launch the app with BLOCKTANK_REGTEST_AUTH=user:password." (core: 401 Unauthorized).
SIMCTL_CHILD_BLOCKTANK_REGTEST_AUTH set Deposit of 100,000 sats (txid 931471e3…7a83) and 3 blocks mined, and Savings went 200,000 → 300,000.
Credential in logs 0 hits for the value, "Basic" or "authorization" in both app logs. The built Info.plist doesn't contain it.

Device gate: 4b872daa6 — 3 cases passed (iPhone 17 Pro, staging regtest); perf: not relevant; Android: synonymdev/bitkit-android#1867 gate running.

Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Auto-merge on: approved by @jvsena42 on 4b872da with all three device cases passing. @pwltr's rc14-range point is answered: after #909 the bump is rc13 to rc14 only. No open threads. It merges once the required checks finish green.

@ovitrif
ovitrif enabled auto-merge November 11, 2026 14:45

@pwltr pwltr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed 4b872da.

No consequential correctness finding was established by the static General review.

Review details

General review
Model: GPT-6.1 Sol
Reasoning effort: high

@ovitrif
ovitrif merged commit 2f697c1 into master Nov 11, 2026
24 checks passed
@ovitrif
ovitrif deleted the feat/regtest-auth branch November 11, 2026 15:27
@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

@pwltr Here's the follow-up on the extra rc14 runs I said I'd post. They're incomplete: the test Mac was overloaded, and the simulator UI automation kept timing out. Only Trezor emulator pairing over Bridge ran, and it passed on rc14 (synonymdev/bitkit-core#192). The hardware-wallet send, the boost and the BIP21 duplicate-key scan didn't run. Since #909 those changes come from master's rc13, not from this PR's rc13 to rc14 bump, so I'm not rerunning them here.

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.

bug: blocktank regtest actions fail with 401 on staging

4 participants