Skip to content

Conversation

@dimxy
Copy link
Collaborator

@dimxy dimxy commented Aug 1, 2025

Several fixes for interaction with Trezor devices:

  • fix DataError returned for segwit calls by specifying the correct input script (relevant to the Legacy models).
  • do not use EthereumPublicKey message (broken in firmware) - fixes ETH-like coins activation in KW (relevant to the Trezor Safe model)
  • fix serialisation of the address_n field in EVM messages - fixes DataError in the KDF get_new_address call (observed for Safe models)

Partially fixes #2515

dimxy added 3 commits July 2, 2025 20:47
* dev: (21 commits)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (#2454)
  ci(pull-requests): review notification bot (#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538)
  bless clippy (#2560)
  refactor(toolchain): use latest available stable compiler (#2557)
  feat(wallet): implement unified offline private key export API (#2542)
  improve note for docker test start failure (#2550)
  fix(DOCS): add note for macos to fix docker containers startup failure (#2544)
  refactor(toolchain): general stabilization for stable rust (#2528)
  fix(ci): adds nodejs 20 to ci-container (#2536)
  fix(WASM and Debian): fix build failures (#2534)
  improvement(event-streaming): impl DeriveStreamerId trait for all streamers (#2489)
  fix(eth): Propagate structured EIP-1559 fee errors (#2532)
  fix(eth): Correctly implement ETH max withdrawal logic (#2531)
  feat(use-clap-for-cli): use clap to parse CLI-Args #2215 (#2510)
  feat(orderbook): expirable maker orders (#2516)
  improvement(eth): drop parity support (#2527)
  chore(release): finalize changelog for v2.5.0-beta (#2524)
  chore(toolchain): upgrade toolchain to nightly 1.86.0 (#2444)
  ...

# Conflicts:
#	mm2src/coins/lp_coins.rs
#	mm2src/coins/rpc_command/get_new_address.rs
#	mm2src/trezor/src/eth/eth_command.rs
@dimxy dimxy self-assigned this Aug 1, 2025
@dimxy dimxy added priority: high Important tasks that need attention soon. bug: core labels Aug 1, 2025
@dimxy dimxy marked this pull request as ready for review August 1, 2025 07:07
@dimxy dimxy requested review from shamardy and smk762 August 1, 2025 07:07
Comment on lines +4303 to +4304
/// TODO: should we use #[serde(tag = "type", content = "params")] for this PrivKeyActivationPolicy like for the Eth policy,
/// to have them identical in activation requests
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is an issue for this #2522

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe fix this in a separate PR
(will be a breaking change for the API)

shamardy
shamardy previously approved these changes Aug 1, 2025
Copy link
Collaborator

@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.

LGTM!

@shamardy
Copy link
Collaborator

shamardy commented Aug 1, 2025

@smk762 please check / approve this if it resolves the issues with EVM and segwit coins.

@smk762
Copy link

smk762 commented Aug 4, 2025

  • LTC-segwit now creates correct address format and is accepted without error by the Trezor device for confirmation. ✅

  • MATIC activated successfully on Safe 5. ✅

  • The task-based scan_for_new_addresses, create_new_account and account_balance RPCs are functioning as expected for ETH/MATIC, but some RPCs return errors, detailed below.

  • MATIC task-based address creation fails.

{
    "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:556] coin_ops:198] confirm_address:183] client:95]",
            "error_type": "HwError",
            "error_data": "DataError"
        }
    },
    "id": null
}
  • MATIC withdraw attempt without inputting the "from" params led to ambiguous error (should be InvalidParams):
{
    "mmrpc": "2.0",
    "result": {
        "status": "Error",
        "details": {
            "error": "Internal error: DataError",
            "error_path": "eth_withdraw.client",
            "error_trace": "eth_withdraw:473] client:95]",
            "error_type": "InternalError",
            "error_data": "DataError"
        }
    },
    "id": null
}

@dimxy
Copy link
Collaborator Author

dimxy commented Aug 5, 2025

  • MATIC task-based address creation fails.
    "error_data": "DataError"

Confirm this error, investigating...

@dimxy
Copy link
Collaborator Author

dimxy commented Aug 6, 2025

  • MATIC task-based address creation fails.
    "error_data": "DataError"

Fixed MATIC get_new_address DataError in 67604e7 (actually it did not work for all EVM coins)
I also tried withdraw without the "from" param - worked
cc: @smk762

Copy link

@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.

Confirmed working, thanks!

  • MATIC/EVM address creation ✔️
  • MATIC/EVM withdraw without from ✔️
  • MATIC/EVM withdraw with from ✔️

dependencies = [
"bytes",
"prost-derive 0.14.1",
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

We now have 2 prost versions which I don't like. I see that latest tendermint-rs version is 0.13 and it looks that this crate is not maintained anymore, as last time a PR was merged was in May. I will leave this as is for now but we need to open an issue for the Tendermint crate and how to deal with it c.c. @onur-ozkan

Choose a reason for hiding this comment

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

They have PRs created very recently so it seems like it's maintained. I sent a PR to them which bumps prost.

Copy link
Collaborator Author

@dimxy dimxy Aug 13, 2025

Choose a reason for hiding this comment

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

I can downgrade back prost for trezor, if needed (I think this was not needed very much)

Choose a reason for hiding this comment

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

It's fine as we will get the update on tendermint too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

They have PRs created very recently so it seems like it's maintained. I sent a PR to them which bumps prost.

Last merged PR was in May and the recently opened PRs are not replied to.

@shamardy shamardy merged commit 4c830ce into dev Aug 12, 2025
19 of 25 checks passed
@shamardy shamardy deleted the fix-2.5-rc-trezor-issues branch August 12, 2025 17:28
dimxy pushed a commit that referenced this pull request Aug 12, 2025
* dev:
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)

# Conflicts:
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
shamardy added a commit that referenced this pull request Aug 12, 2025
shamardy added a commit that referenced this pull request Aug 12, 2025
dimxy pushed a commit that referenced this pull request Aug 13, 2025
* dev:
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
dimxy pushed a commit that referenced this pull request Aug 15, 2025
* dev: (24 commits)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (#2454)
  ci(pull-requests): review notification bot (#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538)
  bless clippy (#2560)
  refactor(toolchain): use latest available stable compiler (#2557)
  feat(wallet): implement unified offline private key export API (#2542)
  improve note for docker test start failure (#2550)
  fix(DOCS): add note for macos to fix docker containers startup failure (#2544)
  refactor(toolchain): general stabilization for stable rust (#2528)
  ...

# Conflicts:
#	mm2src/coins/eth.rs
#	mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs
#	mm2src/coins/eth/eth_swap_v2/eth_taker_swap_v2.rs
#	mm2src/coins/eth/eth_tests.rs
#	mm2src/coins/eth/eth_withdraw.rs
#	mm2src/coins/eth/v2_activation.rs
#	mm2src/coins/nft.rs
#	mm2src/coins/qrc20.rs
#	mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/rpcs.rs
#	mm2src/mm2_main/src/rpc/lp_commands/tokens.rs
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
dimxy pushed a commit that referenced this pull request Aug 19, 2025
* dev:
  improvement(`static mut`s): `static mut` removal (#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (#2454)
  ci(pull-requests): review notification bot (#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538)
  bless clippy (#2560)
  refactor(toolchain): use latest available stable compiler (#2557)
  feat(wallet): implement unified offline private key export API (#2542)
  chore(release): v2.3.0-beta (#2284)

# Conflicts:
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
dimxy pushed a commit that referenced this pull request Aug 21, 2025
* dev:
  fix build script failing to find .git/HEAD (#2601)
  refactor(EVM): rename fn, fix timeouts, add activation req validation (#2543)
  improvement(`static mut`s): `static mut` removal (#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
dimxy pushed a commit that referenced this pull request Aug 21, 2025
* dev:
  fix build script failing to find .git/HEAD (#2601)
  refactor(EVM): rename fn, fix timeouts, add activation req validation (#2543)
  improvement(`static mut`s): `static mut` removal (#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)

# Conflicts:
#	mm2src/mm2_main/src/lp_swap/taker_swap.rs
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request Aug 21, 2025
* dev: (28 commits)
  fix build script failing to find .git/HEAD (GLEECBTC#2601)
  refactor(EVM): rename fn, fix timeouts, add activation req validation (GLEECBTC#2543)
  improvement(`static mut`s): `static mut` removal (GLEECBTC#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (GLEECBTC#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (GLEECBTC#2580)
  fix(clippy): fix clippy warnings for GLEECBTC#2565 (GLEECBTC#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (GLEECBTC#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (GLEECBTC#2564)
  feat(protocol): [0] solana support (GLEECBTC#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (GLEECBTC#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (GLEECBTC#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (GLEECBTC#2572)
  improvement(dep-stack): security bumps (GLEECBTC#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (GLEECBTC#2563)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (GLEECBTC#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (GLEECBTC#2454)
  ci(pull-requests): review notification bot (GLEECBTC#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (GLEECBTC#2538)
  bless clippy (GLEECBTC#2560)
  refactor(toolchain): use latest available stable compiler (GLEECBTC#2557)
  ...

# Conflicts:
#	mm2src/coins/eth.rs
#	mm2src/coins/eth/eth_rpc.rs
#	mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs
#	mm2src/coins/eth/eth_swap_v2/eth_taker_swap_v2.rs
#	mm2src/coins/eth/eth_withdraw.rs
#	mm2src/coins/eth/v2_activation.rs
#	mm2src/coins/lightning.rs
#	mm2src/coins/lp_coins.rs
#	mm2src/coins/nft.rs
#	mm2src/coins/qrc20.rs
#	mm2src/coins/siacoin.rs
#	mm2src/coins/tendermint/tendermint_token.rs
#	mm2src/coins/test_coin.rs
#	mm2src/coins/utxo/bch.rs
#	mm2src/coins/utxo/qtum.rs
#	mm2src/coins/utxo/slp.rs
#	mm2src/coins/utxo/utxo_standard.rs
#	mm2src/coins/z_coin.rs
#	mm2src/coins_activation/src/bch_with_tokens_activation.rs
#	mm2src/coins_activation/src/erc20_token_activation.rs
#	mm2src/coins_activation/src/eth_with_token_activation.rs
#	mm2src/coins_activation/src/init_erc20_token_activation.rs
#	mm2src/coins_activation/src/init_token.rs
#	mm2src/coins_activation/src/platform_coin_with_tokens.rs
#	mm2src/coins_activation/src/slp_token_activation.rs
#	mm2src/coins_activation/src/tendermint_with_assets_activation.rs
#	mm2src/coins_activation/src/token.rs
#	mm2src/mm2_main/src/lp_swap.rs
#	mm2src/mm2_main/src/lp_swap/check_balance.rs
#	mm2src/mm2_main/src/lp_swap/maker_swap.rs
#	mm2src/mm2_main/src/lp_swap/max_maker_vol_rpc.rs
#	mm2src/mm2_main/src/lp_swap/swap_v2_rpcs.rs
#	mm2src/mm2_main/src/lp_swap/taker_swap.rs
#	mm2src/mm2_main/src/lp_swap/trade_preimage.rs
#	mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs
#	mm2src/mm2_main/src/rpc/lp_commands/legacy.rs
#	mm2src/mm2_main/src/rpc/lp_commands/lr_swap.rs
#	mm2src/mm2_main/src/rpc/lp_commands/lr_swap/lr_impl.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/errors.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/rpcs.rs
#	mm2src/mm2_main/src/rpc/lp_commands/tokens.rs
#	mm2src/mm2_main/tests/docker_tests/docker_tests_common.rs
#	mm2src/mm2_main/tests/docker_tests/docker_tests_inner.rs
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
#	mm2src/mm2_main/tests/integration_tests_common/mod.rs
#	mm2src/trading_api/src/one_inch_api/client.rs
dimxy pushed a commit that referenced this pull request Oct 15, 2025
- UTXO: set InputScriptType (SpendAddress/SpendWitness) to avoid DataError
- EVM: use bitcoin GetPublicKey instead of broken EthereumGetPublicKey (Safe/T)
- EVM proto: mark address_n as non-packed; fix get_new_address DataError
- deps(trezor): bump prost to 0.14.1 (keep 0.12.x for other crates)
dimxy pushed a commit that referenced this pull request Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.6.0-beta bug: core priority: high Important tasks that need attention soon.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants