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(ETH): add eth access list (experimental) #2170

Open
wants to merge 25 commits into
base: dev
Choose a base branch
from
Open

Conversation

dimxy
Copy link
Collaborator

@dimxy dimxy commented Jul 21, 2024

This is an experimental PR to evaluate whether we can use EIP-2930 access lists.
EVM access lists (added to a transaction calling a contract) may reduce gas consumption when a contract is called. Access list specified addresses and EVM slots which this contract is used and EVM preloads them in memory. EVM operations with such preloaded objects are charged with less gas.

This PR allows to use access lists to swap transactions. The access lists are created with eth_createAccessList rpc with some customisation (namely, removing an access list item pointing to the etomic contract itself which does not reduce gas). Tests show that such customised lists provide saving of 200 to 400 gas units for a call.

To use access lists in swaps a bool "use_access_list" param is added to coins.
I added printing of tx gasUsed in one docker eth test where one can see gas with or without access list (by changing "use_access_list" param in eth test coin config).

attn: @cipig

Also added:
'version' to swap negotiation protocol. New features (eip1559 or access list) won't be used in swap txns if other node has older version.

Fixed:
getting max_eth_tx_type and gas_fee_estimator params from the platform coin if enable_eth_with_tokens rpc is used to activate coins (previously worked only when enable rpc). Problems solved: gas fee estimator did not start and eip1559 txns were not created.

