Skip to content

Commit

Permalink
Prevent claiming of overpaid swaps (#667)
Browse files Browse the repository at this point in the history 
* Handle overpayments gracefully

* Fix test mock txs and swaps

* Get actual payer amount from blockchain

* Check first tx output instead of script balance

* Fetch actual payer amount if not available

* Remove user lockup amount verification in recoverer

* Refactor user lockup amount check into appropriate method
  • Loading branch information
@danielgranhao
danielgranhao authored Jan 21, 2025
1 parent 16ac13e commit 0cdfa20
Show file tree
Hide file tree
Showing 7 changed files with 306 additions and 147 deletions.
125 changes: 110 additions & 15 deletions lib/core/src/chain_swap.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{str::FromStr, sync::Arc};

use anyhow::{anyhow, Result};
use anyhow::{anyhow, bail, Result};
use async_trait::async_trait;
use boltz_client::{
boltz::{self},
Expand Down Expand Up @@ -211,6 +211,11 @@ impl ChainSwapHandler {
.update_chain_swap_accept_zero_conf(&id, !zero_conf_rejected)?;
}
if let Some(transaction) = update.transaction.clone() {
let actual_payer_amount =
self.fetch_incoming_swap_actual_payer_amount(swap).await?;
self.persister
.update_actual_payer_amount(&swap.id, actual_payer_amount)?;

self.update_swap_info(&ChainSwapUpdate {
swap_id: id,
to_state: Pending,
Expand All @@ -230,6 +235,11 @@ impl ChainSwapHandler {
return Err(anyhow!("Unexpected payload from Boltz status stream"));
};

if let Err(e) = self.verify_user_lockup_tx(swap).await {
warn!("User lockup transaction for incoming Chain Swap {} could not be verified. err: {}", swap.id, e);
return Err(anyhow!("Could not verify user lockup transaction: {e}",));
}

if let Err(e) = self
.verify_server_lockup_tx(swap, &transaction, false)
.await
Expand Down Expand Up @@ -319,7 +329,7 @@ impl ChainSwapHandler {

// If swap state is unrecoverable, either:
// 1. The transaction failed
// 2. Lockup failed (too little funds were sent)
// 2. Lockup failed (too little/too much funds were sent)
// 3. The claim lockup was refunded
// 4. The swap has expired (>24h)
// We initiate a cooperative refund, and then fallback to a regular one
Expand All @@ -344,7 +354,7 @@ impl ChainSwapHandler {
match swap.refund_tx_id.clone() {
None => {
warn!("Chain Swap {id} is in an unrecoverable state: {swap_state:?}");
match self.verify_user_lockup_tx(swap).await {
match self.verify_user_lockup_tx_exists(swap).await {
Ok(_) => {
info!("Chain Swap {id} user lockup tx was broadcast. Setting the swap to refundable.");
self.update_swap_info(&ChainSwapUpdate {
Expand Down Expand Up @@ -576,6 +586,11 @@ impl ChainSwapHandler {
return Err(anyhow!("Unexpected payload from Boltz status stream"));
};

if let Err(e) = self.verify_user_lockup_tx(swap).await {
warn!("User lockup transaction for outgoing Chain Swap {} could not be verified. err: {}", swap.id, e);
return Err(anyhow!("Could not verify user lockup transaction: {e}",));
}

if let Err(e) = self
.verify_server_lockup_tx(swap, &transaction, false)
.await
Expand Down Expand Up @@ -1165,6 +1180,47 @@ impl ChainSwapHandler {
}
}

async fn fetch_incoming_swap_actual_payer_amount(&self, chain_swap: &ChainSwap) -> Result<u64> {
let swap_script = chain_swap.get_lockup_swap_script()?;
let script_pubkey = swap_script
.as_bitcoin_script()?
.to_address(self.config.network.as_bitcoin_chain())
.map_err(|e| anyhow!("Failed to get swap script address {e:?}"))?
.script_pubkey();

let history = self.fetch_bitcoin_script_history(&swap_script).await?;

// User lockup tx is by definition the first
let first_tx_id = history
.first()
.ok_or(anyhow!(
"No history found for user lockup script for swap {}",
chain_swap.id
))?
.txid
.to_raw_hash()
.into();

// Get full transaction
let txs = self
.bitcoin_chain_service
.lock()
.await
.get_transactions(&[first_tx_id])?;
let user_lockup_tx = txs.first().ok_or(anyhow!(
"No transactions found for user lockup script for swap {}",
chain_swap.id
))?;

// Find output paying to our script and get amount
user_lockup_tx
.output
.iter()
.find(|out| out.script_pubkey == script_pubkey)
.map(|out| out.value.to_sat())
.ok_or(anyhow!("No output found paying to user lockup script"))
}

async fn verify_server_lockup_tx(
&self,
chain_swap: &ChainSwap,
Expand Down Expand Up @@ -1218,7 +1274,7 @@ impl ChainSwapHandler {
// Verify RBF
let rbf_explicit = tx.input.iter().any(|tx_in| tx_in.sequence.is_rbf());
if !verify_confirmation && rbf_explicit {
return Err(anyhow!("Transaction signals RBF"));
bail!("Transaction signals RBF");
}
// Verify amount
let secp = Secp256k1::new();
Expand All @@ -1235,20 +1291,20 @@ impl ChainSwapHandler {
match chain_swap.accepted_receiver_amount_sat {
None => {
if value < claim_details.amount {
return Err(anyhow!(
bail!(
"Transaction value {value} sats is less than {} sats",
claim_details.amount
));
);
}
}
Some(accepted_receiver_amount_sat) => {
let expected_server_lockup_amount_sat =
accepted_receiver_amount_sat + chain_swap.claim_fees_sat;
if value < expected_server_lockup_amount_sat {
return Err(anyhow!(
bail!(
"Transaction value {value} sats is less than accepted {} sats",
expected_server_lockup_amount_sat
));
);
}
}
}
Expand Down Expand Up @@ -1301,7 +1357,7 @@ impl ChainSwapHandler {
Ok(())
}

async fn verify_user_lockup_tx(&self, chain_swap: &ChainSwap) -> Result<String> {
async fn verify_user_lockup_tx_exists(&self, chain_swap: &ChainSwap) -> Result<()> {
let swap_script = chain_swap.get_lockup_swap_script()?;
let script_history = match chain_swap.direction {
Direction::Incoming => self.fetch_bitcoin_script_history(&swap_script).await,
Expand All @@ -1314,7 +1370,6 @@ impl ChainSwapHandler {
.iter()
.find(|h| h.txid.to_hex() == user_lockup_tx_id)
.ok_or(anyhow!("Transaction was not found in script history"))?;
Ok(user_lockup_tx_id)
}
None => {
let txid = script_history
Expand All @@ -1328,9 +1383,37 @@ impl ChainSwapHandler {
user_lockup_tx_id: Some(txid.clone()),
..Default::default()
})?;
Ok(txid)
}
}

Ok(())
}

async fn verify_user_lockup_tx(&self, chain_swap: &ChainSwap) -> Result<()> {
self.verify_user_lockup_tx_exists(chain_swap).await?;

// Verify amount for incoming chain swaps
if chain_swap.direction == Direction::Incoming {
let actual_payer_amount_sat = match chain_swap.actual_payer_amount_sat {
Some(amount) => amount,
None => {
let actual_payer_amount_sat = self
.fetch_incoming_swap_actual_payer_amount(chain_swap)
.await?;
self.persister
.update_actual_payer_amount(&chain_swap.id, actual_payer_amount_sat)?;
actual_payer_amount_sat
}
};
// For non-amountless swaps, make sure user locked up the agreed amount
if chain_swap.payer_amount_sat > 0
&& chain_swap.payer_amount_sat != actual_payer_amount_sat
{
bail!("Invalid user lockup tx - user lockup amount ({actual_payer_amount_sat} sat) differs from agreed ({} sat)", chain_swap.payer_amount_sat);
}
}

Ok(())
}

async fn fetch_bitcoin_script_history(
Expand Down Expand Up @@ -1453,8 +1536,14 @@ mod tests {

for (first_state, allowed_states) in valid_combinations.iter() {
for allowed_state in allowed_states {
let chain_swap =
new_chain_swap(Direction::Incoming, Some(*first_state), false, None, false);
let chain_swap = new_chain_swap(
Direction::Incoming,
Some(*first_state),
false,
None,
false,
false,
);
persister.insert_or_update_chain_swap(&chain_swap)?;

assert!(chain_swap_handler
Expand All @@ -1480,8 +1569,14 @@ mod tests {

for (first_state, disallowed_states) in invalid_combinations.iter() {
for disallowed_state in disallowed_states {
let chain_swap =
new_chain_swap(Direction::Incoming, Some(*first_state), false, None, false);
let chain_swap = new_chain_swap(
Direction::Incoming,
Some(*first_state),
false,
None,
false,
false,
);
persister.insert_or_update_chain_swap(&chain_swap)?;

assert!(chain_swap_handler
Expand Down
9 changes: 5 additions & 4 deletions lib/core/src/persist/chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ mod tests {
async fn test_writing_stale_swap() -> Result<()> {
create_persister!(storage);

let chain_swap = new_chain_swap(Direction::Incoming, None, false, None, false);
let chain_swap = new_chain_swap(Direction::Incoming, None, false, None, false, false);
storage.insert_or_update_chain_swap(&chain_swap)?;

// read - update - write works if there are no updates in between
Expand All @@ -543,7 +543,7 @@ mod tests {
create_persister!(storage);

let chain_swap_local_with_sync_state =
new_chain_swap(Direction::Incoming, None, false, None, false);
new_chain_swap(Direction::Incoming, None, false, None, false, false);
storage.insert_or_update_chain_swap(&chain_swap_local_with_sync_state)?;
storage.set_sync_state(SyncState {
data_id: chain_swap_local_with_sync_state.id.clone(),
Expand All @@ -553,10 +553,11 @@ mod tests {
})?;

let chain_swap_local_no_sync_state =
new_chain_swap(Direction::Incoming, None, false, None, false);
new_chain_swap(Direction::Incoming, None, false, None, false, false);
storage.insert_or_update_chain_swap(&chain_swap_local_no_sync_state)?;

let chain_swap_not_local = new_chain_swap(Direction::Incoming, None, false, None, false);
let chain_swap_not_local =
new_chain_swap(Direction::Incoming, None, false, None, false, false);
storage.insert_or_update_chain_swap(&chain_swap_not_local)?;
storage.set_sync_state(SyncState {
data_id: chain_swap_not_local.id,
Expand Down
2 changes: 1 addition & 1 deletion lib/core/src/recover/recoverer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ impl Recoverer {
Ok(res)
}

/// Reconstruct Chain Receive Swap tx IDs from the onchain data and the immutable data data
/// Reconstruct Chain Receive Swap tx IDs from the onchain data and the immutable data
fn recover_receive_chain_swap_tx_ids(
&self,
tx_map: &TxMap,
Expand Down
Loading

0 comments on commit 0cdfa20

Please sign in to comment.