Skip to content

fix(nft): make update_nft work with hd wallets using the enabled address#2386

Merged
shamardy merged 5 commits intodevfrom
fix-update-nft-hd
Apr 15, 2025
Merged

fix(nft): make update_nft work with hd wallets using the enabled address#2386
shamardy merged 5 commits intodevfrom
fix-update-nft-hd

Conversation

@shamardy
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown

@CharlVS CharlVS left a comment

Choose a reason for hiding this comment

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

@shamardy we're getting a different error now. This is with both the coin and the NFT enabled.

{
  "mmrpc": "2.0",
  "error": "Expected 'SingleAddress' derivation method",
  "error_path": "nft.eth.lp_coins",
  "error_trace": "nft:266] nft:746] eth:6637] lp_coins:4219]",
  "error_type": "GetNftInfoError",
  "error_data": {
    "error_type": "GetEthAddressError",
    "error_data": {
      "UnexpectedDerivationMethod": "ExpectedSingleAddress"
    }
  },
  "id": null
}

@shamardy Is there anything I can do to help you reproduce the issue or aid in debugging?

@shamardy
Copy link
Copy Markdown
Collaborator Author

Is there anything I can do to help you reproduce the issue or aid in debugging?

@CharlVS it would be helpful to know which RPCs you used to enable the coin and NFT alongside the requests. Do you use this https://github.com/KomodoPlatform/coins/blob/aa1fca0fca621b13369422d133bef6a838958420/coins#L17228-L17245 in the coins config? Additionally, what other RPCs related to HD wallets or NFTs do you call until you encounter this error?

@laruh
Copy link
Copy Markdown

laruh commented Mar 12, 2025

{
  "mmrpc": "2.0",
  "error": "Expected 'SingleAddress' derivation method",
  "error_path": "nft.eth.lp_coins",
  "error_trace": "nft:266] nft:746] eth:6637] lp_coins:4219]",
  "error_type": "GetNftInfoError",
  "error_data": {
    "error_type": "GetEthAddressError",
    "error_data": {
      "UnexpectedDerivationMethod": "ExpectedSingleAddress"
    }
  },
  "id": null
}

As I can see you could enable nft. But can you successfully enable it using both enable_eth_with_tokens and update_nft in HD mode?

@smk762
Copy link
Copy Markdown

smk762 commented Mar 13, 2025

Using the following MM2.json:

{
    "gui": "mm2_777",
    "enable_hd": true,
    "netid": 8762,
    "rpcport": 8778,
    "1inch_api": "https://api.1inch.dev",
    "use_trading_proto_v2": true,
    "gas_api": {"provider": "Blocknative", "url": "https://api.blocknative.com"},
    "rpc_password": "RPC_CONTRoL_USERP@SSW0RD",
    "passphrase": "****",
    "event_stream_configuration": {
      "access_control_allow_origin": "*",
      "active_events": {
        "NETWORK": { "stream_interval_seconds": 1.5 },
        "COIN_BALANCE": {}
      }
   }
}

and MATIC activation with

  {
    "userpass": "{{userpass}}",
    "method": "enable_eth_with_tokens",
    "mmrpc": "2.0",
    "params": {
      "ticker": "MATIC",
      "get_balances": false,
      "tx_history": false,
      "gas_station_url": "https://gasstation-mainnet.matic.network/",
      "swap_contract_address": "0x9130b257D37A52E52F21054c4DA3450c72f595CE",
      "fallback_swap_contract": "0x9130b257D37A52E52F21054c4DA3450c72f595CE",
      "swap_v2_contracts": {
	    "maker_swap_v2_contract": "0x9130b257D37A52E52F21054c4DA3450c72f595CE",
	    "taker_swap_v2_contract": "0x9130b257D37A52E52F21054c4DA3450c72f595CE",
	    "nft_maker_swap_v2_contract": "0x9130b257D37A52E52F21054c4DA3450c72f595CE"
      },
      "nft_req": {
        "provider": {
          "type": "Moralis",
          "info": {
            "url": "https://moralis-proxy.komodo.earth"
          }
        }
      },
      "nodes": [
        {
          "url": "https://polygon-rpc.com"
        },
        {
          "url": "https://electrum3.cipig.net:18755"
        },
        {
          "url": "https://block-proxy.komodo.earth/rpc/matic"
        }
      ],
      "erc20_tokens_requests": [
        {
          "ticker": "PGX-PLG20",
          "required_confirmations": 4
        },
        {
          "ticker": "AAVE-PLG20",
          "required_confirmations": 4
        }
      ],
      "required_confirmations": 5,
      "requires_notarization": false
    }
  }

