Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(legacy-swap): wait for confirmation of taker spend maker payment #2206

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions mm2src/coins/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2257,7 +2257,7 @@ impl MarketCoinOps for EthCoin {
}
}

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not u8 or u16?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too keep consistence with the field pub confirmations: u64,

macro_rules! update_status_with_error {
($status: ident, $error: ident) => {
match $error.get_inner() {
Expand Down Expand Up @@ -2328,7 +2328,8 @@ impl MarketCoinOps for EthCoin {
Ok(conf) => {
if conf == confirmed_at {
status.append(" Confirmed.");
break Ok(());
// Let's avoid risking additional errors by calling `current_block` and instead return required_confirms
break Ok(required_confirms.as_u64());
}
},
Err(e) => {
Expand Down
6 changes: 3 additions & 3 deletions mm2src/coins/lightning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ impl MarketCoinOps for LightningCoin {

// Todo: Add waiting for confirmations logic for the case of if the channel is closed and the htlc can be claimed on-chain
// Todo: The above is postponed and might not be needed after this issue is resolved https://github.com/lightningdevkit/rust-lightning/issues/2017
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
let payment_hash = try_f!(payment_hash_from_slice(&input.payment_tx).map_err(|e| e.to_string()));
let payment_hex = hex::encode(payment_hash.0);

Expand All @@ -1139,7 +1139,7 @@ impl MarketCoinOps for LightningCoin {
Ok(Some(payment)) => {
match payment.payment_type {
PaymentType::OutboundPayment { .. } => match payment.status {
HTLCStatus::Pending | HTLCStatus::Succeeded => return Ok(()),
HTLCStatus::Pending | HTLCStatus::Succeeded => return Ok(1u64),
HTLCStatus::Claimable => {
return ERR!(
"Payment {} has an invalid status of {} in the db",
Expand All @@ -1153,7 +1153,7 @@ impl MarketCoinOps for LightningCoin {
HTLCStatus::Failed => return ERR!("Lightning swap payment {} failed", payment_hex),
},
PaymentType::InboundPayment => match payment.status {
HTLCStatus::Claimable | HTLCStatus::Succeeded => return Ok(()),
HTLCStatus::Claimable | HTLCStatus::Succeeded => return Ok(1u64),
HTLCStatus::Pending => info!("Payment {} not received yet!", payment_hex),
HTLCStatus::Failed => return ERR!("Lightning swap payment {} failed", payment_hex),
},
Expand Down
3 changes: 2 additions & 1 deletion mm2src/coins/lp_coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2085,7 +2085,8 @@ pub trait MarketCoinOps {
/// Signs raw utxo transaction in hexadecimal format as input and returns signed transaction in hexadecimal format
async fn sign_raw_tx(&self, args: &SignRawTransactionRequest) -> RawTransactionResult;

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send>;
/// Wait for confirmations and return confirmation number
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send>;

fn wait_for_htlc_tx_spend(&self, args: WaitForHTLCTxSpendArgs<'_>) -> TransactionFut;

Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/qrc20.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1250,7 +1250,7 @@ impl MarketCoinOps for Qrc20Coin {
utxo_common::sign_raw_tx(self, args).await
}

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
let tx: UtxoTx = try_fus!(deserialize(input.payment_tx.as_slice()).map_err(|e| ERRL!("{:?}", e)));
let selfi = self.clone();
let fut = async move {
Expand Down
6 changes: 3 additions & 3 deletions mm2src/coins/qrc20/swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,9 +374,9 @@ impl Qrc20Coin {
requires_nota: bool,
wait_until: u64,
check_every: u64,
) -> Result<(), String> {
) -> Result<u64, String> {
let tx_hash = H256Json::from(qtum_tx.hash().reversed());
try_s!(
let confirmations = try_s!(
self.utxo
.rpc_client
.wait_for_confirmations(
Expand Down Expand Up @@ -415,7 +415,7 @@ impl Qrc20Coin {
try_s!(check_if_contract_call_completed(&receipt));
}

Ok(())
Ok(confirmations)
}

/// Generate `ContractCallOutput` outputs required to send a swap payment.
Expand Down
5 changes: 4 additions & 1 deletion mm2src/coins/siacoin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,10 @@ impl MarketCoinOps for SiaCoin {
#[inline(always)]
async fn sign_raw_tx(&self, _args: &SignRawTransactionRequest) -> RawTransactionResult { unimplemented!() }

fn wait_for_confirmations(&self, _input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(
&self,
_input: ConfirmPaymentInput,
) -> Box<dyn Future<Item = u64, Error = String> + Send> {
unimplemented!()
}

Expand Down
5 changes: 4 additions & 1 deletion mm2src/coins/solana.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,10 @@ impl MarketCoinOps for SolanaCoin {
})
}

fn wait_for_confirmations(&self, _input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(
&self,
_input: ConfirmPaymentInput,
) -> Box<dyn Future<Item = u64, Error = String> + Send> {
unimplemented!()
}

Expand Down
5 changes: 4 additions & 1 deletion mm2src/coins/solana/spl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,10 @@ impl MarketCoinOps for SplToken {
})
}

fn wait_for_confirmations(&self, _input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(
&self,
_input: ConfirmPaymentInput,
) -> Box<dyn Future<Item = u64, Error = String> + Send> {
unimplemented!()
}

Expand Down
5 changes: 3 additions & 2 deletions mm2src/coins/tendermint/tendermint_coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2560,7 +2560,7 @@ impl MarketCoinOps for TendermintCoin {
})
}

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
// Sanity check
let _: TxRaw = try_fus!(Message::decode(input.payment_tx.as_slice()));

Expand All @@ -2581,7 +2581,8 @@ impl MarketCoinOps for TendermintCoin {

if let Some(tx_status_code) = tx_status_code {
return match tx_status_code {
cosmrs::tendermint::abci::Code::Ok => Ok(()),
// Tendermint uses Byzantine Fault Tolerance (BFT), achieve instant finality once a block is committed.
cosmrs::tendermint::abci::Code::Ok => Ok(1u64),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better this function wait_for_confirmations return Optional u8 or 16 and we can return None for tendermint. cc @onur-ozkan

cosmrs::tendermint::abci::Code::Err(err_code) => Err(format!(
"Got error code: '{}' for tx: '{}'. Broadcasted tx isn't valid.",
err_code, tx_hash
Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/tendermint/tendermint_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl MarketCoinOps for TendermintToken {
})
}

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
self.platform_coin.wait_for_confirmations(input)
}

Expand Down
5 changes: 4 additions & 1 deletion mm2src/coins/test_coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ impl MarketCoinOps for TestCoin {
#[inline(always)]
async fn sign_raw_tx(&self, _args: &SignRawTransactionRequest) -> RawTransactionResult { unimplemented!() }

fn wait_for_confirmations(&self, _input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(
&self,
_input: ConfirmPaymentInput,
) -> Box<dyn Future<Item = u64, Error = String> + Send> {
unimplemented!()
}

Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/utxo/bch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1225,7 +1225,7 @@ impl MarketCoinOps for BchCoin {
utxo_common::sign_raw_tx(self, args).await
}

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
utxo_common::wait_for_confirmations(&self.utxo_arc, input)
}

Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/utxo/qtum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ impl MarketCoinOps for QtumCoin {
utxo_common::sign_raw_tx(self, args).await
}

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
utxo_common::wait_for_confirmations(&self.utxo_arc, input)
}

Expand Down
4 changes: 2 additions & 2 deletions mm2src/coins/utxo/rpc_clients.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl UtxoRpcClientEnum {
requires_notarization: bool,
wait_until: u64,
check_every: u64,
) -> Box<dyn Future<Item = (), Error = String> + Send> {
) -> Box<dyn Future<Item = u64, Error = String> + Send> {
let selfi = self.clone();
let mut tx_not_found_retries = TX_NOT_FOUND_RETRIES;
let fut = async move {
Expand All @@ -185,7 +185,7 @@ impl UtxoRpcClientEnum {
t.rawconfirmations.unwrap_or(t.confirmations)
};
if tx_confirmations >= confirmations {
return Ok(());
return Ok(tx_confirmations as u64);
} else {
info!(
"Waiting for tx {:?} confirmations, now {}, required {}, requires_notarization {}",
Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/utxo/slp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,7 @@ impl MarketCoinOps for SlpToken {
utxo_common::sign_raw_tx(self, args).await
}

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
self.platform_coin.wait_for_confirmations(input)
}

Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/utxo/utxo_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2844,7 +2844,7 @@ async fn sign_raw_utxo_tx<T: AsRef<UtxoCoinFields> + UtxoTxGenerationOps>(
pub fn wait_for_confirmations(
coin: &UtxoCoinFields,
input: ConfirmPaymentInput,
) -> Box<dyn Future<Item = (), Error = String> + Send> {
) -> Box<dyn Future<Item = u64, Error = String> + Send> {
let mut tx: UtxoTx = try_fus!(deserialize(input.payment_tx.as_slice()).map_err(|e| ERRL!("{:?}", e)));
tx.tx_hash_algo = coin.tx_hash_algo;
coin.rpc_client.wait_for_confirmations(
Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/utxo/utxo_standard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ impl MarketCoinOps for UtxoStandardCoin {
utxo_common::sign_raw_tx(self, args).await
}

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
utxo_common::wait_for_confirmations(&self.utxo_arc, input)
}

Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/z_coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ impl MarketCoinOps for ZCoin {
utxo_common::sign_raw_tx(self, args).await
}

fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = (), Error = String> + Send> {
fn wait_for_confirmations(&self, input: ConfirmPaymentInput) -> Box<dyn Future<Item = u64, Error = String> + Send> {
utxo_common::wait_for_confirmations(self.as_ref(), input)
}

Expand Down
24 changes: 24 additions & 0 deletions mm2src/mm2_main/src/lp_swap/taker_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1824,6 +1824,30 @@ impl TakerSwap {

let tx_hash = transaction.tx_hash_as_bytes();
info!("Maker payment spend tx {:02x}", tx_hash);

// we should wait for only one confirmation to make sure spend maker payment transaction is not failed
let confirm_maker_payment_spend_input = ConfirmPaymentInput {
payment_tx: transaction.tx_hex(),
confirmations: std::cmp::min(1, self.r().data.maker_payment_confirmations),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to involve 'maker_payment_confirmations' if we always want exactly 1 conf?
(And what would be if maker_payment_confirmations set to 0? Like if someone wants to run the whole swap with no confs at all?)

requires_nota: self.r().data.maker_payment_requires_nota.unwrap_or(false),
wait_until: self.wait_refund_until(),
check_every: WAIT_CONFIRM_INTERVAL_SEC,
};
let wait_fut = self
.maker_coin
.wait_for_confirmations(confirm_maker_payment_spend_input);
let confirmations = match wait_fut.compat().await {
Ok(conf) => conf,
Err(err) => {
return Ok((Some(TakerSwapCommand::Finish), vec![
TakerSwapEvent::MakerPaymentSpendFailed(
ERRL!("!wait for maker payment spend confirmations: {}", err).into(),
),
]));
},
};
info!("Maker payment spend confirmed. Confirmation number: {}", confirmations);

let tx_ident = TransactionIdentifier {
tx_hex: BytesJson::from(transaction.tx_hex()),
tx_hash,
Expand Down
Loading