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

[r2r] Include maker/taker pubkeys in MM2.db stats_swaps table #1665

Merged
merged 22 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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: 5 additions & 0 deletions mm2src/mm2_main/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ fn migration_7() -> Vec<(&'static str, Vec<String>)> {
db_common::sqlite::execute_batch(stats_swaps::ADD_COINS_PRICE_INFOMATION)
}

fn migration_8() -> Vec<(&'static str, Vec<String>)> {
db_common::sqlite::execute_batch(stats_swaps::ADD_MAKER_TAKER_PUBKEYS)
}

async fn statements_for_migration(ctx: &MmArc, current_migration: i64) -> Option<Vec<(&'static str, Vec<String>)>> {
match current_migration {
1 => Some(migration_1(ctx).await),
Expand All @@ -106,6 +110,7 @@ async fn statements_for_migration(ctx: &MmArc, current_migration: i64) -> Option
5 => Some(migration_5()),
6 => Some(migration_6()),
7 => Some(migration_7()),
8 => Some(migration_8()),
_ => None,
}
}
Expand Down
20 changes: 18 additions & 2 deletions mm2src/mm2_main/src/database/stats_swaps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,23 @@ const INSERT_STATS_SWAP: &str = "INSERT INTO stats_swaps (
taker_amount,
is_success,
maker_coin_usd_price,
taker_coin_usd_price
) VALUES (:maker_coin, :maker_coin_ticker, :maker_coin_platform, :taker_coin, :taker_coin_ticker, :taker_coin_platform, :uuid, :started_at, :finished_at, :maker_amount, :taker_amount, :is_success, :maker_coin_usd_price, :taker_coin_usd_price)";
taker_coin_usd_price,
maker_pubkey,
taker_pubkey
) VALUES (:maker_coin, :maker_coin_ticker, :maker_coin_platform, :taker_coin, :taker_coin_ticker,
:taker_coin_platform, :uuid, :started_at, :finished_at, :maker_amount, :taker_amount, :is_success,
:maker_coin_usd_price, :taker_coin_usd_price, :maker_pubkey, :taker_pubkey)";

pub const ADD_COINS_PRICE_INFOMATION: &[&str] = &[
"ALTER TABLE stats_swaps ADD COLUMN maker_coin_usd_price DECIMAL;",
"ALTER TABLE stats_swaps ADD COLUMN taker_coin_usd_price DECIMAL;",
];

pub const ADD_MAKER_TAKER_PUBKEYS: &[&str] = &[
"ALTER TABLE stats_swaps ADD COLUMN maker_pubkey VARCHAR(255);",
"ALTER TABLE stats_swaps ADD COLUMN taker_pubkey VARCHAR(255);",
Comment on lines +59 to +60
Copy link
Member

Choose a reason for hiding this comment

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

Can't just "ALTER TABLE stats_swaps ADD COLUMN maker_pubkey VARCHAR(255), ADD COLUMN taker_pubkey VARCHAR(255); work?

Copy link
Member Author

Choose a reason for hiding this comment

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

];

