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 6 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.to_string(),
":taker_pubkey": pubkeys.taker.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.to_string(),
":taker_pubkey": pubkeys.taker.to_string(),
};
Some((INSERT_STATS_SWAP, params))
}
Expand Down
36 changes: 36 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 @@ -711,12 +712,25 @@ pub struct NegotiationDataV3 {
taker_coin_htlc_pub: Vec<u8>,
}

#[derive(Clone, Debug, Eq, Deserialize, PartialEq, Serialize)]
pub struct NegotiationDataV4 {
started_at: u64,
payment_locktime: u64,
secret_hash: Vec<u8>,
maker_coin_swap_contract: Vec<u8>,
taker_coin_swap_contract: Vec<u8>,
maker_coin_htlc_pub: Vec<u8>,
taker_coin_htlc_pub: Vec<u8>,
persistent_pubkey: Vec<u8>,

Choose a reason for hiding this comment

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

Please try to avoid adding NegotiationDataV4 message. I even think this new variant might break backwards compatibility.
I personally think it's better to store maker_coin_htlc_pub as maker_pubkey and taker_coin_htlc_pub as taker_pubkey. @shamardy what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

waiting for @shamardy feedback on this...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with @sergeyboyko0791 here. Using persistent_pubkey also leaks private information for private ARRR swaps.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@borngraced is this PR ready for review? NegotiationDataV4 is still not removed from the code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh sorry, I missed that! and yes it's ready for review.

}

#[derive(Clone, Debug, Eq, Deserialize, PartialEq, Serialize)]
#[serde(untagged)]
pub enum NegotiationDataMsg {
V1(NegotiationDataV1),
V2(NegotiationDataV2),
V3(NegotiationDataV3),
V4(NegotiationDataV4),
}

impl NegotiationDataMsg {
Expand All @@ -725,6 +739,7 @@ impl NegotiationDataMsg {
NegotiationDataMsg::V1(v1) => v1.started_at,
NegotiationDataMsg::V2(v2) => v2.started_at,
NegotiationDataMsg::V3(v3) => v3.started_at,
NegotiationDataMsg::V4(v4) => v4.started_at,
}
}

Expand All @@ -733,6 +748,7 @@ impl NegotiationDataMsg {
NegotiationDataMsg::V1(v1) => v1.payment_locktime,
NegotiationDataMsg::V2(v2) => v2.payment_locktime,
NegotiationDataMsg::V3(v3) => v3.payment_locktime,
NegotiationDataMsg::V4(v4) => v4.payment_locktime,
}
}

Expand All @@ -741,6 +757,7 @@ impl NegotiationDataMsg {
NegotiationDataMsg::V1(v1) => &v1.secret_hash,
NegotiationDataMsg::V2(v2) => &v2.secret_hash,
NegotiationDataMsg::V3(v3) => &v3.secret_hash,
NegotiationDataMsg::V4(v4) => &v4.secret_hash,
}
}

Expand All @@ -749,6 +766,7 @@ impl NegotiationDataMsg {
NegotiationDataMsg::V1(v1) => &v1.persistent_pubkey,
NegotiationDataMsg::V2(v2) => &v2.persistent_pubkey,
NegotiationDataMsg::V3(v3) => &v3.maker_coin_htlc_pub,
NegotiationDataMsg::V4(v4) => &v4.maker_coin_htlc_pub,
}
}

Expand All @@ -757,6 +775,7 @@ impl NegotiationDataMsg {
NegotiationDataMsg::V1(v1) => &v1.persistent_pubkey,
NegotiationDataMsg::V2(v2) => &v2.persistent_pubkey,
NegotiationDataMsg::V3(v3) => &v3.taker_coin_htlc_pub,
NegotiationDataMsg::V4(v4) => &v4.taker_coin_htlc_pub,
}
}

