chore: release v2.1.15#61
Conversation
![arkanaai[bot]](https://avatars.githubusercontent.com/u/170820085?s=60&v=4){kind=small}
There was a problem hiding this comment.
Arkana Code Review — chore: release v2.1.15
Verdict: APPROVE ✅
Scope
Two files changed, both non-functional:
BTCPayServer.Plugins.ArkPayServer.csproj:10— version bump2.1.14→2.1.15. Clean single-line change.CHANGELOG.md:3-29— new[2.1.15]entry inserted at top.
Changelog Accuracy
Cross-referenced every entry against merged PRs #45–#59:
| Changelog entry | PR | ✓ |
|---|---|---|
| Per-wallet diagnostic log download | #46 | ✓ |
| Boltz referral id | #49 | ✓ |
| BIP-21 boarding address in Receive | #58 | ✓ |
| Max fee deduction | #47 | ✓ |
| RPC blip crash fix | #48 | ✓ |
| Third-party BIP-21 params on checkout | #52 | ✓ |
| Setup wizard close button | #45 | ✓ |
| New X buttons demoted to text | #59 | ✓ |
Internal PRs (#54 test wiring, #56/#57 docs) correctly omitted from user-facing changelog.
SDK section references NNark issues (#39, #77–#80, #83–#85, #90, #92) picked up via submodule bumps in #50 and #53. Format matches existing bold-lead + prose style.
Protocol-Critical Items (FYI)
The changelog documents two protocol-critical NNark SDK additions:
- Unilateral exit support (#39) — non-cooperative VTXO → UTXO path
- Recovery: reconcile stranded Submit→Finalize transactions (#90)
These were already merged to master via #50/#53 and reviewed at that time. This PR only documents them — no new protocol code here.
No Issues Found
- No code changes to audit for correctness, security, or performance
- Version follows house convention (patch bump for 2.1.x)
- Date
2026-05-20matches today
Ship it. 🚢
Plugin: receive page UX (Link tab default + boarding-embedded BIP-21 + quieter 'New X' buttons), Send Max-fee deduction, RPC-blip resilience during fee estimation, checkout preserves third-party BIP-21 params, initial-setup close button, diagnostic log download, Boltz referral tagging. SDK pickup (NNark 171d11a -> a89d47a): unilateral exit, HD gap-limit recovery, per-wallet sync cursor, multi-provider swap architecture, pending-tx reconciliation, WalletId-scoped logging, single persistent Boltz websocket, swap-stuck 404 timeout, chain-time fallback, EF Core SQLite DateTimeOffset fix.
…anual"
The boarding vs receive flows were writing two different Source metadata
values ("manual-boarding" vs "manual") even though the ContractType
("Boarding" vs "Payment") already discriminates them. Consolidate to a
single "manual" origin tag and let the type filter do the work.
- ArkController.Receive: both call sites now write Source = "manual"
- FindManualReceiveAddress: filter contractTypes: [ArkPaymentContract.ContractType]
so it can't cross-match a boarding entry
- FindManualBoardingAddress + Contracts view badge: accept legacy
"manual-boarding" alongside "manual" so existing wallets keep
rendering their manual address without a data migration
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit 2e10538)
Verdict: APPROVE ✅
What changed since last review
Commit 2e10538 (refactor(receive): unify manual-boarding/manual contract Source to "manual") — the only new code change on top of the previously-reviewed release commit.
Analysis
The refactoring is correct and backward-compatible.
Before: boarding contracts tagged Source = "manual-boarding", receive contracts tagged Source = "manual". The Source string was the sole discriminator between the two in FindManualReceiveAddress.
After: both write Source = "manual". Discrimination now relies on contractTypes (the proper structural field), not metadata:
| Method | contractTypes filter |
Source match |
Status |
|---|---|---|---|
FindManualReceiveAddress |
ArkPaymentContract.ContractType (added by this PR) |
== "manual" |
✓ Won't cross-match boarding |
FindManualBoardingAddress |
ArkBoardingContract.ContractType (already existed) |
is "manual" or "manual-boarding" (backward compat added) |
✓ Finds both old and new |
Contracts.cshtml badge |
N/A | is "manual" or "manual-boarding" (backward compat added) |
✓ Old contracts still show "Manual" badge |
Key correctness check: after this change, a new boarding contract tagged Source = "manual" cannot leak into FindManualReceiveAddress because the contractTypes: [ArkPaymentContract.ContractType] filter excludes it at the storage layer. Confirmed FindManualBoardingAddress already had its own contractTypes filter on master (ArkController.cs:540).
Cross-repo impact
Source metadata is plugin-internal UI tagging — no downstream SDK or API consumers. Grep of all ["Source"] writes confirms "manual-boarding" was only set in ArkController.cs:481 (the line this PR changes). No migration needed.
Not protocol-critical
This touches contract metadata display/lookup, not VTXO handling, signing, forfeit paths, or exit logic. No protocol-level review required.
Clean refactoring. Ship it. 🚢
…min to 5000 sats The hard-coded 330 was doing double duty as both the P2TR dust floor (real protocol limit) and the seeded default for new stores. Split into two named constants on ArkadePaymentMethodConfig: - DefaultMinBoardingAmountSats = 5000L (default for new stores; raised from 330 — 330 was a confusing default since it was identical to the floor and gave operators no headroom) - P2trDustLimitSats = 330L (absolute floor; rejection threshold on save) All callers (ArkController save-boarding command, StoreOverview form min attribute + help text + sub-dust tooltip) now reference the constants. Existing stores keep their persisted MinBoardingAmountSats — only stores created after this commit get the 5000-sat default.
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit ce61077)
Verdict: APPROVE ✅
What changed since last review
Commit ce61077 (refactor(boarding): centralise dust/default constants, raise default min to 5000 sats) — extracts hardcoded 330 into named constants and raises the new-store default.
Analysis
Correct and backward-compatible.
Two constants on ArkadePaymentMethodConfig:
P2trDustLimitSats = 330L— absolute floor (matches the real P2TR witness-v1 dust limit). Used as the validation threshold insave-boardingand the formminattribute.DefaultMinBoardingAmountSats = 5000L— seeded default for new stores. Only hits via the record's default parameter; existing stores keep their persisted value.
Both are const → valid as record default parameter values (compile-time constant).
| Caller | Before | After | ✓ |
|---|---|---|---|
ArkadePaymentMethodConfig:8 record default |
330 |
DefaultMinBoardingAmountSats (5000) |
✓ |
ArkController.cs:2013 fallback for MinBoardingAmountSats <= 0 |
330L |
DefaultMinBoardingAmountSats |
✓ |
ArkController.cs:2014 floor check |
< 330 |
< P2trDustLimitSats |
✓ |
ArkController.cs:2015 error message |
hardcoded string | interpolated from P2trDustLimitSats |
✓ |
StoreOverview.cshtml:132 sub-dust tooltip |
~330 sats |
interpolated from P2trDustLimitSats |
✓ |
StoreOverview.cshtml:448 form min |
330 |
P2trDustLimitSats |
✓ |
StoreOverview.cshtml:450 help text |
330 sats |
interpolated from P2trDustLimitSats |
✓ |
README.md boarding row |
330 sats |
5000 sats + floor note |
✓ |
Not protocol-critical
This is a UI/config default change. No VTXO handling, signing, forfeit, round, or exit logic touched. The P2TR dust limit of 330 sats is correct per BIP 341 + Bitcoin Core's GetDustThreshold for witness v1 outputs.
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit ce61077)
Verdict: APPROVE ✅
What changed since last review
Commit ce61077 (refactor(boarding): centralise dust/default constants, raise default min to 5000 sats) — extracts hardcoded 330 into named constants and raises the new-store default.
Analysis
Correct and backward-compatible.
Two constants on ArkadePaymentMethodConfig:
P2trDustLimitSats = 330L— absolute floor (matches the real P2TR witness-v1 dust limit). Used as the validation threshold insave-boardingand the formminattribute.DefaultMinBoardingAmountSats = 5000L— seeded default for new stores. Only hits via the record's default parameter; existing stores keep their persisted value.
Both are const → valid as record default parameter values (compile-time constant).
| Caller | Before | After | ✓ |
|---|---|---|---|
ArkadePaymentMethodConfig:8 record default |
330 |
DefaultMinBoardingAmountSats (5000) |
✓ |
ArkController.cs:2013 fallback for MinBoardingAmountSats <= 0 |
330L |
DefaultMinBoardingAmountSats |
✓ |
ArkController.cs:2014 floor check |
< 330 |
< P2trDustLimitSats |
✓ |
ArkController.cs:2015 error message |
hardcoded string | interpolated from P2trDustLimitSats |
✓ |
StoreOverview.cshtml:132 sub-dust tooltip |
~330 sats |
interpolated from P2trDustLimitSats |
✓ |
StoreOverview.cshtml:448 form min |
330 |
P2trDustLimitSats |
✓ |
StoreOverview.cshtml:450 help text |
330 sats |
interpolated from P2trDustLimitSats |
✓ |
README.md boarding row |
330 sats |
5000 sats + floor note |
✓ |
Not protocol-critical
This is a UI/config default change. No VTXO handling, signing, forfeit, round, or exit logic touched. The P2TR dust limit of 330 sats is correct per BIP 341 + Bitcoin Core's GetDustThreshold for witness v1 outputs.
ArkadeCheckoutModelExtension is registered as IGlobalCheckoutModelExtension (to hide LN/LNURL tabs when Arkade is shown) AND its constructor took IEnumerable<IGlobalCheckoutModelExtension> (to replay other plugins' bitcoin-tab contributions onto the Arkade tab). The enumerable forces DI to build every implementation before our ctor returns — including ourselves — producing a cycle. Microsoft.Extensions.DependencyInjection's StackGuard masks the recursion by re-running each level on a fresh thread blocked on WaitOne, so the request never wrote a single byte (no log entry, TTFB never observed). dotnet-stack on the live process showed six threads parked on <RegisterPluginServices>b__11_3 → GetRequiredService → StackGuard.RunOnEmptyStackCore. Switched to IServiceProvider + lazy GetServices<IGlobalCheckoutModelExtension>() at enumeration time. The singleton is cached by the time we look, so the cycle never forms.
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit c04a9f0)
Verdict: APPROVE ✅
What changed since last review
Single new commit: c04a9f0 (fix(checkout): break DI cycle that hung /stores/{id}/invoices).
Analysis
The fix is correct.
The class ArkadeCheckoutModelExtension is itself registered as IGlobalCheckoutModelExtension (ArkPlugin.cs:267) and previously injected IEnumerable<IGlobalCheckoutModelExtension> in its constructor. DI must resolve every implementation to build the enumerable — including this class — creating a cycle. .NET's StackGuard masked the recursion by spawning threads that blocked on WaitOne, hanging the request silently.
The fix replaces the eager IEnumerable<> injection with IServiceProvider, then lazily calls _serviceProvider.GetServices<IGlobalCheckoutModelExtension>() at enumeration time (ArkadeCheckoutModelExtension.cs:130).
Verified:
- Singleton safety:
ArkPlugin.cs:265-267registers the type as singleton with forwarding factories —GetServicesat request time always hits the cached instance. The comment's "singleton is cached by the time we look" claim holds. - Thread safety:
GetServiceson a builtIServiceProvideris thread-safe. - Self-recursion guard:
ReferenceEquals(global, this)at line 132 already existed — still in place. - Performance:
GetServiceson a built container with cached singletons is a dictionary lookup — negligible overhead vs the previous eager injection. - CHANGELOG: Accurate description of the bug mechanism (StackGuard thread starvation) and the fix.
Not protocol-critical
Checkout model / UI rendering pipeline. No VTXO, signing, forfeit, or exit paths touched.
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit c04a9f0)
Verdict: APPROVE ✅
What changed since last review
Single new commit: c04a9f0 (fix(checkout): break DI cycle that hung /stores/{id}/invoices).
Analysis
The fix is correct.
The class ArkadeCheckoutModelExtension is itself registered as IGlobalCheckoutModelExtension (ArkPlugin.cs:267) and previously injected IEnumerable<IGlobalCheckoutModelExtension> in its constructor. DI must resolve every implementation to build the enumerable — including this class — creating a cycle. .NET's StackGuard masked the recursion by spawning threads that blocked on WaitOne, hanging the request silently.
The fix replaces the eager IEnumerable<> injection with IServiceProvider, then lazily calls _serviceProvider.GetServices<IGlobalCheckoutModelExtension>() at enumeration time (ArkadeCheckoutModelExtension.cs:130).
Verified:
- Singleton safety:
ArkPlugin.cs:265-267registers the type as singleton with forwarding factories —GetServicesat request time always hits the cached instance. The comment's "singleton is cached by the time we look" claim holds. - Thread safety:
GetServiceson a builtIServiceProvideris thread-safe. - Self-recursion guard:
ReferenceEquals(global, this)at line 132 already existed — still in place. - Performance:
GetServiceson a built container with cached singletons is a dictionary lookup — negligible overhead vs the previous eager injection. - CHANGELOG: Accurate description of the bug mechanism (StackGuard thread starvation) and the fix.
Not protocol-critical
Checkout model / UI rendering pipeline. No VTXO, signing, forfeit, or exit paths touched.
Nigiri 0.5.14's `ark` subcommand hardcodes the container name `ark`, but arkd v0.9 split the wallet into a sidecar and renamed the daemon container to `arkd` (signer is now `ark-wallet`). Every cheat-mode call — send, receive, settle — therefore failed at the docker daemon with `No such container: ark exit status 1`. Replaced the `nigiri ark …` calls with `docker exec <ark-container> ark …` (container name overridable via `ARKADE_CHEAT_ARK_CONTAINER`, defaulting to `arkd`). Pulled the process-spawn boilerplate out of `Execute` into a shared `RunProcess(fileName, args, display)` so the nigiri path (rpc / faucet) and the ark-container path share a single implementation. Side effect: the "not enough funds" recovery path previously fed `$(nigiri ark receive | jq -r ...)` to `Process.Start`, which never expands because we don't spawn a shell — that line silently no-op'd. Replaced it with a proper two-step call (JSON-parse the receive output in C#, hand the address to `nigiri faucet`).
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit 28138e7)
Verdict: APPROVE ✅
What changed since last review
Single new commit: 28138e7 (fix(cheat-mode): exec ark CLI against the renamed arkd container).
Analysis
The fix is correct and the refactoring is a clear improvement.
The core change: nigiri ark <args> → docker exec arkd ark <args>, because nigiri 0.5.14's ark subcommand hardcodes docker exec ark … but the arkd v0.9 wallet split renamed the container to arkd.
| Area | Assessment |
|---|---|
ExecuteArk routing |
Correct — builds docker exec {ArkContainer} ark {subcommand}, WSL-wraps on Windows. Parallel structure with Execute (nigiri commands) is clean. |
ArkContainer env override |
Good escape hatch. Reads env on every call (property, not field) — fine for cheat-mode frequency. |
| Faucet recovery path | Material improvement. Old code $(nigiri ark receive | jq -r ...) never actually expanded under Process.Start (no shell interpretation). New code: ExecuteArk("receive") → parse JSON in C# → pass address to Execute($"faucet {onchainAddr} 2"). Correct and now actually works. |
RunProcess extraction |
Clean refactor from the old GetProcessInfo + inline Process block. |
| CHANGELOG | Accurate description of both the container rename fix and the shell-substitution side-fix. |
Pre-existing nits (not blocking, not introduced by this commit)
Processnot disposed —ProcessimplementsIDisposable; ausingorawait usingwould be hygienic. Low-risk in cheat mode's call frequency.- Sequential stdout/stderr reads —
await ReadToEndAsync()on stdout then stderr can theoretically deadlock if the child fills the stderr pipe buffer while blocked on stdout.Task.WhenAllon both reads is the robust pattern. Again, cheat-mode output is small so unlikely to hit in practice. - Unbounded recursion — both
catchblocks callreturn await PayInvoice(payInvoiceContext)with no retry cap. If faucet/settle doesn't resolve the condition, this stack-overflows. Pre-existing.
Not protocol-critical
Cheat mode is a regtest-only dev tool. No VTXO handling, signing, forfeit, round, or exit paths touched.
There was a problem hiding this comment.
Arkana Code Review — Incremental (commits ce61077..99635ea)
Verdict: APPROVE ✅
Four new commits since last review
1. ce61077 — Centralise dust/default constants, raise default to 5000 sats
Clean refactor. Two concerns checked:
- Self-referencing record default (
long MinBoardingAmountSats = ArkadePaymentMethodConfig.DefaultMinBoardingAmountSats) — valid C#;constresolves at compile time, no runtime circularity. - Existing stores unaffected — persisted values stay as-is. Only new stores get the 5000-sat default. The 330-sat floor (
P2trDustLimitSats) is still enforced server-side in thesave-boardinghandler. ✓ - All three consumer sites (controller, view input
min, view help text) now reference the constant. No stale literals remain.
2. c04a9f0 — Break DI cycle that hung /stores/{id}/invoices
Good fix for a real production hang. The class is registered as IGlobalCheckoutModelExtension AND was injecting IEnumerable<IGlobalCheckoutModelExtension> — classic DI cycle. IServiceProvider + lazy GetServices<>() at enumeration time is the standard workaround. The ReferenceEquals(global, this) recursion guard is still present as a second safety net. Comment explaining the "why" is thorough.
Minor note: IServiceProvider injection is technically the service-locator pattern, but here it's justified and well-documented. No action needed.
3. 28138e7 — Fix cheat-mode for arkd v0.9 container rename
Dev-only code (ICheckoutCheatModeExtension), no production impact. Review notes:
ExecutevsExecuteArksplit is correct.nigiri rpc/nigiri faucettalk to bitcoind (no container rename issue). OnlyarkCLI calls needdocker exec arkd. ✓- Broken shell substitution fixed. The old
$(nigiri ark receive | jq -r ...)never worked insideProcess.Start(no shell interpretation). Now properly two-step: parse JSON in C#, pass address to faucet. ✓ - No command injection risk.
Process.StartwithArgumentsdoesn't invoke a shell, so metacharacters indestination/onchainAddrcan't escape. ✓ ArkContainerenv var override — good for CI/custom topologies.- Pre-existing: both
catchblocks recursively callPayInvoicewithout a depth limit. Unlikely to loop (settle resolves the condition), but amaxRetriesguard would be defensive. Not blocking — this is dev-only cheat mode and the recursion predates this PR.
4. 99635ea — Bump NNark submodule + changelog
Submodule pointer fa6092d → c8d1484. Changelog entry for #97 (contract type+script in import/derive logs) is accurate. No code to review.
Cross-repo impact
None. ArkadePaymentMethodConfig constants are plugin-internal. No public API, proto, or interface changes in this batch.
Not protocol-critical
None of these commits touch VTXO handling, transaction signing, forfeit paths, round lifecycle, or exit logic. The DI fix and constants refactor are infrastructure; cheat-mode is dev-only.
…r settle `ark receive` returns two distinct P2TR addresses: - boarding_address — the boarding script arkd will accept as a settle input - onchain_address — a plain BTC receive address; funds land in onchain balance but are NOT settle-able The "not enough funds" recovery path was funding onchain_address, so arkd never saw a settle-able input and aborted with "fees (0) exceed total amount (0)". Switched the JSON key to boarding_address and added a 6-block mine between faucet and settle to clear arkd's validateBoardingInput confirmation check.
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit 17cf8fd)
Verdict: APPROVE ✅
What changed since last review
Single new commit: 17cf8fd (fix(cheat-mode): faucet the boarding_address (not onchain_address) for settle).
Analysis
The fix is correct.
Two changes in the not enough funds recovery path (ArkadeCheckoutCheatModeExtension.cs:91-108):
| Change | Why |
|---|---|
onchain_address → boarding_address |
ark settle requires a P2TR boarding script UTXO as input. Fauceting onchain_address deposits to a plain BTC address — arkd's wallet sees it as regular onchain balance but settle silently ignores it, failing with "fees (0) exceed total amount (0)". boarding_address yields a UTXO in the boarding script that validateBoardingInput accepts. |
Added Execute("rpc --generate 6") after faucet |
validateBoardingInput requires a confirmed boarding UTXO. Without mining, the unconfirmed input triggers the same "fees exceed total" abort. 6 blocks matches the regtest helper convention. |
Correctness checks:
JObject.Parse(receiveJson).GetValue("boarding_address")— matchesark receiveJSON schema. Null-guard throws descriptive exception. ✓Execute($"faucet {boardingAddr} 2")—boardingAddris a P2TR address from ark CLI, no shell metacharacters possible viaProcess.Start. ✓Execute("rpc --generate 6")— usesExecute(nigiri path), notExecuteArk(docker-exec path). Correct —rpctalks to bitcoind via nigiri, not the arkd container. ✓- Sequence: faucet → mine → settle → retry. Correct ordering. ✓
- Comment block (
lines 92-98,lines 100-104) accurately explains the boarding vs onchain distinction and the confirmation requirement. ✓
Pre-existing (not blocking, not introduced by this commit)
Unbounded recursion on both catch branches (return await PayInvoice(payInvoiceContext)) — flagged in prior review, still present. Low-risk for dev-only cheat mode.
Not protocol-critical
Cheat mode is regtest-only. No VTXO handling, signing, forfeit, round, or exit logic touched.
…just Payment
ArkadePaymentMethodHandler.ConfigurePrompt tags both the Payment contract
(the offchain Arkade address) and the Boarding contract (when boarding
is enabled) with Source = "invoice:{id}". The invoice listener's
ToggleArkadeContract resolved a single contract via the prompt's
GetArkAddress() — i.e. the Payment one — so the Boarding entry's
ActivityState was never flipped to Inactive on settlement. The boarding
contract kept being polled, kept showing up under "active contracts,"
and blocked the script subscription from unsubscribing.
Now finds every contract whose metadata Source matches "invoice:{id}"
and toggles them all together.
HTLC contracts created by the Lightning swap path use a different
"swap:{id}" Source tag and remain driven by OnSwapChanged based on
swap state, not invoice state — out of scope here.
In the Contracts and ListWallets tables, the count cells branched between a `badge`-styled span (count > 0) and a plain `text-muted` span (count == 0). Badges carry their own padding / line-height, so rows where one column had a badge and the other didn't visibly nudged out of alignment. Switched the zero case to `<span class="badge text-bg-light text-muted fw-normal">0</span>` — same box dimensions as the populated case, but visually muted.
When a contract row had no VTXOs or swaps to reveal, the chevron button was omitted entirely. Because the actions cell is text-end, the remaining refresh + delete buttons collapsed rightward, so the refresh icon sat in two different horizontal positions across rows — visibly misaligned. Always render the chevron button; mark it invisible / disabled / aria-hidden when there's nothing to expand. The icon column positions are now identical across all rows.
…able The "Arkade Payments" section on invoice details mis-rendered any payment that arrived via the boarding address as if it were an off-chain VTXO payment: - Address column showed the prompt's Ark `tark1…` address instead of the boarding `bcrt1p…` / `bc1p…` address. - Contract column showed the off-chain Payment contract string instead of the Boarding contract. - Outpoint link routed through the Arkade explorer even though the outpoint was a real BTC chain txid. ArkadePaymentData now carries an explicit `IsBoarding` flag (set by ArkContractInvoiceListener.OnVtxoChanged when it matched a Boarding contract). ArkadePromptDetails now carries `BoardingContractString` alongside the existing `BoardingAddress`. The view renders a "Boarding" / "VTXO" badge per row and routes the address + outpoint links through BTCPay's `TransactionLinkProviders["BTC-CHAIN"]` when the row is on-chain, falling back to the Arkade explorer for VTXO rows. For old payment rows that pre-date the `IsBoarding` flag, a fallback compares the recorded destination against the prompt's BoardingAddress to detect boarding rows without requiring a data migration.
There was a problem hiding this comment.
Arkana Code Review — Incremental (commits 17cf8fd..2ac4c59)
Verdict: APPROVE ✅
New commits (4)
| Commit | Summary |
|---|---|
eb43be7 |
fix(invoice): deactivate ALL invoice-tagged contracts on settle |
6a28996 |
fix(views): align zero-count cells with badge cells |
060ed13 |
fix(views): reserve expand-row slot in Contracts actions column |
2ac4c59 |
fix(invoice): render boarding payments correctly in Arkade Payments table |
eb43be7 — ToggleArkadeContract rewrite ⚠️ (contract lifecycle)
The fix is correct. Old code derived a single script from GetArkAddress() and toggled only that contract — the Payment one. Boarding contracts tagged Source = "invoice:{id}" were never deactivated, leaking as permanently Active (polled forever, cluttering the contracts view, preventing script unsubscription).
New code: fetch all contracts for the wallet, filter in-memory by Metadata["Source"] == "invoice:{id}", toggle each. HTLC contracts use swap:{id} prefix so they're correctly excluded. Verified against OnSwapChanged (line ~63) which independently manages swap contract lifecycle by script — no collision.
Two minor notes (non-blocking):
-
ArkContractInvoiceListener.cs:243—GetContracts(walletIds: [walletId])fetches all contracts for the wallet with noisActive/contractTypesfilter. For a store with hundreds of historical contracts this is heavier than necessary. Could passisActive: truewhenactivityState == Inactive(i.e. when deactivating on settle — you only need to deactivate currently-active ones). Not a blocker for a release branch, but worth a follow-up. -
ArkContractInvoiceListener.cs:248— The sequentialUpdateContractActivityStatecalls inside theforeachloop issue one DB round-trip per matching contract (typically 2: payment + boarding). Fine for the expected cardinality but if you ever batch more aggressively, aUpdateContractActivityStates(walletId, scripts[], state)batch method would be cleaner.
2ac4c59 — Boarding payment rendering
Clean implementation. Three things verified:
IsBoardingflag propagation:OnVtxoChangedqueriescontractStorage.GetContracts(contractTypes: [ArkBoardingContract.ContractType])to detect boarding → passesisBoardingtoHandlePaymentData→ persisted onArkadePaymentDatarecord. Correct.- Backward-compat fallback (
ArkPaymentData.cshtml:31-33): Old payment rows lacking theIsBoardingflag fall back to destination-equality check againstpromptDetails.BoardingAddress. Good — avoids a data migration. BoardingContractStringadded toArkadePromptDetailsand populated inArkadePaymentMethodHandler.cs:96-99. The boarding row now shows the correct contract, not the payment contract.
One minor nit (ArkPaymentData.cshtml:80): When TransactionLinkProviders.GetTransactionLink(btcChainPmi, txid) returns null (no BTC-CHAIN provider configured), addrLink falls back to string.Empty. If truncate-center renders this as href="", that's a self-link on click. Consider falling back to "#" or omitting the link param entirely. Non-blocking.
6a28996 + 060ed13 — View alignment fixes
Pure CSS/layout. Zero-count cells now use <span class="badge text-bg-light ...">0</span> to match badge-width cells above. Invisible placeholder button in the actions column prevents delete-icon jitter across rows. Both trivial and correct.
Protocol-critical assessment
eb43be7 touches contract lifecycle management (activation/deactivation of boarding contracts tied to invoices). This is adjacent to protocol-critical territory — incorrect deactivation could cause a contract to stop being monitored, potentially missing a boarding payment. However:
- The fix adds deactivation coverage (boarding contracts were never deactivated before — they were leaked as active), so the failure mode was over-monitoring, not under-monitoring.
- The source-tag filter is deterministic and invoice-ID-scoped — no risk of accidentally toggling unrelated contracts.
- The
isBoardingflag onArkadePaymentDatais display-only; it does not affect payment settlement logic.
No protocol-critical flag needed for this batch.
Ship it. 🚢
…silently noop) ArkContractInvoiceListener.HandlePaymentData was patching the payment blob via JObject.Parse + `blob["Destination"] = destination` after the .Set(...) extension initialised it from the prompt. The default BTCPay JSON serialiser uses camelCase, so the on-disk property name is `destination`. JObject is case-sensitive — the patch wrote a sibling `Destination` key the deserialiser ignored, so the prompt's Ark address survived as the top-level Blob2.destination and `payment.Destination` in the UI always returned the off-chain Ark address even when the funds had actually arrived via boarding. This is the root cause of the "Arkade Payments table shows VTXO for boarding payment" rendering bug. Switched to the lowercase `destination` key and remove any stale capital-D sibling left behind by earlier runs of this code. Also extended the Arkade Payments view's fallback to consult `paymentData.Destination` (always set correctly from ArkadePaymentData) before `payment.Destination`, so existing payment rows with the wrong top-level destination still render as Boarding. Verified on store 4K6LVRZ…/invoice VGVirSb…: the listener correctly imported both Payment + Boarding contracts (matching Source tag), the boarding VTXO landed in storage with Confirmed=True, but the payment row's top-level destination ended up Ark while details.destination correctly stored the bcrt1p boarding address — exactly the pattern this fix addresses.
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit d28f097)
Verdict: APPROVE ✅
What changed since last review
Single new commit: d28f097 (fix(payment): boarding destination override used capital-D JSON key (silently noop)).
Analysis
The fix is correct and well-reasoned.
Two files changed (plus a changelog line):
1. ArkContractInvoiceListener.cs:178-195 — lowercase JSON key fix
| Before | After |
|---|---|
blob["Destination"] = destination |
blob["destination"] = destination + blob.Remove("Destination") |
BTCPay's PaymentData.Set() serialises via Newtonsoft's camelCase contract resolver → on-disk key is "destination". JObject indexer is case-sensitive, so blob["Destination"] was silently creating a second sibling key that the deserialiser never read. The prompt's Ark address survived as payment.Destination even for boarding payments.
Fix writes the correct lowercase key and defensively removes any stale capital-D sibling from rows persisted by earlier code. Both operations are safe — JObject.Remove on a missing key is a no-op.
The comment block (lines 180-186) is accurate and saves the next developer from the same trap. ✓
2. ArkPaymentData.cshtml:29-40 — view fallback chain
var paymentDestination = paymentData.Destination ?? payment.Destination;
var isBoarding = paymentData.IsBoarding
|| (!string.IsNullOrEmpty(promptDetails.BoardingAddress)
&& string.Equals(paymentDestination, promptDetails.BoardingAddress, ...));Before: the isBoarding fallback compared payment.Destination (the broken top-level blob field) against the boarding address — always false for old rows since the bug stored the Ark address there. Now: prefers paymentData.Destination (from ArkadePaymentData.details, which was always written correctly) before falling back to the top-level. This correctly heals old rows without a data migration. ✓
The address and contractString derivations also use paymentDestination consistently. ✓
Edge cases verified
- New boarding payments (post-fix):
IsBoarding = trueinArkadePaymentData, lowercasedestinationcorrect in blob. All three detection paths agree. ✓ - Old boarding payments (pre-fix, capital-D bug):
IsBoarding = false(flag didn't exist),payment.Destination= wrong Ark address, butpaymentData.Destination= correct boarding address. Fallback chain:paymentData.Destination ?? payment.Destination→ correct boarding address → matchespromptDetails.BoardingAddress→isBoarding = true. ✓ - VTXO payments (non-boarding):
IsBoarding = false,paymentData.Destination= Ark address ≠BoardingAddress→isBoarding = false. ✓ - Very old payments (no
Destinationin details at all):paymentData.Destination= null → falls back topayment.Destination→ Ark address ≠ boarding address →isBoarding = false→ renders as VTXO. Correct — these predated boarding support. ✓
Not protocol-critical
This touches payment display rendering and blob metadata, not VTXO handling, transaction signing, forfeit paths, round lifecycle, or exit logic.
Ship it. 🚢
…anged events
Adds InvoicePaymentLatencyTests.CheatModePay_DirectArkTx_InvoiceSettlesWithinThreshold
to NArk.E2E.Tests. The test:
- Creates a merchant store + Arkade wallet via the existing helper
- Once-per-suite, initialises the in-arkd `ark` CLI and funds it via
a single boarding+settle (kept out of the timed window so the
measurement isolates the plugin's invoice-settle latency rather than
arkd's batch round)
- Creates a 5000-sat ARKADE invoice and triggers BTCPay's cheat-mode
POST /i/{id}/test-payment route, which routes through the plugin's
ArkadeCheckoutCheatModeExtension and `docker exec arkd ark send`
(an out-of-round Ark tx — essentially instant on arkd's side)
- Polls GetInvoice() at 100ms resolution until status == Settled
- Two assertion modes:
- HardTimeout (30s) → Assert.Fail with a "missed-event regression"
message if the invoice never settles (catches a broken
VtxosChanged → OnVtxoChanged → AddPayment chain)
- LatencyThreshold (12s) → Assert.True against the elapsed time when
it does settle (catches a slowness regression — currently observed
real-world is ~6s)
Threshold deliberately wide to absorb CI jitter today; tighten as the
in-handler gap between UpsertVtxo and invoice_paymentSettled is
localised and fixed.
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit c50e740)
Verdict: APPROVE ✅
What changed since last review
Single new file: NArk.E2E.Tests/InvoicePaymentLatencyTests.cs (295 lines) — an E2E test that times cheat-mode invoice payment latency and guards against missed VtxosChanged events.
Analysis
Well-structured regression test with clear dual purpose:
- Latency regression — asserts invoice settles within 12s (2× the observed ~6s baseline). Tight 100ms polling loop gives sub-second resolution. ✓
- Missed-event regression — 30s hard timeout catches the case where
VtxosChangednever fires at all, failing as a hard error rather than a CI flake. ✓
Correctness checks:
| Area | Status |
|---|---|
EnsureArkdCliReadyAsync double-checked locking via SemaphoreSlim + _arkdCliReady flag |
✓ Classic async DCL pattern, correct |
Setup sequence: ark init → ark receive → nigiri faucet → mine 6 → ark settle |
✓ Matches the boarding→settle flow. 6-block confirm is consistent with cheat-mode fix in 17cf8fd |
| Balance threshold check (10,000 sats) after settle | ✓ Defensive — catches silent settle failures |
GetArkdOffchainSatsAsync JSON parsing with try/catch JsonException |
✓ Handles uninitialised CLI gracefully |
Invoice created with Amount = 5000m, Currency = "SATS" |
✓ Matches cheat-mode extension's routing |
Cheat pay via Playwright APIRequest.PostAsync to /i/{id}/test-payment |
✓ Correct BTCPay cheat-mode endpoint |
InvoiceStatus.Invalid early-exit with descriptive assertion |
✓ Separates "payment rejected" from "payment slow" |
Timer starts at t0 before POST, stops on Settled poll |
✓ Measures full end-to-end including network |
Minor observations (non-blocking):
-
InvoicePaymentLatencyTests.cs:62—_arkdCliSetupLockand_arkdCliReadyarestatic, so they survive across test class instantiations within the same process. This is intentional (fund once per suite), and there's only one[Fact]in this class today. If more facts are added later, the static state is correct — the semaphore serialises and the bool gates. Fine as-is. -
InvoicePaymentLatencyTests.cs:128— The polling loop measuresDateTimeOffset.UtcNow - t0at the momentSettledis observed, which includes the last poll interval (~100ms jitter). For a 12s threshold this is negligible but worth knowing if the threshold is ever tightened below 1s. -
InvoicePaymentLatencyTests.cs:198—containerresolution happens inside the lock, so thedocker execcalls are serialised. If a future test class also callsEnsureArkdCliReadyAsync, theSemaphoreSlim(1,1)correctly prevents concurrent arkd setup races.
Not protocol-critical
Pure test infrastructure. No VTXO handling, transaction signing, forfeit paths, round lifecycle, or exit logic modified.
Ship it. 🚢
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit c50e740)
Verdict: APPROVE ✅
What changed since last review
Single new file: NArk.E2E.Tests/InvoicePaymentLatencyTests.cs (295 lines) — an E2E test that times cheat-mode invoice payment latency and guards against missed VtxosChanged events.
Analysis
Well-structured regression test with clear dual purpose:
- Latency regression — asserts invoice settles within 12s (2× the observed ~6s baseline). Tight 100ms polling loop gives sub-second resolution. ✓
- Missed-event regression — 30s hard timeout catches the case where
VtxosChangednever fires at all, failing as a hard error rather than a CI flake. ✓
Correctness checks:
| Area | Status |
|---|---|
EnsureArkdCliReadyAsync double-checked locking via SemaphoreSlim + _arkdCliReady flag |
✓ Classic async DCL pattern, correct |
Setup sequence: ark init → ark receive → nigiri faucet → mine 6 → ark settle |
✓ Matches the boarding→settle flow. 6-block confirm is consistent with cheat-mode fix in 17cf8fd |
| Balance threshold check (10,000 sats) after settle | ✓ Defensive — catches silent settle failures |
GetArkdOffchainSatsAsync JSON parsing with try/catch JsonException |
✓ Handles uninitialised CLI gracefully |
Invoice created with Amount = 5000m, Currency = "SATS" |
✓ Matches cheat-mode extension's routing |
Cheat pay via Playwright APIRequest.PostAsync to /i/{id}/test-payment |
✓ Correct BTCPay cheat-mode endpoint |
InvoiceStatus.Invalid early-exit with descriptive assertion |
✓ Separates "payment rejected" from "payment slow" |
Timer starts at t0 before POST, stops on Settled poll |
✓ Measures full end-to-end including network |
Minor observations (non-blocking):
-
InvoicePaymentLatencyTests.cs:62—_arkdCliSetupLockand_arkdCliReadyarestatic, so they survive across test class instantiations within the same process. This is intentional (fund once per suite), and there's only one[Fact]in this class today. If more facts are added later, the static state is correct — the semaphore serialises and the bool gates. Fine as-is. -
InvoicePaymentLatencyTests.cs:128— The polling loop measuresDateTimeOffset.UtcNow - t0at the momentSettledis observed, which includes the last poll interval (~100ms jitter). For a 12s threshold this is negligible but worth knowing if the threshold is ever tightened below 1s. -
InvoicePaymentLatencyTests.cs:198—containerresolution happens inside the lock, so thedocker execcalls are serialised. If a future test class also callsEnsureArkdCliReadyAsync, theSemaphoreSlim(1,1)correctly prevents concurrent arkd setup races.
Not protocol-critical
Pure test infrastructure. No VTXO handling, transaction signing, forfeit paths, round lifecycle, or exit logic modified.
Ship it. 🚢
…ers + antiforgery Three issues found running the test against the live nigiri stack: 1. ASP.NET MVC controllers default to FORM binding for complex parameters without [FromBody]; `UIInvoiceController.TestPayment(FakePaymentRequest)` silently defaulted PaymentMethodId="BTC" when the body was JSON, and GetCheatModeExtension(... "BTC") returned null on an Arkade-only store → 400 "No ICheatModeExtension registered". Switched to application/x-www-form-urlencoded. 2. BTCPay enforces antiforgery on every UI-controller POST via the UIControllerAntiforgeryTokenAttribute global filter (registered in Startup); missing token = 400 with an empty body. Grab the token off the Arkade overview page and pass it in the RequestVerificationToken header. 3. (Minor) Document the matching binding rules in comments so the next person hitting an empty-body 400 on this endpoint can find the cause. Local run on the nigiri stack: test passes with measured latency ~2s, well under the 12s threshold. Plugin debug log decomposes as: arkd push T=0 UpsertVtxo +1.1s (750ms intentional delay) invoice_paymentSettled +1.3s (185ms from upsert)
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit 6a8297d)
Verdict: APPROVE ✅
What changed since last review
Single commit 6a8297d (test(e2e): fix InvoicePaymentLatencyTests POST to match BTCPay's binders + antiforgery) — test-only, no production code touched.
Analysis
Two fixes to InvoicePaymentLatencyTests.cs, both correct:
-
Content-Type switched from JSON to
application/x-www-form-urlencoded(InvoicePaymentLatencyTests.cs:116-121). BTCPay'sUIInvoiceController.TestPaymentbinds from form data (no[FromBody]), so the previous JSON POST silently fell back to defaults (PaymentMethodId="BTC") and the Arkade cheat extension never matched. The form-encoded payloadAmount=5000&CryptoCode=SATS&PaymentMethodId=ARKADEmatches the MVC binder correctly. -
Antiforgery token added (
InvoicePaymentLatencyTests.cs:95-100). Navigates to the Arkade overview to seed aRequestVerificationToken, then passes it in the header. Matches the pattern used inSpendingTests,ApiEndpointTests,SwapsTests, andFundedWalletTests— all call the sameGetAntiforgeryTokenAsync()fromPlaywrightBaseTest.
Not protocol-critical
Test infrastructure only — no VTXO, signing, forfeit, or exit code changed.
…r claim) Picks up two SDK perf merges since #97 landed: - #99 perf(sync): single immediate poll per stream push, drop 750ms/3s/8s retry schedule — perceived VTXO arrival latency drops to one network round-trip on a healthy connection, no more "still nothing" log churn. - #100 perf(wallet): cache OutputDescriptor.Parse + BIP-39 master ExtKey — per-receive claim ~800x faster; biggest impact on HD wallets where the KDF dominated cold claim cost. CHANGELOG v2.1.15 SDK NNark section updated accordingly. No plugin code changes required — both are internal perf wins that downstream consumers get for free. Build clean against the new pointer.
There was a problem hiding this comment.
Arkana Code Review — Incremental (new commit aed2290)
Verdict: APPROVE ✅
What changed since last review
Single commit aed2290 (chore: bump NNark to b55c886 (perf: sync responsiveness + ~800x faster claim)) — submodule bump + two new CHANGELOG entries. No btcpay-arkade production code changed.
NNark submodule analysis (c8d1484..b55c886 — 2 commits)
1. b55c886 — perf(sync): single immediate poll per stream push, drop 750ms/3s/8s retry schedule (#99)
VtxoSynchronizationService.cs: removes the FirePollSchedule method and its StreamPushPollSchedule delays (750ms/3s/8s). Replaced with a single immediate _readyToPoll.Writer.WriteAsync on stream push. The RoutinePoll at 5s interval (VtxoSynchronizationService.cs:213) acts as the safety net if arkd's indexer lags the event.
✅ Correct simplification. The retry schedule was defensive against arkd v0.9.0-rc.1 indexer lag; on v0.9.5+ it's dead weight. Worst case: immediate poll misses → routine 5s poll catches it. No VTXO loss vector.
2. a6aa8a3 — perf(wallet): cache OutputDescriptor.Parse and BIP-39 master extKey (#100)
Three caches introduced:
| Cache | Location | Key | Value | Thread-safe | Bounded |
|---|---|---|---|---|---|
_descriptorCache |
KeyExtensions.cs |
(descriptor_string, network_id) |
OutputDescriptor |
ConcurrentDictionary ✓ |
By unique descriptors (small) ✓ |
_extKeyCache |
HierarchicalDeterministicWalletSigner.cs |
mnemonic string | ExtKey (master private key) |
ConcurrentDictionary ✓ |
By unique wallets (tiny) ✓ |
| Removed wasted parse | HierarchicalDeterministicAddressProvider.cs:27 |
N/A | N/A | N/A | N/A |
Security check on _extKeyCache: The mnemonic string is already held in wallet.Secret (in-memory ArkWalletInfo). The ExtKey (master private key) was already derived and used transiently on every call. Promoting it to a static cache doesn't expand the attack surface — the key material lifetime is already process-scoped. The cache just avoids re-running PBKDF2-HMAC-SHA512 × 2048 on every Sign/SignMusig/GetPubKey/GenerateNonces. ✓
Logic-preservation check on VHTLCContractTransformer.cs: The CanTransform refactor unwraps the short-circuit && into nested if blocks for instrumentation. Logic is identical:
- Before:
if (preimage != null && IsOurs(Receiver)) → return GetSigner() != null - After:
if (preimage != null) { if (IsOurs(Receiver)) { return GetSigner() != null } }✓
IsOurs fix (HierarchicalDeterministicAddressProvider.cs:27): The original OutputDescriptor.Parse(wallet.AccountDescriptor, network) was a pure side-effect-free call whose return value was discarded — a ~500ms no-op on every IsOurs invocation. Replaced with a string.IsNullOrEmpty null-guard that preserves the validation intent. ✓
Diagnostic probes (non-blocking observation)
~15 TEMP latency probe Stopwatch + LogTrace blocks added across CoinService, SpendingService, SweeperService, DefaultWalletProvider, VHTLCContractTransformer. All gated behind LogTrace (off by default). Comments say "TEMP" — should be cleaned up in a follow-up once the bottleneck analysis is complete. Fine for now.
CHANGELOG accuracy
Two new entries under "SDK (NNark)":
- VTXO sync responsiveness → matches #99 ✓
- Per-receive claim ~800× faster → matches #100 ✓
Not protocol-critical
Performance optimizations only. No changes to VTXO handling, transaction construction, forfeit paths, round lifecycle, or exit logic. The signing flow is unchanged — only the KDF and parse hot paths are cached.
Ship it. 🚢
…container
The plugin's ArkadeCheckoutCheatModeExtension defaults to docker
exec'ing into 'arkd' — the SDK's documented v0.9-split topology
(arkd-override compose: container_name: ${ARK_CONTAINER:-arkd}).
nigiri's base compose, however, ships the daemon as 'ark' and that
service name wins in CI; start-env.sh's auto-derivation in
.env.defaults didn't promote the override to 'arkd'. Result:
CheatModePay_DirectArkTx_InvoiceSettlesWithinThreshold has been
failing on the branch since ~11:30 UTC today with 'No such container:
arkd'. The plugin exposes ARKADE_CHEAT_ARK_CONTAINER for exactly this
case; pinning to 'ark' matches what nigiri actually creates.
Pre-existing CI failure, not caused by the submodule bump in
aed2290 — same test failed identically on 26225732848 (last run
before my push).
There was a problem hiding this comment.
Arkana Code Review — Incremental (13 new commits since 2e10538)
Verdict: APPROVE ✅
Scope of new changes
| Commit | Area | Risk |
|---|---|---|
ce61077 |
Centralise dust/default constants, raise default boarding min to 5000 | Low |
c04a9f0 |
Break DI cycle that hung /stores/{id}/invoices |
Medium — DI lifecycle |
28138e7 |
Cheat-mode: exec ark CLI against renamed arkd container |
Low (test infra) |
99635ea |
Bump NNark to c8d1484 (contract type+script in import/derive logs) | Low |
17cf8fd |
Cheat-mode: faucet boarding_address (not onchain_address) |
Low (test infra) |
eb43be7 |
Deactivate ALL invoice-tagged contracts on settle | High — contract lifecycle |
6a28996 |
Views: align zero-count cells with badge cells | Cosmetic |
060ed13 |
Views: reserve expand-row slot in Contracts actions column | Cosmetic |
2ac4c59 |
Render boarding payments correctly in Arkade Payments table | Medium — view correctness |
d28f097 |
Fix boarding destination override: Destination → destination JSON key |
High — payment data integrity |
c50e740 |
E2E: latency test for cheat-mode pay | Low (test) |
6a8297d |
E2E: fix POST binding to match BTCPay's MVC binder | Low (test) |
aed2290 |
Bump NNark to b55c886 (perf: sync + 800× faster claim) | Low |
b79ecc2 |
CI: set ARKADE_CHEAT_ARK_CONTAINER=ark for nigiri |
Low (CI) |
Protocol-adjacent findings (reviewed, all correct)
1. ToggleArkadeContract now deactivates ALL invoice-tagged contracts (eb43be7)
Old code only toggled the Payment contract (derived from prompt's GetArkAddress()); the Boarding contract stayed Active forever after settlement — leaking a subscription and polluting the active-contracts list.
New code queries all contracts for the wallet, filters in-memory by Source == "invoice:{id}", and toggles each. Functionally correct: ConfigurePrompt tags both Payment and Boarding contracts with the same Source = "invoice:{id}" metadata, and HTLC contracts use a different "swap:{id}" tag so they're excluded. The activityState is derived from invoice.Status == New ? Active : Inactive — correct state machine.
Minor perf note (non-blocking): GetContracts(walletIds: [walletId]) fetches the entire contract set for the wallet with no isActive/contractTypes filter because we need to find contracts in any state (activate vs deactivate depends on invoice state). IContractStorage.GetContracts doesn't support metadata filtering, so the in-memory Where is the only option today. For stores with thousands of contracts this is O(n) per invoice state change. Fine for now; if it shows up in profiles, a metadata filter parameter on GetContracts would be the right fix in NNark.
2. HandlePaymentData destination JSON key fix (d28f097)
Set() serialises ArkadePaymentData via BTCPay's camelCase JsonSerializer, producing "destination" (lowercase). The old code patched blob["Destination"] (capital-D), which — because JObject is case-sensitive — added a shadow key the deserialiser ignored. The boarding address was silently lost on read. Fix is correct: write blob["destination"], remove stale "Destination". Defensive and backward-compatible.
3. IsBoarding flag propagation (2ac4c59, d28f097)
ArkadePaymentData gains IsBoarding, ArkadePromptDetails gains BoardingContractString. Both flow through OnVtxoChanged → HandlePaymentData → ArkadePaymentData → Blob2 and are consumed in ArkPaymentData.cshtml to render the correct badge ("Boarding" vs "VTXO") and route outpoint links to TransactionLinkProviders["BTC-CHAIN"] for on-chain payments. The view has a two-level fallback for old rows lacking the flag: paymentData.Destination → prompt's BoardingAddress comparison. Correct.
4. DI cycle fix (c04a9f0)
ArkadeCheckoutModelExtension is both an ICheckoutModelExtension and an IGlobalCheckoutModelExtension. Injecting IEnumerable<IGlobalCheckoutModelExtension> into its constructor caused the DI container to resolve all implementations (including itself) before the constructor returned — a cycle that StackGuard papered over by spinning up threads, deadlocking /stores/{id}/invoices. Switching to IServiceProvider + lazy GetServices<>() at enumeration time breaks the cycle cleanly. The ReferenceEquals(global, this) guard is retained. Correct.
5. Cheat-mode container rename (28138e7, 17cf8fd)
nigiri ark ... → docker exec {ArkContainer} ark ... with ArkContainer defaulting to "arkd" (overridable via ARKADE_CHEAT_ARK_CONTAINER). No shell injection risk: Process.Start doesn't invoke a shell. The recovery path's broken shell substitution ($(nigiri ark receive | jq -r ...) — never expanded under Process.Start) is now a proper two-step call: parse JSON in C#, pass the address into nigiri faucet. Also correctly faucets boarding_address instead of onchain_address, and mines 6 blocks before settle (required: validateBoardingInput needs confirmed UTXOs). All test infra, not production.
6. Boarding minimum default raised (ce61077)
DefaultMinBoardingAmountSats = 5000L, P2trDustLimitSats = 330L. Constants centralised, all callers reference them. Existing stores keep their persisted value; only new stores get 5000. README updated. Clean.
E2E test review (InvoicePaymentLatencyTests.cs)
Solid regression test. Double-check locking via SemaphoreSlim + _arkdCliReady for one-time arkd CLI setup. Form-urlencoded POST matches BTCPay's MVC binder (not JSON — the old JSON path silently fell back to PaymentMethodId="BTC"). 12s threshold with 2× headroom over observed 6s is reasonable for CI jitter. Hard 30s timeout catches missed-event regressions. No concerns.
Cross-repo impact
Sourcemetadata is plugin-internal — no downstream consumers.ArkadePaymentData.IsBoardingandArkadePromptDetails.BoardingContractStringare new fields with defaults (false,null) — backward-compatible with existing serialised data.- NNark submodule bump is a fast-forward; the plugin already consumed the new APIs in prior commits.
- No public API surface changes that would affect ts-sdk, go-sdk, or other consumers.
Not protocol-critical
None of these changes touch VTXO construction, transaction signing, forfeit paths, round lifecycle, connector trees, or exit paths. The contract lifecycle changes (activate/deactivate) are metadata-level bookkeeping — they control which contracts the plugin polls, not how VTXOs are created or redeemed. No human sign-off required beyond normal review.
Ship it. 🚢
1 participant
Release v2.1.15
Bumps
BTCPayServer.Plugins.ArkPayServerto 2.1.15 and refreshesCHANGELOG.mdfor everything that's landed on master since v2.1.14, plus the NNark SDK pickup pinned at master tipfa6092d(which now includes #96).Patch bump matches the project's house convention (2.1.x has been bumped patch even for feature-y entries like 2.1.5's new SDK API).
What's in the release
Plugin (#45–#64):
btcpay-arkadereferral id (feat: tag Boltz swap-creation requests with 'btcpay-arkade' referral #49).IBitcoinBlockchainfactory detects an NBXplorer-incompatible explorer provider (type-nameElectrumorRPCClient == null) and falls back toEsploraBlockchainagainst the network's default Esplora endpoint. Needs companion fix(electrum): republish NewOnChainTransactionEvent on every wallet tx Kukks/BTCPayServerPlugins#132 for inbound on-chain events (already merged).SDK pickup (NNark
171d11a→fa6092d):ArkNetworkConfig(verified at the protocol layer against live Fulcrum hosts).VtxoSynchronizationServiceroutine poll logs (24 INFO lines/min of noise on idle wallets → Debug; productive polls + cold-start catch-up stay Info).AddBoltzProviderregistersBoltzClientso direct-DI consumers don't get "Unable to resolve BoltzClient".DateTimeOffset → long ticksfor SQLite (#92).Internal-only PRs (docs cleanup #56 / #57, CI/test wiring #54 / #60) intentionally omitted from the user-facing changelog.
Test plan
fa6092d.<Version>= 2.1.15.v2.1.15and push to trigger the GitHub Release workflow.