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

feat(trading-proto-upgrade): fees fixes among other things #2109

Open
wants to merge 42 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
13a798e
use u8 for forkid
mariocynicys Apr 24, 2024
9a7d81b
add fixmes, amend me
mariocynicys Apr 27, 2024
a844f62
Negotiate the taker payment spend fee
mariocynicys May 2, 2024
151f5a8
Use `big_decimal_from_sat` & `sat_from_big_decimal` for conversions t…
mariocynicys May 3, 2024
9f722fb
fix tests
mariocynicys May 3, 2024
ef28a6c
can't clamp the fee to 1 since it's probably higher than the fee
mariocynicys May 3, 2024
0bd984a
remove wrong transition
mariocynicys May 3, 2024
ad21f4e
add some tests
mariocynicys May 4, 2024
ff05d06
add a calculator for taker payment spend transaction size
mariocynicys May 8, 2024
c20bac2
don't actually sign anything to avoid unwrapping
mariocynicys May 8, 2024
2ff43ad
account for the sighash byte
mariocynicys May 8, 2024
7a92123
get tx size methods to MmCoin trait
mariocynicys May 8, 2024
225b776
get_taker_payment_spend_tx_size -> get_taker_payment_tx_size
mariocynicys May 8, 2024
b221def
calculate the taker payment spend tx size as per the new protocol
mariocynicys May 9, 2024
481ba3e
fix formatting
mariocynicys May 9, 2024
e0f649c
first batch of renamings
mariocynicys May 10, 2024
d1f2d1b
always wait for maker payment before proceeding with taker payment spent
mariocynicys May 10, 2024
d90ad0e
stop un-needed propagation of `maker_payment_spend_fee`
mariocynicys May 10, 2024
0a4248c
more consistent renaming on maker side
mariocynicys May 10, 2024
3bd51d1
wrong name in the protobuf file
mariocynicys May 10, 2024
f4d18d6
format
mariocynicys May 10, 2024
d18bf96
fix the names in tests
mariocynicys May 10, 2024
4848cb0
suggestions from aline: use is_zero, parse string instead of float to…
mariocynicys May 20, 2024
b6ee698
merge with origin/dev
mariocynicys Jun 14, 2024
4f5a04a
fix tests not compiling after merge
mariocynicys Jun 18, 2024
130fc88
claimation -> claiming
mariocynicys Jul 2, 2024
ec3ffe3
remove unused method
mariocynicys Jul 2, 2024
9cb9818
move the fixme to the appropriate place
mariocynicys Jul 3, 2024
c86bade
set preimage & maker payment fee as a todo
mariocynicys Jul 3, 2024
a2e4bfa
remove fixme about the locktime used for btc
mariocynicys Jul 3, 2024
c383862
sighashtype is encoded as four bytes even tho it's just one
mariocynicys Jul 3, 2024
a08a981
move the forkid fixme into a todo for later
mariocynicys Jul 3, 2024
ba0037f
cap fee to the one instruced by taker, allow using less
mariocynicys Jul 4, 2024
c970e25
implement other fee calculation methods
mariocynicys Jul 4, 2024
0ea4f0c
use get_taker_payment_fee instead of const size get_htlc_spend_fee
mariocynicys Jul 4, 2024
db11ed9
use a more idiomatic syntax for func generic bounds
mariocynicys Jul 5, 2024
28fc442
broadcast every 30s instead of 600s
mariocynicys Jul 5, 2024
638d90e
add a missing fixme
mariocynicys Jul 29, 2024
12e1b66
use `get_sender_trade_fee` for non-htlc TXs
mariocynicys Jul 29, 2024
bc32198
merge with origin/dev
mariocynicys Aug 13, 2024
241af5a
merge with origin/dev
mariocynicys Oct 13, 2024
100951b
merge with dev
mariocynicys Nov 11, 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
51 changes: 44 additions & 7 deletions mm2src/coins/lp_coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1259,6 +1259,8 @@ pub struct SendTakerFundingArgs<'a> {
pub maker_pub: &'a [u8],
/// DEX fee
pub dex_fee: &'a DexFee,
/// The extra fee added to the trading volume to cover the taker payment transaction fees
pub taker_payment_fee: BigDecimal,
shamardy marked this conversation as resolved.
Show resolved Hide resolved
/// Additional reward for maker (premium)
pub premium_amount: BigDecimal,
/// Actual volume of taker's payment
Expand All @@ -1280,7 +1282,7 @@ pub struct RefundFundingSecretArgs<'a, Coin: ParseCoinAssocTypes + ?Sized> {
}

