BtcSwapTx: update refund tx to consider all utxos

[ok300]

This PR updates BtcSwapTx and its new_refund method to consider all available utxos.

Fixes #72

[ok300]

@i5hi as I'm still testing this, I marked it as a draft and wanted to ask for your high level thoughts and feedback:

• is this the proper way of addressing this, or do you see a better way?
• should the two BtcSwapTx fields (utxo and utxos) be merged into one?
• are there any other implications or considerations for this PR that I might have missed?

[i5hi]

The approach is good!

We don't need the old utxo field anymore.

[ok300]

Alright, I continued with this approach and removed the utxo field in 3587e3c .

Short summary of the changes:

• removed utxo field
• updated new_refund to use fetch_utxos instead of fetch_utxo
• updated sign_claim to rely only on the 1st utxo in utxos (*)
• updated sign_refund
  • cooperative branch: rely only on the 1st utxo in utxos (*)
  • non-cooperative branch: use all available utxos as inputs

I'm not fully sure about the parts marked with (*), more specifically:

• Can more than 1 utxos be claimed?
  • I assume no, because as soon as 1 utxo shows up on the lockup address, the Boltz Backend advances the swap to the next state.
• Can more than 1 utxos be refunded in cooperative mode?
  • I assume no, because this needs interaction with Boltz and intuitively it makes sense to only be able to refund what can be claimed (see above).

What do you think @i5hi? Are my assumptions correct?

[i5hi]

• Can more than 1 utxo be claimed?
  Technically, yes but in practice, No.

Boltz will never lockup more than 1 utxo, so wont need to. However, for consistency the code should attempt to spend all utxos, incase for whatever reason there is more than 1 - no harm I suppose.

• Can more than 1 utxo be coop refunded?
  Yes.

We refund what we lock* up - so incase we lock up more than 1 utxo, we must refund both.

[i5hi]

@michael1011 Need confirmation from you on this

[i5hi]

Infact, we should stick with your original approach of usingfetch_utxo for new_claim and always expect and spend just 1 utxo.

while new_refund will use fetch_utxos and always attempt to spend all.

[michael1011]

Can more than 1 utxos be claimed?

In theory, yes. But it's very likely never going to happen (unless there is a grave bug) that we lock more than one UTXO for a swap.

Can more than 1 utxos be refunded in cooperative mode?

Yes. When asking for a cooperative refund signature, one of the parameters is index which is the number of the input you want to have a partial signature for. That way, you can do multiple cooperative refunds (for a single swap or multiple ones) in a single transaction. When we have a swap in a failed state, we are happy to create as many refund signatures as you would like.

[ok300]

@i5hi @michael1011 thanks for your feedback. I addressed your points and added one question about sign_refund.

[i5hi]

@ok300 Have you tried running the integration submarine tests and send 2 payments to the address to see if this all works well?

[i5hi]

Also ensure that standard refunds are working as expected

[ok300]

@i5hi I ran cargo test and some tests passed, others were skipped with "ignored, requires invoice and refund address".

Are these the tests you mean and if so, what's the proper way to run them? Just change the invoice and refund address fields, then remove the #ignore annotation?

[i5hi]

you dont need to remove ignore, for each submarine test you have to use a fresh invoice or you will get a 400 from boltz:

cargo test --test bitcoin bitcoin_v2_submarine -- --nocapture --ignored 

[ok300]

I tried bitcoin_v2_submarine with a fresh testnet invoice from Phoenix, but running the command you posted failed with "invoice not for current active network: testnet3".

Do I have to set anything else?

[i5hi]

Just tried a pheonix testnet invoice; worked fine.

Try this

lntb5125180n1pnwmt4zpp5mvhqku7k4m4smvhdft67qq0kfpsg3jktsp99uaws60rq6m9wumfqcqpjsp5qlw0j3wlpnzx4cmrk3n8f09fqra2fmwjm4jk0j2l8uqeclag8c9q9q7sqqqqqqqqqqqqqqqqqqqsqqqqqysgqdqqmqz9gxqyjw5qrzjqwfn3p9278ttzzpe0e00uhyxhned3j5d9acqak5emwfpflp8z2cnflctr6qq3f9n3gqqqqlgqqqqqeqqjqeh4hcz22hnr9xc0tat79q0aujefdreewpseqvh6z4afk2qyd067r5z8zq863m5lgkj9nkrdmmtalf6j7zejla0xur3e22mq4dtev9tqpkhanae

[ok300]

I ran two kinds of tests:

First, I used this branch to trigger (and complete) refunds for Incoming Chain Swaps (BTC -> LBTC) in breez/breez-sdk-liquid#471 . Both cooperative and non-cooperative claims (to LBTC, as final step of the chain swap) and refunds (to BTC) were successful.

Second, I used @i5hi 's suggestion above to run the bitcoin_v2_submarine test. It essentially worked (the new invoice was paid), even though the test printed an error HTTP("mempool min fee not met, 224 < 4543") at L170, which is when calling boltz_api_v2.post_submarine_claim_tx_details. The testnet mempool fee estimation is generally erratic. I also didn't find a way to set custom (higher) feerates for the claim tx in the bitcoin_v2_submarine test. IMO this is a separate issue related to feerates on testnet.

The PR is ready for final review 🙏

[ok300]

@michael1011 @i5hi anything else I can do to help with the review?

We are using this branch in the Liquid Breez SDK and it would make life easier to get it merged 🙏

[michael1011]

utACK