[feature request] HD Wallet (multi-account)

[ca333]

Multi Account wallet BIP44 integration alongside BIP39 will be nice enhancement - GUI developers and CLI users would not worry about key-management anymore. Furthermore we can extend this beyond wallet into the DEX - will increase user/dev experience. I envision it one day user can chose which "account" (derivation path/key) to use for specific atomic trades or wallet operations. And as positive effect users can reuse mnemonic from other BIP32/39/44 wallets.

resources:
https://www.reddit.com/r/Bitcoin/comments/2srxr2/what_is_the_difference_between_bip_32_and_bip_44/cnspxwq/
rust lib: https://github.com/rust-bitcoin/rust-wallet (not secure code reviewed)

[artemii235]

@ca333 Thanks for creating the issue, it will be nice enhancement indeed!

[Milerius]

By implementing PR for offline staking of QTUM I realized the need for this feature.

In general, offline staking does not mix well with active trading.

My idea to go to multi-address without having to refactor everything and break everything would rather be to make this feature work only on the new compatible bip39 / bip44 seed and by definition to have the bip39 address + the derived bip44 address.

I imagine an option when starting AtomicDEX to no longer use the Iguana derivation, but bip39 and from there add an optional address collection next to our main address. After that, we could refactor the functions one by one to automatically fall back to the primary address in case.

So WithdrawRequest could became:

#[allow(dead_code)]
#[derive(Deserialize)]
pub struct WithdrawRequest {
    coin: String,
    to: String,
    #[serde(default)]
    amount: BigDecimal,
    #[serde(default)]
    max: bool,
    fee: Option<WithdrawFee>,
    // If not None used as from address instead of the default one
    from: Option<String>
}

and an RPC get_accounts that return the list of public addresses for the given coin.

In the same idea, later my_balance will be able to return the balance of all the addresses, but by default keep the same behaviour. And we can abuse the V2 of the RPC for this kind of operation.

Otherwise, we can also introduce RPC like accounts_infos which will basically regroup balance and other useful information about each account of a given coin

[artemii235]

I like the idea of having the from: Option<String> in the WithdrawRequest. I think it should even be Option<Vec<String>> to allow withdrawal from multiple addresses at once. Overall, your proposal seems a good point to kickstart HD wallets integration. Please complete your ongoing tasks (QTUM delegation and Solana wallet only) and we can possibly prioritize this then.

[artemii235]

@sergeyboyko0791 Could you please post a comprehensive test plan on testing the global HD feature? It should contain reference wallets to check against, etc.

[sergeyboyko0791]

Global HD account feature allows the user to use the BIP32/BIP44/BIP49/BIP84 derivation scheme at the following path:
m/purpose'/coin'/0'/0/hd_account_id, where hd_account_id is set globally at the mm2 config, and purpose, coin values vary and depend on a coin configuration.

