Page MenuHomePhabricator

[electrum] fix hardware wallets (except Trezor)
ClosedPublic

Authored by PiRK on Oct 9 2023, 07:14.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCb7518269852e: [electrum] fix hardware wallets (except Trezor)
Summary

It seems like I did all my recent hardware wallet test tih a trezor device, and meanwhile the other were broken because of recent change.

The main two issues that this diff fixes are:

  • KeyStore.get_tx_derivations now returns a dict indexed by xpubkeys as bytes, and the code still expected it to use hex strings
  • updating the signatures in the coin dict returned by Transaction.inputs() no longer has any effect, as the dicts are build on demand for convenience but are no longer used by Transaction

In addition to this, D14567 introduced a bug for satochip when computing the hash of the tx preimage by hashing the hex version of the preimage instead of bytes. The Hash function supports both bytes and strings as inputs, but it does not assume that strings are hex encoded bytes but rather text to be encoded as "utf-8"

Test Plan

With a satochip device and a ledger device, test sending a transaction. Test creating and signing an unsigned tx in a multisig setup.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
fix_hw
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 25297
Build 50179: Build Diffelectrum-tests
Build 50178: arc lint + arc unit

Event Timeline

PiRK requested review of this revision.Oct 9 2023, 07:14
PiRK edited the summary of this revision. (Show Details)

remove a stray typehint. I put this here for debugging to let my IDE highlight any potential issue with the txin API

electrum/electrumabc_plugins/ledger/ledger.py
495 ↗(On Diff #42593)

sequence is always defined for a TxInput.

This revision is now accepted and ready to land.Oct 9 2023, 17:47