Expand All @@ -765,6 +784,7 @@ impl NegotiationDataMsg {
NegotiationDataMsg::V1(_) => None,
NegotiationDataMsg::V2(v2) => Some(&v2.maker_coin_swap_contract),
NegotiationDataMsg::V3(v3) => Some(&v3.maker_coin_swap_contract),
NegotiationDataMsg::V4(v4) => Some(&v4.maker_coin_swap_contract),
}
}

Expand All @@ -773,6 +793,16 @@ impl NegotiationDataMsg {
NegotiationDataMsg::V1(_) => None,
NegotiationDataMsg::V2(v2) => Some(&v2.taker_coin_swap_contract),
NegotiationDataMsg::V3(v3) => Some(&v3.taker_coin_swap_contract),
NegotiationDataMsg::V4(v4) => Some(&v4.taker_coin_swap_contract),
}
}

pub fn persistent_pubkey(&self) -> Option<&[u8]> {
match self {
NegotiationDataMsg::V1(v1) => Some(&v1.persistent_pubkey),
NegotiationDataMsg::V2(v2) => Some(&v2.persistent_pubkey),
NegotiationDataMsg::V4(v4) => Some(&v4.persistent_pubkey),
NegotiationDataMsg::V3(_) => None,
}
}
}
Expand Down Expand Up @@ -1380,6 +1410,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: H264Json,
pub taker: H264Json,
}

#[cfg(all(test, not(target_arch = "wasm32")))]
mod lp_swap_tests {
use super::*;
Expand Down
27 changes: 23 additions & 4 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, NegotiationDataV4, SwapPubkeys};
use coins::{CanRefundHtlc, CheckIfMyPaymentSentArgs, FeeApproxStage, FoundSwapTxSpend, MmCoinEnum,
PaymentInstructions, PaymentInstructionsErr, SearchForSwapTxSpendInput, SendMakerPaymentArgs,
SendMakerRefundsPaymentArgs, SendMakerSpendsTakerPaymentArgs, TradeFee, TradePreimageValue,
Expand Down Expand Up @@ -84,6 +84,7 @@ async fn save_my_maker_swap_event(ctx: &MmArc, swap: &MakerSwap, event: MakerSav
Ok(Some(swap)) => swap,
Ok(None) => SavedSwap::Maker(MakerSavedSwap {
uuid: swap.uuid,
my_persistence_pub: swap.my_persistent_pub.into(),
my_order_uuid: swap.my_order_uuid,
maker_amount: Some(swap.maker_amount.clone()),
maker_coin: Some(swap.maker_coin.ticker().to_owned()),
Expand Down Expand Up @@ -121,6 +122,7 @@ pub struct TakerNegotiationData {
pub taker_coin_swap_contract_addr: Option<BytesJson>,
pub maker_coin_htlc_pubkey: Option<H264Json>,
pub taker_coin_htlc_pubkey: Option<H264Json>,
pub my_persistance_pubkey: H264Json,
}

impl TakerNegotiationData {
Expand Down Expand Up @@ -419,14 +421,15 @@ impl MakerSwap {
taker_coin_swap_contract,
})
} else {
NegotiationDataMsg::V3(NegotiationDataV3 {
NegotiationDataMsg::V4(NegotiationDataV4 {
started_at: r.data.started_at,
payment_locktime: r.data.maker_payment_lock,
secret_hash,
maker_coin_swap_contract,
taker_coin_swap_contract,
maker_coin_htlc_pub: self.my_maker_coin_htlc_pub().into(),
taker_coin_htlc_pub: self.my_taker_coin_htlc_pub().into(),
persistent_pubkey: r.data.my_persistent_pub.0.to_vec(),
})
}
}
Expand Down Expand Up @@ -656,6 +659,7 @@ impl MakerSwap {
)]));
};