/// Helper struct wrapping arguments for [TakerCoinSwapOpsV2::gen_taker_funding_spend_preimage]
pub struct GenTakerFundingSpendArgs<'a, Coin: ParseCoinAssocTypes + ?Sized> {
pub struct GenTakerPaymentPreimageArgs<'a, Coin: ParseCoinAssocTypes + ?Sized> {
/// Taker payment transaction serialized to raw bytes
pub funding_tx: &'a Coin::Tx,
/// Maker's pubkey
Expand All @@ -1295,6 +1297,8 @@ pub struct GenTakerFundingSpendArgs<'a, Coin: ParseCoinAssocTypes + ?Sized> {
pub taker_payment_time_lock: u64,
/// The hash of the secret generated by maker
pub maker_secret_hash: &'a [u8],
/// The extra fee added to the trading volume to cover the taker payment transaction fees
pub taker_payment_fee: BigDecimal,
shamardy marked this conversation as resolved.
Show resolved Hide resolved
}

/// Helper struct wrapping arguments for [TakerCoinSwapOpsV2::validate_taker_funding]
Expand All @@ -1309,6 +1313,8 @@ pub struct ValidateTakerFundingArgs<'a, Coin: ParseCoinAssocTypes + ?Sized> {
pub other_pub: &'a Coin::Pubkey,
/// DEX fee amount
pub dex_fee: &'a DexFee,
/// The extra fee added to the trading volume to cover the taker payment transaction fees
pub taker_payment_fee: BigDecimal,
shamardy marked this conversation as resolved.
Show resolved Hide resolved
/// Additional reward for maker (premium)
pub premium_amount: BigDecimal,
/// Actual volume of taker's payment
Expand Down Expand Up @@ -1417,6 +1423,8 @@ impl From<UtxoRpcError> for ValidateSwapV2TxError {
/// Enum covering error cases that can happen during taker funding spend preimage validation.
#[derive(Debug, Display, EnumFromStringify)]
pub enum ValidateTakerFundingSpendPreimageError {
/// Error during conversion of BigDecimal amount to coin's specific monetary units (satoshis, wei, etc.).
NumConversion(String),
/// Funding tx has no outputs
FundingTxNoOutputs,
/// Actual preimage fee is either too high or too small
Expand All @@ -1437,6 +1445,10 @@ pub enum ValidateTakerFundingSpendPreimageError {
Rpc(String),
}

impl From<NumConversError> for ValidateTakerFundingSpendPreimageError {
fn from(err: NumConversError) -> Self { ValidateTakerFundingSpendPreimageError::NumConversion(err.to_string()) }
}

impl From<TxGenError> for ValidateTakerFundingSpendPreimageError {
fn from(err: TxGenError) -> Self { ValidateTakerFundingSpendPreimageError::TxGenError(format!("{:?}", err)) }
}
Expand Down Expand Up @@ -1667,6 +1679,12 @@ pub trait MakerCoinSwapOpsV2: ParseCoinAssocTypes + Send + Sync + 'static {

/// Spend maker payment transaction
async fn spend_maker_payment_v2(&self, args: SpendMakerPaymentArgs<'_, Self>) -> Result<Self::Tx, TransactionErr>;

/// Get the fee to be paid for the processing of the maker payment transaction.
async fn get_maker_payment_fee(&self, stage: &FeeApproxStage) -> TradePreimageResult<TradeFee>;

/// Get the fee to be paid for the processing of the maker payment spend transaction aka the claimation transaction.
shamardy marked this conversation as resolved.
Show resolved Hide resolved
async fn get_maker_payment_spend_fee(&self, stage: &FeeApproxStage) -> TradePreimageResult<TradeFee>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why would we need the get_maker_payment_spend_fee function in the future? I understand that get_maker_payment_fee will be used so that the taker can cover this fee on the funding transaction, but I don't see a need for get_maker_payment_spend_fee

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this is just to calculate fee from the taker side in a right way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it should be used here:

let maker_payment_spend_fee = match state_machine.maker_coin.get_receiver_trade_fee(stage).compat().await {

In place of get_receiver_trade_fee, but I am trying to figure out why didn't i use it O_O

}

#[async_trait]
Expand Down Expand Up @@ -1809,24 +1827,24 @@ pub trait TakerCoinSwapOpsV2: ParseCoinAssocTypes + Send + Sync + 'static {
) -> Result<Option<FundingTxSpend<Self>>, SearchForFundingSpendErr>;

/// Generates and signs a preimage spending funding tx to the combined taker payment
async fn gen_taker_funding_spend_preimage(
async fn gen_taker_payment_preimage(
&self,
args: &GenTakerFundingSpendArgs<'_, Self>,
args: &GenTakerPaymentPreimageArgs<'_, Self>,
swap_unique_data: &[u8],
) -> GenPreimageResult<Self>;

/// Validates taker funding spend preimage generated and signed by maker
async fn validate_taker_funding_spend_preimage(
async fn validate_taker_payment_preimage(
&self,
gen_args: &GenTakerFundingSpendArgs<'_, Self>,
gen_args: &GenTakerPaymentPreimageArgs<'_, Self>,
preimage: &TxPreimageWithSig<Self>,
) -> ValidateTakerFundingSpendPreimageResult;

/// Generates and signs a preimage spending funding tx to the combined taker payment
async fn sign_and_send_taker_funding_spend(
async fn sign_and_send_taker_payment(
&self,
preimage: &TxPreimageWithSig<Self>,
args: &GenTakerFundingSpendArgs<'_, Self>,
args: &GenTakerPaymentPreimageArgs<'_, Self>,
swap_unique_data: &[u8],
) -> Result<Self::Tx, TransactionErr>;

Expand Down Expand Up @@ -1867,6 +1885,25 @@ pub trait TakerCoinSwapOpsV2: ParseCoinAssocTypes + Send + Sync + 'static {

/// Derives an HTLC key-pair and returns a public key corresponding to that key.
fn derive_htlc_pubkey_v2(&self, swap_unique_data: &[u8]) -> Self::Pubkey;

/// Get the fee to be paid for the processing of the funding transaction.
/// Txs in success case (no refund) go as follows:
/// Chain A: funding -> taker payment -> taker payment spend (aka preimage) (to the maker & dex)
/// Chain B: maker payment -> maker payment spend (aka claimation) (to the taker)
async fn get_funding_fee(&self, stage: &FeeApproxStage) -> TradePreimageResult<TradeFee>;

/// Get the fee to be paid for the processing of the funding-spend transaction aka taker payment transaction.
///
/// Don't use this method, use [TakerCoinSwapOpsV2::get_taker_payment_fee] instead.
async fn get_funding_spend_fee(&self, stage: &FeeApproxStage) -> TradePreimageResult<TradeFee> {
self.get_taker_payment_fee(stage).await
}

/// Get the fee to be paid for the processing of the taker payment transaction.
async fn get_taker_payment_fee(&self, stage: &FeeApproxStage) -> TradePreimageResult<TradeFee>;
shamardy marked this conversation as resolved.
Show resolved Hide resolved

/// Get the fee to be paid for the processing of the taker payment spend transaction aka the preimage transaction.
async fn get_taker_payment_spend_fee(&self, stage: &FeeApproxStage) -> TradePreimageResult<TradeFee>;
}

/// Operations that coins have independently from the MarketMaker.
Expand Down
20 changes: 13 additions & 7 deletions mm2src/coins/test_coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::{CoinBalance, FundingTxSpend, HistorySyncState, MarketCoinOps, MmCoin
WaitForTakerPaymentSpendError};
use crate::coin_errors::ValidatePaymentResult;
use crate::{coin_errors::MyAddressError, BalanceFut, CanRefundHtlc, CheckIfMyPaymentSentArgs, CoinFutSpawner,
ConfirmPaymentInput, FeeApproxStage, FoundSwapTxSpend, GenPreimageResult, GenTakerFundingSpendArgs,
ConfirmPaymentInput, FeeApproxStage, FoundSwapTxSpend, GenPreimageResult, GenTakerPaymentPreimageArgs,
GenTakerPaymentSpendArgs, MakerSwapTakerCoin, MmCoinEnum, NegotiateSwapContractAddrErr,
ParseCoinAssocTypes, PaymentInstructionArgs, PaymentInstructions, PaymentInstructionsErr,
RawTransactionResult, RefundFundingSecretArgs, RefundPaymentArgs, RefundResult, SearchForSwapTxSpendInput,
Expand Down Expand Up @@ -492,26 +492,26 @@ impl TakerCoinSwapOpsV2 for TestCoin {
todo!()
}

async fn gen_taker_funding_spend_preimage(
async fn gen_taker_payment_preimage(
&self,
args: &GenTakerFundingSpendArgs<'_, Self>,
args: &GenTakerPaymentPreimageArgs<'_, Self>,
swap_unique_data: &[u8],
) -> GenPreimageResult<Self> {
todo!()
}

async fn validate_taker_funding_spend_preimage(
async fn validate_taker_payment_preimage(
&self,
gen_args: &GenTakerFundingSpendArgs<'_, Self>,
gen_args: &GenTakerPaymentPreimageArgs<'_, Self>,
preimage: &TxPreimageWithSig<Self>,
) -> ValidateTakerFundingSpendPreimageResult {
todo!()
}

async fn sign_and_send_taker_funding_spend(
async fn sign_and_send_taker_payment(
&self,
preimage: &TxPreimageWithSig<Self>,
args: &GenTakerFundingSpendArgs<'_, Self>,
args: &GenTakerPaymentPreimageArgs<'_, Self>,
swap_unique_data: &[u8],
) -> Result<Self::Tx, TransactionErr> {
todo!()
Expand Down Expand Up @@ -557,4 +557,10 @@ impl TakerCoinSwapOpsV2 for TestCoin {
}

fn derive_htlc_pubkey_v2(&self, swap_unique_data: &[u8]) -> Self::Pubkey { todo!() }

async fn get_funding_fee(&self, stage: &FeeApproxStage) -> TradePreimageResult<TradeFee> { todo!() }

async fn get_taker_payment_fee(&self, stage: &FeeApproxStage) -> TradePreimageResult<TradeFee> { todo!() }

async fn get_taker_payment_spend_fee(&self, stage: &FeeApproxStage) -> TradePreimageResult<TradeFee> { todo!() }
}
2 changes: 1 addition & 1 deletion mm2src/coins/utxo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ pub struct UtxoCoinConf {
/// Address and privkey checksum type
pub checksum_type: ChecksumType,
/// Fork id used in sighash
pub fork_id: u32,
pub fork_id: u8,
/// Signature version
pub signature_version: SignatureVersion,
pub required_confirmations: AtomicU64,
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 @@ -1018,7 +1018,7 @@ impl UtxoSignerOps for QtumCoin {
})
}

fn fork_id(&self) -> u32 { self.utxo_arc.conf.fork_id }
fn fork_id(&self) -> u8 { self.utxo_arc.conf.fork_id }

fn branch_id(&self) -> u32 { self.utxo_arc.conf.consensus_branch_id }

Expand Down
18 changes: 10 additions & 8 deletions mm2src/coins/utxo/utxo_builder/utxo_conf_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ pub enum UtxoConfError {
InvalidVersionGroupId(String),
InvalidAddressFormat(String),
InvalidDecimals(String),
#[display(fmt = "Invalid fork id: {}, valid fork ids are values between 0x00 - 0xff", _0)]
InvalidForkId(String),
}

impl From<Bip32Error> for UtxoConfError {
Expand Down Expand Up @@ -91,8 +93,8 @@ impl<'a> UtxoConfBuilder<'a> {
let tx_fee_volatility_percent = self.tx_fee_volatility_percent();
let version_group_id = self.version_group_id(tx_version, overwintered)?;
let consensus_branch_id = self.consensus_branch_id(tx_version)?;
let signature_version = self.signature_version();
let fork_id = self.fork_id();
let signature_version = self.signature_version()?;
let fork_id = self.fork_id()?;

// should be sufficient to detect zcash by overwintered flag
let zcash = overwintered;
Expand Down Expand Up @@ -244,23 +246,23 @@ impl<'a> UtxoConfBuilder<'a> {
Ok(consensus_branch_id)
}

fn signature_version(&self) -> SignatureVersion {
let default_signature_version = if self.ticker == "BCH" || self.fork_id() != 0 {
fn signature_version(&self) -> UtxoConfResult<SignatureVersion> {
let default_signature_version = if self.ticker == "BCH" || self.fork_id()? != 0 {
SignatureVersion::ForkId
} else {
SignatureVersion::Base
};
json::from_value(self.conf["signature_version"].clone()).unwrap_or(default_signature_version)
Ok(json::from_value(self.conf["signature_version"].clone()).unwrap_or(default_signature_version))
}

fn fork_id(&self) -> u32 {
fn fork_id(&self) -> UtxoConfResult<u8> {
let default_fork_id = match self.ticker {
"BCH" => "0x40",
_ => "0x0",
};
let hex_string = self.conf["fork_id"].as_str().unwrap_or(default_fork_id);
let fork_id = u32::from_str_radix(hex_string.trim_start_matches("0x"), 16).unwrap();
fork_id
u8::from_str_radix(hex_string.trim_start_matches("0x"), 16)
.map_to_mm(|e| UtxoConfError::InvalidForkId(e.to_string()))
}

fn required_confirmations(&self) -> u64 {
Expand Down
Loading
Loading