Page MenuHomePhabricator

[electrum] Update trezor firmware
AbandonedPublicDraft

Authored by Fabien on Fri, Mar 7, 21:45.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

Automated process for updating the firmware of Trezor devices. This lets you install the latest Trezor firmware, a custom firmware from a file or an eCash firmware (not available for model 1 yet).

Test Plan

I ran it successfully on all the supported devices

Diff Detail

Repository
rABC Bitcoin ABC
Branch
electrum_update_trezor_firmware
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 32675
Build 64838: Build Diffelectrum-tests
Build 64837: arc lint + arc unit

Event Timeline

PiRK added inline comments.
electrum/electrumabc_plugins/hw_wallet/qt.py
246–249 ↗(On Diff #52975)

Using a different attribute for the two different dialogs would make these checks unnecessary (just if self.waiting_dialog is not None)

electrum/electrumabc_plugins/trezor/qt.py
942 ↗(On Diff #52975)

move comment to previous line so it doesn't affect the formatting

electrum/electrumabc_plugins/hw_wallet/qt.py
246–249 ↗(On Diff #52975)

I suppose the idea was to have only a single attribute to clear in self.clear_dialog() ? I guess if you are sure that there will never be both dialogs used at the same time, this is fine. If for some reason both dialogs are activated at the same time, I'm not sure what would happen. Maybe one of them will not be cleared as the self.dialog reference will point to the second one?

I think in the past it was a bit dangerous to have a QWidget without a live python reference to it, as the garbage collector was triggering a call to the c++ destructor while signals could still be connected. I think these days PyQt is handling these situations better, without segfaults.

electrum/electrumabc_plugins/trezor/qt.py
1085 ↗(On Diff #52975)

you could use a simpler HBox/VBoxLayout here

electrum/electrumabc_plugins/hw_wallet/qt.py
246–249 ↗(On Diff #52975)

The idea is that you should not have both at the same time (actually the same assumption holds for all the other dialogs as well). I will consider making this another attribute but not sure if this is an improvement or a hazard

electrum/electrumabc_plugins/trezor/qt.py
942 ↗(On Diff #52975)

yup, copy pasta

1085 ↗(On Diff #52975)

I think it should go to the advanced tab in the end, it was just easier for testing to add a tab

Rebase, feature complete

Apparently I did something during cleanup that broke the feature for model 1, investigating

Fix firmware header type when empty, unbreaking model 1 update

The data files probably need to be added to package_data in setup.py if we want them to be in the release

electrum/electrumabc_plugins/trezor/qt.py
1042 ↗(On Diff #53037)

remove

Catch duplicated bootloader unlock, remove debug print

I still think the setup.py is going to need another line to package the firmware data. I'm building another AppImage to test it with the f-string fixed.

electrum/electrumabc_plugins/trezor/qt.py
978 ↗(On Diff #53038)

While testing the AppImage release (python 3.11):

  File "/tmp/.mount_ElectrvuJaIu/usr/lib/python3.11/site-packages/electrumabc_plugins/trezor/qt.py", line 978
    f"The latest Trezor firmware is version {".".join([str(v) for v in version])}\n\n"
                                              ^
SyntaxError: f-string: expecting '}'
electrum/electrumabc_plugins/trezor/clientbase.py
393 ↗(On Diff #53038)
  File "/tmp/.mount_Electr0wzneO/usr/lib/python3.11/site-packages/electrumabc_plugins/trezor/trezor.py", line 45, in <module>
    from .clientbase import TrezorClientBase, parse_path
  File "/tmp/.mount_Electr0wzneO/usr/lib/python3.11/site-packages/electrumabc_plugins/trezor/clientbase.py", line 393
    f"Firmware vendor: {firmware_vendor or ("unknown" if model != "1" else "not embedded in Model 1 firmware")}\n"
                                             ^^^^^^^
SyntaxError: f-string: unmatched '('
electrum/electrumabc_plugins/trezor/clientbase.py
499 ↗(On Diff #53038)

I confirm that this is needed for release binaries:

diff --git a/electrum/setup.py b/electrum/setup.py
index df99211620..64b9ebb697 100644
--- a/electrum/setup.py
+++ b/electrum/setup.py
@@ -212,7 +212,7 @@ setup(
             "locale/*/LC_MESSAGES/electron-cash.mo",
         ],
         "electrumabc_plugins.fusion": ["*.svg", "*.png"],
-        "electrumabc_plugins.trezor": ["homescreen/*.jpg"],
+        "electrumabc_plugins.trezor": ["homescreen/*.jpg", "firmware/*.bin"],
         # On Linux and Windows this means adding electrumabc_gui/qt/data/*.ttf
         # On Darwin we don't use that font, so we don't add it to save space.
         **platform_package_data,

Without:

Screenshot from 2025-03-12 20-39-18.png (169×465 px, 13 KB)