feat(ETH): eip1559 gas fee estimator and rpcs

[dimxy]

Add eth EIP1559 priority fee estimator (WiP):

• [x] simple priority gas fee estimator based on eth_feeHistory call
• [x] loop running the estimator with blocktime period and storing values in a ctx
• [x] RPCs to start/stop loop for a enabled eth coin (ETH, USDT)
• [x] RPC to get estimated values

Add tx priority fee support for withdraw and swaps, improve gas limit for swap txns:

• [x] add eip1559 max priorty fee and max base fee v2 props to eth txns for withdraw and swaps
• [ ] consider adding GUI notifications based on websocket streaming channel (developed in PR #2041).
• [x] add alternative gas fee estimates from an API provider (a paid service) - added as test calls. In real it will be service on a mm2 eth provider
• [x] improve gas limit calculation to ask less gas from user: we cannot use eth_estimateGas for our etomic contract before swap started (as token spending approval is needed) so we use different consts for different swap operations
• [ ] do real-world test and adjust new gas limit consts if needed
• [x] add tests for priority fee using the new dockerized geth node
• [x] merge eip1559 support in https://github.com/KomodoPlatform/rust-web3/pull/1 and https://github.com/KomodoPlatform/mm2-parity-ethereum/pull/1 and fix deps
• [x] add 'coins' param with transaction type (legacy, 1, 2) supported for different eth coins: ETH, ETC etc
• [x] decide where to have FeeEstimatorContext
• [ ] decide where to store gas api provider config (currently in MM2.json
• [x] make config which chains are supported for gas estimation with provider and which use only internal estimator (added 'gas_fee_estimator' in coins)
• [ ] add eip1559 for trezor (when PR #1979 merged)
• [ ] improve gas fee estimates update: use eth_subscribe to get estimates when a new block comes, make a config with interval value if a gas api provides frequent updates

This PR covers requirements in #1848 issue for Eth coin and ERC20 tokens, allowing more predictable priority-based gas fee and lesser gas limit for swap transactions

[dimxy]

I have a question about how the feature is supposed to work in general: is it really required to run the background loop and keep additional context? What are the benefits of this approach over using "on demand" approach?

This is essentially how other wallets work. I believe calls to a gas api provider could last relatively long plus, if estimates are not expected to change until a new block comes it's no reason to ping it more often. Also the context with estimated data is shared among all eth coins/tokens. Thus gui could fetch the recent results quickly, without forwarding calls to a provider

[artemii235]

I see, thanks for the explanation :slightly_smiling_face:

[laruh]

@dimxy please keep in mind, that PR started to have conflicts in eth.rs

[dimxy]

I have tried to simplify the estimator_loop as much as I could...

Thank you for this, studying... I understand this code could be much simple. The use case I tried to implement was like that:

• Fee estimation loop starts when user opens a page with a transaction creation and finishes when this page is closed.
• Also, user could open concurrently 2 or more pages for different eth tokens (for tx creation), so the same estimation loop should be running and shared between those tokens. When all pages closed the loop ends.

If this is an overkill we can certainly reduce my code and just allow for GUI to handle estimation itself (when it needs it)

[artemii235]

@dimxy Yeah, I understand the idea. The proposal code actually implements it in a bit different way, considering the following:

1. Once the loop is started for ETH, it implicitly enables estimation for all ERC20 tokens, with "platform_coin":"ETH". So there is no need to keep using_coins state in Rust. This should be documented, so GUI developers/users are aware.
2. run_state can be simplified anyway because there seem to be only Started or Stopped state.
3. abort_handler: AsyncMutex<Option<AbortOnDropHandle>> covers both "stop by RPC" and "application stop" cases: it is explicitly taken from FeeEstimatorContext on RPC call or dropped with the context when stop RPC is invoked. I used Arc<AsyncMutex<FeePerGasEstimated>> as fee_estimator_loop argument to ensure that it doesn't keep the additional reference to FeeEstimatorContext, preventing it from dropping on stop.
4. Also, (just IMHO) wrapping mpsc::Receiver in Arc<Mutex> is a potential design problem. The Single Consumer pattern is not intended to be concurrently used in multiple threads (explicitly marked !Sync).

[dimxy]

4. Also, (just IMHO) wrapping mpsc::Receiver in Arc<Mutex> is a potential design problem. The Single Consumer pattern is not intended to be concurrently used in multiple threads (explicitly marked !Sync).

I understand now Arc is not good here. I guess I should have used simply Mutex. Thank you for that.

[dimxy]

@dimxy Yeah, I understand the idea. The proposal code actually implements it in a bit different way..

Yes I think it's better to use your code and let the GUI to control fee per gas estimation running

[artemii235]

I understand now Arc is not good here. I guess I should have used simply Mutex. Thank you for that.

If possible, Mutex should be avoided too :slightly_smiling_face: The Receiver can be moved to the handling thread or future without wrappers. It has to be determined on case by case basis.

[dimxy]

I understand now Arc is not good here. I guess I should have used simply Mutex. Thank you for that.

If possible, Mutex should be avoided too 🙂 The Receiver can be moved to the handling thread or future without wrappers. It has to be determined on case by case basis.

For future reference: I believe this property !Sync for Receiver exists for std::sync::mpsc channel kind (and I used futures::channel::mpsc for which there is no such marker). I tried to make a test for std::sync::mpsc though: the compiler handles improper Receiver use pretty well, like wrapping it with Arc and sending Arc to a spawned thread. Besides, I guess !Sync does not prohibit sending Receiver itself to another thread (and compiler allowed to do this in my test). https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=82c1b216daddc1f104b15647739e4102)

[dimxy]

Latest commits:

• added eip1559 priority fee to eth withdraw and swaps. Priority fee is set for eth coin or token with "set_swap_transaction_fee_policy" rpc
• added customised gas limit consts for eth swap operations
• changed tx fee in trade_preimage rpc (TradePreimageResponse::base_coin_fee and rel_coin_fee): now it does not include tx fee for possible refund. But taker_max_vol rpc still returns total tx fee to prevent too low balance to pay for refund (TODO: add a test for this)

Note: maybe in future we will want to have custom gas limit for different tokens if there are special recommendations. For now I found TUSD suggests gas limit of 150K and currently this value used for all erc20, so for this time we do not need this param yet

[dimxy]

Thanks for the PR! I have a question

It's temporarily, I have added PRs with eip1559 support into our eth libs and temp use my repo until those PRs are merged

[dimxy]

Above there are few fixes on review notes, also implemented couple of todos (coins params whether gas provider and which tx type are supported) plus some refactor of gas limit constants

[laruh]

ps @dimxy pr started to have conflicts

[shamardy]

@dimxy please fix conflicts in this PR so that we can continue with the review.

[laruh]

@dimxy note that PR started to have conflicts 👀

[dimxy]

@dimxy note that PR started to have conflicts 👀

updated

[dimxy]

@KomodoPlatform/qa this PR adds support for eth EIP-1559 gas priority fee, for withdrawals and swaps.

Allow eip-1559 transactions for coins To allow eip-1559 transactions (which have type 2) there is a new "max_eth_tx_type" parameter in the coins file:

{
  "coin": "ETH",
  ...
  "chain_id": 1,
  "max_eth_tx_type": 2
}

Note, that the "chain_id" param must be also set, when the max_eth_tx_type is set to 2. (I guess we have recently set chain_id always mandatory).

Withdrawals with new priority fee per gas For withdrawals a new WithdrawFee::EthGasEip1559 option is added with max_priority_fee_per_gas and max_fee_per_gas fields (in gwei, BigDecimal). WithdrawFee::EthGasEip1559 variant also has a new "gas_option" enum field with EthGasLimitOption::Set(u64) or EthGasLimitOption::Calc options to set custom gas limit or make the API calculate it. See sample.

Setting swap policy to eip-1559 GUI can set to use the eip-1559 policy for swap transactions by a new "set_swap_transaction_fee_policy" rpc, with options to use Internal, Low, Medium and High policies. 'Internal' means to create legacy transactions with gas price obtained from eth nodes (default option). Low, Medium and High are eip-1559 tx priority levels. For eip-1559 variants MM2 will be querying the configured gas provider and use the gas fee values for the policy the user set with this rpc. There is also an "get_swap_transaction_fee_policy" rpc to query the currently set policy.

Getting suggested fee per gas There is a gas fee estimator added to API, which returns suggested gas fees for low, medium, high priority levels, either from supported gas api providers (Infura or BlockNative) or from an internal "simple" gas fee estimator based on fee history.

To configure the gas fee provider url a new param in MM2.json was added: "gas_api": {"provider": "Blocknative", "url": "https://api.provider.com"} or "gas_api": {"provider": "Infura", "url": "https://api.provider.com"} There is no configuration needed for the internal "simple" gas fee estimator, it needs only web3 api configured for the platform coin.

In coins file there is a new parameter added indicating which gas api provider should be used:

{
  "coin": "ETH",
  ...
  "gas_fee_estimator": estimator_type
}

where estimator_type could be "provider" or "simple".

When gas fee estimator configured, GUI should first start it with a "start_eth_fee_estimator" rpc with a "coin" param. (There is also "stop_eth_fee_estimator" to stop it when GUI does not need it). The estimator is always started for the platform coin (ETH) although in the start param it can be a token, so the platform coin must be first enabled for estimator to work. The running estimator periodically queries the configured gas fee provider or calls the simple estimator, if the gas fee provider fails.

GUI can query the suggested gas fees with "get_eth_estimated_fee_per_gas" rpc (with "coin" param) which returns a serialised FeePerGasEstimated struct with low, medium and high levels with max fees per gas and max priority fees.

New param in sign_raw_transaction rpc to support eip-1559 The "sign_raw_transaction" rpc supports eip-1559 transaction with a new optional "pay_for_gas" field in the request params:

"tx": {
            ...
            "gas_limit": "21000",
            "pay_for_gas": {
                "tx_type": "Eip1559",
                "max_fee_per_gas": "1234.567",
                "max_priority_fee_per_gas": "1.2",
            }
        } 

See sample. The "tx_type" param can be also set to "legacy" with old "gas_price":

"tx": {
            ...
            "gas_limit": "21000",
            "pay_for_gas": {
                "tx_type": "Legacy",
                "gas_price": "1234.567"
            }
        } 

[dimxy]

@KomodoPlatform/qa this PR changes trade fees as part of initiatives to solve problems with high transaction fees. This all affects ETH only.

"trade_preimage" rpc does not include refund fee The "trade_preimage" rpc now does not include refund fee in the transaction preimage trade fee. This is intended not to show too big transaction fee to the user, what may discourage him from starting the swap.

At the same time "max_taker_vol" and "max_maker_vol" rpc still include refund fee (what will prevent user from creation of a swap for his whole amount which he won't be able to refund back).

To discuss: maybe it is worth to add a param to the "trade_preimage" request to allow for GUI to choose to include or not the refund fee in the trade_fee. Or, maybe always return both trade and refund fees separately.

"get_trade_fee" rpc unchanged Note: there is yet another "get_trade_fee" rpc and it was not changed (calculated based on max trade gas of 150,000). Apparently this rpc is not used in the GUI.

Gas limits adjusted for swap operations Instead of using a constant of 150,000 multiple constants were added for various swap stages and coins/tokens. The new constants are here.

This all is experimental at this stage and needs to be tested and discussed.

[dimxy]

Please add comments documenting new coin configs and new APIs, or API changes for QA team.

done in https://github.com/KomodoPlatform/komodo-defi-framework/pull/2051#issuecomment-2117979019