Skip to content

Fix Firo Open CryptoPay minimum fee rejecting Spark payments - #4305

Merged
joshuakrueger-dfx merged 3 commits into
developfrom
fix/ocp-firo-min-fee
Jul 23, 2026
Merged

Fix Firo Open CryptoPay minimum fee rejecting Spark payments#4305
joshuakrueger-dfx merged 3 commits into
developfrom
fix/ocp-firo-min-fee

Conversation

@joshuakrueger-dfx

@joshuakrueger-dfx joshuakrueger-dfx commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Problem

During Stack Wallet OCP testing on Firo (FIRO mainnet, dfx.shop test product), sending was blocked with "Open CryptoPay requires at least 2.068 sat/vB fee". That advertised transferAmounts[].minFee was derived from DFX's own payout fee rate — estimateSmartFee(1) times the CPFP/default margin. That CPFP margin exists only for DFX's outbound spends (where DFX may child-pays-for-parent its own batching); a paying customer cannot CPFP an inbound payment, so charging the margin on the customer-facing minimum was wrong on principle.

The current OCP Firo deposit address (PAYMENT_FIRO_ADDRESS) is transparent. A Stack Wallet payment is therefore a Spark-spend to a transparent address, whose fee sits at Firo's relay floor and cannot be raised by the user — so a margin-multiplied minimum could never be met.

Fix

Decouple the customer-facing minimum from the payout margin on both UTXO chains, using each network's own next-block rate (estimateSmartFee(1), no CPFP margin), floored at the relay minimum (MIN_FEE_RATE_SAT_VB = 1 sat/vB) so the advertised minimum is always relayable.

  • Bitcoin: max(getRecommendedFeeRate(), relayFloor) — adapts to congestion, floored.
  • Firo: same recipe. Firo does not congest and its node usually returns no estimate on the quiet chain, so this resolves to the relay floor in practice — exactly what a Spark-spend to the transparent deposit address pays.

The CPFP/default margin stays on DFX's own payout path (getCurrentFeeRategetSendFeeRate, used by sendUtxoToMany), unchanged.

Firo fail-closed vs quiet-node

PayoutFiroService.getRecommendedFeeRate() returns estimateSmartFee(1) ?? relayFloor:

  • null (node answered with no estimate — the normal quiet-Firo state) → relay floor.
  • thrown error (node down / RPC failure) → propagates, so Firo drops out of the fee cache and the chain is not offered — fail-closed, matching Bitcoin. A dead node is never advertised as available at minFee = 1.

Acceptance itself is independently fenced by the node's real relay floor (testMempoolAccept for the hex path, getRawTx for the Firo tx-id path), so the advertised minimum cannot admit an unrelayable tx.

Changes

  • payment-link-fee.service.ts: Bitcoin + Firo minimum = max(recommended rate, relay floor), no payout margin
  • payout-bitcoin.service.ts / payout-firo.service.ts: new getRecommendedFeeRate() (Firo null-safe → relay floor, node error fails closed)
  • bitcoin-based-fee.service.ts: export the shared MIN_FEE_RATE_SAT_VB relay floor
  • tests: payment-link-fee.service.spec.ts (both chains, relay-floor clamp) and payout-firo.service.spec.ts (estimate / null→floor / error-propagation branches)

PR completeness

  • Migration: none (no entity/column change)
  • Environment: none — the earlier FIRO_MIN_FEE_RATE env var was dropped; the minimum is now derived from the node
  • Service/frontend sync: none (transferAmounts[].minFee DTO shape unchanged)

Test plan

  • format / lint / type-check: clean
  • npm test payment-link-fee.service payout-firo.service: 22 passing
  • Strict relaxation vs the old margin-inflated minimum: no previously-successful payment can fail; sub-relay-min txs are still rejected by the node at broadcast.

Out of scope

Firo credits the merchant at TX_MEMPOOL (before confirmation) with no reversal path — pre-existing, governed by minCompletionStatus, unaffected by this change. A dedicated Spark (sm1…) deposit address — whose protocol-capped fee would want a relay-floor cap rather than a congestion-adaptive minimum — is a separate, larger piece of work (new shielded inbound rail) and is not part of this PR.

The Firo customer-facing Open CryptoPay minimum was taken from DFX's own
payout fee rate (estimateSmartFee times the CPFP/default margin, ~2.068
sat/vB). Firo Spark transactions carry a protocol-fixed fee at the network
relay minimum (~1 sat/vB) that the user cannot raise, so valid Spark
payments were rejected.

Use Firo's relay floor as the customer minimum (configurable via
FIRO_MIN_FEE_RATE, default 1 sat/vB); the payout margin stays on DFX's own
payout path. Bitcoin keeps its margin-based minimum because its fees are
user-adjustable.
@joshuakrueger-dfx
joshuakrueger-dfx marked this pull request as ready for review July 22, 2026 10:47
@davidleomay

