fix(fee): remove fixed 0.0001 min threshold for TakerFee

[shamardy]

fixes #884

[cipig]

thanks for the fix, looks good

before:

dexfee 2.66 USD

after

dexfee 0.35 USD

swap worked fine

this was dexfee: https://polygonscan.com/tx/0x8c914293305b1eff4a2a462fd77af73467a9932b56e6187ee07e23229d56b054
0.00001325 ($0.35)

question: is this #1635 related? can we fix that too? i can't post orders with an amount lower 0.0101 WBTC-PLG20 atm, which is ~260 USD

[artemii235]

It's a great improvement!
Only one comment: dex_fee_threshold still exists in several tests' code. Is it worth renaming it to min_tx_amount for uniformity?

[shamardy]

dex_fee_threshold still exists in several tests' code. Is it worth renaming it to min_tx_amount for uniformity?

Done

[shamardy]

question: is this #1635 related? can we fix that too? i can't post orders with an amount lower 0.0101 WBTC-PLG20 atm, which is ~260 USD

It's unrelated to this PR, this is due to how minimum trading volume is calculated currently for EVM coins/tokens

komodo-defi-framework/mm2src/coins/eth.rs

Lines 2136 to 2139 in 51c44f6

	fn min_trading_vol(&self) -> MmNumber {
	let pow = self.decimals / 3;
	MmNumber::from(1) / MmNumber::from(10u64.pow(pow as u32))
	}

Since this is wrapped BTC, the decimals is 8 and the minimum is 0.01, we shouldn't depend on the decimal to calculate the minimum since what is of essence here is either the usd price or the gas price. We should work on this separately from this PR anyways, I see that @artemii235 suggested depending on the gas price in the past #873 (comment) when we didn't have the price service to get ERC20 price in relation to ETH, maybe we can look at this solution again or propose a different solution :)

[artemii235]

🔥

[artemii235]

@shamardy

It's unrelated to this PR, this is due to how minimum trading volume is calculated currently for EVM coins/tokens

let pow = self.decimals / 3; looks quite strange, but it turned out that I implemented it 2 years ago 😄 Now, I think it's not justified to have such constraint because it allows e.g. 0.000001 volume, which still has little sense to use due to fees. So, it doesn't really prevent users from doing absurd trades interfering sane $260 order.

Should we maybe just use let pow = self.decimals?

[shamardy]

Should we maybe just use let pow = self.decimals?

Sure, this has no drawbacks compared to the current approach, makers get to decide the minimum volume anyways using min_base_vol. I will change this in this PR.

[shamardy]

There is also this fixed minimum price but I will leave it until we face a problem related to it

komodo-defi-framework/mm2src/mm2_main/src/lp_ordermatch.rs

Line 1791 in 8fd29ec

let min_price = MmNumber::from(BigRational::new(1.into(), 100_000_000.into()));

@artemii235 I actually think that the validate price function should be removed completely but I want to get your opinion on this first.

[shamardy]

@cipig can you please check if this issue #1635 is fixed by the latest commits?

[cipig]

@cipig can you please check if this issue #1635 is fixed by the latest commits?

It's fixed, thanks a lot.

Could trade WBTC-PLG20 worth 25 USD:

[artemii235]

@shamardy

I actually think that the validate price function should be removed completely but I want to get your opinion on this first.

I agree because this threshold may prevent a real trade between e.g. BTC and a very cheap asset. But it's worth checking that the price is above zero, as MmNumber can be negative.

[artemii235]

One minor optimization 🙂

[shamardy]

But it's worth checking that the price is above zero, as MmNumber can be negative.

I kept the validate_price function but made the minimum allowed to be any value above zero.

[artemii235]

👍