feat(hd_wallet): utxo and evm hd wallet and trezor #1962

shamardy · 2023-09-07T22:44:15Z

Intermediate PR part of #1838

Full UTXO HD wallet functionalities are implemented. UTXO HD wallet now uses the same APIs used for Trezor except for withdraw were withdraw is used for HD wallet and task::withdraw for Trezor. APIs used for HD wallet:
- Activation:
  - task::enable_utxo
  - task::enable_qtum
- Accounts:
  - task::create_new_account
  - account_balance
  - task::account_balance
- Addresses:
  - get_new_address
  - task::scan_for_new_addresses
- Tx History:
  - my_tx_history v2
- Withdraw:
  - withdraw both v1 and v2
path_to_address in activation now uses HDAccountAddressId struct, StandardHDCoinAddress was removed to reduce redundancies.
- The account_id'/chain/address_id specified in activation by path_to_address is enabled/added during the activation.
task::create_new_account can now take an optional parameter account_id to specify the new account index instead of just incrementing the account index. If not specified, incrementing will be used as before.
WithdrawFrom also uses HDAccountAddressId instead of StandardHDCoinAddress, a full derivation path can be also used to specify the address to withdraw from, this makes specifying the address more easy since the full derivation path is what is returned in the response of activation/account_balance/etc..

… HD UTXO

…TXO coins

…actor

artemii235

Great work! I have only a few minor comments.

mm2src/coins/hd_wallet.rs

mm2src/coins/utxo/slp.rs

mm2src/mm2_main/src/wasm_tests.rs

mm2src/mm2_main/tests/mm2_tests/bch_and_slp_tests.rs

mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs

laruh

Great work!
Just want to add some doc coms for main types and have question also about doc coms for now.

mm2src/coins/utxo.rs

mm2src/coins/hd_wallet.rs

mm2src/coins/lp_coins.rs

mm2src/mm2_test_helpers/src/for_tests.rs

smk762 · 2024-02-28T08:29:17Z

@shamardy the method can_get_new_address which was previously documented now returns

{
    "mmrpc": "2.0",
    "error": "No such method",
    "error_path": "dispatcher",
    "error_trace": "dispatcher:217]",
    "error_type": "NoSuchMethod",
    "id": null
}

Can you please confirm if this has been renamed or removed?

smk762 · 2024-02-28T10:23:12Z

task::withdraw::status is returning an error like it expects the "from address" as part of the query, in addition to the task_id

Can you please confirm if this is correct (currently not documented) or a bug?

shamardy · 2024-02-28T17:33:39Z

the method can_get_new_address which was previously documented now returns, Can you please confirm if this has been renamed or removed?

It was never there before this PR, there is a get_new_address and task::get_new_address though.

shamardy · 2024-02-28T17:41:10Z

task::withdraw::status is returning an error like it expects the "from address" as part of the query, in addition to the task_id, Can you please confirm if this has been renamed or removed?

It expects from address as part of the task::withdraw::init request. I can modify this to use the enabled_address if from is not provided, what do you think?

shamardy · 2024-02-28T17:43:21Z

@smk762 Just a note, these 2 PRs #1979 , #2005 will get merged to this one once code review for them is finished, so you can postpone testing for now if you want to test HD functionality alongside trezor for both EVM and UTXO in the future.

smk762 · 2024-02-29T04:48:48Z

task::withdraw::status is returning an error like it expects the "from address" as part of the query, in addition to the task_id, Can you please confirm if this has been renamed or removed?

It expects from address as part of the task::withdraw::init request. I can modify this to use the enabled_address if from is not provided, what do you think?

I see - I was using an example that did not include this, though confirm docs have the from fields (and indicated as optional, HD wallets only). How is the enabled_address defined?

While testing task::withdraw::init, I got NoTrezorDeviceAvailable for HD wallet activated without Trezor (tho withdraw_v2 works).

HD-withdraw.mp4

Can you please confirm the following:

task::withdraw methods can only be used with hardware wallets (e.g. Trezor) and ZHTLC coins (regardless of how they were activated).
HD wallets not activated with a Trezor must use the withdraw v2 method (unless coin is ZHTLC).
Non-HD wallets can use withdraw v1 or v2 for all coins except ZHTLC.