I get the response:

{
  "mmrpc": "2.0",
  "error": "Unexpected derivation method: Expected 'SingleAddress' derivation method",
  "error_path": "platform_coin_with_tokens.eth_with_token_activation.v2_activation.lp_coins",
  "error_trace": "platform_coin_with_tokens:470] eth_with_token_activation:305] v2_activation:518] lp_coins:4219]",
  "error_type": "UnexpectedDerivationMethod",
  "error_data": "Expected 'SingleAddress' derivation method",
  "id": null
}

KDF console logs include
13 08:44:26, mm2_main::rpc::dispatcher:124] ERROR RPC error response: platform_coin_with_tokens:470] eth_with_token_activation:305] v2_activation:518] lp_coins:4219] Unexpected derivation method: Expected 'SingleAddress' derivation method

@smk762
Copy link
Copy Markdown

smk762 commented Mar 13, 2025

Doing the same activation request, but with nft_req section removed:

  • in KDF console logs:
13 08:47:59, coins::coin_balance::common_impl:486] INFO MATIC HD wallet hasn't been enabled before. Create default HD account
13 08:50:04, coins::coin_balance::common_impl:574] INFO Generate '1' addresses: ticker=MATIC account_id=0, chain=External
13 08:50:09, coins_activation::platform_coin_with_tokens:475] INFO MATIC current block 68991794

Activation took 2 mins, 15 sec.

From here, sent req:

{
  "userpass": "{{userpass}}",
  "method": "enable_nft",
  "mmrpc": "2.0",
  "params": {
    "ticker": "NFT_MATIC",
    "activation_params": {
      "provider":{
        "type": "Moralis",
        "info": {
          "url": "https://moralis-proxy.komodo.earth",
          "proxy_auth": true
        }
      }
    }
  }
}

and got resp:

{
    "mmrpc": "2.0",
    "result": {
        "nfts": {},
        "platform_coin": "MATIC"
    },
    "id": null
}

in around 2 seconds (no NFTs in wallet yet).
Next request:

{
  "userpass": "{{userpass}}",
  "method": "update_nft",
  "mmrpc": "2.0",
  "params": {
    "chains": [
      "POLYGON"
    ],
    "proxy_auth": false,
    "url": "https://moralis-proxy.komodo.earth",
    "url_antispam": "https://nft.antispam.dragonhound.info"
  }
}

Same error response in 84ms:

{
    "mmrpc": "2.0",
    "error": "Expected 'SingleAddress' derivation method",
    "error_path": "nft.eth.lp_coins",
    "error_trace": "nft:266] nft:746] eth:6637] lp_coins:4219]",
    "error_type": "GetNftInfoError",
    "error_data": {
        "error_type": "GetEthAddressError",
        "error_data": {
            "UnexpectedDerivationMethod": "ExpectedSingleAddress"
        }
    },
    "id": null
}

and with KDF console log:
13 08:52:56, mm2_main::rpc::dispatcher:124] ERROR RPC error response: nft:266] nft:746] eth:6637] lp_coins:4219] Expected 'SingleAddress' derivation method

@laruh
Copy link
Copy Markdown

laruh commented Mar 13, 2025

Thanks for additional logs @smk762

13 08:47:59, coins::coin_balance::common_impl:486] INFO MATIC HD wallet hasn't been enabled before. Create default HD account
13 08:50:04, coins::coin_balance::common_impl:574] INFO Generate '1' addresses: ticker=MATIC account_id=0, chain=External

It shows that its first time of HD wallet activation and it created one address with account_id=0, but it unfortunately doesn't show address_id.
You didnt use "path_to_address" in activation, so smth like default I would expect with zeros ids:

impl Default for HDPathAccountToAddressId {
    fn default() -> Self {
        HDPathAccountToAddressId {
            account_id: 0,
            chain: Bip44Chain::External,
            address_id: 0,
        }
    }
}

and we actually use default of path_to_address: &HDPathAccountToAddressId, when we get nft address here
https://github.com/KomodoPlatform/komodo-defi-framework/blob/884d1e206781e82ea9261dded399cb0c633165de/mm2src/coins/eth.rs#L6636-L6637

@shamardy may be the issue that when we dont set "path_to_address" in activation it generates not account_id: 0 address_id: 0 wallet (not Default)?
the eth:6637 line let my_address btw is where err occurs