For example, if the user wants to activate a 3rd HD account (m/purpose'/coin'/0'/0/3), the config should look like:

{
  "gui": "DEVDOCS_CLI",
  "netid": 7777,
  "rpc_password": "YOUR_PASS_HERE",
  "passphrase": "BIP39 MNEMONIC PHRASE",
  // ...
  "hd_account_id": 3
}

Please note that if hd_account_id is set, the passphrase field must follow the BIP39 mnemonic sentence.
Currently, English words are supported only.

Coin activation

In order to activate coins, they have to contain derivation_path field to determine purpose and coin values at the derivation path. Coin types can be found at https://github.com/satoshilabs/slips/blob/master/slip-0044.md
For example, BTC should have:

{
  "coin": "BTC",
  "name": "bitcoin",
  // ...
  "derivation_path": "m/44'/0'"
}

Testing plan

• All coins and protocols that support the Iguana derivation method, should be able to be activated with HD derivation.
  Please also check if the addresses of activated coins should match the addresses of popular wallets activated with the same mnemonic phrase. For example, Trust wallet.
• Test swaps of different coin pairs where Maker - activated with HD, Taker - activated with Iguana, and vice versa
• Test swaps of different coin pairs where Maker and Taker activated with HD

1. Standard UTXO. For example, RICK, KMD, BTC, QTUM using electrum, enable (with a local Node) RPCs.
   purpose = 44.
2. ETH, ERC20, BNB, ..., using enable, enable_eth_with_tokens RPCs.
   purpose = 44.
3. QRC20 using electrum, enable RPCs
   purpose = 44.
4. ZCoin using task::enable_z_coin::init.
   In addition to the derivation_path field, there is also z_derivation_path that should be present at coin_conf["protocol"]["protocol_data"]. For example,

{
    "coin": "ZOMBIE",
    "asset": "ZOMBIE",
    "txversion": 4,
    "overwintered": 1,
    "mm2": 1,
    "protocol": {
        "type": "ZHTLC",
        "protocol_data": {
            "consensus_params": {
                "overwinter_activation_height": 0,
                "sapling_activation_height": 1,
                "blossom_activation_height": null,
                "heartwood_activation_height": null,
                "canopy_activation_height": null,
                "coin_type": 133,
                "hrp_sapling_extended_spending_key": "secret-extended-key-main",
                "hrp_sapling_extended_full_viewing_key": "zxviews",
                "hrp_sapling_payment_address": "zs",
                "b58_pubkey_address_prefix": [28, 184],
                "b58_script_address_prefix": [28, 189]
            },
            "check_point_block": {
                "height": 290000,
                "time": 1664200629,
                "hash": "106BAA72C53E7FA52E30E6D3D15B37001207E3CF3B9FCE9BAB6C6D4AF9ED9200",
                "sapling_tree": "017797D05B070D29A47EFEBE3FAD3F29345D31BE608C46A5131CD55D201A631C13000D000119CE6220D0CB0F82AD6466B677828A0B4C2983662DAB181A86F913F7E9FB9C28000139C4399E4CA741CBABBDDAEB6DCC3541BA902343E394160EEECCDF20C289BA65011823D28B592E9612A6C3CF4778F174E10B1B714B4FF85E6E58EE19DD4A0D5734016FA4682B0007E61B63A0442B85E0B8C0CE2409E665F219013B5E24E385F6066B00000001A325043E11CD6A431A0BD99141C4C6E9632A156185EB9B0DBEF665EEC803DD6F00000103C11FCCC90C2EC1A126635F708311EDEF9B93D3E752E053D3AA9EFA0AF9D526"
            },
            "z_derivation_path": "m/32'/133'",
        }
    },
    "required_confirmations": 0,
    "derivation_path": "m/44'/133'",
}

Please note:
derivation_path should have purpose = 44.
z_derivation_path should have purpose = 32 (this is required to make mm2 compatible with other zcash wallets).
Wallets: https://z.cash/wallets/
5. BCH, SLP using enable_bch_with_tokens RPC.
purpose = 44
6. Tendermint, TendermintToken using enable_tendermint_with_assets, enable_tendermint_token RPCs.
purpose = 44, coin = 566
7. Solana, SPL token using enable_solana_with_tokens (not required as no GUI supported them)
purpose = 44
8. Lightning using task::enable_lightning::init. Lightning is based on UTXO-segwit coin, so the config and the address should be the same as the platform ones.

cc: @smk762 @tonymorony

[shamardy]

8. Lightning using task::enable_lightning::init. Lightning is based on UTXO-segwit coin, so the config and the address should be the same as the platform ones.

Please note that lightning derivation path is still an open for research topic after I discussed it with @sergeyboyko0791 on DM. Different lightning node implementations use different derivation paths, e.g.:

• LDK advises using m/535h (purpose = 535) or a custom path. This is for a BTC/lightning only HD wallet, which is different from our case as we use a derivation path for BTC first and not the seed directly.
• LND uses purpose = 1017.
• Core Lightning allows the user to provide a secret that is used to generate a seed . This seed is used to generate a master extended key then /0'/0/ path is used to get an extended key that will be used for the lightning node.

In addition to how the master lightning node keys are generated, channel backups are not compatible between different node implementations as well.

so the config and the address should be the same as the platform ones

Currently in AtomicDEX lightning implementation the node's master key is generated from the platform's private key, a bit similar to how it's done in Core Lightning but only one child level is used to get the node's secret that generates the node's publickey/address. Different hardened childs are used to generate more master keys that are important for the node's operation.

[smk762]

Testing completed prior to merge, though there have since been changes to z_coin code so will recheck that this week