If so, I'll add some extra notes to docs to reduce the confusion.

dimxy · 2024-04-09T11:56:24Z

mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs

@@ -311,23 +358,22 @@ pub trait UtxoFieldsWithHardwareWalletBuilder: UtxoCoinBuilderCommonOps {
        let recently_spent_outpoints = AsyncMutex::new(RecentlySpentOutPoints::new(my_script_pubkey));


I just have taken a look how RecentlySpentOutPoints works.
One of its tasks is that it allows to immediately spend an output sent to self ('my_address') from a just broadcasted transaction. This is needed because the electrum provider may not know immediately about just sent txns. Basically this was added for 'change' outputs. But I think the current RecentlySpentOutPoints implementation (when it is created only with the initially activated 'my_address') does not cover cases with HD accounts:

when a user withdraws from some HD account the change is sent to this 'from' account (not to 'my_address').

another case is when a user sends coins from one HD account to another his account.

(Not sure if we need this fixed though. Maybe it's okay for the user to just wait for a couple of min until the provider db is actualised)

It will be easier done in this PR #2053 or after it's merged where UnspentInfo has the script_pubkey. Add this to the checklist here #1838 (comment)

This commit implements all HD wallet and trezor methods for EVM coins/tokens. It also adds `init_platform_coin_with_tokens` and `init_token` methods for initializing EVM coins/tokens using task manager which is the prefered way for HD/Trezor.

shamardy · 2024-04-25T18:21:52Z

@KomodoPlatform/qa latest commits allow EVM coins/tokens to work with HD wallet and trezor methods same as how utxo works, so this methods/requests/responses #1962 (comment), #1962 (comment), #1962 (comment), #1962 (comment), #1962 (comment) should work the same for EVM. Transaction history is still not implemented for EVM so these #1962 (comment) should not work. New methods for EVM coins/tokens activation were added similar to these #1962 (comment) but differ in requests and responses, so I will detail them below.

Enable EVM coin with tokens in HD mode (or trezor):

Start mm2 with "enable_hd":true
task::enable_eth methods are the only methods that should be used to enable utxo in HD or trezor mode, since it will do address scanning and returns addresses and balances for the enabled accounts. Note that we use the task methods since scanning for balances and fetching them can take time.

`task::enable_eth::init`

Request

{
    "userpass": "{{userpass}}",
    "mmrpc": "2.0",
    "method": "task::enable_eth::init",
    "params": {
        "ticker": "MATIC",
        "gas_station_url": "https://gasstation-mainnet.matic.network/",
        "swap_contract_address": "0x9130b257D37A52E52F21054c4DA3450c72f595CE",
        "fallback_swap_contract": "0x9130b257D37A52E52F21054c4DA3450c72f595CE",
        "nodes": [
            {
                "url": "https://polygon-rpc.com"
            },
            {
                "url": "https://poly-rpc.gateway.pokt.network"
            }
        ],
        "erc20_tokens_requests": [
            {
                "ticker": "PGX-PLG20",
                "required_confirmations": 4
            },
            {
                "ticker": "AAVE-PLG20",
                "required_confirmations": 4
            }
        ],
        "required_confirmations": 5,
        // These are the new parameters that are added for this method which are different from `enable_eth_with_tokens`
        "priv_key_policy": "ContextPrivKey", // Optional, defaults to "ContextPrivKey", Accepted values: "ContextPrivKey", "Trezor"
        "path_to_address": { // defaults to 0'/0/0
            "account_id": 0,
            "chain": "External", // Accepted values: "External", "Internal"
            "address_id": 1
        },
        "gap_limit": 20, // Optional, defaults to 20 
        "scan_policy": "scan_if_new_wallet", // Optional, defaults to "scan_if_new_wallet", Accepted values: "do_not_scan", "scan_if_new_wallet", "scan"
        "min_addresses_number": 3 // Optional, Number of addresses to generate, if not specified addresses will be generated up to path_to_address::address_index
    }
}

Response

{
    "mmrpc": "2.0",
    "result": {
        "task_id": 0
    },
    "id": null
}

`task::enable_eth::status`

Request

{
    "userpass": "{{userpass}}",
    "mmrpc": "2.0",
    "method": "task::enable_eth::status",
    "params": {
        "task_id": 0
    }
}

Response

{
    "mmrpc": "2.0",
    "result": {
        "status": "InProgress",
        "details": "ActivatingCoin" // `ActivatingCoin` or `RequestingWalletBalance`
    },
    "id": null
}

Response

{
    "mmrpc": "2.0",
    "result": {
        "status": "Ok",
        "details": {
            "current_block": 56246116,
            "ticker": "MATIC",
            "wallet_balance": {
                "wallet_type": "HD",
                "accounts": [
                    {
                        "account_index": 0,
                        "derivation_path": "m/44'/966'/0'",
                        "total_balance": {
                            "PGX-PLG20": {
                                "spendable": "0",
                                "unspendable": "0"
                            },
                            "AAVE-PLG20": {
                                "spendable": "0",
                                "unspendable": "0"
                            },
                            "MATIC": {
                                "spendable": "0",
                                "unspendable": "0"
                            }
                        },
                        "addresses": [
                            {
                                "address": "0xa305D76bB19C80035e50015eABA4Ae6265C3B21c",
                                "derivation_path": "m/44'/966'/0'/0/0",
                                "chain": "External",
                                "balance": {
                                    "MATIC": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    },
                                    "PGX-PLG20": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    },
                                    "AAVE-PLG20": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    }
                                }
                            },
                            {
                                "address": "0x9535A7A6E03E3061ec3D902bc94900E56F3791fB",
                                "derivation_path": "m/44'/966'/0'/0/1",
                                "chain": "External",
                                "balance": {
                                    "PGX-PLG20": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    },
                                    "MATIC": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    },
                                    "AAVE-PLG20": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    }
                                }
                            },
                            {
                                "address": "0x9BdC9d32fC5de748b1d4f5f3191719D94709CdD5",
                                "derivation_path": "m/44'/966'/0'/0/2",
                                "chain": "External",
                                "balance": {
                                    "MATIC": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    },
                                    "AAVE-PLG20": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    },
                                    "PGX-PLG20": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    }
                                }
                            }
                        ]
                    }
                ]
            },
            "nfts_infos": {}
        }
    },
    "id": null
}