…etamask, add conf to use access list, edit lists to remove contract address item (no gas reducing)
* dev:
  feat(nft-swap): add standalone maker contract and proxy support (#2100)
  feat(ETH): add `gas_limit` coins param to override default values (#2137)
  feat(tendermint): implement better sequence resolving logic (#2164)
  ci(artifact): add target for macos on apple silicon (#2163)
  fix(helpers): extend http to ws address conversion (#2166)
  fix(makerbot): add "testcoin" to provider options (#2161)
  fix(hd_wallet): make extended pubkey of hd wallet generic (#2159)
  fix(docker-tests): implement containers runtime directories (#2162)
  feat(tendermint): improve the `max` handling for tendermint withdraw (#2155)
  revert #2158 (comment) (#2160)
  ci(artifacts): upload build artifacts with in-tree script (#2158)
  test(tendermint): migrate to local/offline containerized testnets (#2128)
  use easingthemes/ssh-deploy@v5.0.3 for all builds except windows (#2157)
  chore(bin): rename mm2 binaries to kdf (#2126)
@dimxy dimxy added the in progress Changes will be made from the author label Jul 21, 2024
* dev:
  chore(mm2_main): replace lib.rs by mm2.rs as the root lib (#2178)
  chore(release): bump mm2 version to 2.2.0-beta (#2188)
  ci(docker-tests): ignore tendermint IBC tests for now (#2185)
  feat(nft-swap): complete refund methods (#2129)
  chore(release): add changelog entries for v2.1.0-beta (#2165)
  fix(zcoin): don't force low r signing to generate htlc pubkey for zcoin (#2184)
  chore(rust-analyzer): add rust-analyzer into the workspace toolchain (#2179)
  chore: migrate .cargo/config to .cargo/config.toml to avoid deprecation warning (#2177)
  fix(swaps): ensure taker payment spend confirmations (#2176)
Copy link
Member

@cipig cipig left a comment

Choose a reason for hiding this comment

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

using this on all my makers... works fine, also with old takers

@dimxy
Copy link
Collaborator Author

dimxy commented Aug 21, 2024

This PR becomes important because evm tx type activation by version is added here - allows to do evm swaps with old nodes.
Also a few fixes for that code related to settings in the coins file was added here.
@shamardy

@smk762
Copy link

smk762 commented Sep 13, 2024

I tried testing this for KomodoPlatform/komodo-docs-mdx#293 but I end up with

{
    "mmrpc": "2.0",
    "error": "Gas fee estimation not supported for this coin",
    "error_path": "get_estimated_fees",
    "error_trace": "get_estimated_fees:206]",
    "error_type": "CoinNotSupported",
    "id": null
}

from request

{
    "userpass": "{{userpass}}",
    "mmrpc": "2.0",
    "method": "start_eth_fee_estimator",
    "params": {
        "coin": "ETH"
    }
    // "id": null // Accepted values: Integers
}

Is something needed in the coins file, or specific params needed during the coin activation? Which coins are (or will be) supported? I've attempted with both enable and enable_eth_with_tokens methods on 2.2.0-beta_35de663

@dimxy
Copy link
Collaborator Author

dimxy commented Sep 13, 2024

Is something needed in the coins file, or specific params needed during the coin activation?

Gas fee estimator is supported for ETH-like coins with EIP-1559 implemented.
To enabled it platform coin ('ETH') should have param: "gas_fee_estimator": "provider" (or "simple")
If "provider" is set then MM2.json needs a config for the provider:

"gas_api": {"provider": "Blocknative", "url": "https://api.blocknative.com"},

(btw proposals to improve this or make similar to other configs appreciated)

* dev:
  chore(RPCs): rename `get_peers_info` RPC to `get_directly_connected_peers` (#2195)
  chore(WASM-builds): remove `wasm-opt` overriding (#2200)
  fix(coins): add p2p feature to mm2_net dependency (#2210)
  chore(test): turn on debug assertion (#2204)
  feat(sia): extract sia lib to external repo (#2167)
  feat(eth-swap): eth tpu v2 methods, eth docker test enhancements (#2169)
  fix(cors): allow OPTIONS request to KDF server (#2191)
  docs(README): update commit badges to use dev branch (#2193)
  use default value for `komodo_proxy` (#2192)
  feat(cosmos): komodo-defi-proxy support (#2173)
@dimxy dimxy added under review and removed in progress Changes will be made from the author labels Sep 23, 2024
@KomodoPlatform KomodoPlatform deleted a comment from Ethiqque Sep 26, 2024
* dev:
  fix(orders): fix cancel order race condition using time-based cache (#2232)
  fix(legacy-swap): taker failed spend maker payment marked as failed (#2199)
  chore(adex-cli): deprecate adex-cli (#2234)
  feat(new-RPC): connection healthcheck implementation for peers  (#2194)
  fix(proxy-signature): add message lifetime overflows (#2233)
  feat(CI): handle remote files in a safer way (#2217)
  chore(doc): update issue address in README (#2227)
  fix(merge): remove duplicated db_root function (#2229)
  feat(wallets): add `get_wallet_names` rpc (#2202)
  chore(tests): don't use `.wait()` and use `block_on` instead (#2220)
  fix(native-rpc): remove escaped response body (#2219)
  fix(clippy): fix coins mod clippy warnings in wasm (#2224)
  feat(core): handling CTRL-C signal with graceful shutdown (#2213)
  docs(README): fix typos (#2212)
  remove the non-sense arguments (#2216)
  fix(db): stop creating the all-zeroes dir on KDF start (#2218)
* dev:
  feat(cosmos-offline-tests): prepare IBC channels inside the container  (#2246)
  fix(cosmos): fix tx broadcasting error (#2238)
  chore(solana): remove solana implementation (#2239)
  chore(cli): remove leftover subcommands from help message (#2235)
@laruh
Copy link
Member

laruh commented Nov 6, 2024

@dimxy @shamardy could you please clarify what the priority of this feature (when its preferable to have it in production)? I see this pr is focusing on legacy swap.
also @dimxy fix the conflicts please if you think that this feature is r2r

@dimxy
Copy link
Collaborator Author

dimxy commented Nov 7, 2024

@dimxy @shamardy could you please clarify what the priority of this feature (when its preferable to have it in production)? I see this pr is focusing on legacy swap. also @dimxy fix the conflicts please if you think that this feature is r2r

This PR initially was optional, for studying eth access lists in swaps.
However when testing it I realised that we need remote-node-version activated support for eip2930/eip1559 eth transactions (this is needed not only for this PR but for already merged code for priority fee). So I added this version activation in this PR and would like to have it merged

* dev:
  fix(nft): add token_id field to the tx history primary key, fix balance (#2209)
  feat(cosmos): support IBC types in tx history implementation (#2245)
  fix(hd-wallet): use `CoinBalanceMap` for UTXO and QTUM (#2259)
  fix(tests): add more sepolia endpoints in tests (#2262)
  fix(legacy-swap): check for confirmations on recover taker (#2242)
  fix(legacy-swap): remove the need for takers to confirm their payment (#2249)
  refactor(P2P): types and modules (#2256)
  fix(evm): correctly display eth addr in iguana v2 activation result (#2254)
  feat(utxo): prioritize electrum connections (#1966)
  refactor(SwapOps): make all methods async (#2251)
  refactor(SwapOps): make `send_maker_payment` async (#2250)
  remove old p2p implementation (#2248)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants