From 0fec5d556d78d525f5516700ce610c1deff373f2 Mon Sep 17 00:00:00 2001 From: Mohan <86064887+binarybaron@users.noreply.github.com> Date: Sat, 1 Nov 2025 00:44:58 +0100 Subject: [PATCH] refactor(alice): concretize enc sig publication requirement (#664) * refactor(alice): concretize enc sig publication requirement * add changelog entry --- CHANGELOG.md | 2 ++ libp2p-rendezvous-node/Cargo.toml | 4 ++-- swap-core/src/bitcoin/cancel.rs | 4 ---- swap-env/src/env.rs | 8 ++++++++ swap/src/protocol/alice/swap.rs | 27 ++++++++++++++------------- 5 files changed, 26 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a981f04..f4dadf60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- ASB: Fix an issue where we would not redeem the Bitcoin and force a refund even though it was still possible to do so. + ## [3.2.7] - 2025-10-28 ## [3.2.6] - 2025-10-27 diff --git a/libp2p-rendezvous-node/Cargo.toml b/libp2p-rendezvous-node/Cargo.toml index 116f7e01..49812b42 100644 --- a/libp2p-rendezvous-node/Cargo.toml +++ b/libp2p-rendezvous-node/Cargo.toml @@ -11,9 +11,9 @@ tor-hsservice = { workspace = true } tor-rtcompat = { workspace = true } # Async +backoff = { workspace = true, features = ["tokio"] } futures = { workspace = true } tokio = { workspace = true, features = ["rt-multi-thread", "time", "macros", "sync", "process", "fs", "net", "io-util"] } -backoff = { workspace = true, features = ["tokio"] } # Libp2p libp2p = { workspace = true, features = ["rendezvous", "tcp", "yamux", "dns", "noise", "ping", "websocket", "tokio", "macros"] } @@ -28,6 +28,6 @@ tracing-subscriber = { workspace = true, default-features = false, features = [" anyhow = "1" # Other -swap-env = { path = "../swap-env" } atty = "0.2" structopt = { version = "0.3", default-features = false } +swap-env = { path = "../swap-env" } diff --git a/swap-core/src/bitcoin/cancel.rs b/swap-core/src/bitcoin/cancel.rs index 8a523757..fc143987 100644 --- a/swap-core/src/bitcoin/cancel.rs +++ b/swap-core/src/bitcoin/cancel.rs @@ -45,10 +45,6 @@ impl CancelTimelock { pub const fn new(number_of_blocks: u32) -> Self { Self(number_of_blocks) } - - pub fn half(&self) -> CancelTimelock { - Self(self.0 / 2) - } } impl Add for BlockHeight { diff --git a/swap-env/src/env.rs b/swap-env/src/env.rs index 79881059..0636dbe7 100644 --- a/swap-env/src/env.rs +++ b/swap-env/src/env.rs @@ -9,6 +9,9 @@ pub struct Config { pub bitcoin_lock_mempool_timeout: Duration, pub bitcoin_lock_confirmed_timeout: Duration, pub bitcoin_finality_confirmations: u32, + /// The upper bound for the number of blocks that will be mined before our + /// Bitcoin transaction is included in a block + pub bitcoin_blocks_till_confirmed_upper_bound_assumption: u32, pub bitcoin_avg_block_time: Duration, pub bitcoin_cancel_timelock: u32, pub bitcoin_punish_timelock: u32, @@ -52,6 +55,9 @@ impl GetConfig for Mainnet { bitcoin_lock_mempool_timeout: 10.std_minutes(), bitcoin_lock_confirmed_timeout: 2.std_hours(), bitcoin_finality_confirmations: 1, + // We assume that a transaction that was constructed to be confirmed within one block + // will be confirmed within at most 6 blocks + bitcoin_blocks_till_confirmed_upper_bound_assumption: 6, bitcoin_avg_block_time: 10.std_minutes(), bitcoin_cancel_timelock: 72, bitcoin_punish_timelock: 144, @@ -73,6 +79,7 @@ impl GetConfig for Testnet { bitcoin_lock_mempool_timeout: 10.std_minutes(), bitcoin_lock_confirmed_timeout: 1.std_hours(), bitcoin_finality_confirmations: 1, + bitcoin_blocks_till_confirmed_upper_bound_assumption: 6, bitcoin_avg_block_time: 10.std_minutes(), bitcoin_cancel_timelock: 12 * 3, bitcoin_punish_timelock: 24 * 3, @@ -92,6 +99,7 @@ impl GetConfig for Regtest { bitcoin_lock_mempool_timeout: 30.std_seconds(), bitcoin_lock_confirmed_timeout: 5.std_minutes(), bitcoin_finality_confirmations: 1, + bitcoin_blocks_till_confirmed_upper_bound_assumption: 6, bitcoin_avg_block_time: 5.std_seconds(), bitcoin_cancel_timelock: 100, bitcoin_punish_timelock: 50, diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index bcc0455a..d6ede89e 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -451,26 +451,25 @@ where .build(); match backoff::future::retry_notify(backoff.clone(), || async { - // We can only redeem the Bitcoin if we are sure that our Bitcoin redeem transaction - // will be confirmed before the timelock expires - // - // We only publish the transaction if we have more than half of the timelock left. let tx_lock_status = bitcoin_wallet .status_of_script(&state3.tx_lock.clone()) .await?; - if tx_lock_status.is_confirmed_with(state3.cancel_timelock.half()) { + // If the cancel timelock is expired, it it not safe to publish the Bitcoin redeem transaction anymore + // + // TODO: In practice this should be redundant because the logic above will trigger for a superset of the cases where this is true + if tx_lock_status.is_confirmed_with(state3.cancel_timelock) { return Ok(None); } - // If the cancel timelock is expired, it it not safe to publish the Bitcoin redeem transaction anymore + // We can only redeem the Bitcoin if we are fairly sure that our Bitcoin redeem transaction + // will be confirmed before the cancel timelock expires // - // TODO: This should never be true if the logic above is not true but we do it anyway to be safe - // TODO: Remove this - if !matches!( - state3.expired_timelocks(&*bitcoin_wallet).await?, - ExpiredTimelocks::None { .. } - ) { + // We make an assumption that it will take at most `env_config.bitcoin_blocks_till_confirmed_upper_bound_assumption` blocks + // until our transaction is included in a block. If this assumption is not satisfied, we will not publish the transaction. + // + // We will instead wait for the cancel timelock to expire and then refund. + if tx_lock_status.blocks_left_until(state3.cancel_timelock) < env_config.bitcoin_blocks_till_confirmed_upper_bound_assumption { return Ok(None); } @@ -494,6 +493,7 @@ where // We wait until we see the transaction in the mempool before transitioning to the next state Some((txid, subscription)) => match subscription.wait_until_seen().await { Ok(_) => AliceState::BtcRedeemTransactionPublished { state3, transfer_proof }, + // TODO: No need to bail here, we should just retry? Err(e) => { // We extract the txid and the hex representation of the transaction // this'll allow the user to manually re-publish the transaction @@ -503,7 +503,8 @@ where } }, - // More than half of the cancel timelock expired before we could publish the redeem transaction + // It is not safe to publish the Bitcoin redeem transaction anymore + // We wait for the cancel timelock to expire and then refund None => { tracing::error!("We were unable to publish the Bitcoin redeem transaction before the timelock expired.");