[electrum] Better support for Trezor safe devices
ClosedPublic
Actions

Authored by Fabien on Thu, Dec 5, 15:45.

Details

Reviewers

bytesofman
PiRK

Group Reviewers

Restricted Project

Commits

rABCd275d0e46129: [electrum] Better support for Trezor safe devices

Summary

This bumps trezorlib to support the safe 5 and improve the general handling of the safe series.

Test Plan

Run electrum with verson 0.13.9.
Note that it's been tested against emulators with mitigated success for the Safe 5.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

electrum_trezor_safe

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 31530
Build 62557: Build Diff	electrum-tests
Build 62556: arc lint + arc unit

Event Timeline

Fabien created this revision.Thu, Dec 5, 15:45

Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Dec 5, 15:45

Fabien requested review of this revision.Thu, Dec 5, 15:45

Harbormaster completed remote builds in B31530: Diff 51410.Thu, Dec 5, 15:48

bytesofman added a subscriber: bytesofman.Thu, Dec 5, 18:28

bytesofman added inline comments.

electrum/electrumabc_plugins/trezor/trezor.py
265	are we more than "pretty sure" now ? seems like we have some more confidence with this update

Fabien added inline comments.Thu, Dec 5, 18:37

electrum/electrumabc_plugins/trezor/trezor.py
265	are we more than "pretty sure" now ? seems like we have some more confidence with this update I have no idea but this seems like a bugfix anyway since there were no Safe series when this was introduced

bytesofman requested changes to this revision.Thu, Dec 5, 19:12

bytesofman added inline comments.

electrum/electrumabc_plugins/trezor/trezor.py
265

This revision now requires changes to proceed.Thu, Dec 5, 19:12

Fix comment

Harbormaster completed remote builds in B31535: Diff 51416.Thu, Dec 5, 19:39

bytesofman accepted this revision.Thu, Dec 5, 19:51

This revision is now accepted and ready to land.Thu, Dec 5, 19:51

At some point we can also change the regular (non-deterministic) requirements.txt to force >= 0.13.9 and drop the compat.py code. It can be done in a separate diff, though. I'm still trying to think of potential downsides of doing so. Electrum maintains support for trezor>=0.13.0 mainly because there are packagers on various Linux distros who have to work with older trezor packages. We don't have this issue with Electrum ABC.

I will build a beta AppImage and test that the release actually works with this bump. But I really see no reason why it wouldn"t. Trezorlib is pure python so no weird compatibility surprises to be expected related to compiled libs. And it supports all python versions >=8 (we package with 3.11

In D17294#392907, @PiRK wrote:

I will build a beta AppImage and test that the release actually works with this bump. But I really see no reason why it wouldn"t. Trezorlib is pure python so no weird compatibility surprises to be expected related to compiled libs. And it supports all python versions >=8 (we package with 3.11

I built the linux release btw and no issue

Closed by commit rABCd275d0e46129: [electrum] Better support for Trezor safe devices (authored by Fabien). · Explain WhyThu, Dec 5, 22:16

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABCd275d0e46129: [electrum] Better support for Trezor safe devices.