From 15f7932f7f52ffc042136f867d0a051797a19fc8 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Thu, 15 Oct 2020 13:10:31 +1100 Subject: [PATCH] Replace monero::CheckTransfer with monero::WatchForTransfer Instead of checking once to see if Monero's `TxLock` has been published, the new trait should keep looking until the transaction has been found. The new trait also allows the caller to set an expected number of confirmations on the transaction. The implementation of the trait is currently part of test code, but it should be similar to what we will eventually do for an application. --- xmr-btc/Cargo.toml | 1 + xmr-btc/src/bob.rs | 14 +++-- xmr-btc/src/lib.rs | 15 ++++-- xmr-btc/src/monero.rs | 21 +++++--- xmr-btc/tests/harness/wallet/monero.rs | 72 +++++++++++++++++--------- 5 files changed, 84 insertions(+), 39 deletions(-) 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<()> { - let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into()); - - let cli = self.0.bob_wallet_rpc_client(); + expected_amount: Amount, + expected_confirmations: u32, + ) -> Result<(), InsufficientFunds> { + enum Error { + TxNotFound, + InsufficientConfirmations, + InsufficientFunds { expected: Amount, actual: Amount }, + } - let res = cli - .check_tx_key( - &String::from(transfer_proof.tx_hash()), - &transfer_proof.tx_key().to_string(), - &address.to_string(), - ) - .await?; + let wallet = self.0.bob_wallet_rpc_client(); + let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into()); - if res.received != u64::from(amount) { - bail!( - "tx_lock doesn't pay enough: expected {:?}, got {:?}", - res.received, - amount - ) - } + 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))?; + + if proof.received != expected_amount.as_piconero() { + return Err(backoff::Error::Permanent(Error::InsufficientFunds { + expected: expected_amount, + actual: Amount::from_piconero(proof.received), + })); + } + + 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(()) }