From 4903169f4cf7f45a16c4e945388d5aa992fecaa6 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 17 Dec 2020 18:32:06 +1100 Subject: [PATCH] Check error to determine if tx_cancel has already been published If the error code suggests that the tx_cancel is already on the blockchain, continue with the protocol. This change removed the need to check for tx_cancel before attemting to publish it. Removed the tx_cancel field from the Cancelled state as it can be computed from state3. Co-authored-by: rishflab --- Cargo.lock | 1 + swap/Cargo.toml | 1 + swap/src/alice/steps.rs | 52 ++------------------------------------- swap/src/alice/swap.rs | 50 +++++++++++++++----------------------- swap/src/bob/swap.rs | 22 ++++++++++------- swap/src/lib.rs | 2 ++ xmr-btc/src/alice.rs | 22 ++++++++++++++++- xmr-btc/src/bitcoin.rs | 34 ++++++++++++++++++++++++++ xmr-btc/src/bob.rs | 54 +++++++++-------------------------------- 9 files changed, 105 insertions(+), 133 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c9de62a8..10b6e272 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3320,6 +3320,7 @@ dependencies = [ "genawaiter", "get-port", "hyper", + "jsonrpc_client", "libp2p", "libp2p-tokio-socks5", "log", diff --git a/swap/Cargo.toml b/swap/Cargo.toml index f30ae7e4..fa0a34ec 100644 --- a/swap/Cargo.toml +++ b/swap/Cargo.toml @@ -19,6 +19,7 @@ derivative = "2" ecdsa_fun = { git = "https://github.com/LLFourn/secp256kfun", rev = "cdfbc766045ea678a41780919d6228dd5acee3be", features = ["libsecp_compat", "serde"] } futures = { version = "0.3", default-features = false } genawaiter = "0.99.1" +jsonrpc_client = { git = "https://github.com/thomaseizinger/rust-jsonrpc-client", rev = "c7010817e0f86ab24b3dc10d6bb0463faa0aace4", features = ["reqwest"] } libp2p = { version = "0.29", default-features = false, features = ["tcp-tokio", "yamux", "mplex", "dns", "noise", "request-response"] } libp2p-tokio-socks5 = "0.4" log = { version = "0.4", features = ["serde"] } diff --git a/swap/src/alice/steps.rs b/swap/src/alice/steps.rs index 30006f88..73873c83 100644 --- a/swap/src/alice/steps.rs +++ b/swap/src/alice/steps.rs @@ -20,8 +20,8 @@ use xmr_btc::{ alice::State3, bitcoin::{ poll_until_block_height_is_gte, BlockHeight, BroadcastSignedTransaction, - EncryptedSignature, GetRawTransaction, TransactionBlockHeight, TxCancel, TxLock, TxRefund, - WaitForTransactionFinality, WatchForRawTransaction, + EncryptedSignature, TxCancel, TxLock, TxRefund, WaitForTransactionFinality, + WatchForRawTransaction, }, config::Config, cross_curve_dleq, @@ -203,54 +203,6 @@ where .await } -pub async fn publish_cancel_transaction( - tx_lock: TxLock, - a: bitcoin::SecretKey, - B: bitcoin::PublicKey, - refund_timelock: u32, - tx_cancel_sig_bob: bitcoin::Signature, - bitcoin_wallet: Arc, -) -> Result -where - W: GetRawTransaction + TransactionBlockHeight + BlockHeight + BroadcastSignedTransaction, -{ - // First wait for t1 to expire - let tx_lock_height = bitcoin_wallet - .transaction_block_height(tx_lock.txid()) - .await; - poll_until_block_height_is_gte(bitcoin_wallet.as_ref(), tx_lock_height + refund_timelock).await; - - let tx_cancel = bitcoin::TxCancel::new(&tx_lock, refund_timelock, a.public(), B); - - // If Bob hasn't yet broadcasted the tx cancel, we do it - if bitcoin_wallet - .get_raw_transaction(tx_cancel.txid()) - .await - .is_err() - { - // TODO(Franck): Maybe the cancel transaction is already mined, in this case, - // the broadcast will error out. - - let sig_a = a.sign(tx_cancel.digest()); - let sig_b = tx_cancel_sig_bob.clone(); - - let tx_cancel = tx_cancel - .clone() - .add_signatures(&tx_lock, (a.public(), sig_a), (B, sig_b)) - .expect("sig_{a,b} to be valid signatures for tx_cancel"); - - // TODO(Franck): Error handling is delicate, why can't we broadcast? - bitcoin_wallet - .broadcast_signed_transaction(tx_cancel) - .await?; - - // TODO(Franck): Wait until transaction is mined and returned mined - // block height - } - - Ok(tx_cancel) -} - pub async fn wait_for_bitcoin_refund( tx_cancel: &TxCancel, cancel_tx_height: u32, diff --git a/swap/src/alice/swap.rs b/swap/src/alice/swap.rs index 6690e5c7..7d64434d 100644 --- a/swap/src/alice/swap.rs +++ b/swap/src/alice/swap.rs @@ -6,8 +6,8 @@ use crate::{ steps::{ build_bitcoin_punish_transaction, build_bitcoin_redeem_transaction, extract_monero_private_key, lock_xmr, negotiate, publish_bitcoin_punish_transaction, - publish_bitcoin_redeem_transaction, publish_cancel_transaction, - wait_for_bitcoin_encrypted_signature, wait_for_bitcoin_refund, wait_for_locked_bitcoin, + publish_bitcoin_redeem_transaction, wait_for_bitcoin_encrypted_signature, + wait_for_bitcoin_refund, wait_for_locked_bitcoin, }, }, bitcoin, @@ -16,9 +16,9 @@ use crate::{ state, state::{Alice, Swap}, storage::Database, - SwapAmounts, + SwapAmounts, TRANSACTION_ALREADY_IN_BLOCKCHAIN_ERROR_CODE, }; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use async_recursion::async_recursion; use futures::{ future::{select, Either}, @@ -31,7 +31,7 @@ use tracing::info; use uuid::Uuid; use xmr_btc::{ alice::{State0, State3}, - bitcoin::{TransactionBlockHeight, TxCancel, TxRefund, WatchForRawTransaction}, + bitcoin::{TransactionBlockHeight, TxRefund, WatchForRawTransaction}, config::Config, monero::CreateWalletForOutput, Epoch, @@ -67,7 +67,6 @@ pub enum AliceState { BtcRedeemed, BtcCancelled { state3: State3, - tx_cancel: TxCancel, }, BtcRefunded { spend_key: monero::PrivateKey, @@ -167,19 +166,7 @@ impl TryFrom for AliceState { encrypted_signature, }, Alice::T1Expired(state3) => AliceState::T1Expired { state3 }, - Alice::BtcCancelled(state) => { - let tx_cancel = bitcoin::TxCancel::new( - &state.tx_lock, - state.refund_timelock, - state.a.public(), - state.B, - ); - - BtcCancelled { - state3: state, - tx_cancel, - } - } + Alice::BtcCancelled(state3) => BtcCancelled { state3 }, Alice::BtcPunishable(state) => { let tx_cancel = bitcoin::TxCancel::new( &state.tx_lock, @@ -486,20 +473,22 @@ pub async fn run_until( .await } AliceState::T1Expired { state3 } => { - let tx_cancel = publish_cancel_transaction( - state3.tx_lock.clone(), - state3.a.clone(), - state3.B, - state3.refund_timelock, - state3.tx_cancel_sig_bob.clone(), - bitcoin_wallet.clone(), - ) - .await?; + if let Err(error) = state3.submit_tx_cancel(bitcoin_wallet.as_ref()).await { + let json_rpc_err = error + .downcast_ref::() + .ok_or_else(|| anyhow!("Failed to downcast JsonRpcError"))?; + if json_rpc_err.code == TRANSACTION_ALREADY_IN_BLOCKCHAIN_ERROR_CODE { + info!("Failed to send cancel transaction, assuming that is was already published by the other party..."); + } else { + return Err(error); + } + }; - let state = AliceState::BtcCancelled { state3, tx_cancel }; + let state = AliceState::BtcCancelled { state3 }; let db_state = (&state).into(); db.insert_latest_state(swap_id, Swap::Alice(db_state)) .await?; + run_until( state, is_target_state, @@ -512,7 +501,8 @@ pub async fn run_until( ) .await } - AliceState::BtcCancelled { state3, tx_cancel } => { + AliceState::BtcCancelled { state3 } => { + let tx_cancel = state3.tx_cancel(); let tx_cancel_height = bitcoin_wallet .transaction_block_height(tx_cancel.txid()) .await; diff --git a/swap/src/bob/swap.rs b/swap/src/bob/swap.rs index 19a0bbd0..d86e0da4 100644 --- a/swap/src/bob/swap.rs +++ b/swap/src/bob/swap.rs @@ -3,9 +3,9 @@ use crate::{ state, state::{Bob, Swap}, storage::Database, - SwapAmounts, + SwapAmounts, TRANSACTION_ALREADY_IN_BLOCKCHAIN_ERROR_CODE, }; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use async_recursion::async_recursion; use rand::{CryptoRng, RngCore}; use std::{convert::TryFrom, fmt, sync::Arc}; @@ -343,13 +343,17 @@ where .await } BobState::T1Expired(state4) => { - if state4 - .check_for_tx_cancel(bitcoin_wallet.as_ref()) - .await - .is_err() - { - state4.submit_tx_cancel(bitcoin_wallet.as_ref()).await?; - } + let result = state4.submit_tx_cancel(bitcoin_wallet.as_ref()).await; + if let Err(error) = result { + let json_rpc_err = error + .downcast_ref::() + .ok_or_else(|| anyhow!("Failed to downcast JsonRpcError"))?; + if json_rpc_err.code == TRANSACTION_ALREADY_IN_BLOCKCHAIN_ERROR_CODE { + info!("Failed to send cancel transaction, assuming that is was already included by the other party..."); + } else { + return Err(error); + } + }; let state = BobState::Cancelled(state4); db.insert_latest_state(swap_id, state::Swap::Bob(state.clone().into())) diff --git a/swap/src/lib.rs b/swap/src/lib.rs index b71d005f..7b95a0d4 100644 --- a/swap/src/lib.rs +++ b/swap/src/lib.rs @@ -15,6 +15,8 @@ pub mod trace; pub type Never = std::convert::Infallible; +const TRANSACTION_ALREADY_IN_BLOCKCHAIN_ERROR_CODE: i64 = -27; + /// Commands sent from Bob to the main task. #[derive(Clone, Copy, Debug)] pub enum Cmd { diff --git a/xmr-btc/src/alice.rs b/xmr-btc/src/alice.rs index 74557f5d..3ac3cc24 100644 --- a/xmr-btc/src/alice.rs +++ b/xmr-btc/src/alice.rs @@ -28,7 +28,8 @@ use std::{ use tokio::{sync::Mutex, time::timeout}; use tracing::{error, info}; pub mod message; -use crate::bitcoin::{BlockHeight, TransactionBlockHeight}; +use crate::bitcoin::{BlockHeight, TransactionBlockHeight, TxCancel}; +use ::bitcoin::Txid; pub use message::{Message, Message0, Message1, Message2}; #[derive(Debug)] @@ -710,6 +711,25 @@ impl State3 { (false, false) => Ok(Epoch::T2), } } + + pub fn tx_cancel(&self) -> TxCancel { + crate::bitcoin::TxCancel::new(&self.tx_lock, self.refund_timelock, self.a.public(), self.B) + } + + pub async fn submit_tx_cancel(&self, bitcoin_wallet: &W) -> Result + where + W: BroadcastSignedTransaction, + { + crate::bitcoin::publish_cancel_transaction( + self.tx_lock.clone(), + self.a.clone(), + self.B, + self.refund_timelock, + self.tx_punish_sig_bob.clone(), + bitcoin_wallet, + ) + .await + } } #[derive(Clone, Debug, Deserialize, Serialize)] diff --git a/xmr-btc/src/bitcoin.rs b/xmr-btc/src/bitcoin.rs index 562b262f..1c959c6a 100644 --- a/xmr-btc/src/bitcoin.rs +++ b/xmr-btc/src/bitcoin.rs @@ -243,3 +243,37 @@ where tokio::time::delay_for(std::time::Duration::from_secs(1)).await; } } + +pub async fn publish_cancel_transaction( + tx_lock: TxLock, + secret_key: crate::bitcoin::SecretKey, + public_key: crate::bitcoin::PublicKey, + refund_timelock: u32, + tx_cancel_sig: crate::bitcoin::Signature, + bitcoin_wallet: &W, +) -> Result +where + W: BroadcastSignedTransaction, +{ + let tx_cancel = + crate::bitcoin::TxCancel::new(&tx_lock, refund_timelock, public_key, secret_key.public()); + + let sig_theirs = tx_cancel_sig.clone(); + let sig_ours = secret_key.sign(tx_cancel.digest()); + + let tx_cancel_txn = tx_cancel + .clone() + .add_signatures( + &tx_lock, + (public_key, sig_theirs), + (secret_key.public(), sig_ours), + ) + .expect( + "sig_{a,b} to be valid signatures for + tx_cancel", + ); + + bitcoin_wallet + .broadcast_signed_transaction(tx_cancel_txn) + .await +} diff --git a/xmr-btc/src/bob.rs b/xmr-btc/src/bob.rs index 61c072ab..f57576fc 100644 --- a/xmr-btc/src/bob.rs +++ b/xmr-btc/src/bob.rs @@ -34,10 +34,10 @@ use tracing::error; pub mod message; use crate::{ - bitcoin::{BlockHeight, GetRawTransaction, Network, TransactionBlockHeight}, + bitcoin::{BlockHeight, Network, TransactionBlockHeight}, monero::{CreateWalletForOutput, WatchForTransfer}, }; -use ::bitcoin::{Transaction, Txid}; +use ::bitcoin::Txid; pub use message::{Message, Message0, Message1, Message2, Message3}; #[allow(clippy::large_enum_variant)] @@ -660,51 +660,19 @@ impl State4 { self.b.encsign(self.S_a_bitcoin, tx_redeem.digest()) } - pub async fn check_for_tx_cancel(&self, bitcoin_wallet: &W) -> Result - where - W: GetRawTransaction, - { - let tx_cancel = - bitcoin::TxCancel::new(&self.tx_lock, self.refund_timelock, self.A, self.b.public()); - - let sig_a = self.tx_cancel_sig_a.clone(); - let sig_b = self.b.sign(tx_cancel.digest()); - - let tx_cancel = tx_cancel - .clone() - .add_signatures(&self.tx_lock, (self.A, sig_a), (self.b.public(), sig_b)) - .expect( - "sig_{a,b} to be valid signatures for - tx_cancel", - ); - - let tx = bitcoin_wallet.get_raw_transaction(tx_cancel.txid()).await?; - - Ok(tx) - } - pub async fn submit_tx_cancel(&self, bitcoin_wallet: &W) -> Result where W: BroadcastSignedTransaction, { - let tx_cancel = - bitcoin::TxCancel::new(&self.tx_lock, self.refund_timelock, self.A, self.b.public()); - - let sig_a = self.tx_cancel_sig_a.clone(); - let sig_b = self.b.sign(tx_cancel.digest()); - - let tx_cancel = tx_cancel - .clone() - .add_signatures(&self.tx_lock, (self.A, sig_a), (self.b.public(), sig_b)) - .expect( - "sig_{a,b} to be valid signatures for - tx_cancel", - ); - - let tx_id = bitcoin_wallet - .broadcast_signed_transaction(tx_cancel) - .await?; - Ok(tx_id) + crate::bitcoin::publish_cancel_transaction( + self.tx_lock.clone(), + self.b.clone(), + self.A, + self.refund_timelock, + self.tx_cancel_sig_a.clone(), + bitcoin_wallet, + ) + .await } pub async fn watch_for_redeem_btc(&self, bitcoin_wallet: &W) -> Result