`task::enable_erc20::init`

This initialize an EVM token using the same priv_key_policy that was used to initialize the platform coin, this is used to initialize more tokens that were not initialized with task::enable_eth::init

Request

{
    "userpass": "{{userpass}}",
    "mmrpc": "2.0",
    "method": "task::enable_erc20::init",
    "params": {
        "ticker": "USDT-PLG20",
        "activation_params": {
            "required_confirmations": 1, // Optional, defaults to coins config
            "path_to_address": { // defaults to 0'/0/0
                "account_id": 0,
                "chain": "External", // Accepted values: "External", "Internal"
                "address_id": 1
            },
            "scan_policy": "scan_if_new_wallet", // Optional, defaults to "scan_if_new_wallet", Accepted values: "do_not_scan", "scan_if_new_wallet", "scan"
            "min_addresses_number": 3 // Optional, Number of addresses to generate, if not specified addresses will be generated up to path_to_address::address_index}
        }
    }
}

Response

{
    "mmrpc": "2.0",
    "result": {
        "task_id": 1
    },
    "id": null
}

`task::enable_erc20::status`

Request

{
    "userpass": "{{userpass}}",
    "mmrpc": "2.0",
    "method": "task::enable_erc20::status",
    "params": {
        "task_id": 1
    }
}

Response

{
    "mmrpc": "2.0",
    "result": {
        "status": "Ok",
        "details": {
            "ticker": "USDT-PLG20",
            "platform_coin": "MATIC",
            "token_contract_address": "0xc2132d05d31c914a87c6611c10748aeb04b58e8f",
            "current_block": 56246189,
            "required_confirmations": 1,
            "wallet_balance": {
                "wallet_type": "HD",
                "accounts": [
                    {
                        "account_index": 0,
                        "derivation_path": "m/44'/966'/0'",
                        "total_balance": {
                            "USDT-PLG20": {
                                "spendable": "0",
                                "unspendable": "0"
                            }
                        },
                        "addresses": [
                            {
                                "address": "0xa305D76bB19C80035e50015eABA4Ae6265C3B21c",
                                "derivation_path": "m/44'/966'/0'/0/0",
                                "chain": "External",
                                "balance": {
                                    "USDT-PLG20": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    }
                                }
                            },
                            {
                                "address": "0x9535A7A6E03E3061ec3D902bc94900E56F3791fB",
                                "derivation_path": "m/44'/966'/0'/0/1",
                                "chain": "External",
                                "balance": {
                                    "USDT-PLG20": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    }
                                }
                            },
                            {
                                "address": "0x9BdC9d32fC5de748b1d4f5f3191719D94709CdD5",
                                "derivation_path": "m/44'/966'/0'/0/2",
                                "chain": "External",
                                "balance": {
                                    "USDT-PLG20": {
                                        "spendable": "0",
                                        "unspendable": "0"
                                    }
                                }
                            }
                        ]
                    }
                ]
            }
        }
    },
    "id": null
}

