Closed dangeross closed 5 months ago
Massive!
Thanks @dangeross !
Will review over the weekend.
@dangeross couldnt' find tests for this. Attempting to put one together now.
couldnt' find tests for this. Attempting to put one together now.
Thanks @i5hi, let me know if I can help at all.
It would be cool if @michael1011 could confirm the boltz states used in the chain swaps.
@dangeross When creating a ChainSwap from BTC to LBTC
we would need to use the BtcSwapScript/TxV2 structs right?
Additionally, its a bit tricky for the user of the library to know who part of the ChainSwapResponse
to use as ChainSwapDetails
in this function:
pub fn chain_from_swap_resp(
side: Side,
chain_swap_details: ChainSwapDetails,
our_pubkey: PublicKey,
)
There is a claim and lockup - ChainSwapDetails
- so maybe better for this function to take the ChainSwapResponse
object and let the method handle which of the two ChainSwapDetails
to use?
When creating a ChainSwap
from BTC to LBTC
we would need to use the BtcSwapScript/TxV2 structs right?
The lockup side is BtcSwapScript, the claim side is LBtcSwapScript
so maybe better for this function to take the
ChainSwapResponse
object and let the method handle which of the twoChainSwapDetails
to use?
Could work, maybe also having two wrapper functions, one for claim and one for lockup. E.g. chain_claim_from_swap_resp
and chain_lockup_from_swap_resp
If it helps @i5hi, here is our current usage. I think having 2 functions is a better approach, then passing the Side
is not required. But the Side
or maybe details_type Claim/Lockup need to be stored to switch the order of the musig key aggregation
https://github.com/breez/breez-liquid-sdk/blob/bcb47432608f8b91e40a1806b47c1f496ccc76c6/lib/core/src/model.rs#L369-L403
@dangeross The sign_claim methods now use a new cooperative Cooperative
:
pub fn sign_claim(
&self,
keys: &Keypair,
preimage: &Preimage,
absolute_fees: Amount,
is_cooperative: Option<Cooperative>,
) -> Result<Transaction, Error>
The updated tests in liquid/bitcoin_v2.rs have been updated as follows:
let tx = claim_tx
.sign_claim(
&our_keys,
&preimage,
Amount::from_sat(1000),
Some(Cooperative {
boltz_api: &boltz_api_v2,
swap_id: swap_id.clone(),
pub_nonce: None,
partial_sig: None,
}),
)
.unwrap();
This would fail as pub_nonce
and partial_sig
are not provided.
I went through the breez code base to understand how to use this properly. Can you confirm that this would be correct usage?
let claim_tx = LBtcSwapTxV2::new_claim(
claim_script.clone(),
claim_address.clone(),
&ElectrumConfig::default_liquid(),
BOLTZ_TESTNET_URL_V2.to_string(),
swap_id.clone(),
)
.unwrap();
let refund_tx = BtcSwapTxV2::new_refund(
lockup_script.clone(),
&refund_address,
&ElectrumConfig::default_bitcoin(),
)
.unwrap();
let claim_tx_response =
boltz_api_v2.get_chain_claim_tx_details(&swap_id).unwrap();
let (partial_sig, pub_nonce) = refund_tx
.partial_sig(
&our_refund_keys,
&claim_tx_response.pub_nonce,
&claim_tx_response.transaction_hash,
)
.unwrap();
let tx = claim_tx
.sign_claim(
&our_claim_keys,
&preimage,
Amount::from_sat(1000),
Some(Cooperative {
boltz_api: &boltz_api_v2,
swap_id: swap_id.clone(),
pub_nonce: Some(pub_nonce),
partial_sig: Some(partial_sig),
}),
)
.unwrap();
I've noticed this actually won't be an issue with LN swaps
let partial_sig_resp = match self.swap_script.swap_type {
SwapType::Chain => match (pub_nonce, partial_sig) {
(Some(pub_nonce), Some(partial_sig)) => boltz_api.post_chain_claim_tx_details(
&swap_id,
preimage,
pub_nonce,
partial_sig,
ToSign {
pub_nonce: claim_pub_nonce.serialize().to_lower_hex_string(),
transaction: claim_tx_hex,
index: 0,
},
),
_ => Err(Error::Protocol(
"Chain swap claim needs a partial_sig".to_string(),
)),
},
SwapType::ReverseSubmarine => boltz_api.get_reverse_partial_sig(
&swap_id,
&preimage,
&claim_pub_nonce,
&claim_tx_hex,
),
_ => Err(Error::Protocol(format!(
"Cannot get partial sig for {:?} Swap",
self.swap_script.swap_type
))),
}?;
So i will not change the the LN swap tests. Let me know if the previous comment is the correct flow for chain swaps.
@i5hi the pub_nonce
and partial_sig
are only used in the chain swap claim where there is this double exchange of nonce's/sig's. So all other swap types work without these set in Cooperative
And the pub_nonce
and partial_sig
are from the refund_tx
right?
And when refunding they would be from the claim_tx?
And the
pub_nonce
andpartial_sig
are from therefund_tx
right?
Yes
And when refunding they would be from the claim_tx?
They are only needed for the claim flow
Damn! I merged michael's update to the function name and it broke the test because its usage needs to be updated.
Sorry guys! I will just merge and fix it on trunk immediately.
Can I get an ACK on doing it this way?
Or @dangeross would you be able to fix it from your branch?
Or recommend another strategy?
ACK @i5hi. Feel free to commit to the PR
Patched on trunk! Cheers!
This PR adds Chain Swaps. It should include:
Tested: