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): taker failed spend maker payment marked as failed #2199

Open
wants to merge 27 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1461212
WIP: new fn confirm_maker_payment_spend, Taker command and events
laruh Aug 23, 2024
31292be
Merge remote-tracking branch 'origin/dev' into taker-failed-swaps-mar…
laruh Aug 25, 2024
0ede740
update events in recreate_taker_swap_taker_expected.json
laruh Aug 26, 2024
9c6c892
update events in recreate_taker_swap_taker_payment_wait_confirm_faile…
laruh Aug 26, 2024
b716806
update events in for_tests.rs consts, taker_swap.rs mod taker_swap_te…
laruh Aug 26, 2024
5c47ef0
add "MakerPaymentSpendConfirmed" to TAKER_ACTUAL_EVENTS_TAKER_SPENDS_…
laruh Aug 26, 2024
3117c5f
Merge remote-tracking branch 'origin/dev' into taker-failed-swaps-mar…
laruh Sep 5, 2024
eca0bda
provide "MakerPaymentSpendConfirmed" success event in more test jsons
laruh Sep 5, 2024
0e9a807
log events
laruh Sep 5, 2024
eb16235
TakerSwapEvent::MakerPaymentSpentByWatcher(_) => Some(TakerSwapComman…
laruh Sep 5, 2024
1a1a775
in taker_restart.rs provide TakerSwapCommand::ConfirmMakerPaymentSpen…
laruh Sep 5, 2024
3af96a5
move TakerSwapEvent::MakerPaymentSpendConfirmFailed in MakerSavedEvent
laruh Sep 5, 2024
1c3c935
use TakerSwapCommand::SpendMakerPayment => check_maker_payment_spend_…
laruh Sep 5, 2024
7829b4b
return TakerSwapEvent::MakerPaymentSpentByWatcher(_) => Some(TakerSwa…
laruh Sep 5, 2024
075de97
add "MakerPaymentSpendConfirmed" to TAKER_USING_WATCHERS_SUCCESS_EVENTS
laruh Sep 5, 2024
cc7c963
add "MakerPaymentSpendConfirmed" to TAKER_USING_WATCHERS_SUCCESS_EVEN…
laruh Sep 5, 2024
69ed68e
remove logs
laruh Sep 5, 2024
b2446b9
self.r().data.maker_payment_requires_nota.unwrap_or(false)
laruh Sep 6, 2024
2c119be
provide number of confirmations in wait_for_confirmations
laruh Sep 8, 2024
b9ba150
Merge remote-tracking branch 'origin/dev' into taker-failed-swaps-mar…
laruh Sep 8, 2024
165ddfe
provide requires_nota:false in confirm_maker_payment_spend
laruh Sep 26, 2024
b50b2a3
review: make doc com of wait_for_confirmations clearer
laruh Sep 26, 2024
6fa527a
Merge remote-tracking branch 'origin/dev' into taker-failed-swaps-mar…
laruh Sep 26, 2024
0073fe8
Revert "provide number of confirmations in wait_for_confirmations"
laruh Sep 26, 2024
aed8d4c
review: for old saved swaps check "TakerSwapEvent::MakerPaymentSpent"…
laruh Sep 27, 2024
ed91569
review: add "MakerPaymentSpendConfirmed" event in const FINISHED_TAKE…
laruh Sep 27, 2024
0661ee2
review: cover ConfirmMakerPaymentSpend for MakerPaymentSpentByWatcher
laruh Sep 27, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,14 @@
"Started",
"Negotiated",
"TakerFeeSent",
"TakerPaymentInstructionsReceived",
"MakerPaymentReceived",
"MakerPaymentWaitConfirmStarted",
"MakerPaymentValidatedAndConfirmed",
"TakerPaymentSent",
"TakerPaymentSpent",
"MakerPaymentSpent",
"MakerPaymentSpendConfirmed",
"Finished"
],
"error_events": [
Expand All @@ -164,8 +166,12 @@
"TakerPaymentDataSendFailed",
"TakerPaymentWaitForSpendFailed",
"MakerPaymentSpendFailed",
"MakerPaymentSpendConfirmFailed",
"TakerPaymentWaitRefundStarted",
"TakerPaymentRefundStarted",
"TakerPaymentRefunded",
"TakerPaymentRefundFailed"
"TakerPaymentRefundedByWatcher",
"TakerPaymentRefundFailed",
"TakerPaymentRefundFinished"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,6 @@
"taker_coin":"MORTY",
"gui":"atomicDEX 0.5.1 iOS",
"mm_version":"1b065636a",
"success_events":["Started","Negotiated","TakerFeeSent","TakerPaymentInstructionsReceived","MakerPaymentReceived","MakerPaymentWaitConfirmStarted","MakerPaymentValidatedAndConfirmed","TakerPaymentSent","TakerPaymentSpent","MakerPaymentSpent","Finished"],
"success_events":["Started","Negotiated","TakerFeeSent","TakerPaymentInstructionsReceived","MakerPaymentReceived","MakerPaymentWaitConfirmStarted","MakerPaymentValidatedAndConfirmed","TakerPaymentSent","TakerPaymentSpent","MakerPaymentSpent","MakerPaymentSpendConfirmed","Finished"],
"error_events":["StartFailed","NegotiateFailed","TakerFeeSendFailed","MakerPaymentValidateFailed","MakerPaymentWaitConfirmFailed","TakerPaymentTransactionFailed","TakerPaymentWaitConfirmFailed","TakerPaymentDataSendFailed","TakerPaymentWaitForSpendFailed","MakerPaymentSpendFailed","TakerPaymentWaitRefundStarted","TakerPaymentRefunded","TakerPaymentRefundedByWatcher","TakerPaymentRefundFailed"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@
"taker_coin":"MORTY",
"gui":"atomicDEX 0.5.1 iOS",
"mm_version":"1b065636a",
"success_events":["Started","Negotiated","TakerFeeSent","TakerPaymentInstructionsReceived","MakerPaymentReceived","MakerPaymentWaitConfirmStarted","MakerPaymentValidatedAndConfirmed","TakerPaymentSent","TakerPaymentSpent","MakerPaymentSpent","Finished"],
"success_events":["Started","Negotiated","TakerFeeSent","TakerPaymentInstructionsReceived","MakerPaymentReceived","MakerPaymentWaitConfirmStarted","MakerPaymentValidatedAndConfirmed","TakerPaymentSent","TakerPaymentSpent","MakerPaymentSpent","MakerPaymentSpendConfirmed","Finished"],
"error_events":["StartFailed","NegotiateFailed","TakerFeeSendFailed","MakerPaymentValidateFailed","MakerPaymentWaitConfirmFailed","TakerPaymentTransactionFailed","TakerPaymentWaitConfirmFailed","TakerPaymentDataSendFailed","TakerPaymentWaitForSpendFailed","MakerPaymentSpendFailed","TakerPaymentWaitRefundStarted","TakerPaymentRefunded","TakerPaymentRefundedByWatcher","TakerPaymentRefundFailed"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,6 @@
"taker_coin":"MORTY",
"gui":null,
"mm_version":"",
"success_events":["Started","Negotiated","TakerFeeSent","TakerPaymentInstructionsReceived","MakerPaymentReceived","MakerPaymentWaitConfirmStarted","MakerPaymentValidatedAndConfirmed","TakerPaymentSent","TakerPaymentSpent","MakerPaymentSpent","Finished"],
"error_events":["StartFailed","NegotiateFailed","TakerFeeSendFailed","MakerPaymentValidateFailed","MakerPaymentWaitConfirmFailed","TakerPaymentTransactionFailed","TakerPaymentWaitConfirmFailed","TakerPaymentDataSendFailed","TakerPaymentWaitForSpendFailed","MakerPaymentSpendFailed","TakerPaymentWaitRefundStarted","TakerPaymentRefundStarted","TakerPaymentRefunded","TakerPaymentRefundedByWatcher","TakerPaymentRefundFailed","TakerPaymentRefundFinished"]
"success_events":["Started","Negotiated","TakerFeeSent","TakerPaymentInstructionsReceived","MakerPaymentReceived","MakerPaymentWaitConfirmStarted","MakerPaymentValidatedAndConfirmed","TakerPaymentSent","TakerPaymentSpent","MakerPaymentSpent","MakerPaymentSpendConfirmed","Finished"],
"error_events":["StartFailed","NegotiateFailed","TakerFeeSendFailed","MakerPaymentValidateFailed","MakerPaymentWaitConfirmFailed","TakerPaymentTransactionFailed","TakerPaymentWaitConfirmFailed","TakerPaymentDataSendFailed","TakerPaymentWaitForSpendFailed","MakerPaymentSpendFailed","MakerPaymentSpendConfirmFailed","TakerPaymentWaitRefundStarted","TakerPaymentRefundStarted","TakerPaymentRefunded","TakerPaymentRefundedByWatcher","TakerPaymentRefundFailed","TakerPaymentRefundFinished"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@
"taker_coin":"MORTY",
"gui":null,
"mm_version":"",
"success_events":["Started","Negotiated","TakerFeeSent","TakerPaymentInstructionsReceived","MakerPaymentReceived","MakerPaymentWaitConfirmStarted","MakerPaymentValidatedAndConfirmed","TakerPaymentSent","TakerPaymentSpent","MakerPaymentSpent","Finished"],
"error_events":["StartFailed","NegotiateFailed","TakerFeeSendFailed","MakerPaymentValidateFailed","MakerPaymentWaitConfirmFailed","TakerPaymentTransactionFailed","TakerPaymentWaitConfirmFailed","TakerPaymentDataSendFailed","TakerPaymentWaitForSpendFailed","MakerPaymentSpendFailed","TakerPaymentWaitRefundStarted","TakerPaymentRefundStarted","TakerPaymentRefunded","TakerPaymentRefundedByWatcher","TakerPaymentRefundFailed","TakerPaymentRefundFinished"]
"success_events":["Started","Negotiated","TakerFeeSent","TakerPaymentInstructionsReceived","MakerPaymentReceived","MakerPaymentWaitConfirmStarted","MakerPaymentValidatedAndConfirmed","TakerPaymentSent","TakerPaymentSpent","MakerPaymentSpent","MakerPaymentSpendConfirmed","Finished"],
"error_events":["StartFailed","NegotiateFailed","TakerFeeSendFailed","MakerPaymentValidateFailed","MakerPaymentWaitConfirmFailed","TakerPaymentTransactionFailed","TakerPaymentWaitConfirmFailed","TakerPaymentDataSendFailed","TakerPaymentWaitForSpendFailed","MakerPaymentSpendFailed","MakerPaymentSpendConfirmFailed","TakerPaymentWaitRefundStarted","TakerPaymentRefundStarted","TakerPaymentRefunded","TakerPaymentRefundedByWatcher","TakerPaymentRefundFailed","TakerPaymentRefundFinished"]
}
2 changes: 1 addition & 1 deletion mm2src/mm2_main/src/lp_swap/maker_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,7 @@ impl MakerSwapEvent {
},
MakerSwapEvent::TakerPaymentSpent(_) => "Taker payment spent...".to_owned(),
MakerSwapEvent::TakerPaymentSpendFailed(_) => "Taker payment spend failed...".to_owned(),
MakerSwapEvent::TakerPaymentSpendConfirmStarted => "Taker payment send wait confirm started...".to_owned(),
MakerSwapEvent::TakerPaymentSpendConfirmStarted => "Taker payment spend confirm started...".to_owned(),
MakerSwapEvent::TakerPaymentSpendConfirmed => "Taker payment spend confirmed...".to_owned(),
MakerSwapEvent::TakerPaymentSpendConfirmFailed(_) => "Taker payment spend confirm failed...".to_owned(),
MakerSwapEvent::MakerPaymentWaitRefundStarted { wait_until } => {
Expand Down
2 changes: 2 additions & 0 deletions mm2src/mm2_main/src/lp_swap/recreate_swap_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ fn convert_taker_to_maker_events(
| TakerSwapEvent::MakerPaymentWaitConfirmStarted
| TakerSwapEvent::MakerPaymentValidatedAndConfirmed
| TakerSwapEvent::MakerPaymentSpent(_)
| TakerSwapEvent::MakerPaymentSpendConfirmed
| TakerSwapEvent::MakerPaymentSpendConfirmFailed(_)
| TakerSwapEvent::MakerPaymentSpentByWatcher(_)
| TakerSwapEvent::MakerPaymentSpendFailed(_)
// We don't know the reason at the moment, so we rely on the errors handling above.
Expand Down
3 changes: 2 additions & 1 deletion mm2src/mm2_main/src/lp_swap/taker_restart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pub async fn get_command_based_on_maker_or_watcher_activity(
Err(e) => ERR!("Error {} when trying to find taker payment spend", e),
},
TakerSwapCommand::SpendMakerPayment => check_maker_payment_spend_and_add_event(ctx, swap, saved).await,
TakerSwapCommand::ConfirmMakerPaymentSpend => Ok(command),
Copy link
Collaborator

Choose a reason for hiding this comment

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

The below is related to this change in get_command_based_on_maker_or_watcher_activity.

I think we should return TakerSwapCommand::ConfirmMakerPaymentSpend instead of TakerSwapCommand::Finish here

Copy link
Collaborator

Choose a reason for hiding this comment

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

c.c. @dimxy

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems so.
search_for_swap_tx_spend_other fn may return unconfirmed txns.
So we need (with the created event MakerPaymentSpentByWatcher) to go to ConfirmMakerPaymentSpend command.
I think it's better to fix also get_command() to ensure MakerPaymentSpentByWatcher event triggers ConfirmMakerPaymentSpend.
And good to have a new swap watcher test for this to ensure the flow works okay

Copy link
Member Author

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 to fix also get_command() to ensure MakerPaymentSpentByWatcher event triggers ConfirmMakerPaymentSpend.

yep, if we want to return TakerSwapCommand::ConfirmMakerPaymentSpend in check_maker_payment_spend_and_add_event, then we also need to cover confirmation step in get_command for MakerPaymentSpentByWatcher

Copy link
Member Author

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 to fix also get_command() to ensure MakerPaymentSpentByWatcher event triggers ConfirmMakerPaymentSpend.

yep, if we want to return TakerSwapCommand::ConfirmMakerPaymentSpend in check_maker_payment_spend_and_add_event, then we also need to cover confirmation step in get_command for MakerPaymentSpentByWatcher

Hmm it means that we also need to update fn is_recoverable according to the same note as here #2199 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

@shamardy one moment about watchers, do we actually use this feature in production? Im not sure that it is fully functional in prod.

If nope, we shouldn't change MakerPaymentSpentByWatcher handle in legacy get_command()function

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, if we want to return TakerSwapCommand::ConfirmMakerPaymentSpend in check_maker_payment_spend_and_add_event, then we also need to cover confirmation step in get_command for MakerPaymentSpentByWatcher

It will be done here

let event = TakerSwapEvent::MakerPaymentSpentByWatcher(tx_ident);
let to_save = TakerSavedEvent {
timestamp: now_ms(),
event,
};
swap.apply_event(to_save.event.clone());
saved.events.push(to_save);
let new_swap = SavedSwap::Taker(saved);
try_s!(new_swap.save_to_db(ctx).await);
info!("{}", MAKER_PAYMENT_SPENT_BY_WATCHER_LOG);
Ok(TakerSwapCommand::Finish)
by this change #2199 (comment)

Hmm it means that we also need to update fn is_recoverable according to the same note as here #2199 (comment)

Yes, I suppose.

one moment about watchers, do we actually use this feature in production? Im not sure that it is fully functional in prod.
If nope, we shouldn't change MakerPaymentSpentByWatcher handle in legacy get_command()function

It's used for utxo <-> utxo swap in production. We need to collect data about it though to know how many users used it etc.. , we should leave a comment about this to @KomodoPlatform/qa

Copy link
Collaborator

Choose a reason for hiding this comment

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

We might need to handle it here too

TakerSwapEvent::MakerPaymentSpentByWatcher(_) => Some(TakerSwapCommand::Finish),

Copy link
Member Author

Choose a reason for hiding this comment

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

It's used for utxo <-> utxo swap in production.

Got it, then I also update MakerPaymentSpentByWatcher cases

Copy link
Member Author

Choose a reason for hiding this comment

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

We might need to handle it here too

TakerSwapEvent::MakerPaymentSpentByWatcher(_) => Some(TakerSwapCommand::Finish),

yep, its what dimxy was referencing about

TakerSwapCommand::PrepareForTakerPaymentRefund | TakerSwapCommand::RefundTakerPayment => {
#[cfg(not(any(test, feature = "run-docker-tests")))]
{
Expand Down Expand Up @@ -141,7 +142,7 @@ pub async fn check_maker_payment_spend_and_add_event(
let new_swap = SavedSwap::Taker(saved);
try_s!(new_swap.save_to_db(ctx).await);
info!("{}", MAKER_PAYMENT_SPENT_BY_WATCHER_LOG);
Ok(TakerSwapCommand::Finish)
Ok(TakerSwapCommand::ConfirmMakerPaymentSpend)
}

pub async fn check_taker_payment_spend(swap: &TakerSwap) -> Result<Option<FoundSwapTxSpend>, String> {
Expand Down
102 changes: 82 additions & 20 deletions mm2src/mm2_main/src/lp_swap/taker_swap.rs

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions mm2src/mm2_main/tests/docker_tests/swap_watcher_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ fn test_taker_saves_the_swap_as_successful_after_restart_panic_at_wait_for_taker
"WatcherMessageSent",
"TakerPaymentSpent",
"MakerPaymentSpentByWatcher",
"MakerPaymentSpendConfirmed",
"Finished",
];
check_actual_events(&mm_alice, &uuids[0], &expected_events);
Expand Down Expand Up @@ -467,6 +468,7 @@ fn test_taker_saves_the_swap_as_successful_after_restart_panic_at_maker_payment_
"WatcherMessageSent",
"TakerPaymentSpent",
"MakerPaymentSpentByWatcher",
"MakerPaymentSpendConfirmed",
"Finished",
];
check_actual_events(&mm_alice, &uuids[0], &expected_events);
Expand Down
4 changes: 2 additions & 2 deletions mm2src/mm2_main/tests/docker_tests/swaps_file_lock_tests.rs

Large diffs are not rendered by default.

15 changes: 10 additions & 5 deletions mm2src/mm2_test_helpers/src/for_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub const MAKER_ERROR_EVENTS: [&str; 15] = [
"MakerPaymentRefundFinished",
];

pub const TAKER_SUCCESS_EVENTS: [&str; 11] = [
pub const TAKER_SUCCESS_EVENTS: [&str; 12] = [
"Started",
"Negotiated",
"TakerFeeSent",
Expand All @@ -92,10 +92,11 @@ pub const TAKER_SUCCESS_EVENTS: [&str; 11] = [
"TakerPaymentSent",
"TakerPaymentSpent",
"MakerPaymentSpent",
"MakerPaymentSpendConfirmed",
"Finished",
];

pub const TAKER_USING_WATCHERS_SUCCESS_EVENTS: [&str; 13] = [
pub const TAKER_USING_WATCHERS_SUCCESS_EVENTS: [&str; 14] = [
"Started",
"Negotiated",
"TakerFeeSent",
Expand All @@ -108,11 +109,12 @@ pub const TAKER_USING_WATCHERS_SUCCESS_EVENTS: [&str; 13] = [
"TakerPaymentSpent",
"MakerPaymentSpent",
"MakerPaymentSpentByWatcher",
"MakerPaymentSpendConfirmed",
"Finished",
];

// Taker using watchers and watcher spends maker payment
pub const TAKER_ACTUAL_EVENTS_WATCHER_SPENDS_MAKER_PAYMENT: [&str; 12] = [
pub const TAKER_ACTUAL_EVENTS_WATCHER_SPENDS_MAKER_PAYMENT: [&str; 13] = [
"Started",
"Negotiated",
"TakerFeeSent",
Expand All @@ -124,11 +126,12 @@ pub const TAKER_ACTUAL_EVENTS_WATCHER_SPENDS_MAKER_PAYMENT: [&str; 12] = [
"WatcherMessageSent",
"TakerPaymentSpent",
"MakerPaymentSpentByWatcher",
"MakerPaymentSpendConfirmed",
"Finished",
];

// Taker using watchers and spends maker payment instead of watcher
pub const TAKER_ACTUAL_EVENTS_TAKER_SPENDS_MAKER_PAYMENT: [&str; 12] = [
pub const TAKER_ACTUAL_EVENTS_TAKER_SPENDS_MAKER_PAYMENT: [&str; 13] = [
"Started",
"Negotiated",
"TakerFeeSent",
Expand All @@ -140,10 +143,11 @@ pub const TAKER_ACTUAL_EVENTS_TAKER_SPENDS_MAKER_PAYMENT: [&str; 12] = [
"WatcherMessageSent",
"TakerPaymentSpent",
"MakerPaymentSpent",
"MakerPaymentSpendConfirmed",
"Finished",
];

pub const TAKER_ERROR_EVENTS: [&str; 16] = [
pub const TAKER_ERROR_EVENTS: [&str; 17] = [
"StartFailed",
"NegotiateFailed",
"TakerFeeSendFailed",
Expand All @@ -154,6 +158,7 @@ pub const TAKER_ERROR_EVENTS: [&str; 16] = [
"TakerPaymentDataSendFailed",
"TakerPaymentWaitForSpendFailed",
"MakerPaymentSpendFailed",
"MakerPaymentSpendConfirmFailed",
"TakerPaymentWaitRefundStarted",
"TakerPaymentRefundStarted",
"TakerPaymentRefunded",
Expand Down
Loading