170: Cancel and refund commands r=da-kami a=da-kami

I plugged the cancel and refund logic into the current state/state-machine logic of the swap.

## Follow ups  (out of scope)

We might want to record issues to be tackled later, since we are on a tight time budget :)
Please let me know what you think @D4nte @rishflab 

### Problems with `ack` after sending a message

Alice was waiting forever when awaiting the `ack` from bob when sending the lock proof in case she runs into a dial error. It seems the `acks` can cause the program to hang. This is a severe problem that we most probably will encountered in production at some point. For this PR I wrapped the `ack` of Alice upon sending the `encsig` in a timeout to work around this problem, see 7463081f88 - but **we might want to consider to remove all `ack` message. I don't see much value in them if we don't have a resilient retry strategy.**

### Do not require Monero wallet for cancel/refund

The cancel/refund commands don't require a monero wallet.
In this PR we re-uses the builder which requires the monero wallet as well - and we check for the monero balance upon wallet initialization, so the command will fail if no monero wallet is started.

### Save Alice connection info in Bob DB

Save Alice's peer-id/address in DB: It's cumbersome for the user to lookup those details again.




Co-authored-by: Daniel Karzel <daniel@comit.network>
This commit is contained in:
bors[bot] 2021-02-09 02:40:47 +00:00 committed by GitHub
commit 27bb9498d5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 678 additions and 265 deletions

View File

@ -134,6 +134,9 @@ jobs:
punish,
refund_restart_alice_cancelled,
refund_restart_alice,
bob_refunds_using_cancel_and_refund_command,
bob_refunds_using_cancel_and_refund_command_timelock_not_exired,
bob_refunds_using_cancel_and_refund_command_timelock_not_exired_force,
]
runs-on: ubuntu-latest
steps:

430
Cargo.lock generated

File diff suppressed because it is too large Load Diff

View File

@ -50,6 +50,8 @@ pub enum Command {
},
History,
Resume(Resume),
Cancel(Cancel),
Refund(Refund),
}
#[derive(structopt::StructOpt, Debug)]
@ -79,6 +81,48 @@ pub enum Resume {
},
}
#[derive(structopt::StructOpt, Debug)]
pub enum Cancel {
BuyXmr {
#[structopt(long = "swap-id")]
swap_id: Uuid,
// TODO: Remove Alice peer-id/address, it should be saved in the database when running swap
// and loaded from the database when running resume/cancel/refund
#[structopt(long = "counterpart-peer-id")]
alice_peer_id: PeerId,
#[structopt(long = "counterpart-addr")]
alice_addr: Multiaddr,
#[structopt(flatten)]
config: Config,
#[structopt(short, long)]
force: bool,
},
}
#[derive(structopt::StructOpt, Debug)]
pub enum Refund {
BuyXmr {
#[structopt(long = "swap-id")]
swap_id: Uuid,
// TODO: Remove Alice peer-id/address, it should be saved in the database when running swap
// and loaded from the database when running resume/cancel/refund
#[structopt(long = "counterpart-peer-id")]
alice_peer_id: PeerId,
#[structopt(long = "counterpart-addr")]
alice_addr: Multiaddr,
#[structopt(flatten)]
config: Config,
#[structopt(short, long)]
force: bool,
},
}
#[derive(structopt::StructOpt, Debug)]
pub struct Config {
#[structopt(

View File

@ -13,11 +13,12 @@
#![allow(non_snake_case)]
use crate::{
cli::{Command, Options, Resume},
cli::{Cancel, Command, Options, Refund, Resume},
config::{
initial_setup, query_user_for_initial_testnet_config, read_config, ConfigNotInitialized,
},
execution_params::GetExecutionParams,
protocol::bob::cancel::CancelError,
};
use anyhow::{Context, Result};
use database::Database;
@ -28,7 +29,7 @@ use protocol::{alice, bob, bob::Builder, SwapAmounts};
use std::{path::PathBuf, sync::Arc};
use structopt::StructOpt;
use trace::init_tracing;
use tracing::info;
use tracing::{error, info, warn};
use uuid::Uuid;
pub mod bitcoin;
@ -209,6 +210,86 @@ async fn main() -> Result<()> {
tokio::spawn(async move { event_loop.run().await });
bob::run(swap).await?;
}
Command::Cancel(Cancel::BuyXmr {
swap_id,
alice_peer_id,
alice_addr,
config,
force,
}) => {
// TODO: Optimization: Only init the Bitcoin wallet, Monero wallet unnecessary
let (bitcoin_wallet, monero_wallet) =
init_wallets(config.path, bitcoin_network, monero_network).await?;
let bob_factory = Builder::new(
seed,
db_path,
swap_id,
Arc::new(bitcoin_wallet),
Arc::new(monero_wallet),
alice_addr,
alice_peer_id,
execution_params,
);
let (swap, event_loop) = bob_factory.build().await?;
tokio::spawn(async move { event_loop.run().await });
match bob::cancel(
swap.swap_id,
swap.state,
swap.bitcoin_wallet,
swap.db,
force,
)
.await?
{
Ok((txid, _)) => {
info!("Cancel transaction successfully published with id {}", txid)
}
Err(CancelError::CancelTimelockNotExpiredYet) => error!(
"The Cancel Transaction cannot be published yet, \
because the timelock has not expired. Please try again later."
),
Err(CancelError::CancelTxAlreadyPublished) => {
warn!("The Cancel Transaction has already been published.")
}
}
}
Command::Refund(Refund::BuyXmr {
swap_id,
alice_peer_id,
alice_addr,
config,
force,
}) => {
let (bitcoin_wallet, monero_wallet) =
init_wallets(config.path, bitcoin_network, monero_network).await?;
// TODO: Optimize to only use the Bitcoin wallet, Monero wallet is unnecessary
let bob_factory = Builder::new(
seed,
db_path,
swap_id,
Arc::new(bitcoin_wallet),
Arc::new(monero_wallet),
alice_addr,
alice_peer_id,
execution_params,
);
let (swap, event_loop) = bob_factory.build().await?;
tokio::spawn(async move { event_loop.run().await });
bob::refund(
swap.swap_id,
swap.state,
swap.execution_params,
swap.bitcoin_wallet,
swap.db,
force,
)
.await??;
}
};
Ok(())