To hot fix current issue we should complete this todo
https://github.com/KomodoPlatform/komodo-defi-framework/blob/884d1e206781e82ea9261dded399cb0c633165de/mm2src/coins/eth.rs#L6618-L6621

But i totally agree with you (we discussed in dm this) that there are more important and deep issues/questions related to the below note:


Note: Its still a mistery right now, why enable_eth_with_tokens with nft didnt work and standalone enable_nft worked, as they reuse same initialize_global_nft function, which calls self.derivation_method.single_addr_or_err() where self is platform coin instance

image

https://github.com/KomodoPlatform/komodo-defi-framework/blob/884d1e206781e82ea9261dded399cb0c633165de/mm2src/coins/eth/v2_activation.rs#L517-L518

@smk762
Copy link
Copy Markdown

smk762 commented Apr 8, 2025

@laruh is this one ready for re-review?

@laruh
Copy link
Copy Markdown

laruh commented Apr 8, 2025

@laruh is this one ready for re-review?

yes please retest: enable_eth_with_tokens -> enable nft (separate req) -> update nft rpc

enable_eth_with_tokens with nft params still doesnt work, well you also can test it to see

Copy link
Copy Markdown
Collaborator Author

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

@laruh changes look good to me for enabling nft with a separate request. Let's merge it like this after it's tested since it's needed for 2.4.0-beta release.

@smk762
Copy link
Copy Markdown

smk762 commented Apr 14, 2025

@laruh is this one ready for re-review?

yes please retest: enable_eth_with_tokens -> enable nft (separate req) -> update nft rpc

enable_eth_with_tokens with nft params still doesnt work, well you also can test it to see

enable_eth_with_tokens -> enable nft (separate req) -> update nft rpc now returns {"mmrpc":"2.0","result":null,"id":null} as expected on a fresh wallet. I'll dig up one that has some NFTs in it to confirm expected output in this case aslo.

@smk762 smk762 self-requested a review April 14, 2025 12:46
Copy link
Copy Markdown

@smk762 smk762 left a comment

Choose a reason for hiding this comment

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

Though including the nft_req param in enable_eth_with_tokens still returns UnexpectedDerivationMethod, if not using this param and rather doing enable_nft after activation, NFTs are visible in enable_nft response as expected, and update_nft still returns {"mmrpc":"2.0","result":null,"id":null}, now with no errors sighted in console logs.

@shamardy shamardy merged commit 6ad71d7 into dev Apr 15, 2025
22 of 23 checks passed
@shamardy shamardy deleted the fix-update-nft-hd branch April 15, 2025 18:16
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request May 13, 2025
* dev: (26 commits)
  chore(deps): remove base58 and replace it completely with bs58 (GLEECBTC#2427)
  feat(tron): initial groundwork for full TRON integration (GLEECBTC#2425)
  fix(UTXO): improve tx fee calculation and min relay fee handling (GLEECBTC#2316)
  deps(timed-map): bump to 1.3.1 (GLEECBTC#2413)
  improvement(tendermint): safer IBC channel handler (GLEECBTC#2298)
  chore(release): complete v2.4.0-beta changelogs  (GLEECBTC#2436)
  fix(event-streaming): initial addresses registration in utxo balance streaming (GLEECBTC#2431)
  improvement(watchers): re-write use-watchers handling (GLEECBTC#2430)
  fix(evm): make withdraw_nft work in HD mode (GLEECBTC#2424)
  feat(taproot): support parsing taproot output address types
  chore(RPC): use consistent param name for QTUM delegation (GLEECBTC#2419)
  fix(makerbot): add LiveCoinWatch price provider (GLEECBTC#2416)
  chore(release): add changelog entries for v2.4.0-beta (GLEECBTC#2415)
  fix(wallets): prevent path traversal in `wallet_file_path` and update file extension (GLEECBTC#2400)
  fix(nft): make `update_nft` work with hd wallets using the enabled address (GLEECBTC#2386)
  fix(wasm): unify error handling for mm2_main (GLEECBTC#2389)
  fix(tx-history): token information and query (GLEECBTC#2404)
  test(electrums): fix failing test_one_unavailable_electrum_proto_version (GLEECBTC#2412)
  improvement(network): remove static IPs from seed lists (GLEECBTC#2407)
  improvement(best-orders): return an rpc error when we can't find best orders (GLEECBTC#2318)
  ...
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