Note: chain_id is now needed for all evm coins/tokens. chain_id should be part of platform coin config only and retrieved from platform coin, this is for v2 methods, but we should keep it in all tokens configs as well for v1 methods.

* dev: docs(README): remove outdated information from the README (KomodoPlatform#2097) fix(sia): fix sia compilation after hd wallet PR merge (KomodoPlatform#2103) feat(hd_wallet): utxo and evm hd wallet and trezor (KomodoPlatform#1962) feat(sia): initial Sia integration (KomodoPlatform#2086) fix(BCH): deserialize BCH header that uses KAWPOW version correctly (KomodoPlatform#2099) fix(eth_tests): remove ETH_DEV_NODE from tests (KomodoPlatform#2101)

* dev: feat(app-dir): implement root application dir `.kdf` (KomodoPlatform#2102) fix tendermint fee calculation (KomodoPlatform#2106) update dockerfile (KomodoPlatform#2104) docs(README): remove outdated information from the README (KomodoPlatform#2097) fix(sia): fix sia compilation after hd wallet PR merge (KomodoPlatform#2103) feat(hd_wallet): utxo and evm hd wallet and trezor (KomodoPlatform#1962) feat(sia): initial Sia integration (KomodoPlatform#2086) fix(BCH): deserialize BCH header that uses KAWPOW version correctly (KomodoPlatform#2099) fix(eth_tests): remove ETH_DEV_NODE from tests (KomodoPlatform#2101) feat(coin): support nucleus as an alternative to iris HTLC (KomodoPlatform#2079)

shamardy added 16 commits September 1, 2023 20:09

wip: account_balance, my_tx_history v2, get_new_address, withdraw for…

d961a14

… HD UTXO

Specify account index when creating a new account, test enabling of U…

bfcc3c9

…TXO coins

Merge remote-tracking branch 'origin/dev' into hd-account-balance

fe64afa

fix trade_test_rick_and_morty and trade_test_electrum_and_eth_coins

8d6980e

get swaps working again for HD UTXO

19e29d0

revert changes to EnabledCoinBalanceParams

c6aa703

derive addresses up to the one to be enabled

5e0f90d

remove unneeded derive_secp256k1_secret_from_full_path function

dacfc9b

Merge remote-tracking branch 'origin/dev' into hd-account-balance

0597876

remove StandardHDCoinAddress and use HDAccountAddressId instead + ref…

d8f2b42

…actor

use chain from the activation request in enable_hd_wallet

9aa2b88

remove unneeded todos

38afa8e

remove hd_wallet_derived_pubkey function

de14171

refactor xpub_extractor to be used only when trezor

6cf756e

fix wasm tests

10513c4

Ignore tests that still need implementations and remove unneeded todos

44813a2

shamardy added the in progress Changes will be made from the author label Sep 7, 2023

shamardy changed the title ~~feat(hd_wallet): Complete UTXO HD wallet functionalities~~ feat(hd_wallet): complete utxo hd wallet functionalities Sep 7, 2023

shamardy added the under review label Sep 7, 2023

shamardy requested review from artemii235 and dimxy September 7, 2023 22:54

shamardy added 2.1.0-beta and removed in progress Changes will be made from the author labels Sep 7, 2023

artemii235 reviewed Sep 8, 2023

View reviewed changes

laruh requested changes Sep 11, 2023

View reviewed changes

shamardy added in progress Changes will be made from the author and removed under review labels Sep 12, 2023

shamardy added 3 commits September 12, 2023 19:57

handle HDAccountAddressId::to_derivation_path errors

d56d2ea

update get_enabled_address doc comment

fe7b84e

return error if from is used in withdraw slp request

fb70886

Merge remote-tracking branch 'origin/dev' into hd-account-balance

3d517b1

shamardy added 2 commits March 8, 2024 15:03

Merge remote-tracking branch 'dev' into hd-account-balance

97f6bd6

Merge remote-tracking branch 'dev' into hd-account-balance

aed4c7a

shamardy mentioned this pull request Apr 9, 2024

Implementing Per-Coin Database Paths Based on PubkeyHash for Enhanced Modularization #2094

Open

dimxy reviewed Apr 9, 2024

View reviewed changes

shamardy and others added 4 commits April 16, 2024 05:43

Merge remote-tracking branch 'dev' into hd-account-balance

25b146e

Merge remote-tracking branch 'dev' into hd-account-balance

93a2cd2

Review fixes: streamline derivation path structs names

a6b133d

shamardy mentioned this pull request Apr 23, 2024

[bug] HD Wallet implementation #1838

Open

shamardy added 5 commits April 23, 2024 03:00

try using wasm_bindgen_test_runner for wasm CI tests

8761221

fix: remove verbose attribute in wasm test command

920e2db

remove wasm_bindgen_test_runner and increase timeout instead

4e50190

Merge remote-tracking branch 'dev' into hd-account-balance

d00e2e9

fix fmt

2f17f7b

shamardy added sec-review no-deps labels Apr 25, 2024

shamardy changed the title ~~feat(hd_wallet): complete utxo hd wallet functionalities~~ feat(hd_wallet): utxo and evm hd wallet and trezor Apr 25, 2024

shamardy merged commit 065213b into dev Apr 25, 2024
26 of 29 checks passed

shamardy deleted the hd-account-balance branch April 25, 2024 18:53

shamardy mentioned this pull request May 16, 2024

feat(ETH): eip1559 gas fee estimator and rpcs #2051

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(hd_wallet): utxo and evm hd wallet and trezor #1962

feat(hd_wallet): utxo and evm hd wallet and trezor #1962

shamardy commented Sep 7, 2023 •

edited

artemii235 left a comment

laruh left a comment

smk762 commented Feb 28, 2024

smk762 commented Feb 28, 2024

shamardy commented Feb 28, 2024

shamardy commented Feb 28, 2024

shamardy commented Feb 28, 2024 •

edited

smk762 commented Feb 29, 2024 •

edited

dimxy Apr 9, 2024

shamardy Apr 23, 2024

shamardy commented Apr 25, 2024

		@@ -311,23 +358,22 @@ pub trait UtxoFieldsWithHardwareWalletBuilder: UtxoCoinBuilderCommonOps {
		let recently_spent_outpoints = AsyncMutex::new(RecentlySpentOutPoints::new(my_script_pubkey));

feat(hd_wallet): utxo and evm hd wallet and trezor #1962

feat(hd_wallet): utxo and evm hd wallet and trezor #1962

Conversation

shamardy commented Sep 7, 2023 • edited

artemii235 left a comment

Choose a reason for hiding this comment

laruh left a comment

Choose a reason for hiding this comment

smk762 commented Feb 28, 2024

smk762 commented Feb 28, 2024

shamardy commented Feb 28, 2024

shamardy commented Feb 28, 2024

shamardy commented Feb 28, 2024 • edited

smk762 commented Feb 29, 2024 • edited

dimxy Apr 9, 2024

Choose a reason for hiding this comment

shamardy Apr 23, 2024

Choose a reason for hiding this comment

shamardy commented Apr 25, 2024

Enable EVM coin with tokens in HD mode (or trezor):

task::enable_eth::init

Request

Response

task::enable_eth::status

Request

Response

Response

task::enable_erc20::init

Request

Response

task::enable_erc20::status

Request

Response

shamardy commented Sep 7, 2023 •

edited

shamardy commented Feb 28, 2024 •

edited

smk762 commented Feb 29, 2024 •

edited

`task::enable_eth::init`

`task::enable_eth::status`

`task::enable_erc20::init`

`task::enable_erc20::status`