atomicDEX-API icon indicating copy to clipboard operation
atomicDEX-API copied to clipboard
verified publisher icon KomodoPlatform

Trezor Functionality Report & Issues from v2.5.0-RC tests

Open smk762 opened this issue 6 months ago • 14 comments

Trezor support in KDF is not universal, and some inconsistencies, "success should be error" and "error should be success" were detected. I'll report below with a comment for each protocol tested so this is more easily digested.

In summary:

  • ETH protocol activation is not functioning.
  • Segwit coins return an error on task::get_new_address with Wrong derivation path for selected account. m/84/2'/0'/0/1 shown on device. This was tested and confirmed for LTC-segwit, BTC-segwit, SYS-segwit, and assumed to apply to all segwit variants.
  • QTUM returns an error on task::get_new_address with Confirmation address mismatch - but non-task get_new_address works and the created address is functional for deposits/withdrawals.
  • ZHTLC and Tendermint allow the priv_key_policy to be set to trezor, then activate as if it wasnt there (i.e. in SingleAddress mode). Subsequent attempts to use task based address, account and withdraw methods all return CoinIsActivatedNotWithHDWallet or CoinDoesntSupportInitWithdraw
  • QTUM and UTXO require priv_key_policy as a string, but ETH requires it as an object. Incidentally, Tendermint correctly accepts an object for walletconnect, though it is not trezor compatible. Ideally, all usage of priv_key_policy should use the object format.

Update: ETH activation error was limited to Trezor Safe 5. No such issue was present using a Trezor Model 1.

smk762 avatar Jul 02 '25 04:07 smk762

task::enable_tendermint::init

Not Trezor compatible, but fails to return an error when privkey policy is set to Trezor. This may mislead users.

ATOM:

  • ATOM activates via task::enable_tendermint::init without regard to priv_key_policy. Accepts both string, object, or None.

  • Addresses:

    • with priv_key_policy as string:
    • with priv_key_policy as object: {'wallet_type': 'Iguana', 'address': 'cosmos1p....ddj8p6t'}
    • with priv_key_policy as None:
    • Response does not indicate wallet_type.

  • task::account_balance::init

{
  "status": "Error",
  "details": {
    "error": "Coin is expected to be activated with the HD wallet derivation method",
    "error_path": "init_account_balance",
    "error_trace": "init_account_balance:86]",
    "error_type": "CoinIsActivatedNotWithHDWallet"
  }
}
  • task::create_new_account::init
{
  "status": "Error",
  "details": {
    "error": "Coin is expected to be activated with the HD wallet derivation method",
    "error_path": "init_create_account",
    "error_trace": "init_create_account:330]",
    "error_type": "CoinIsActivatedNotWithHDWallet"
  }
}
  • task::scan_for_new_addresses::init
{
  "status": "Error",
  "details": {
    "error": "Coin is expected to be activated with the HD wallet derivation method",
    "error_path": "init_scan_for_new_addresses",
    "error_trace": "init_scan_for_new_addresses:105]",
    "error_type": "CoinIsActivatedNotWithHDWallet"
  }
}
  • task::get_new_address::init
  "status": "Error",
  "details": {
    "error": "Coin is expected to be activated with the HD wallet derivation method",
    "error_path": "get_new_address",
    "error_trace": "get_new_address:353]",
    "error_type": "CoinIsActivatedNotWithHDWallet"
  }
}
  • task::withdraw::init
{
  "status": "Error",
  "details": {
    "error": "'ATOM' coin doesn't support 'init_withdraw' yet. Consider using 'withdraw' request instead",
    "error_path": "init_withdraw",
    "error_trace": "init_withdraw:142]",
    "error_type": "CoinDoesntSupportInitWithdraw",
    "error_data": {
      "coin": "ATOM"
    }
  }
}

smk762 avatar Jul 02 '25 04:07 smk762

task::enable_utxo::init

LTC-segwit:

  • LTC-segwit activates via task::enable_utxo::init with priv_key_policy as STRING. Using object for priv_key_policy returns
{
    "mmrpc": "2.0",
    "error": "Error parsing request: unknown variant `type`, expected `ContextPrivKey` or `Trezor`",
    "error_path": "dispatcher",
    "error_trace": "dispatcher:124]",
    "error_type": "InvalidRequest",
    "error_data": "unknown variant `type`, expected `ContextPrivKey` or `Trezor`",
    "id": null
}

  • Addresses:

    • with priv_key_policy as string: 'address': 'ltc1....gzz9m', 'derivation_path': "m/84'/2'/0'/0/0"
    • Response indicates 'wallet_type': 'HD'

  • task::account_balance::init returns successful result

{
    "mmrpc": "2.0",
    "result": {
        "status": "Ok",
        "details": {
            "account_index": 0,
            "derivation_path": "m/84'/2'/0'",
            "total_balance": {
                "LTC-segwit": {
                    "spendable": "0.01",
                    "unspendable": "0"
                }
            },
            "addresses": [
                {
                    "address": "ltc1qru........gj42r5",
                    "derivation_path": "m/84'/2'/0'/0/0",
                    "chain": "External",
                    "balance": {
                        "LTC-segwit": {
                            "spendable": "0.01",
                            "unspendable": "0"
                        }
                    }
                }
            ]
        }
    },
    "id": null
}
  • task::create_new_account::init returns successful result
{
  "status": "Ok",
  "details": {
    "account_index": 1,
    "derivation_path": "m/84'/2'/1'",
    "total_balance": {},
    "addresses": []
  }
}
  • task::scan_for_new_addresses::init returns successful result
{
  "status": "Ok",
  "details": {
    "account_index": 0,
    "derivation_path": "m/84'/2'/0'",
    "new_addresses": []
  }
}
  • task::get_new_address::init

With segwit coin, gets STUCK at

{
  "status": "InProgress",
  "details": {
    "ConfirmAddress": {
      "expected_address": "ltc1..........uyw5"
    }
  }
}

Trezor device (after PIN input) reports: Wrong derivation path for selected account. m/84/2'/0'/0/1

After confirming this on Trezor, staus reports

{
  "status": "Error",
  "details": {
    "error": "User cancelled action",
    "error_path": "get_new_address.coin_ops.confirm_address.response.client",
    "error_trace": "get_new_address:509] coin_ops:187] confirm_address:159] response:122] client:93]",
    "error_type": "HwError",
    "error_data": "UserCancelled"
  }
}
  • task::withdraw::init returns "successful" result, yet no new address
{
  "status": "Ok",
  "details": {
    "account_index": 0,
    "derivation_path": "m/84'/2'/0'",
    "new_addresses": []
  }
}

Withdraw tested on LTC-segwit & KMD. In both cases:

  • returns "successful" result, and raw hex returns txid when broadcast appears and confirms on block explorer.

FWIW, trezor data for LTC at https://github.com/trezor/trezor-common/blob/master/defs/bitcoin/litecoin.json I cant see the derivation paths there, except for the SLIP44: 2 bit, which leads to https://github.com/satoshilabs/slips/blob/master/slip-0044.md

No mention of the 44 / 84 to differentiate segwit/legacy.

smk762 avatar Jul 02 '25 05:07 smk762

task::enable_z_coin::init

ZHTLC:

  • ARRR activates via task::enable_z_coin::init without regard to priv_key_policy. Accepts both string, object, or None.

  • Addresses:

    • with priv_key_policy as string: {'wallet_type': 'Iguana', 'address': 'zs1lkrlj8ewen5yqk377lm9qdr4gulkgmr3fgun5kj8j929uygueq20vspwy8xfrqyexx642d58wsj'}
    • with priv_key_policy as object: {'wallet_type': 'Iguana', 'address': 'zs1lkrlj8ewen5yqk377lm9qdr4gulkgmr3fgun5kj8j929uygueq20vspwy8xfrqyexx642d58wsj'}
    • with priv_key_policy as None:

  • task::account_balance::init

{
  "status": "Error",
  "details": {
    "error": "Coin is expected to be activated with the HD wallet derivation method",
    "error_path": "init_account_balance",
    "error_trace": "init_account_balance:86]",
    "error_type": "CoinIsActivatedNotWithHDWallet"
  }
}
  • task::create_new_account::init
{
  "status": "Error",
  "details": {
    "error": "Coin is expected to be activated with the HD wallet derivation method",
    "error_path": "init_create_account",
    "error_trace": "init_create_account:330]",
    "error_type": "CoinIsActivatedNotWithHDWallet"
  }
}
  • task::scan_for_new_addresses::init
{
  "status": "Error",
  "details": {
    "error": "Coin is expected to be activated with the HD wallet derivation method",
    "error_path": "init_scan_for_new_addresses",
    "error_trace": "init_scan_for_new_addresses:105]",
    "error_type": "CoinIsActivatedNotWithHDWallet"
  }
}
  • task::get_new_address::init
{
  "status": "Error",
  "details": {
    "error": "Coin is expected to be activated with the HD wallet derivation method",
    "error_path": "get_new_address",
    "error_trace": "get_new_address:353]",
    "error_type": "CoinIsActivatedNotWithHDWallet"
  }
}
  • task::withdraw::init PENDING (need ARRR balance)

smk762 avatar Jul 02 '25 06:07 smk762

task::enable_eth::init

Unable to activate. Status query returns the following on ETH, AVAX. BNB & MATIC.

{
    "mmrpc": "2.0",
    "result": {
        "status": "Error",
        "details": {
            "error": "DataError",
            "error_path": "lib.platform_coin_with_tokens.eth_with_token_activation.coin_balance.mod.pubkey.client",
            "error_trace": "lib:104] platform_coin_with_tokens:474] eth_with_token_activation:438] coin_balance:493] mod:417] pubkey:180] client:93]",
            "error_type": "Internal",
            "error_data": "DataError"
        }
    },
    "id": null
}

smk762 avatar Jul 02 '25 06:07 smk762

task::enable_qtum::init

QTUM:

  • QTUM activates via task::enable_qtum::init with priv_key_policy as STRING

  • Addresses:

    • with priv_key_policy as string: {'address': 'Qc...ZT', 'derivation_path': "m/44'/2301'/0'/0/0"
    • Response indicates 'wallet_type': 'HD'

  • task::account_balance::init returns successful result

{
  "status": "Ok",
  "details": {
    "account_index": 0,
    "derivation_path": "m/44'/2301'/0'",
    "total_balance": {
      "QTUM": {
        "spendable": "0",
        "unspendable": "0"
      }
    },
    "addresses": [
      {
        "address": "Qc....ZT",
        "derivation_path": "m/44'/2301'/0'/0/0",
        "chain": "External",
        "balance": {
          "QTUM": {
            "spendable": "0",
            "unspendable": "0"
          }
        }
      }
    ]
  }
}
  • task::create_new_account::init returns successful result
{
  "status": "Ok",
  "details": {
    "account_index": 1,
    "derivation_path": "m/44'/2301'/1'",
    "total_balance": {},
    "addresses": []
  }
}
  • task::scan_for_new_addresses::init returns successful result
{
  "status": "Ok",
  "details": {
    "account_index": 0,
    "derivation_path": "m/44'/2301'/0'",
    "new_addresses": []
  }
}
  • task::get_new_address::init ERROR
{
  "method": "task::get_new_address::init",
  "mmrpc": "2.0",
  "params": {
    "coin": "QTUM",
    "account_id": 0,
    "chain": "External"
  },
  "id": 0,
  "userpass": "RPC_UserP@SSW0RD"
}

After confirming this on Trezor, status reports


{
  "mmrpc": "2.0",
  "result": {
    "status": "Error",
    "details": {
      "error": "Internal: Confirmation address mismatched: expected 'QZVhE3d4umpa1KFGSXRcJASaACAemMEJvN, found 'QdSZm1FgZvtFr4y68TgfhjiwZbNT4zpiHG''",
      "error_path": "get_new_address.coin_ops.confirm_address",
      "error_trace": "get_new_address:509] coin_ops:187] confirm_address:174]",
      "error_type": "Internal",
      "error_data": "Confirmation address mismatched: expected 'QZVhE3d4umpa1KFGSXRcJASaACAemMEJvN, found 'QdSZm1FgZvtFr4y68TgfhjiwZbNT4zpiHG''"
    }
  },
  "id": 0
}

Rechecking task::account_balance reveals no new additional addresses.

Using non-task get_new_address returns

{
    "mmrpc": "2.0",
    "result": {
        "new_address": {
            "address": "QdSZm1FgZvtFr4y68TgfhjiwZbNT4zpiHG",
            "derivation_path": "m/44'/2301'/0'/0/1",
            "chain": "External",
            "balance": {
                "QTUM": {
                    "spendable": "0",
                    "unspendable": "0"
                }
            }
        }
    },
    "id": null
}

Rechecking task::account_balance reveals the non-task generated address is now also present.

  • task::withdraw::init returns "successful" result, and raw hex returns txid when broadcast appears and confirms on QTUM block explorer.
  • task::withdraw::init also returns "successful" result using the non-task generated address as source, and raw hex returns txid when broadcast appears and confirms on QTUM block explorer.

smk762 avatar Jul 02 '25 06:07 smk762

A quick fix for trezor task::enable_eth::init (not getting public key on a Safe device): https://github.com/KomodoPlatform/komodo-defi-framework/commit/eeaee97594698dfdfcae36a34b5715f508b2c37e cc: @smk762

dimxy avatar Jul 02 '25 15:07 dimxy

A quick fix for trezor task::enable_eth::init (not getting public key on a Safe device): eeaee97 cc: @smk762

Started testing, noticed it now showed WatingForTrezorToConnect if device not unlocked. Proceeded to unlock device, rechecked status, and got Device not found (or similar wording).

Reran init (without restarting device), and status returned EnterTrezorPassphrase, so the earlier fail to find appears to be logic, not the cable.

From here, passphrase was accepted and confirmed as expected, and ETH also activated 🎉

smk762 avatar Jul 03 '25 02:07 smk762

Some irregularities (using TS5):

I see another DataError response when attempting to create additional address for MATIC/BNB/EVM.

{
    "mmrpc": "2.0",
    "result": {
        "status": "Error",
        "details": {
            "error": "Got data error",
            "error_path": "get_new_address.coin_ops.confirm_address.client",
            "error_trace": "get_new_address:514] coin_ops:194] confirm_address:168] client:93]",
            "error_type": "HwError",
            "error_data": "DataError"
        }
    },
    "id": null
}

Trezor suite specifies the segwit derivation path in its details, so it would be expected that BTC-segwit in KDF would match.

Image

An attempt to create a new address for BTC showed a different "expected address" in the status response than was shown on Trezor. I ran task::get_new_address::cancel.

A second attempt, which involved a newline between task_id key and value (this shouldnt matter, but mentioning just in case), led to the following response:

{
    "mmrpc": "2.0",
    "result": {
        "status": "Error",
        "details": {
            "error": "Internal: bad magic in v1 read: 0x3f346e instead of 0x3f2323",
            "error_path": "get_new_address.coin_ops.confirm_address.hw_ctx.protocol",
            "error_trace": "get_new_address:514] coin_ops:194] confirm_address:156] hw_ctx:177] protocol:88]",
            "error_type": "Internal",
            "error_data": "bad magic in v1 read: 0x3f346e instead of 0x3f2323"
        }
    },
    "id": null
}

And after that, trezor_connection_status gave a previously undiscovered error:

{
    "mmrpc": "2.0",
    "result": {
        "status": "Unreachable"
    },
    "id": null
}

Attempting to run task::init_trezor again (without restarting the device) led to another unique error not yet seen in the wild.

{
    "mmrpc": "2.0",
    "result": {
        "status": "Error",
        "details": {
            "error": "Internal: Received unexpected message type: Success",
            "error_path": "init_hw.crypto_ctx.hw_ctx.client",
            "error_trace": "init_hw:158] crypto_ctx:247] crypto_ctx:361] hw_ctx:58] client:107]",
            "error_type": "Internal",
            "error_data": "Received unexpected message type: Success"
        }
    },
    "id": null
}

Restarting the device (confirm: tried unplugging it and plugging it in again), which did not allow a restoration of conn.

{
    "mmrpc": "2.0",
    "result": {
        "status": "Error",
        "details": {
            "error": "Internal: Resource busy",
            "error_path": "init_hw.crypto_ctx.hw_client.usb.libusb",
            "error_trace": "init_hw:158] crypto_ctx:247] crypto_ctx:361] hw_client:176] hw_client:146] usb:84] libusb:138]",
            "error_type": "Internal",
            "error_data": "Resource busy"
        }
    },
    "id": null
}

After this, I restarted KDF and was once again able to connect to the Trezor.

smk762 avatar Jul 03 '25 03:07 smk762

BCH Is also failing to activate, confirmed on both the Safe 5 and the Model 1:

{
    "mmrpc": "2.0",
    "result": {
        "status": "Error",
        "details": {
            "error": "Error on platform coin BCH creation: FirmwareError",
            "error_path": "lib.init_standalone_coin.common_impl.coin_balance.mod.pubkey.client",
            "error_trace": "lib:104] init_standalone_coin:224] common_impl:68] coin_balance:496] mod:421] pubkey:163] client:93]",
            "error_type": "CoinCreationError",
            "error_data": {
                "ticker": "BCH",
                "error": "FirmwareError"
            }
        }
    },
    "id": null
}

smk762 avatar Jul 03 '25 07:07 smk762

Some irregularities (using TS5):

I see another DataError response when attempting to create additional address for MATIC/BNB/EVM.

Did this happen even after the quick fix for getting eth addresses (https://github.com/KomodoPlatform/komodo-defi-framework/commit/eeaee97594698dfdfcae36a34b5715f508b2c37e)? I received a positive response on "task::create_new_account::init" for tETH:

{"mmrpc":"2.0","result":{"status":"Ok","details":{"account_index":1,"derivation_path":"m/44'/1'/1'","total_balance":{},"addresses":[]}},"id":null}

(used the core emulator - for Safe/Model T models)

dimxy avatar Jul 04 '25 17:07 dimxy

An attempt to create a new address for BTC showed a different "expected address" in the status response than was shown on Trezor. I ran task::get_new_address::cancel.

Apparently this occurs because the new firmware validates the spending script type vs the derivation path (e.g. for m/84' the script should be Spendwitness) but we always use None for the script type.

I believe I fixed the above in f73e66a48d3b1eebd479d4dfdacfa71c6c45dda9 by adding the actual script type to the proto_bitcoin::GetAddress call (cc @shamardy) BTW if you did fixes in the coins file while testing (like fixing "derivation_path") you need to delete the kdf DB ($HOME/.kdf/DB) otherwise it appears to may have wrong accounts @smk762

dimxy avatar Jul 05 '25 12:07 dimxy

Thanks - confirmed no error in https://github.com/KomodoPlatform/komodo-defi-framework/commit/f73e66a48d3b1eebd479d4dfdacfa71c6c45dda9 with safe 5 creating addresses for LTC-segwit, BTC-segwit & BTC in expected format with kdf/trezor matching

smk762 avatar Jul 07 '25 10:07 smk762

BTW do you still see 'DataError' for EVM coins when calling create_new_account @smk762? (I hoped this was fixed in https://github.com/KomodoPlatform/komodo-defi-framework/commit/eeaee97594698dfdfcae36a34b5715f508b2c37e as DataEror was returned for the EthereumGetAddress Trezor call which should not be used in this fix)

dimxy avatar Jul 08 '25 12:07 dimxy

BTW do you still see 'DataError' for EVM coins when calling create_new_account @smk762? (I hoped this was fixed in eeaee97 as DataEror was returned for the EthereumGetAddress Trezor call which should not be used in this fix)

I'm failing to build that commit now:

   Compiling base-x v0.2.11
   Compiling asn1_der v0.7.6
error[E0282]: type annotations needed for `Box<_>`
  --> /home/smk/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/time-0.3.20/src/format_description/parse/mod.rs:83:9
   |
83 |     let items = format_items
   |         ^^^^^
...
86 |     Ok(items.into())
   |              ---- type must be known at this point
   |
   = note: this is an inference error on crate `time` caused by an API change in Rust 1.80.0; update `time` to version `>=0.3.35` by calling `cargo update`

   Compiling tower-service v0.3.2
   Compiling multibase v0.9.1
   Compiling rand v0.6.5
   Compiling secp256k1-sys v0.4.2
   Compiling ahash v0.7.6
   Compiling memoffset v0.6.4
   Compiling instant v0.1.12
   Compiling semver v1.0.6
   Compiling try-lock v0.2.2
   Compiling want v0.3.0
For more information about this error, try `rustc --explain E0282`.
error: could not compile `time` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...

Do you have a branch / PR for these changes so I can grab bins from CI?

smk762 avatar Jul 18 '25 05:07 smk762