diff --git a/xmr-btc/Cargo.toml b/xmr-btc/Cargo.toml index 35475ab1..d82790f5 100644 --- a/xmr-btc/Cargo.toml +++ b/xmr-btc/Cargo.toml @@ -21,6 +21,7 @@ thiserror = "1" tracing = "0.1" [dev-dependencies] +backoff = { version = "0.2", features = ["tokio"] } base64 = "0.12" bitcoin-harness = { git = "https://github.com/coblox/bitcoin-harness-rs", rev = "d402b36d3d6406150e3bfb71492ff4a0a7cb290e" } futures = "0.3" diff --git a/xmr-btc/src/bob.rs b/xmr-btc/src/bob.rs index 75c13bd3..2d4105d7 100644 --- a/xmr-btc/src/bob.rs +++ b/xmr-btc/src/bob.rs @@ -5,7 +5,7 @@ use crate::{ WatchForRawTransaction, }, monero, - monero::{CheckTransfer, CreateWalletForOutput}, + monero::{CreateWalletForOutput, WatchForTransfer}, transport::{ReceiveMessage, SendMessage}, }; use anyhow::{anyhow, Result}; @@ -27,7 +27,7 @@ pub use message::{Message, Message0, Message1, Message2, Message3}; pub async fn next_state< R: RngCore + CryptoRng, B: WatchForRawTransaction + SignTxLock + BuildTxLockPsbt + BroadcastSignedTransaction, - M: CreateWalletForOutput + CheckTransfer, + M: CreateWalletForOutput + WatchForTransfer, T: SendMessage + ReceiveMessage, >( bitcoin_wallet: &B, @@ -347,7 +347,7 @@ pub struct State3 { impl State3 { pub async fn watch_for_lock_xmr(self, xmr_wallet: &W, msg: alice::Message2) -> Result where - W: monero::CheckTransfer, + W: monero::WatchForTransfer, { let S_b_monero = monero::PublicKey::from_private_key(&monero::PrivateKey::from_scalar( self.s_b.into_ed25519(), @@ -355,7 +355,13 @@ impl State3 { let S = self.S_a_monero + S_b_monero; xmr_wallet - .check_transfer(S, self.v.public(), msg.tx_lock_proof, self.xmr) + .watch_for_transfer( + S, + self.v.public(), + msg.tx_lock_proof, + self.xmr, + monero::MIN_CONFIRMATIONS, + ) .await?; Ok(State4 { diff --git a/xmr-btc/src/lib.rs b/xmr-btc/src/lib.rs index a26269a9..f69f1a7a 100644 --- a/xmr-btc/src/lib.rs +++ b/xmr-btc/src/lib.rs @@ -103,11 +103,15 @@ pub fn action_generator_bob( ) -> GenBoxed where N: ReceiveTransferProof + Send + Sync, - M: monero::CheckTransfer + Send + Sync, + M: monero::WatchForTransfer + Send + Sync, B: bitcoin::WatchForRawTransaction + Send + Sync, { + enum SwapFailedRefund { + InsufficientXMR(monero::InsufficientFunds), + } + Gen::new_boxed(|co| async move { - let swap_result: Result<(), ()> = { + let swap_result: Result<(), SwapFailedRefund> = async { co.yield_(Action::LockBitcoin(tx_lock.clone())).await; // the source of this could be the database, this layer doesn't care @@ -121,9 +125,9 @@ where // TODO: We should require a specific number of confirmations on the lock // transaction monero_ledger - .check_transfer(S, v.public(), transfer_proof, xmr) + .watch_for_transfer(S, v.public(), transfer_proof, xmr, 10) .await - .expect("TODO: implementor of this trait must make it infallible by retrying"); + .map_err(|e| SwapFailedRefund::InsufficientXMR(e))?; let tx_redeem = bitcoin::TxRedeem::new(&tx_lock, &redeem_address); let tx_redeem_encsig = b.encsign(S_a_bitcoin.clone(), tx_redeem.digest()); @@ -159,7 +163,8 @@ where .await; Ok(()) - }; + } + .await; // NOTE: swap result should only be `Err` if we have reached the // `refund_timelock`. Therefore, we should always yield the refund action diff --git a/xmr-btc/src/monero.rs b/xmr-btc/src/monero.rs index 459fa708..241666c7 100644 --- a/xmr-btc/src/monero.rs +++ b/xmr-btc/src/monero.rs @@ -1,10 +1,11 @@ -use anyhow::Result; use async_trait::async_trait; pub use curve25519_dalek::scalar::Scalar; pub use monero::{Address, PrivateKey, PublicKey}; use rand::{CryptoRng, RngCore}; use std::ops::Add; +pub const MIN_CONFIRMATIONS: u32 = 10; + pub fn random_private_key(rng: &mut R) -> PrivateKey { let scalar = Scalar::random(rng); @@ -107,18 +108,26 @@ pub trait Transfer { public_spend_key: PublicKey, public_view_key: PublicViewKey, amount: Amount, - ) -> Result<(TransferProof, Amount)>; + ) -> anyhow::Result<(TransferProof, Amount)>; } #[async_trait] -pub trait CheckTransfer { - async fn check_transfer( +pub trait WatchForTransfer { + async fn watch_for_transfer( &self, public_spend_key: PublicKey, public_view_key: PublicViewKey, transfer_proof: TransferProof, amount: Amount, - ) -> Result<()>; + expected_confirmations: u32, + ) -> Result<(), InsufficientFunds>; +} + +#[derive(Debug, Clone, Copy, thiserror::Error)] +#[error("transaction does not pay enough: expected {expected:?}, got {actual:?}")] +pub struct InsufficientFunds { + pub expected: Amount, + pub actual: Amount, } #[async_trait] @@ -127,5 +136,5 @@ pub trait CreateWalletForOutput { &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, - ) -> Result<()>; + ) -> anyhow::Result<()>; } diff --git a/xmr-btc/tests/harness/wallet/monero.rs b/xmr-btc/tests/harness/wallet/monero.rs index ca5e9039..9faf7dd8 100644 --- a/xmr-btc/tests/harness/wallet/monero.rs +++ b/xmr-btc/tests/harness/wallet/monero.rs @@ -1,11 +1,12 @@ -use anyhow::{bail, Result}; +use anyhow::Result; use async_trait::async_trait; +use backoff::{future::FutureOperation as _, ExponentialBackoff}; use monero::{Address, Network, PrivateKey}; use monero_harness::Monero; use std::str::FromStr; use xmr_btc::monero::{ - Amount, CheckTransfer, CreateWalletForOutput, PrivateViewKey, PublicKey, PublicViewKey, - Transfer, TransferProof, TxHash, + Amount, CreateWalletForOutput, InsufficientFunds, PrivateViewKey, PublicKey, PublicViewKey, + Transfer, TransferProof, TxHash, WatchForTransfer, }; #[derive(Debug)] @@ -66,33 +67,56 @@ impl CreateWalletForOutput for AliceWallet<'_> { pub struct BobWallet<'c>(pub &'c Monero<'c>); #[async_trait] -impl CheckTransfer for BobWallet<'_> { - async fn check_transfer( +impl WatchForTransfer for BobWallet<'_> { + async fn watch_for_transfer( &self, public_spend_key: PublicKey, public_view_key: PublicViewKey, transfer_proof: TransferProof, - amount: Amount, - ) -> Result<()> { + expected_amount: Amount, + expected_confirmations: u32, + ) -> Result<(), InsufficientFunds> { + enum Error { + TxNotFound, + InsufficientConfirmations, + InsufficientFunds { expected: Amount, actual: Amount }, + } + + let wallet = self.0.bob_wallet_rpc_client(); let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into()); - let cli = self.0.bob_wallet_rpc_client(); + let res = (|| async { + // NOTE: Currently, this is conflating IO errors with the transaction not being + // in the blockchain yet, or not having enough confirmations on it. All these + // errors warrant a retry, but the strategy should probably differ per case + let proof = wallet + .check_tx_key( + &String::from(transfer_proof.tx_hash()), + &transfer_proof.tx_key().to_string(), + &address.to_string(), + ) + .await + .map_err(|_| backoff::Error::Transient(Error::TxNotFound))?; - let res = cli - .check_tx_key( - &String::from(transfer_proof.tx_hash()), - &transfer_proof.tx_key().to_string(), - &address.to_string(), - ) - .await?; + if proof.received != expected_amount.as_piconero() { + return Err(backoff::Error::Permanent(Error::InsufficientFunds { + expected: expected_amount, + actual: Amount::from_piconero(proof.received), + })); + } - if res.received != u64::from(amount) { - bail!( - "tx_lock doesn't pay enough: expected {:?}, got {:?}", - res.received, - amount - ) - } + if proof.confirmations < expected_confirmations { + return Err(backoff::Error::Transient(Error::InsufficientConfirmations)); + } + + Ok(proof) + }) + .retry(ExponentialBackoff::default()) + .await; + + if let Err(Error::InsufficientFunds { expected, actual }) = res { + return Err(InsufficientFunds { expected, actual }); + }; Ok(()) }