View File

@ -1,4 +1,5 @@
use crate::{
execution_params::ExecutionParams,
network::{transport::SwapTransport, TokioExecutor},
protocol::{
alice::{Behaviour, OutEvent, State0, State3, SwapResponse, TransferProof},
@ -9,7 +10,10 @@ use anyhow::{anyhow, Context, Result};
use libp2p::{
core::Multiaddr, futures::FutureExt, request_response::ResponseChannel, PeerId, Swarm,
};
use tokio::sync::mpsc::{Receiver, Sender};
use tokio::{
sync::mpsc::{Receiver, Sender},
time::timeout,
};
use tracing::{error, trace};
#[allow(missing_debug_implementations)]
@ -91,13 +95,27 @@ impl EventLoopHandle {
Ok(())
}
pub async fn send_transfer_proof(&mut self, bob: PeerId, msg: TransferProof) -> Result<()> {
pub async fn send_transfer_proof(
&mut self,
bob: PeerId,
msg: TransferProof,
execution_params: ExecutionParams,
) -> Result<()> {
let _ = self.send_transfer_proof.send((bob, msg)).await?;
self.recv_transfer_proof_ack
.recv()
.await
.ok_or_else(|| anyhow!("Failed to receive transfer proof ack from Bob"))?;
// TODO: Re-evaluate if these acknowledges are necessary at all.
// If we don't use a timeout here and Alice fails to dial Bob she will wait
// indefinitely for this acknowledge.
if timeout(
execution_params.bob_time_to_act,
self.recv_transfer_proof_ack.recv(),
)
.await
.is_err()
{
error!("Failed to receive transfer proof ack from Bob")
}
Ok(())
}
}

View File

@ -96,6 +96,7 @@ pub async fn lock_xmr<W>(
state3: alice::State3,
event_loop_handle: &mut EventLoopHandle,
monero_wallet: Arc<W>,
execution_params: ExecutionParams,
) -> Result<()>
where
W: Transfer,
@ -117,9 +118,13 @@ where
// Otherwise Alice might publish the lock tx twice!
event_loop_handle
.send_transfer_proof(bob_peer_id, TransferProof {
tx_lock_proof: transfer_proof,
})
.send_transfer_proof(
bob_peer_id,
TransferProof {
tx_lock_proof: transfer_proof,
},
execution_params,
)
.await?;
Ok(())

View File

@ -165,6 +165,7 @@ async fn run_until_internal(
*state3.clone(),
&mut event_loop_handle,
monero_wallet.clone(),
execution_params,
)
.await?;
@ -416,9 +417,16 @@ async fn run_until_internal(
pin_mut!(punish_tx_finalised);
pin_mut!(refund_tx_seen);
match select(punish_tx_finalised, refund_tx_seen).await {
Either::Left(_) => {
let state = AliceState::BtcPunished;
match select(refund_tx_seen, punish_tx_finalised).await {
Either::Left((published_refund_tx, _)) => {
let spend_key = extract_monero_private_key(
published_refund_tx,
tx_refund,
state3.s_a,
state3.a.clone(),
state3.S_b_bitcoin,
)?;
let state = AliceState::BtcRefunded { spend_key, state3 };
let db_state = (&state).into();
db.insert_latest_state(swap_id, database::Swap::Alice(db_state))
.await?;
@ -434,15 +442,8 @@ async fn run_until_internal(
)
.await
}
Either::Right((published_refund_tx, _)) => {
let spend_key = extract_monero_private_key(
published_refund_tx,
tx_refund,
state3.s_a,
state3.a.clone(),
state3.S_b_bitcoin,
)?;
let state = AliceState::BtcRefunded { spend_key, state3 };
Either::Right(_) => {
let state = AliceState::BtcPunished;
let db_state = (&state).into();
db.insert_latest_state(swap_id, database::Swap::Alice(db_state))
.await?;

View File

@ -20,8 +20,10 @@ use tracing::{debug, info};
use uuid::Uuid;
pub use self::{
cancel::cancel,
encrypted_signature::EncryptedSignature,
event_loop::{EventLoop, EventLoopHandle},
refund::refund,
state::*,
swap::{run, run_until},
swap_request::*,
@ -29,9 +31,11 @@ pub use self::{
pub use execution_setup::{Message0, Message2, Message4};
use libp2p::request_response::ResponseChannel;
pub mod cancel;
mod encrypted_signature;
pub mod event_loop;
mod execution_setup;
pub mod refund;
pub mod state;
pub mod swap;
mod swap_request;

View File

@ -0,0 +1,63 @@
use crate::{
bitcoin::{timelocks::ExpiredTimelocks, Txid, Wallet},
database::{Database, Swap},
protocol::bob::BobState,
};
use anyhow::{bail, Result};
use std::sync::Arc;
use uuid::Uuid;
#[derive(Debug, thiserror::Error, Clone, Copy)]
pub enum CancelError {
#[error("The cancel timelock has not expired yet.")]
CancelTimelockNotExpiredYet,
#[error("The cancel transaction has already been published.")]
CancelTxAlreadyPublished,
}
pub async fn cancel(
swap_id: Uuid,
state: BobState,
bitcoin_wallet: Arc<Wallet>,
db: Database,
force: bool,
) -> Result<Result<(Txid, BobState), CancelError>> {
let state4 = match state {
BobState::BtcLocked(state3) => state3.state4(),
BobState::XmrLockProofReceived { state, .. } => state.state4(),
BobState::XmrLocked(state4) => state4,
BobState::EncSigSent(state4) => state4,
BobState::CancelTimelockExpired(state4) => state4,
_ => bail!(
"Cannot cancel swap {} because it is in state {} which is not refundable.",
swap_id,
state
),
};
if !force {
if let ExpiredTimelocks::None = state4.expired_timelock(bitcoin_wallet.as_ref()).await? {
return Ok(Err(CancelError::CancelTimelockNotExpiredYet));
}
if state4
.check_for_tx_cancel(bitcoin_wallet.as_ref())
.await
.is_ok()
{
let state = BobState::BtcCancelled(state4);
let db_state = state.into();
db.insert_latest_state(swap_id, Swap::Bob(db_state)).await?;
return Ok(Err(CancelError::CancelTxAlreadyPublished));
}
}
let txid = state4.submit_tx_cancel(bitcoin_wallet.as_ref()).await?;
let state = BobState::BtcCancelled(state4);
let db_state = state.clone().into();
db.insert_latest_state(swap_id, Swap::Bob(db_state)).await?;
Ok(Ok((txid, state)))
}

View File

@ -0,0 +1,56 @@
use crate::{
bitcoin::Wallet,
database::{Database, Swap},
execution_params::ExecutionParams,
protocol::bob::BobState,
};
use anyhow::{bail, Result};
use std::sync::Arc;
use uuid::Uuid;
#[derive(thiserror::Error, Debug, Clone, Copy)]
#[error("Cannot refund because swap {0} was not cancelled yet. Make sure to cancel the swap before trying to refund.")]
pub struct SwapNotCancelledYet(Uuid);
pub async fn refund(
swap_id: Uuid,
state: BobState,
execution_params: ExecutionParams,
bitcoin_wallet: Arc<Wallet>,
db: Database,
force: bool,
) -> Result<Result<BobState, SwapNotCancelledYet>> {
let state4 = if force {
match state {
BobState::BtcLocked(state3) => state3.state4(),
BobState::XmrLockProofReceived { state, .. } => state.state4(),
BobState::XmrLocked(state4) => state4,
BobState::EncSigSent(state4) => state4,
BobState::CancelTimelockExpired(state4) => state4,
BobState::BtcCancelled(state4) => state4,
_ => bail!(
"Cannot refund swap {} because it is in state {} which is not refundable.",
swap_id,
state
),
}
} else {
match state {
BobState::BtcCancelled(state4) => state4,
_ => {
return Ok(Err(SwapNotCancelledYet(swap_id)));
}
}
};
state4
.refund_btc(bitcoin_wallet.as_ref(), execution_params)
.await?;
let state = BobState::BtcRefunded(state4);
let db_state = state.clone().into();
db.insert_latest_state(swap_id, Swap::Bob(db_state)).await?;
Ok(Ok(state))
}

View File

@ -0,0 +1,66 @@
pub mod testutils;
use swap::protocol::{alice, bob, bob::BobState};
use testutils::{bob_run_until::is_btc_locked, FastCancelConfig};
#[tokio::test]
async fn given_bob_manually_refunds_after_btc_locked_bob_refunds() {
testutils::setup_test(FastCancelConfig, |mut ctx| async move {
let (alice_swap, _) = ctx.new_swap_as_alice().await;
let (bob_swap, bob_join_handle) = ctx.new_swap_as_bob().await;
let alice_handle = alice::run(alice_swap);
let alice_swap_handle = tokio::spawn(alice_handle);
let bob_state = bob::run_until(bob_swap, is_btc_locked).await.unwrap();
assert!(matches!(bob_state, BobState::BtcLocked { .. }));
let (bob_swap, bob_join_handle) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
// Ensure Bob's timelock is expired
if let BobState::BtcLocked(state3) = bob_swap.state.clone() {
state3
.wait_for_cancel_timelock_to_expire(bob_swap.bitcoin_wallet.as_ref())
.await
.unwrap();
} else {
panic!("Bob in unexpected state {}", bob_swap.state);
}
// Bob manually cancels
let (_, state) = bob::cancel(
bob_swap.swap_id,
bob_swap.state,
bob_swap.bitcoin_wallet,
bob_swap.db,
false,
)
.await
.unwrap()
.unwrap();
assert!(matches!(state, BobState::BtcCancelled { .. }));
let (bob_swap, _) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::BtcCancelled { .. }));
// Bob manually refunds
let bob_state = bob::refund(
bob_swap.swap_id,
bob_swap.state,
bob_swap.execution_params,
bob_swap.bitcoin_wallet,
bob_swap.db,
false,
)
.await
.unwrap()
.unwrap();
ctx.assert_bob_refunded(bob_state).await;
let alice_state = alice_swap_handle.await.unwrap().unwrap();
ctx.assert_alice_refunded(alice_state).await;
})
.await;
}

View File

@ -0,0 +1,58 @@
pub mod testutils;
use bob::cancel::CancelError;
use swap::protocol::{alice, bob, bob::BobState};
use testutils::{bob_run_until::is_btc_locked, SlowCancelConfig};
#[tokio::test]
async fn given_bob_manually_cancels_when_timelock_not_expired_errors() {
testutils::setup_test(SlowCancelConfig, |mut ctx| async move {
let (alice_swap, _) = ctx.new_swap_as_alice().await;
let (bob_swap, bob_join_handle) = ctx.new_swap_as_bob().await;
let alice_handle = alice::run(alice_swap);
tokio::spawn(alice_handle);
let bob_state = bob::run_until(bob_swap, is_btc_locked).await.unwrap();
assert!(matches!(bob_state, BobState::BtcLocked { .. }));
let (bob_swap, bob_join_handle) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::BtcLocked { .. }));
// Bob tries but fails to manually cancel
let result = bob::cancel(
bob_swap.swap_id,
bob_swap.state,
bob_swap.bitcoin_wallet,
bob_swap.db,
false,
)
.await
.unwrap()
.err()
.unwrap();
assert!(matches!(result, CancelError::CancelTimelockNotExpiredYet));
let (bob_swap, bob_join_handle) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::BtcLocked { .. }));
// Bob tries but fails to manually refund
bob::refund(
bob_swap.swap_id,
bob_swap.state,
bob_swap.execution_params,
bob_swap.bitcoin_wallet,
bob_swap.db,
false,
)
.await
.unwrap()
.err()
.unwrap();
let (bob_swap, _) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::BtcLocked { .. }));
})
.await;
}

View File

@ -0,0 +1,54 @@
pub mod testutils;
use swap::protocol::{alice, bob, bob::BobState};
use testutils::{bob_run_until::is_btc_locked, SlowCancelConfig};
#[tokio::test]
async fn given_bob_manually_forces_cancel_when_timelock_not_expired_errors() {
testutils::setup_test(SlowCancelConfig, |mut ctx| async move {
let (alice_swap, _) = ctx.new_swap_as_alice().await;
let (bob_swap, bob_join_handle) = ctx.new_swap_as_bob().await;
let alice_handle = alice::run(alice_swap);
tokio::spawn(alice_handle);
let bob_state = bob::run_until(bob_swap, is_btc_locked).await.unwrap();
assert!(matches!(bob_state, BobState::BtcLocked { .. }));
let (bob_swap, bob_join_handle) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::BtcLocked { .. }));
// Bob forces a cancel that will fail
let is_error = bob::cancel(
bob_swap.swap_id,
bob_swap.state,
bob_swap.bitcoin_wallet,
bob_swap.db,
true,
)
.await
.is_err();
assert!(is_error);
let (bob_swap, bob_join_handle) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::BtcLocked { .. }));
// Bob forces a refund that will fail
let is_error = bob::refund(
bob_swap.swap_id,
bob_swap.state,
bob_swap.execution_params,
bob_swap.bitcoin_wallet,
bob_swap.db,
true,
)
.await
.is_err();
assert!(is_error);
let (bob_swap, _) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::BtcLocked { .. }));
})
.await;
}