println!("MAKER PUB{:?}", taker_data.persistent_pubkey().unwrap_or_default());
Ok((Some(MakerSwapCommand::WaitForTakerFee), vec![
MakerSwapEvent::Negotiated(TakerNegotiationData {
taker_payment_locktime: taker_data.payment_locktime(),
Expand All @@ -666,6 +670,7 @@ impl MakerSwap {
taker_coin_swap_contract_addr,
maker_coin_htlc_pubkey: Some(taker_data.maker_coin_htlc_pub().into()),
taker_coin_htlc_pubkey: Some(taker_data.taker_coin_htlc_pub().into()),
my_persistance_pubkey: taker_data.persistent_pubkey().unwrap_or_default().into(),
}),
]))
}
Expand Down Expand Up @@ -1702,6 +1707,7 @@ impl MakerSwapStatusChanged {
#[derive(Debug, Default, PartialEq, Serialize, Deserialize)]
pub struct MakerSavedSwap {
pub uuid: Uuid,
pub my_persistence_pub: H264Json,
pub my_order_uuid: Option<Uuid>,
pub events: Vec<MakerSavedEvent>,
pub maker_amount: Option<BigDecimal>,
Expand Down Expand Up @@ -1756,6 +1762,7 @@ impl MakerSavedSwap {
});
MakerSavedSwap {
uuid: Default::default(),
my_persistence_pub: Default::default(),
my_order_uuid: None,
events,
maker_amount: Some(maker_amount.to_decimal()),
Expand Down Expand Up @@ -1875,6 +1882,18 @@ 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();
for data in &self.events {
if let MakerSwapEvent::Negotiated(negotiated) = &data.event {
swap_pubkeys.maker = self.my_persistence_pub;
swap_pubkeys.taker = negotiated.my_persistance_pubkey
}
}

swap_pubkeys
}
}

#[allow(clippy::large_enum_variant)]
Expand Down
4 changes: 4 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 @@ -82,6 +82,7 @@ pub async fn recreate_swap_data(ctx: MmArc, args: RecreateSwapRequest) -> Recrea
fn recreate_maker_swap(ctx: MmArc, taker_swap: TakerSavedSwap) -> RecreateSwapResult<MakerSavedSwap> {
let mut maker_swap = MakerSavedSwap {
uuid: taker_swap.uuid,
my_persistence_pub: taker_swap.swap_pubkeys().maker,
my_order_uuid: taker_swap.my_order_uuid,
events: Vec::new(),
maker_amount: taker_swap.maker_amount,
Expand Down Expand Up @@ -164,6 +165,7 @@ fn recreate_maker_swap(ctx: MmArc, taker_swap: TakerSavedSwap) -> RecreateSwapRe
taker_coin_swap_contract_addr: negotiated_event.taker_coin_swap_contract_addr,
maker_coin_htlc_pubkey: started_event.maker_coin_htlc_pubkey,
taker_coin_htlc_pubkey: started_event.taker_coin_htlc_pubkey,
my_persistance_pubkey: negotiated_event.my_persistance_pubkey,
});
maker_swap.events.push(MakerSavedEvent {
timestamp: negotiated_event_timestamp,
Expand Down Expand Up @@ -281,6 +283,7 @@ fn convert_taker_to_maker_events(
async fn recreate_taker_swap(ctx: MmArc, maker_swap: MakerSavedSwap) -> RecreateSwapResult<TakerSavedSwap> {
let mut taker_swap = TakerSavedSwap {
uuid: maker_swap.uuid,
my_persistence_pub: maker_swap.swap_pubkeys().taker,
my_order_uuid: Some(maker_swap.uuid),
events: Vec::new(),
maker_amount: maker_swap.maker_amount,
Expand Down Expand Up @@ -363,6 +366,7 @@ async fn recreate_taker_swap(ctx: MmArc, maker_swap: MakerSavedSwap) -> Recreate
taker_coin_swap_contract_addr: negotiated_event.taker_coin_swap_contract_addr,
maker_coin_htlc_pubkey: started_event.maker_coin_htlc_pubkey,
taker_coin_htlc_pubkey: started_event.taker_coin_htlc_pubkey,
my_persistance_pubkey: negotiated_event.my_persistance_pubkey,
});
taker_swap.events.push(TakerSavedEvent {
timestamp: negotiated_timestamp,
Expand Down
Loading