[enhancement] global fee calculation

[ca333]

current behaviour: network fees are much higher than the actual recommended/required network fee. This applies to normal withdraw transactions and swaps - and affects pretty much all assets.

reproduction steps: create transaction / swap and compare relevant transaction fee(s) with other wallets (same asset).
tested wallets: ledger, trezor, metamask, keplr

expected behaviour: API evaluates the fee based on actual network metrics / simulations and uses a dynamic value (propose 3 levels: low priority, default/normal priority, high priority). Furthermore, and as part of the tx-preimage gen or TX preparation / simulation, we want to provide additional information such as the size of the transaction (in bytes), the actual fee and price per byte (sat per byte, ...) etc. and in order to align with other implementations.

ultimate goal: We need to reliably propose a affordable and competitive TX fee - for all assets and all transaction types.

related:
#710
#1847
#1835
https://docs.cosmos.network/v0.47/basics/gas-fees
https://docs.cosmos.network/main/run-node/txs#simulating-a-transaction

[shamardy]

Thanks a lot for bringing our attention to this very important enhancement/fix. I already started fixing UTXO fees here #1842, I shall continue working on this and fixing fees for other coin types in the upcoming sprints.

expected behaviour: API evaluates the fee based on actual network metrics / simulations and uses a dynamic value (propose 3 levels: low priority, default/normal priority, high priority).

We can borrow the approach used for lightning channels/on-chain operations where we specify a target number of blocks for every target/priority to make this configurable
https://github.com/KomodoPlatform/atomicDEX-API/blob/ebdc8c214c2e4b5d5a6f02b356b679a1130199e8/mm2src/coins/lightning/ln_conf.rs#L4-L9

For swaps, I am not sure if it's the right approach to allow priorities other than high priority since locktimes are involved and we don't want swap transactions to take time to confirm specially the spending transactions.

Furthermore, and as part of the tx-preimage gen or TX preparation / simulation, we want to provide additional information such as the size of the transaction (in bytes), the actual fee and price per byte (sat per byte, ...) etc. and in order to align with other implementations.

As far as I know, transaction size is only important for UTXO fees, we can add that as part of the UTXO transaction details.

ultimate goal: We need to reliably propose a affordable and competitive TX fee - for all assets and all transaction types.

Totally agree, in addition to what was proposed in this issue, we should also try to optimize the etomic swap contract to use less fees if possible.

[cipig]

the expected fees we show for EVM swaps are higher then the actual fees used in the real tx: #853

[cipig]

related #854

[laruh]

Hi
I was working on tx fees for NFTs and found that we dont do check for eip-1559 transactions when we save Eth history in database (despite the fact that currently GUI dont use my history for eth).

Right now we send two requests to calculate total_fee: transaction() to get Transaction structure and transaction_receipt() to get Receipt.
We use gas_used and gas_price fields from these structures to calculate fee, it is legacy calculation.
https://github.com/KomodoPlatform/komodo-defi-framework/blob/main/mm2src/coins/eth.rs#L2460

let web3_tx = match self
                    .web3
                    .eth()
                    .transaction(TransactionId::Hash(trace.transaction_hash.unwrap()))
                    .await
                {
                    Ok(tx) => tx,
                    Err(e) => {
                        ctx.log.log(
                            "",
                            &[&"tx_history", &self.ticker],
                            &ERRL!(
                                "Error {} on getting transaction {:?}",
                                e,
                                trace.transaction_hash.unwrap()
                            ),
                        );
                        continue;
                    },
                };
                let web3_tx = match web3_tx {
                    Some(t) => t,
                    None => {
                        ctx.log.log(
                            "",
                            &[&"tx_history", &self.ticker],
                            &ERRL!("No such transaction {:?}", trace.transaction_hash.unwrap()),
                        );
                        continue;
                    },
                };

                mm_counter!(ctx.metrics, "tx.history.response.count", 1, "coin" => self.ticker.clone(), "method" => "tx_detail_by_hash");

                let receipt = match self
                    .web3
                    .eth()
                    .transaction_receipt(trace.transaction_hash.unwrap())
                    .await
                {
                    Ok(r) => r,
                    Err(e) => {
                        ctx.log.log(
                            "",
                            &[&"tx_history", &self.ticker],
                            &ERRL!(
                                "Error {} on getting transaction {:?} receipt",
                                e,
                                trace.transaction_hash.unwrap()
                            ),
                        );
                        continue;
                    },
                };
                let fee_coin = match &self.coin_type {
                    EthCoinType::Eth => self.ticker(),
                    EthCoinType::Erc20 { platform, .. } => platform.as_str(),
                };
                let fee_details: Option<EthTxFeeDetails> = match receipt {
                    Some(r) => {
                        let gas_used = r.gas_used.unwrap_or_default();
                        let gas_price = web3_tx.gas_price.unwrap_or_default();
                        // It's relatively safe to unwrap `EthTxFeeDetails::new` as it may fail
                        // due to `u256_to_big_decimal` only.
                        // Also TX history is not used by any GUI and has significant disadvantages.
                        Some(EthTxFeeDetails::new(gas_used, gas_price, fee_coin).unwrap())
                    },
                    None => None,
                };