pub const ADD_SPLIT_TICKERS: &[&str] = &[
"ALTER TABLE stats_swaps ADD COLUMN maker_coin_ticker VARCHAR(255) NOT NULL DEFAULT '';",
"ALTER TABLE stats_swaps ADD COLUMN maker_coin_platform VARCHAR(255) NOT NULL DEFAULT '';",
Expand Down Expand Up @@ -132,6 +141,7 @@ fn insert_stats_maker_swap_sql(swap: &MakerSavedSwap) -> Option<(&'static str, O
let (maker_coin_ticker, maker_coin_platform) = split_coin(&swap_data.maker_coin);
let (taker_coin_ticker, taker_coin_platform) = split_coin(&swap_data.taker_coin);

let pubkeys = &swap.swap_pubkeys();
let params = owned_named_params! {
":maker_coin": swap_data.maker_coin.clone(),
":maker_coin_ticker": maker_coin_ticker,
Expand All @@ -147,7 +157,10 @@ fn insert_stats_maker_swap_sql(swap: &MakerSavedSwap) -> Option<(&'static str, O
":is_success": (is_success as u32).to_string(),
":maker_coin_usd_price": swap.maker_coin_usd_price.as_ref().map(|p| p.to_string()),
":taker_coin_usd_price": swap.taker_coin_usd_price.as_ref().map(|p| p.to_string()),
":maker_pubkey": pubkeys.maker.map(|key|key.to_string()),
":taker_pubkey": pubkeys.taker.map(|key|key.to_string()),
};

Some((INSERT_STATS_SWAP, params))
}

Expand Down Expand Up @@ -205,6 +218,7 @@ fn insert_stats_taker_swap_sql(swap: &TakerSavedSwap) -> Option<(&'static str, O
let (maker_coin_ticker, maker_coin_platform) = split_coin(&swap_data.maker_coin);
let (taker_coin_ticker, taker_coin_platform) = split_coin(&swap_data.taker_coin);

let pubkeys = &swap.swap_pubkeys();
let params = owned_named_params! {
":maker_coin": swap_data.maker_coin.clone(),
":maker_coin_ticker": maker_coin_ticker,
Expand All @@ -220,6 +234,8 @@ fn insert_stats_taker_swap_sql(swap: &TakerSavedSwap) -> Option<(&'static str, O
":is_success": (is_success as u32).to_string(),
":maker_coin_usd_price": swap.maker_coin_usd_price.as_ref().map(|p| p.to_string()),
":taker_coin_usd_price": swap.taker_coin_usd_price.as_ref().map(|p| p.to_string()),
":maker_pubkey": pubkeys.maker.map(|key|key.to_string()),
":taker_pubkey": pubkeys.taker.map(|key|key.to_string()),
};
Some((INSERT_STATS_SWAP, params))
}
Expand Down
7 changes: 7 additions & 0 deletions mm2src/mm2_main/src/lp_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ use my_swaps_storage::{MySwapsOps, MySwapsStorage};
use pubkey_banning::BanReason;
pub use pubkey_banning::{ban_pubkey_rpc, is_pubkey_banned, list_banned_pubkeys_rpc, unban_pubkeys_rpc};
pub use recreate_swap_data::recreate_swap_data;
use rpc::v1::types::H264 as H264Json;
pub use saved_swap::{SavedSwap, SavedSwapError, SavedSwapIo, SavedSwapResult};
pub use swap_watcher::{process_watcher_msg, watcher_topic, TakerSwapWatcherData, MAKER_PAYMENT_SPEND_FOUND_LOG,
MAKER_PAYMENT_SPEND_SENT_LOG, TAKER_PAYMENT_REFUND_SENT_LOG, TAKER_SWAP_ENTRY_TIMEOUT,
Expand Down Expand Up @@ -1380,6 +1381,12 @@ fn detect_secret_hash_algo(maker_coin: &MmCoinEnum, taker_coin: &MmCoinEnum) ->
}
}

#[derive(Debug, Deserialize, PartialEq, Serialize, Default)]
shamardy marked this conversation as resolved.
Show resolved Hide resolved
pub struct SwapPubkeys {
pub maker: Option<H264Json>,
pub taker: Option<H264Json>,
}

#[cfg(all(test, not(target_arch = "wasm32")))]
mod lp_swap_tests {
use super::*;
Expand Down
21 changes: 18 additions & 3 deletions mm2src/mm2_main/src/lp_swap/maker_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ use super::{broadcast_my_swap_status, broadcast_p2p_tx_msg, broadcast_swap_messa
check_other_coin_balance_for_swap, detect_secret_hash_algo, dex_fee_amount_from_taker_coin,
get_locked_amount, recv_swap_msg, swap_topic, taker_payment_spend_deadline, tx_helper_topic,
wait_for_maker_payment_conf_until, AtomicSwap, LockedAmount, MySwapInfo, NegotiationDataMsg,
NegotiationDataV2, NegotiationDataV3, RecoveredSwap, RecoveredSwapAction, SavedSwap, SavedSwapIo,
SavedTradeFee, SecretHashAlgo, SwapConfirmationsSettings, SwapError, SwapMsg, SwapTxDataMsg, SwapsContext,
NegotiationDataV2, RecoveredSwap, RecoveredSwapAction, SavedSwap, SavedSwapIo, SavedTradeFee,
SecretHashAlgo, SwapConfirmationsSettings, SwapError, SwapMsg, SwapTxDataMsg, SwapsContext,
TransactionIdentifier, WAIT_CONFIRM_INTERVAL};
use crate::mm2::lp_dispatcher::{DispatcherContext, LpEvents};
use crate::mm2::lp_network::subscribe_to_topic;
use crate::mm2::lp_ordermatch::{MakerOrderBuilder, OrderConfirmationsSettings};
use crate::mm2::lp_price::fetch_swap_coins_price;
use crate::mm2::lp_swap::{broadcast_swap_message, taker_payment_spend_duration};
use crate::mm2::lp_swap::{broadcast_swap_message, taker_payment_spend_duration, NegotiationDataV3, SwapPubkeys};
use coins::{CanRefundHtlc, CheckIfMyPaymentSentArgs, FeeApproxStage, FoundSwapTxSpend, MmCoinEnum,
PaymentInstructions, PaymentInstructionsErr, SearchForSwapTxSpendInput, SendMakerPaymentArgs,
SendMakerRefundsPaymentArgs, SendMakerSpendsTakerPaymentArgs, TradeFee, TradePreimageValue,
Expand Down Expand Up @@ -1875,6 +1875,21 @@ impl MakerSavedSwap {
self.taker_coin_usd_price = Some(rates.rel);
}
}

pub fn swap_pubkeys(&self) -> SwapPubkeys {
shamardy marked this conversation as resolved.
Show resolved Hide resolved
let mut swap_pubkeys = SwapPubkeys::default();

// TODO: Adjust for private coins when/if they are braodcasted
for data in &self.events {
if let MakerSwapEvent::Started(started) = &data.event {
swap_pubkeys.maker = Some(started.my_persistent_pub);
swap_pubkeys.taker = started.maker_coin_htlc_pubkey;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aren't those 2 pubkeys the maker's? can you please add a unit test to test that the right pubkeys are saved to DB? you should use 3 nodes for the test (taker/maker/seednode) and use stats_swap_status RPC to check that the right pubkeys are saved to the seednode's DB.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

};
}

drop_mutability!(swap_pubkeys);
swap_pubkeys
}
}

#[allow(clippy::large_enum_variant)]
Expand Down
25 changes: 20 additions & 5 deletions mm2src/mm2_main/src/lp_swap/taker_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ use super::trade_preimage::{TradePreimageRequest, TradePreimageRpcError, TradePr
use super::{broadcast_my_swap_status, broadcast_swap_message, broadcast_swap_message_every,
check_other_coin_balance_for_swap, dex_fee_amount_from_taker_coin, dex_fee_rate, dex_fee_threshold,
get_locked_amount, recv_swap_msg, swap_topic, wait_for_maker_payment_conf_until, AtomicSwap, LockedAmount,
MySwapInfo, NegotiationDataMsg, NegotiationDataV2, NegotiationDataV3, RecoveredSwap, RecoveredSwapAction,
SavedSwap, SavedSwapIo, SavedTradeFee, SwapConfirmationsSettings, SwapError, SwapMsg, SwapTxDataMsg,
SwapsContext, TransactionIdentifier, WAIT_CONFIRM_INTERVAL};
MySwapInfo, NegotiationDataMsg, NegotiationDataV2, RecoveredSwap, RecoveredSwapAction, SavedSwap,
SavedSwapIo, SavedTradeFee, SwapConfirmationsSettings, SwapError, SwapMsg, SwapTxDataMsg, SwapsContext,
TransactionIdentifier, WAIT_CONFIRM_INTERVAL};
use crate::mm2::lp_network::subscribe_to_topic;
use crate::mm2::lp_ordermatch::{MatchBy, OrderConfirmationsSettings, TakerAction, TakerOrderBuilder};
use crate::mm2::lp_price::fetch_swap_coins_price;
use crate::mm2::lp_swap::{broadcast_p2p_tx_msg, tx_helper_topic, wait_for_maker_payment_conf_duration,
TakerSwapWatcherData};
NegotiationDataV3, SwapPubkeys, TakerSwapWatcherData};
use coins::{lp_coinfind, CanRefundHtlc, CheckIfMyPaymentSentArgs, FeeApproxStage, FoundSwapTxSpend, MmCoinEnum,
PaymentInstructions, PaymentInstructionsErr, SearchForSwapTxSpendInput, SendSpendPaymentArgs,
SendTakerPaymentArgs, SendTakerRefundsPaymentArgs, SendTakerSpendsMakerPaymentArgs, TradeFee,
Expand Down Expand Up @@ -301,6 +301,21 @@ impl TakerSavedSwap {
self.taker_coin_usd_price = Some(rates.rel);
}
}

pub fn swap_pubkeys(&self) -> SwapPubkeys {
let mut swap_pubkeys = SwapPubkeys::default();

// TODO: Adjust for private coins when/if they are braodcasted
for data in &self.events {
if let TakerSwapEvent::Started(started) = &data.event {
swap_pubkeys.maker = started.maker_coin_htlc_pubkey;
swap_pubkeys.taker = Some(started.my_persistent_pub);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same for those 2 pubkeys, aren't they both the taker's?

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 this is ready for another round of review!

};
}

drop_mutability!(swap_pubkeys);
swap_pubkeys
}
}

#[allow(clippy::large_enum_variant)]
Expand Down Expand Up @@ -1171,7 +1186,7 @@ impl TakerSwap {
maker_payment_locktime: maker_data.payment_locktime(),
// using default to avoid misuse of this field
// maker_coin_htlc_pubkey and taker_coin_htlc_pubkey must be used instead
maker_pubkey: H264Json::default(),
maker_pubkey: Default::default(),
secret_hash: maker_data.secret_hash().into(),
maker_coin_swap_contract_addr,
taker_coin_swap_contract_addr,
Expand Down