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

@@ -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"

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<Message> + ReceiveMessage<alice::Message>,
 >(
     bitcoin_wallet: &B,
@@ -347,7 +347,7 @@ pub struct State3 {
 impl State3 {
     pub async fn watch_for_lock_xmr<W>(self, xmr_wallet: &W, msg: alice::Message2) -> Result<State4>
     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 {

xmr-btc/src/lib.rs

@@ -103,11 +103,15 @@ pub fn action_generator_bob<N, M, B>(
 ) -> GenBoxed<Action, (), ()>
 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

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<R: RngCore + CryptoRng>(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 {
+pub trait WatchForTransfer {
-    async fn check_transfer(
+    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<()>;
 }

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,
+    Amount, CreateWalletForOutput, InsufficientFunds, PrivateViewKey, PublicKey, PublicViewKey,
-    Transfer, TransferProof, TxHash,
+    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<'_> {
+impl WatchForTransfer for BobWallet<'_> {
-    async fn check_transfer(
+    async fn watch_for_transfer(
         &self,
         public_spend_key: PublicKey,
         public_view_key: PublicViewKey,
         transfer_proof: TransferProof,
-        amount: Amount,
+        expected_amount: Amount,
-    ) -> Result<()> {
+        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
+            if proof.received != expected_amount.as_piconero() {
-            .check_tx_key(
+                return Err(backoff::Error::Permanent(Error::InsufficientFunds {
-                &String::from(transfer_proof.tx_hash()),
+                    expected: expected_amount,
-                &transfer_proof.tx_key().to_string(),
+                    actual: Amount::from_piconero(proof.received),
-                &address.to_string(),
+                }));
-            )
+            }
-            .await?;
 
-        if res.received != u64::from(amount) {
+            if proof.confirmations < expected_confirmations {
-            bail!(
+                return Err(backoff::Error::Transient(Error::InsufficientConfirmations));
-                "tx_lock doesn't pay enough: expected {:?}, got {:?}",
+            }
-                res.received,
+
-                amount
+            Ok(proof)
-            )
+        })
-        }
+        .retry(ExponentialBackoff::default())
+        .await;
+
+        if let Err(Error::InsufficientFunds { expected, actual }) = res {
+            return Err(InsufficientFunds { expected, actual });
+        };
 
         Ok(())
     }