View File

@ -16,7 +16,7 @@ async fn given_bob_restarts_after_encsig_is_sent_resume_swap() {
assert!(matches!(bob_state, BobState::EncSigSent { .. }));
let bob_swap = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
let (bob_swap, _) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::EncSigSent { .. }));
let bob_state = bob::run(bob_swap).await.unwrap();

View File

@ -18,7 +18,7 @@ async fn given_bob_restarts_after_lock_proof_received_resume_swap() {
assert!(matches!(bob_state, BobState::XmrLockProofReceived { .. }));
let bob_swap = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
let (bob_swap, _) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(
bob_swap.state,
BobState::XmrLockProofReceived { .. }

View File

@ -16,7 +16,7 @@ async fn given_bob_restarts_after_xmr_is_locked_resume_swap() {
assert!(matches!(bob_state, BobState::XmrLocked { .. }));
let bob_swap = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
let (bob_swap, _) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::XmrLocked { .. }));
let bob_state = bob::run(bob_swap).await.unwrap();

View File

@ -23,7 +23,7 @@ async fn alice_punishes_if_bob_never_acts_after_fund() {
// Restart Bob after Alice punished to ensure Bob transitions to
// punished and does not run indefinitely
let bob_swap = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
let (bob_swap, _) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await;
assert!(matches!(bob_swap.state, BobState::BtcLocked { .. }));
let bob_state = bob::run(bob_swap).await.unwrap();

View File

@ -145,14 +145,14 @@ impl TestContext {
pub async fn stop_and_resume_bob_from_db(
&mut self,
join_handle: BobEventLoopJoinHandle,
) -> bob::Swap {
) -> (bob::Swap, BobEventLoopJoinHandle) {
join_handle.0.abort();
let (swap, event_loop) = self.bob_params.builder().build().await.unwrap();
tokio::spawn(async move { event_loop.run().await });
let join_handle = tokio::spawn(async move { event_loop.run().await });
swap
(swap, BobEventLoopJoinHandle(join_handle))
}
pub async fn assert_alice_redeemed(&self, state: AliceState) {