Although eip-1559 implies backward compatibility

Legacy Ethereum transactions will still work and be included in blocks, but they will not benefit directly from the new pricing system. This is due to the fact that upgrading from legacy transactions to new transactions results in the legacy transaction’s gas_price entirely being consumed either by the base_fee_per_gas and the priority_fee_per_gas.

we can use effective_gas_price value right from Receipt structure to get total Fee: gas_used * effective_gas_price and not to send redundant request transaction(TransactionId::Hash(..)) to get Transaction if effective_gas_price from Receipt is not None.

[smk762]

Relating to the high tendermint fees - these should be able to be derived from https://docs.ethermint.zone/basics/gas.html
As a short term solution for https://github.com/KomodoPlatform/komodo-wallet-desktop/issues/2371 I've set a static "custom" fee in legacy desktop via https://github.com/KomodoPlatform/komodo-wallet-desktop/pull/2386

[dimxy]

TODO: add rpc to set custom gas limit for swap evm calls

[cipig]

TODO: add rpc to set custom gas limit for swap evm calls

May i ask why we need/want that? It sounds dangerous to me... if you set the limits too low, tx will be reverted... if you are the maker and takerpaymentspend is reverted, the swap is not even shown as failed on maker, only on taker... so if you don't watch the swap live, you will not notice that if failed... to refund those swaps, you also need to edit the swap JSON, remove all events after makerpayment from it and refund manually.

[dimxy]

TODO: add rpc to set custom gas limit for swap evm calls

May i ask why we need/want that? It sounds dangerous to me... if you set the limits too low, tx will be reverted... if you are the maker and takerpaymentspend is reverted, the swap is not even shown as failed on maker, only on taker... so if you don't watch the swap live, you will not notice that if failed... to refund those swaps, you also need to edit the swap JSON, remove all events after makerpayment from it and refund manually.

The idea was if a swap failed with low gas the GUI may suggest to the user to increase gas limit.
I agree that it would not be easy for a user to understand on which step the swap failed and where more gas is needed.
Maybe GUI may ask swap tx failed due to low gas. Do you want to try again with 25% or 50% more gas? (select percentage)

[cipig]

problem is that it depends on the other side too... if you are taker and maker has set the limits too low (so that maker can't spend takerpayment), then trying again as taker will lead to the same result... so it only works if one of your own txes is reverted
this happens on taker in such cases:

(swap failed and refunded)
on maker, the swap shows up as successful and is not refunded

[dimxy]

Internally we discussed that this feature can be helpful for extra flexibility with gas management. If your experience suggests it would create more problems than solve, let's not do it.

[cipig]

the feature to try same swap with higher limits can be useful, i just wanted to point out that it will not always work (because other side needs to change the settings too) and that we will need to fix the problem that such failed swaps show up as successful on maker... simply check 28663386-8d19-4d4a-97a2-ecece9398155... failed and refunded on taker, successful on maker, even though he couldn't spend takerpayment because of too low gas limit
from maker:

      {
         "event" : {
            "data" : {
               "tx_hash" : "2d4045feb2f25e0eb351fca6e9ea1a9bba851c178c6e2552a65947377712640b",
               "tx_hex" : "f9010e82022d8506fc23ac1d83013880949130b257d37a52e52f21054c4da3450c72f595ce80b8a402ed292b021257b6f520b2abd747e1d8baaa228c27c17d970107ef7af88259571c2f3d5f0000000000000000000000000000000000000000000000001322805b47bcc1489076b8822991ae2b0a462bfd516b83c63a95920af539b462754c128943e2de740000000000000000000000009de41aff9f55219d5bf4359f167d1d0c772a396d00000000000000000000000015c577f4b9fbe66735a9bff9b0b5d3ea2854bfaa820135a0a6c63fe398930eb97eafb52d68fce4b11ee7915a03c1c44bb4d2455c4b5b3f34a05ec0968cfc975c6023d8068b4e03215f8359e8347734cc80a87c1e72924baa97"
            },
            "type" : "TakerPaymentSpent"
         },
         "timestamp" : 1721773185624
      },

while the tx is actually reverted: https://polygonscan.com/tx/0x2d4045feb2f25e0eb351fca6e9ea1a9bba851c178c6e2552a65947377712640b