Mm2.1 refactor get trade fee #797

sergeyboyko0791 · 2021-01-26T11:43:42Z

Add trade_preimage RPC call that returns the most approximated required fees that should be paid per whole swap
Refactor max_taker_vol using new get_sender_trade_fee, get_receiver_trade_fee, get_fee_to_send_taker_fee

* Implement the get_sender_trade_fee, get_receiver_trade_fee for UTXO

…er_trade_fee * Replace can_i_spend_other_payment implementation with get_receiver_trade_fee

…or UTXO

…erations * Refactor max_taker_vol * Take the trade fee into account on locked_amount called * Move the check_balance_for_[maker/taker]_swap out into additional check * Add Traceable trait * Refactor get_locked_amount, get_locked_amount_by_other_swaps

* Replace TradePreimageValue::Max with TradePreimageValue::UpperBound * Fix and expand test_max_taker_vol_dynamic_trade_fee (replace into docker_tests::qrc20_tests) * Fix poisoning of docker_tests

* Replace 'trade_preimage' into lp_swap.rs * Add 'dust' field to coins config

* Replace trade fees from [Maker/Taker]SwapMut to [Maker/Taker]SwapData

* Add check_base_coin_balance_for_swap

* Separate check_coin_balance_for_swap into check_my_coin_balance_for_swap and check_other_coin_balance_for_swap * Add prepared trade_fee

…lled for QRC20 coin

artemii235

Thanks for the PR! It is first review iteration :)

mm2src/coins/utxo/utxo_common.rs

mm2src/coins/eth.rs

* Change volume within trade_preimage response from BigDecimal to MmNumber * Replace try_other with try_map within generate_transaction * Add the test_trade_preimage_fee_includes_change_output_anyway test

* Add UtxoCommonOps::preimage_trade_fee_required_to_send_outputs that encapsulate the change output fee

…onal) * Replace Address::default() with the declared fee_addr_pub_key

artemii235

Review iteration number 2 🙂

mm2src/coins/eth.rs

mm2src/coins/qrc20.rs

…06 pubkey into the global constant

artemii235

@sergeyboyko0791
Approved. Thanks a lot for your work on this!

sergeyboyko0791 · 2021-02-02T07:06:39Z

I think the changes have to be tested, because they could break the functionality of the mm2.
The most important cases:

Please note these cases should be tested for both dynamic and fixed transaction fee, also please test it for ETH and QRC20 coins

Call max_taker_vol and pass the volume as an argument of the sell/buy requests. It's desirable that the wallet balance become a zero after the swap is finished;
setprice with the max: true field should not fail (if the base and rel coin balances are not locked and sufficient for swap). Swap with the max volume should not fail (this must be guaranteed by the balance checking on the setprice RPC call).
buy, sell, setprice check the base coin balance correctly (ERC20, QRC20);
buy, sell, setprice take into account amount locked by other running swaps;
ETH, ERC20, QRC20 coins pay fee to spend other payment, so it will be nice to check if the eg ETH balance is locked by other swap (or is not sufficient to spend other payment), then buy, sell, setprice must fail.

cipig · 2021-02-03T00:55:41Z

https://dexapi.cipig.net/public/error.php?uuid=753e7602-fdd0-4e5b-a9ad-893a957cb065
Not sufficient balance: lp_swap:556] The total required KMD amount 22350.78678807672891565175067354475431222440756107137493599289272318350995977730050138146788426429371 is larger than available 22350.78678769
with latest mm2 (12cf6a0) on maker
the price was set with setprice and max:true

artemii235 · 2021-02-03T06:38:08Z

The swap started with

"maker_amount": "22350.78677807672891565175067354475431222440756107137493599289272318350995977730050138146788426429371"

somehow, it shouldn't have matched.

@sergeyboyko0791 Could you please check how it could happen? I've pushed the https://github.com/KomodoPlatform/atomicDEX-API/tree/mm2.1-fix-setprice-max branch with test_match_and_trade_max where I tried to recreate the problem. I hope it might help a bit.

sergeyboyko0791 · 2021-02-03T12:13:54Z

trade_preimage in docs PR

cipig · 2021-02-19T05:49:25Z

2 new KMD/BTC with maker_swap:298] !check_balance_for_maker_swap maker_swap:1486] Not sufficient balance: lp_swap:558] but on BTC side (makercoin is BTC)... both maker nodes are mine
https://dexapi.cipig.net/public/error.php?uuid=29c913a1-b9d7-4ab6-98b3-6843b994fbdc
https://dexapi.cipig.net/public/error.php?uuid=34143bf0-6576-4cad-8b86-5a20b1d5de7f

sergeyboyko0791 added 26 commits December 28, 2020 22:16

Add trade_preimage RPC call

2cdc954

* Implement the get_sender_trade_fee, get_receiver_trade_fee for UTXO

Merge branch 'mm2.1' into mm2.1-refactor-get-trade-fee

31831af

Impl get_sender_trade_preimage, get_sender_trade_preimage for QRC20 coin

a13fceb

Impl get_sender_trade_preimage, get_sender_trade_preimage for Eth, ERC20

08e7440

Add the QRC20 balance checking on get_sender_trade_fee()

3d291fc

Add the 'senderRefund' contract call fee for QRC20 tokens

b486994

Add the 'senderRefund' contract call fee for Eth, ERC20

a3b9b37

Add TradePreimageError is returned by get_sender_trade_fee/get_receiv…

441d067

…er_trade_fee * Replace can_i_spend_other_payment implementation with get_receiver_trade_fee

Add and implement get_fee_to_send_taker_fee() to all of coins

54a228c

Remove balance checking on get trade fee for QRC20

18a5774

Remove balance checking on get trade fee for Eth, ERC20

61a03ee

Add 'get_tx_fee' checking before the generate_transaction is called f…

2c94845

…or UTXO

Use estimate_gas by get_fee_to_send_taker_fee

a7de370

Merge branch 'mm2.1' into mm2.1-refactor-get-trade-fee

eaf93cb

Add test_check_balance_on_order_post_base_coin_locked

4f617c6

Fix max_taker_vol:

f8f3cfa

* Replace TradePreimageValue::Max with TradePreimageValue::UpperBound * Fix and expand test_max_taker_vol_dynamic_trade_fee (replace into docker_tests::qrc20_tests) * Fix poisoning of docker_tests

Change 'trade_preimage' RPC call signature

30775e3

* Replace 'trade_preimage' into lp_swap.rs * Add 'dust' field to coins config

Make locked_amount() non-async

4e7cab5

* Replace trade fees from [Maker/Taker]SwapMut to [Maker/Taker]SwapData

Avoid unnecessary balance checking on setprice max

0410980

* Add check_base_coin_balance_for_swap

Avoid duplicate get_[sender/receiver]_trade_fee calls on swap starting

a32c182

* Separate check_coin_balance_for_swap into check_my_coin_balance_for_swap and check_other_coin_balance_for_swap * Add prepared trade_fee

Change the trade_preimage request and response signatures

eb7eff7

Remove print_tx tests

fdbef74

Increase dynamic tx fee and gas price by specified percent

283c108

Add 'get_tx_fee' checking before the generate_qrc20_transaction is ca…

e2c7b97

…lled for QRC20 coin

Minor changes

ed9e5dd

sergeyboyko0791 requested a review from artemii235 January 27, 2021 06:15

sergeyboyko0791 mentioned this pull request Jan 27, 2021

Clarify the get_trade_fee meaning and calculate it differenty. #738

Closed

artemii235 reviewed Jan 28, 2021

View reviewed changes

Fix PR issues

61aae8d

* Change volume within trade_preimage response from BigDecimal to MmNumber * Replace try_other with try_map within generate_transaction * Add the test_trade_preimage_fee_includes_change_output_anyway test

sergeyboyko0791 added 2 commits January 31, 2021 21:22

Remove force_change_output argument from utxo::generate_transaction

1bee5c7

* Add UtxoCommonOps::preimage_trade_fee_required_to_send_outputs that encapsulate the change output fee

Rename AdditionalTxInfo::change into unused_change (also make it opti…

6d018fd

…onal) * Replace Address::default() with the declared fee_addr_pub_key

This was linked to issues Feb 1, 2021

Clarify the get_trade_fee meaning and calculate it differenty. #738

Closed

get_trade_fee shows 3 * higher fee for QRC20 #763

Closed

dust on LTC #778

Closed

BTC fees estimations displaying per kilobyte #710

Closed

Merge branch 'mm2.1' into mm2.1-refactor-get-trade-fee

6314c57

artemii235 reviewed Feb 1, 2021

View reviewed changes

mm2src/coins/eth.rs Outdated Show resolved Hide resolved

mm2src/coins/eth.rs Outdated Show resolved Hide resolved

mm2src/coins/eth.rs Outdated Show resolved Hide resolved

mm2src/coins/qrc20.rs Show resolved Hide resolved

sergeyboyko0791 added 4 commits February 1, 2021 16:41

Reduce the approve contract call gas limit to 50k

b2d75c7

Move 03bc2c7ba671bae4a6fc835244c9762b41647b9827d4780a89a949b984a8ddcc…

6b5fbee

…06 pubkey into the global constant

Do not rely on estimate_gas RPC error code

df6fa99

Minor change: add the comment

608b180

artemii235 approved these changes Feb 1, 2021

View reviewed changes

artemii235 merged commit 25272e5 into mm2.1 Feb 1, 2021

artemii235 deleted the mm2.1-refactor-get-trade-fee branch February 1, 2021 12:52

sergeyboyko0791 mentioned this pull request Feb 9, 2021

Add 'txfee_volatility_percent' for DOGE, DGB, FIRO coins KomodoPlatform/coins#103

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mm2.1 refactor get trade fee #797

Mm2.1 refactor get trade fee #797

sergeyboyko0791 commented Jan 26, 2021

artemii235 left a comment

artemii235 left a comment

artemii235 left a comment

sergeyboyko0791 commented Feb 2, 2021

cipig commented Feb 3, 2021 •

edited

Loading

artemii235 commented Feb 3, 2021

sergeyboyko0791 commented Feb 3, 2021

cipig commented Feb 19, 2021

Mm2.1 refactor get trade fee #797

Mm2.1 refactor get trade fee #797

Conversation

sergeyboyko0791 commented Jan 26, 2021

artemii235 left a comment

Choose a reason for hiding this comment

artemii235 left a comment

Choose a reason for hiding this comment

artemii235 left a comment

Choose a reason for hiding this comment

sergeyboyko0791 commented Feb 2, 2021

cipig commented Feb 3, 2021 • edited Loading

artemii235 commented Feb 3, 2021

sergeyboyko0791 commented Feb 3, 2021

cipig commented Feb 19, 2021

cipig commented Feb 3, 2021 •

edited

Loading