Repository navigation
fix(wallet): top up an RGB-giving swap side with plain BTC inputs - #19
Arshia-r-m wants to merge 1 commit into
Conversation
A swap side that gives RGB only spent the colored UTXOs picked for the asset, so their sats had to cover that side's whole BTC cost (its RGB receive output, plus the network and platform fees for the taker). With rgb-lib's default 1,000-sat colored UTXOs the BTC-for-RGB taker and both RGB-for-RGB sides failed with InsufficientBitcoins, and an RGB-for-RGB maker giving a partial amount was left with zero change, so its RGB change had no output to go to and coloring failed. swap_select_inputs now adds plain BTC inputs from the vanilla wallet (respecting exclude_outpoints and min_confirmations) when the colored inputs fall short. When the inputs carry RGB change, of the swapped asset or of others on the same UTXOs, it also reserves enough for a non-dust change output. The extra inputs are listed with the side's colored ones, so the PSBT shape checks, the signing guards and coloring (which only looks at the RGB state of the declared inputs) work unchanged. Tests cover every direction with default-size colored UTXOs, a partial RGB-for-RGB swap where the maker keeps (and later spends) its change, and that the top-up never picks an excluded coin. Closes #11 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🔐 Automated security review🟡 2 medium This repository is public, so details are kept in the private report @cf/zai-org/glm-5.3 · 97,158 in / 14,409 out tokens (≈ $0.20) · 2 files · commit |
🧐 Automated code review🟠 1 high · 🟡 1 medium The change makes an RGB-giving swap side reserve dust for its RGB change output and top up insufficient colored inputs with plain BTC coins. The selection arithmetic, dust reservation and error remapping look correct and consistent with the coloring path, but the new top-up call site introduces two real behavioral regressions (both acknowledged in the PR description but shipped anyway): vanilla funds end up stranded on the colored keychain, and the top-up spends vanilla coins that other in-flight operations have reserved. 1. 🟠 HIGH: Top-up strands the wallet's plain BTC on the colored keychain
An RGB-giving side has a single change output, and per SwapRgbSend::coloring_info (line 4870) that change must sit on the colored (External) keychain whenever it carries RGB change. The new top-up adds a large vanilla input to that side, so the vanilla leftover is returned on a colored-keychain UTXO. Plain BTC operations (send_btc, drain_to) cannot spend colored-keychain coins, so the wallet's vanilla balance becomes unusable for ordinary BTC spending after a routine swap. The PR notes acknowledge this, but there is no workaround short of performing RGB operations, and every default-size colored UTXO (the rgb-lib default, UTXO_SIZE = 1000) triggers the top-up on the common path. Failure scenario: A wallet holds one large vanilla coin (e.g. 1 BTC) and a default 1,000-sat colored UTXO carrying an asset. The owner sells the asset (BTC-for-RGB taker, or RGB-for-RGB). swap_select_inputs selects the 1,000-sat colored UTXO, finds total < needed, and swap_select_btc_inputs adds the 1 BTC vanilla coin. The swap's single change output is on the colored keychain, so after the swap ~1 BTC (minus fee and the amount received) sits on a colored UTXO. A subsequent send_btc or drain_to then fails with InsufficientBitcoins even though the wallet is funded; the tests miss this because total_btc() sums vanilla.future + colored.future. Fix: Add a second, Internal-keychain change output for the vanilla leftover of the top-up (the message-format change the PR defers), or at minimum restrict the top-up to coins small enough that the leftover stays spendable and document the keychain split prominently in the public API docs. Do not ship the current behavior silently: one ordinary swap locks the user's BTC balance out of all plain BTC operations. 2. 🟡 MEDIUM: Top-up selects vanilla coins reserved by in-flight operations
The new call to swap_select_btc_inputs for the RGB-giving side iterates wallet.internal_unspents() without filtering out get_reserved_vanilla_outpoints, unlike create_utxos_begin_impl (line 319) which excludes reserved coins. Before this PR an RGB-giving side never touched vanilla coins, so this double-spend window is newly opened for the RGB path. The PR notes acknowledge it and push the burden onto callers via exclude_outpoints, but nothing in the API forces or even suggests that a caller mid-way through a send_btc/create_utxos operation must pass those outpoints. Failure scenario: A caller runs send_btc begin (PSBT built, vanilla outpoints reserved in the DB, not yet broadcast), then accepts a swap offer selling an RGB asset without passing exclude_outpoints. The top-up selects the reserved vanilla coin, the swap transaction is built and broadcast first, and the pending send_btc operation now spends already-consumed inputs: it fails at broadcast/end with a confusing FailedBroadcast error instead of being prevented at selection, and the caller must manually fail and rebuild it. The same happens between two concurrent swaps that both top up from the same vanilla coin. Fix: In swap_select_btc_inputs (or at the new call site), exclude outpoints returned by get_reserved_vanilla_outpoints, matching what create_utxos_begin_impl already does; alternatively make accept/complete swap APIs automatically treat the wallet's own reserved outpoints as excluded. @cf/zai-org/glm-5.3 · 97,141 in / 26,293 out tokens (≈ $0.25) · 2 files · commit |
Closes #11. Based on
fix/swap-signing-guards(#18). Merge #18 first.Problem
When a swap side gives RGB,
swap_select_inputs(src/wallet/online.rs) only spends the colored UTXOs thatselect_rgb_inputspicks. The sats on those UTXOs have to cover that side's whole BTC cost. rgb-lib creates colored UTXOs with 1,000 sats by default (UTXO_SIZE), which is not enough. On the current code, with those default UTXOs:InsufficientBitcoins { needed: 2000, available: 1000 }InsufficientBitcoins { needed: 3000, available: 1000 }InvalidDetails { "swap inputs carry RGB change but the swap has no change output" }The existing tests missed this because
issue_swap_assetcreates 10,000-sat colored UTXOs. The new tests reproduce the maker row, which the issue had only inferred from the code.Changes
swap_select_inputs, RGB branch:swap_input_rgb_amounts, the same source coloring uses. Dust comes from the external (colored) descriptor'sdust_value().extra_sat(plus that dust when needed), it adds plain BTC inputs with the existingswap_select_btc_inputs, which respectsexclude_outpointsandmin_confirmations.InsufficientBitcoinswith the fullneededandavailable= colored + vanilla.taker_inputs/maker_inputslist.offline.rsor the signing paths:swap_build_psbt,swap_append_side_changeandswap_validate_proposal_psbtwork on the declared input list as a whole.swap_ensure_inputs_foreignandswap_ensure_only_own_inputs_signedtreat every declared input as the side's own.Tests
New tests in
src/wallet/test/onchain_swap.rs. They use a new helper,issue_default_utxo_swap_asset, which callscreate_utxos(.., Some(1), None, ..)to get a 1,000-sat colored UTXO:default_utxo_btc_for_rgb_balances: the taker sells RGB. The test checks:InsufficientBitcoins;default_utxo_rgb_for_rgb_balances: both sides use default UTXOs. Checks BTC and asset balances.default_utxo_rgb_for_btc_balances: checks BTC and asset balances.default_utxo_partial_rgb_for_rgb_keeps_maker_change: the maker uses a default UTXO and gives 300 of 1,000. The taker uses a 10,000-sat UTXO so the maker row is tested on its own. The maker's 700 change settles, and the maker spends it in a second swap.Before the fix, 3 of the 4 failed with the errors in the table above. RGB-for-BTC passed:
After the fix:
Notes / follow-ups
send_btc,drain_to) can't spend it. Keeping that BTC separate needs a second change output per side, which changes the swap message format. That belongs in its own PR.swap_select_btc_inputstakes vanilla coins in BDK order until it has enough. It does not pick the smallest coin that covers the amount.send_btcorcreate_utxoshas reserved (get_reserved_vanilla_outpoints). Swap inputs are not reserved either. Callers running concurrent operations have to useexclude_outpoints.🤖 Generated with Claude Code