Copy link
Copy Markdown
Member

Two questions on this before landing:

  1. Was the original failing tx (Stack Wallet on dfx.shop) actually a Spark tx, or transparent? Prod PAYMENT_FIRO_ADDRESS is a transparent address (aCPr…5wn — matches firoAddressFormat, not firoSparkAddressFormat). A Spark tx can't reach a transparent a… address, so the failing broadcast must have been transparent — in which case it could have paid 2.068+ sat/vB. Worth checking the actual broadcast fee of the rejected tx before landing this; the reported failure might have a different root cause (e.g. Stack Wallet's fee estimator, or a unit/decimal mismatch).

  2. With the current transparent deposit address, does this fix apply? The PR's premise — Spark's protocol-fixed fee — doesn't hold for transparent Firo txs, which have user-adjustable fees. Lowering the advertised minimum to 1 sat/vB on a chain whose users can underpay may cause wallets to follow the hint and broadcast at a fee that gets stuck in mempool during congestion (silent failure: DFX accepts the fee level, but the tx never confirms). If OCP should support Spark, the deployment also needs a Spark deposit address (sm1…) — otherwise the relay-floor argument doesn't apply here.

Possible middle ground: use estimateSmartFee(1) without the CPFP margin for transparent Firo (customer can't CPFP anyway), reserving the 1 sat/vB floor for Spark once a Spark address is actually deployed.

For consistency with the Firo fix, the customer-facing Open CryptoPay
minimum for Bitcoin no longer derives from DFX's payout send rate (which
carries a CPFP/default margin meant only for DFX's own outbound spends).
Use the network's recommended next-block rate, floored at the relay
minimum so the advertised minimum stays relayable. The payout margin
remains on DFX's own payout/payin/dex paths, unchanged.
@joshuakrueger-dfx

Copy link
Copy Markdown
Collaborator Author

Summary of changes

Firo (commit 1): The Open CryptoPay customer-facing minimum for Firo was derived from DFX's own payout send rate (estimateSmartFee × CPFP/default margin, ~2.068 sat/vB). Firo Spark transactions carry a protocol-fixed fee at the network relay minimum (~1 sat/vB) that the user cannot raise, so a valid Spark payment could never meet it. The Firo minimum is now the network relay floor (minRelayTxFee = 1 sat/vB, configurable via FIRO_MIN_FEE_RATE). The now-unused PayoutFiroService dependency was removed.

Bitcoin (commit 2): For consistency, Bitcoin's customer-facing minimum no longer derives from the payout send rate either. It now uses the recommended (next-block) rate, floored at the relay minimum so the advertised minimum stays relayable. The payout margin remains on DFX's own payout/payin/dex paths, unchanged.

Both UTXO chains now follow the same principle — the customer minimum is the network's own minimum for an inbound payment to confirm, without DFX's outbound CPFP margin. Values differ per chain: Bitcoin adapts to congestion (recommended rate + relay-floor clamp); Firo uses a fixed relay floor because Spark fees are protocol-fixed and Firo does not congest.

Reviewed across conformity, logic, and security/failure-mode dimensions (two rounds — the second after adding the Bitcoin change). No blocking findings. Verified: strict relaxation, so no previously-successful payment can now fail; sub-relay-min transactions are still rejected by the node at broadcast; no other blockchain or DFX payout path is affected.

Tests: added payment-link-fee.service.spec.ts covering both chains, the Firo Spark-floor invariant, and the Bitcoin relay-floor clamp. format / lint / type-check clean.

Out of scope (pre-existing, unchanged): Firo credits at TX_MEMPOOL before confirmation with no reversal path — worth a separate look for Spark point-of-sale.

…d floor

The current OCP Firo deposit address is transparent, so a Stack Wallet payment
is a Spark-spend to it whose fee sits at the relay floor and cannot be raised.
Mirror the Bitcoin approach: use Firo's own estimatesmartfee(1) without the
payout CPFP margin, floored at the relay minimum, instead of a hardcoded
FIRO_MIN_FEE_RATE constant.

On a quiet Firo node estimatesmartfee returns null (the normal state) and
degrades to the relay floor; a genuine node/RPC error propagates so Firo fails
closed (drops out of the fee cache) like Bitcoin rather than being advertised at
1. Removes the now-unused FIRO_MIN_FEE_RATE config and adds PayoutFiroService
tests for the estimate/null/error branches.
@joshuakrueger-dfx
joshuakrueger-dfx merged commit 001e543 into develop Jul 23, 2026
7 checks passed
@joshuakrueger-dfx
joshuakrueger-dfx deleted the fix/ocp-firo-min-fee branch July 23, 2026 15:40
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.

2 participants