Adds `derive_priv_key` v2 RPC for HD wallets by smk762 · Pull Request #2541 · GLEECBTC/komodo-defi-framework

smk762 · 2025-07-21T15:06:36Z

The legacy show_priv_key method did not take input for account/address id, and would only return the zero index WIF for HD wallets, so I've added a v2 derive_priv_key method which accepts these inputs.

Request:

{
    "userpass": "{{userpass}}",
    "mmrpc": "2.0",
    "method": "derive_priv_key",
    "params": {
        "coin": "MATIC",
        "account_id": 0,
        "address_id": 3
    }
}

Response:

{
    "mmrpc": "2.0",
    "result": {
        "coin": "MATIC",
        "address": "0x1e8B4aA6a8B8a376E0357504cF2ebC11Bc02288b",
        "derivation_path": "m/44'/60'/0'/0/3",
        "priv_key": "0x932ec93805200317394d6f63216791cf6052e6bb7412153f799c0b0660086e24",
        "pub_key": "0x036b521bb1f9e845301f8bcb1f025151784ac2ea54b95fa50b9c491aced4a34c04"
    },
    "id": null
}

To test:

Use the seed in the test function to launch in HD mode
Activate some coins, and create some addresses, then query these coins and addresses via derive_priv_key.
Confirm the responses match the values returned at https://iancoleman.io/bip39/ with the same seed and selected coin.

onur-ozkan

Thanks!

Some notes from the first review from my end:

onur-ozkan · 2025-07-22T04:41:59Z

mm2src/coins/lp_coins.rs

+    /// Returns the WIF prefix for the coin.
+    fn wif_prefix(&self) -> Option<u8> { None }
+
+    fn is_utxo(&self) -> bool { false }


These shouldn't be defined in MarketCoinOps. I don't think we need to define them anywhere at all; we can simply derive the UTXO coin from MmCoinEnum.

done in 5641673

superceded by 236f9d0

onur-ozkan · 2025-07-22T04:47:18Z

mm2src/coins/priv_key.rs

+    #[display(fmt = "No such coin: {}", _0)]
+    NoSuchCoin(String),
+    #[display(fmt = "Coin {} doesn't support HD wallet derivation", _0)]
+    CoinDoesntSupportDerivation(String),
+    #[display(fmt = "Hardware/remote wallet doesn't allow exporting private keys")]
+    HwWalletNotAllowed,
+    #[display(fmt = "Internal error: {}", _0)]
+    Internal(String),


It would be more clear to use struct-like errors instead if tuple-like ones, e.g.,:

NoSuchCoin { ticker: String, }, Internal { reason: String, }

so we can know what are the inner values are about without having to look use-cases.

done in 221b9fe

onur-ozkan · 2025-07-22T04:51:10Z

mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs

+            "myipaddr": env::var ("BOB_TRADE_IP") .ok(),
+            "rpcip": env::var ("BOB_TRADE_IP") .ok(),
+            "canbind": env::var ("BOB_TRADE_PORT") .ok().map (|s| s.parse::<i64>().unwrap()),


Can you remove white spaces?

We should also unwrap instead of calling ok to fail the test when BOB_TRADE_IP and BOB_TRADE_PORT isn't present.

Done in 79e632c

FWIW, the rest of the file uses .ok() and similar white spacing throughout. Should I fix up those also while I'm here?

onur-ozkan · 2025-07-22T04:56:02Z

mm2src/coins/priv_key.rs

+#[async_trait]
+pub trait DerivePrivKeyV2: MmCoin + CoinWithPrivKeyPolicy + CoinWithDerivationMethod + Sized {
+    async fn derive_priv_key(&self, req: &DerivePrivKeyReq) -> Result<DerivedPrivKey, MmError<DerivePrivKeyError>>;
+}
+
+#[async_trait]
+impl<Coin> DerivePrivKeyV2 for Coin
+where
+    Coin: MmCoin + CoinWithPrivKeyPolicy + CoinWithDerivationMethod + MarketCoinOps + Sync,
+{


If derive_priv_key can take MmCoinEnum as a parameter, that would greatly simplify the implementation so we don't have to do all the generic-dances here (also in priv_key::derive_priv_key).

done in 236f9d0

mariocynicys · 2025-07-22T05:03:58Z

is this impl-ed so to

output the private key to the user (do we really need that for hd wallets?)
or is the result used within KW to do some computation (what would that be?)

gcharang · 2025-07-22T07:39:29Z

@mariocynicys

output the private key to the user (do we really need that for hd wallets?)

yes, this was implemented to let users export privkeys for HD wallet. we do need to do it as users will need the privkeys in case they need to do anything komodo wallet doesn't do. or if they need to access their coins in case the electrum servers we have are down. Even if there are no active electrum servers, users can import their privkey in native wallets with full blockchain history to move their coins

mariocynicys · 2025-07-22T13:57:57Z

i mean if we already have a way to expose the wallet seed, we then dont need to expose individual private keys. i dont know of a wallet that can work with a single private key (if there is, it will probably even consider it as legacy address, so no segwit coins etc... will be visible)

gcharang · 2025-07-22T14:27:08Z

you can import a single private key in komodo ocean wallet for KMD. in any native full node wallet for most other utxo coins. even for ethereum, you can do it in trust wallet or meta mask

onur-ozkan · 2025-07-23T04:57:42Z

mm2src/coins/priv_key.rs

+                })?;
+
+            let private = Private {
+                prefix: 0, // ETH doesn't use WIF format


Is this the only difference compare to UTXO implementation?

smk762 · 2025-07-23T12:36:07Z

closing in favor of #2542

smk762 added 4 commits July 21, 2025 21:36

adds wif_prefix & is_utxo to MarketCoinOps

a085b28

adds wif_prefix & is_utxo to UtxoStandardCoin

d9483cb

adds v2 derive_priv_key rpc

5f878a5

add tests for derive_priv_key

3619acd

smk762 requested review from CharlVS, ca333, gcharang, onur-ozkan and shamardy July 21, 2025 15:06

smk762 mentioned this pull request Jul 21, 2025

Can't export privkeys of every address for a coin in HD wallet mode GLEECBTC/gleec-wallet#2964

Closed

onur-ozkan reviewed Jul 22, 2025

View reviewed changes

smk762 added 4 commits July 22, 2025 23:51

move is_utxo and wif_prefix to macro

5641673

use struct-like errors instead if tuple-like

221b9fe

simplify by using MmCoinEnum as a parameter

236f9d0

rm whitespace and use unwrap

79e632c

smk762 requested a review from onur-ozkan July 22, 2025 16:39

rm blank lines

e640ba0

onur-ozkan reviewed Jul 23, 2025

View reviewed changes

smk762 closed this Jul 23, 2025

shamardy deleted the test-multi-privkey branch July 23, 2025 12:36

Conversation

smk762 commented Jul 21, 2025

Uh oh!

onur-ozkan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smk762 Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mariocynicys commented Jul 22, 2025

Uh oh!

gcharang commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mariocynicys commented Jul 22, 2025

Uh oh!

gcharang commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smk762 commented Jul 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

smk762 Jul 22, 2025 •

edited

Loading

gcharang commented Jul 22, 2025 •

edited

Loading