From 01af9a5676e266cc9d95ed744ab1b221276e5bfd Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Sun, 23 May 2021 12:59:21 +1000 Subject: [PATCH] Bitcoin transaction published state This improves the error handling on the ASB. Once the Bitcoin redeem transaction is seen in mempool, the state machine cannot transition to a cancel scenario anymore because at that point the CLI will have redeemed the Monero. The additional state then waits for transaction finality. --- CHANGELOG.md | 2 ++ swap/src/database/alice.rs | 18 ++++++++++++++- swap/src/protocol/alice/recovery/cancel.rs | 5 ++++- swap/src/protocol/alice/recovery/punish.rs | 3 ++- swap/src/protocol/alice/recovery/redeem.rs | 22 ++++++++++++++++++- swap/src/protocol/alice/recovery/refund.rs | 3 ++- .../protocol/alice/recovery/safely_abort.rs | 1 + swap/src/protocol/alice/state.rs | 12 +++++++++- swap/src/protocol/alice/swap.rs | 16 +++++++++++--- 9 files changed, 73 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ac46f25..57a8671e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 3. ASB is running in resume-only mode and does not accept incoming swap requests - An issue where the monero daemon port used by the `monero-wallet-rpc` could not be specified. The CLI parameter `--monero-daemon-host` was changed to `--monero-daemon-address` where host and port have to be specified. +- An issue where an ASB redeem scenario can transition to a cancel and publish scenario that will fail. + This is a breaking change for the ASB, because it introduces a new state into the database. ### Changed diff --git a/swap/src/database/alice.rs b/swap/src/database/alice.rs index b76e1f85..80c0d0d4 100644 --- a/swap/src/database/alice.rs +++ b/swap/src/database/alice.rs @@ -39,6 +39,9 @@ pub enum Alice { encrypted_signature: EncryptedSignature, state3: alice::State3, }, + BtcRedeemTransactionPublished { + state3: alice::State3, + }, CancelTimelockExpired { monero_wallet_restore_blockheight: BlockHeight, transfer_proof: TransferProof, @@ -119,6 +122,11 @@ impl From<&AliceState> for Alice { state3: state3.as_ref().clone(), encrypted_signature: *encrypted_signature.clone(), }, + AliceState::BtcRedeemTransactionPublished { state3 } => { + Alice::BtcRedeemTransactionPublished { + state3: state3.as_ref().clone(), + } + } AliceState::BtcRedeemed => Alice::Done(AliceEndState::BtcRedeemed), AliceState::BtcCancelled { monero_wallet_restore_blockheight, @@ -212,6 +220,11 @@ impl From for AliceState { state3: Box::new(state), encrypted_signature: Box::new(encrypted_signature), }, + Alice::BtcRedeemTransactionPublished { state3 } => { + AliceState::BtcRedeemTransactionPublished { + state3: Box::new(state3), + } + } Alice::CancelTimelockExpired { monero_wallet_restore_blockheight, transfer_proof, @@ -271,12 +284,15 @@ impl Display for Alice { Alice::XmrLockTransferProofSent { .. } => { f.write_str("Monero lock transfer proof sent") } + Alice::EncSigLearned { .. } => f.write_str("Encrypted signature learned"), + Alice::BtcRedeemTransactionPublished { .. } => { + f.write_str("Bitcoin redeem transaction published") + } Alice::CancelTimelockExpired { .. } => f.write_str("Cancel timelock is expired"), Alice::BtcCancelled { .. } => f.write_str("Bitcoin cancel transaction published"), Alice::BtcPunishable { .. } => f.write_str("Bitcoin punishable"), Alice::BtcRefunded { .. } => f.write_str("Monero refundable"), Alice::Done(end_state) => write!(f, "Done: {}", end_state), - Alice::EncSigLearned { .. } => f.write_str("Encrypted signature learned"), } } } diff --git a/swap/src/protocol/alice/recovery/cancel.rs b/swap/src/protocol/alice/recovery/cancel.rs index cfa1a085..fedab340 100644 --- a/swap/src/protocol/alice/recovery/cancel.rs +++ b/swap/src/protocol/alice/recovery/cancel.rs @@ -34,6 +34,9 @@ pub async fn cancel( (monero_wallet_restore_blockheight, transfer_proof, state3) } + // The redeem transaction was already published, it is not safe to cancel anymore + AliceState::BtcRedeemTransactionPublished { .. } => bail!(" The redeem transaction was already published, it is not safe to cancel anymore"), + // The cancel tx was already published, but Alice not yet in final state AliceState::BtcCancelled { .. } | AliceState::BtcRefunded { .. } @@ -43,7 +46,7 @@ pub async fn cancel( | AliceState::BtcRedeemed | AliceState::XmrRefunded | AliceState::BtcPunished - | AliceState::SafelyAborted => bail!("Cannot cancel swap {} because it is in state {} which is not cancelable", swap_id, state), + | AliceState::SafelyAborted => bail!("Swap is is in state {} which is not cancelable", state), }; tracing::info!(%swap_id, "Trying to manually cancel swap"); diff --git a/swap/src/protocol/alice/recovery/punish.rs b/swap/src/protocol/alice/recovery/punish.rs index 5a06b5ec..6ffacdf5 100644 --- a/swap/src/protocol/alice/recovery/punish.rs +++ b/swap/src/protocol/alice/recovery/punish.rs @@ -50,7 +50,8 @@ pub async fn punish( } // If the swap was refunded it cannot be punished - AliceState::BtcRefunded {..} + AliceState::BtcRedeemTransactionPublished { .. } + | AliceState::BtcRefunded {..} // Alice already in final state | AliceState::BtcRedeemed | AliceState::XmrRefunded diff --git a/swap/src/protocol/alice/recovery/redeem.rs b/swap/src/protocol/alice/recovery/redeem.rs index 12999db9..55450c87 100644 --- a/swap/src/protocol/alice/recovery/redeem.rs +++ b/swap/src/protocol/alice/recovery/redeem.rs @@ -50,19 +50,39 @@ pub async fn redeem( let redeem_tx = state3.signed_redeem_transaction(*encrypted_signature)?; let (txid, subscription) = bitcoin_wallet.broadcast(redeem_tx, "redeem").await?; + subscription.wait_until_seen().await?; + + let state = AliceState::BtcRedeemTransactionPublished { state3 }; + let db_state = (&state).into(); + db.insert_latest_state(swap_id, Swap::Alice(db_state)) + .await?; + if let Finality::Await = finality { subscription.wait_until_final().await?; } let state = AliceState::BtcRedeemed; let db_state = (&state).into(); - db.insert_latest_state(swap_id, Swap::Alice(db_state)) .await?; Ok((txid, state)) } + AliceState::BtcRedeemTransactionPublished { state3 } => { + let subscription = bitcoin_wallet.subscribe_to(state3.tx_redeem()).await; + if let Finality::Await = finality { + subscription.wait_until_final().await?; + } + let state = AliceState::BtcRedeemed; + let db_state = (&state).into(); + db.insert_latest_state(swap_id, Swap::Alice(db_state)) + .await?; + + let txid = state3.tx_redeem().txid(); + + Ok((txid, state)) + } AliceState::Started { .. } | AliceState::BtcLocked { .. } | AliceState::XmrLockTransactionSent { .. } diff --git a/swap/src/protocol/alice/recovery/refund.rs b/swap/src/protocol/alice/recovery/refund.rs index 8168bace..bb26ee67 100644 --- a/swap/src/protocol/alice/recovery/refund.rs +++ b/swap/src/protocol/alice/recovery/refund.rs @@ -56,7 +56,8 @@ pub async fn refund( } // Alice already in final state - AliceState::BtcRedeemed + AliceState::BtcRedeemTransactionPublished { .. } + | AliceState::BtcRedeemed | AliceState::XmrRefunded | AliceState::BtcPunished | AliceState::SafelyAborted => bail!(Error::SwapNotRefundable(state)), diff --git a/swap/src/protocol/alice/recovery/safely_abort.rs b/swap/src/protocol/alice/recovery/safely_abort.rs index 605c5029..29df2814 100644 --- a/swap/src/protocol/alice/recovery/safely_abort.rs +++ b/swap/src/protocol/alice/recovery/safely_abort.rs @@ -22,6 +22,7 @@ pub async fn safely_abort(swap_id: Uuid, db: Arc) -> Result, state3: Box, }, + BtcRedeemTransactionPublished { + state3: Box, + }, BtcRedeemed, BtcCancelled { monero_wallet_restore_blockheight: BlockHeight, @@ -83,6 +86,9 @@ impl fmt::Display for AliceState { write!(f, "xmr lock transfer proof sent") } AliceState::EncSigLearned { .. } => write!(f, "encrypted signature is learned"), + AliceState::BtcRedeemTransactionPublished { .. } => { + write!(f, "bitcoin redeem transaction published") + } AliceState::BtcRedeemed => write!(f, "btc is redeemed"), AliceState::BtcCancelled { .. } => write!(f, "btc is cancelled"), AliceState::BtcRefunded { .. } => write!(f, "btc is refunded"), @@ -449,6 +455,10 @@ impl State3 { bitcoin::TxRefund::new(&self.tx_cancel(), &self.refund_address, self.tx_refund_fee) } + pub fn tx_redeem(&self) -> TxRedeem { + TxRedeem::new(&self.tx_lock, &self.redeem_address, self.tx_redeem_fee) + } + pub fn extract_monero_private_key( &self, published_refund_tx: bitcoin::Transaction, diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index 28290a21..bf71895d 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -204,10 +204,10 @@ where let tx_lock_status = bitcoin_wallet.subscribe_to(state3.tx_lock.clone()).await; match state3.signed_redeem_transaction(*encrypted_signature) { Ok(tx) => match bitcoin_wallet.broadcast(tx, "redeem").await { - Ok((_, subscription)) => match subscription.wait_until_final().await { - Ok(_) => AliceState::BtcRedeemed, + Ok((_, subscription)) => match subscription.wait_until_seen().await { + Ok(_) => AliceState::BtcRedeemTransactionPublished { state3 }, Err(e) => { - bail!("Waiting for Bitcoin transaction finality failed with {}! The redeem transaction was published, but it is not ensured that the transaction was included! You're screwed.", e) + bail!("Waiting for Bitcoin redeem transaction to be in mempool failed with {}! The redeem transaction was published, but it is not ensured that the transaction was included! You're screwed.", e) } }, Err(error) => { @@ -247,6 +247,16 @@ where state3, }, }, + AliceState::BtcRedeemTransactionPublished { state3 } => { + let subscription = bitcoin_wallet.subscribe_to(state3.tx_redeem()).await; + + match subscription.wait_until_final().await { + Ok(_) => AliceState::BtcRedeemed, + Err(e) => { + bail!("The Bitcoin redeem transaction was seen in mempool, but waiting for finality timed out with {}. Manual investigation might be needed to ensure that the transaction was included.", e) + } + } + } AliceState::CancelTimelockExpired { monero_wallet_restore_blockheight, transfer_proof,