⚙ D17573 [electrum] Sign stakes using Trezor devices

Fabien created this revision.Jan 21 2025, 14:22

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 21 2025, 14:22

Harbormaster completed remote builds in B32177: Diff 52327.Jan 21 2025, 14:25

Build Bitcoin ABC Diffs / Diff Testing (electrum-tests) failed.

Tail of the build log:

======================================================================
FAIL: test_proof_data (electrumabc.tests.test_avalanche.TestAvalancheProofFromHex.test_proof_data)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/work/electrum/electrumabc/tests/test_avalanche.py", line 445, in test_proof_data
    proof = Proof.from_hex(expected_proof1)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 58, in from_hex
    return cls.deserialize(BytesIO(bytes.fromhex(hex_str)))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 221, in deserialize
    return Proof(
           ^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 188, in __init__
    self.limitedid = LimitedProofId.build(
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 136, in build
    ss += serialize_sequence(stakes)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 128, in serialize_sequence
    b += obj.serialize()
         ^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 96, in serialize
    assert self.pubkey
AssertionError

======================================================================
FAIL: test_proofid (electrumabc.tests.test_avalanche.TestAvalancheProofFromHex.test_proofid)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/work/electrum/electrumabc/tests/test_avalanche.py", line 388, in test_proofid
    proof = Proof.from_hex(
            ^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 58, in from_hex
    return cls.deserialize(BytesIO(bytes.fromhex(hex_str)))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 221, in deserialize
    return Proof(
           ^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 188, in __init__
    self.limitedid = LimitedProofId.build(
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 136, in build
    ss += serialize_sequence(stakes)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 128, in serialize_sequence
    b += obj.serialize()
         ^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 96, in serialize
    assert self.pubkey
AssertionError

----------------------------------------------------------------------
Ran 334 tests in 30.140s

FAILED (failures=4, errors=4, skipped=4)
Testing `setup.py --version`: OK

ninja: build stopped: cannot make progress due to previous errors.
Build electrum-tests failed with exit code 1

It works

Back out unrelated changes

Harbormaster completed remote builds in B32181: Diff 52336.Jan 22 2025, 15:48

Build Bitcoin ABC Diffs / Diff Testing (electrum-tests) failed.

Tail of the build log:

======================================================================
FAIL: test_proof_data (electrumabc.tests.test_avalanche.TestAvalancheProofFromHex.test_proof_data)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/work/electrum/electrumabc/tests/test_avalanche.py", line 445, in test_proof_data
    proof = Proof.from_hex(expected_proof1)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 58, in from_hex
    return cls.deserialize(BytesIO(bytes.fromhex(hex_str)))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 221, in deserialize
    return Proof(
           ^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 188, in __init__
    self.limitedid = LimitedProofId.build(
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 136, in build
    ss += serialize_sequence(stakes)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 128, in serialize_sequence
    b += obj.serialize()
         ^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 96, in serialize
    assert self.pubkey
AssertionError

======================================================================
FAIL: test_proofid (electrumabc.tests.test_avalanche.TestAvalancheProofFromHex.test_proofid)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/work/electrum/electrumabc/tests/test_avalanche.py", line 388, in test_proofid
    proof = Proof.from_hex(
            ^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 58, in from_hex
    return cls.deserialize(BytesIO(bytes.fromhex(hex_str)))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 221, in deserialize
    return Proof(
           ^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 188, in __init__
    self.limitedid = LimitedProofId.build(
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 136, in build
    ss += serialize_sequence(stakes)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 128, in serialize_sequence
    b += obj.serialize()
         ^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 96, in serialize
    assert self.pubkey
AssertionError

----------------------------------------------------------------------
Ran 334 tests in 22.730s

FAILED (failures=4, errors=5, skipped=4)
Testing `setup.py --version`: OK

ninja: build stopped: cannot make progress due to previous errors.
Build electrum-tests failed with exit code 1

Harbormaster completed remote builds in B32182: Diff 52337.Jan 22 2025, 15:50

Build Bitcoin ABC Diffs / Diff Testing (electrum-tests) failed.

Tail of the build log:

======================================================================
FAIL: test_proof_data (electrumabc.tests.test_avalanche.TestAvalancheProofFromHex.test_proof_data)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/work/electrum/electrumabc/tests/test_avalanche.py", line 445, in test_proof_data
    proof = Proof.from_hex(expected_proof1)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 58, in from_hex
    return cls.deserialize(BytesIO(bytes.fromhex(hex_str)))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 221, in deserialize
    return Proof(
           ^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 188, in __init__
    self.limitedid = LimitedProofId.build(
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 136, in build
    ss += serialize_sequence(stakes)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 128, in serialize_sequence
    b += obj.serialize()
         ^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 96, in serialize
    assert self.pubkey
AssertionError

======================================================================
FAIL: test_proofid (electrumabc.tests.test_avalanche.TestAvalancheProofFromHex.test_proofid)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/work/electrum/electrumabc/tests/test_avalanche.py", line 388, in test_proofid
    proof = Proof.from_hex(
            ^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 58, in from_hex
    return cls.deserialize(BytesIO(bytes.fromhex(hex_str)))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 221, in deserialize
    return Proof(
           ^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 188, in __init__
    self.limitedid = LimitedProofId.build(
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 136, in build
    ss += serialize_sequence(stakes)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/serialize.py", line 128, in serialize_sequence
    b += obj.serialize()
         ^^^^^^^^^^^^^^^
  File "/work/electrum/electrumabc/avalanche/proof.py", line 96, in serialize
    assert self.pubkey
AssertionError

----------------------------------------------------------------------
Ran 334 tests in 42.445s

FAILED (failures=4, errors=5, skipped=4)
Testing `setup.py --version`: OK

ninja: build stopped: cannot make progress due to previous errors.
Build electrum-tests failed with exit code 1

Fix the tests

Harbormaster completed remote builds in B32248: Diff 52432.Jan 29 2025, 15:02

PiRK added a subscriber: PiRK.Jan 31 2025, 12:45

PiRK added inline comments.

electrum/electrumabc/avalanche/proof.py
284 ↗	(On Diff #52432)	Optional[AbstractWallet]?
electrum/electrumabc/tests/test_avalanche.py
227 ↗	(On Diff #52432)	Wrong indentation? I think the test is doing nothing at the moment.

PiRK added inline comments.Jan 31 2025, 13:12

electrum/electrumabc_gui/qt/main_window.py
956–964 ↗	(On Diff #52432)	Is this supposed to dynamically enable the menu action when the device is initialized and the firmware version is known? Or just delay the decision until the time the menu is shown the first time, in the hope that at this time the firmware version number is known? At the moment it can just disable the tools, not enable it if necessary.

PiRK added inline comments.Jan 31 2025, 13:19

electrum/electrumabc_plugins/trezor/clientbase.py
283 ↗	(On Diff #52432)	PublicKey.keydata is already used publicly in other places (e.g. DelegationEditor), so the round trip to/from hex is not necessary.

Fix the stake id not being computed upon import

Fabien edited the summary of this revision. (Show Details)Feb 11 2025, 19:34

Fabien edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B32381: Diff 52620.Feb 11 2025, 19:35

Fabien marked 2 inline comments as done.Feb 11 2025, 19:44

Fabien added inline comments.

electrum/electrumabc/avalanche/proof.py
284 ↗	(On Diff #52432)	why? It's not optional
electrum/electrumabc_gui/qt/main_window.py
956–964 ↗	(On Diff #52432)	This is a delay. When the menu is first built the support status is not known so it needs to be enabled or disabled later (when the user is able to click it).

Feedback

Harbormaster completed remote builds in B32382: Diff 52621.Feb 11 2025, 19:48

Cleanup, use the hw features capabilities to determine is stake signing is supported. This will avoid conflicts with Trezor firmware updates

Fabien published this revision for review.Feb 12 2025, 09:46

Fabien edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B32384: Diff 52623.Feb 12 2025, 09:47

PiRK added inline comments.Feb 12 2025, 10:36

electrum/electrumabc/avalanche/proof.py
284 ↗	(On Diff #52432)	Ok. I think the issue is in init where it shouldn't be optional.

PiRK added inline comments.Feb 12 2025, 10:39

electrum/electrumabc/avalanche/proof.py
249 ↗	(On Diff #52623)	not Optional?

Fabien added inline comments.Feb 12 2025, 12:22

electrum/electrumabc/avalanche/proof.py
249 ↗	(On Diff #52623)	Good catch

Remove uneeded optional

Harbormaster completed remote builds in B32386: Diff 52625.Feb 12 2025, 12:25

Fix signing with a password protected wallet

Harbormaster completed remote builds in B32388: Diff 52627.Feb 12 2025, 20:11

PiRK accepted this revision.Feb 13 2025, 07:50

This revision is now accepted and ready to land.Feb 13 2025, 07:50

PiRK requested changes to this revision.Feb 13 2025, 07:56

PiRK added inline comments.

electrum/electrumabc_plugins/trezor/clientbase.py
6 ↗	(On Diff #52627)	This import is going to cause error with all previous versions of trezorlib

This revision now requires changes to proceed.Feb 13 2025, 07:56

PiRK added inline comments.Feb 13 2025, 07:59

electrum/electrumabc_plugins/trezor/clientbase.py
6 ↗	(On Diff #52627)
272 ↗	(On Diff #52627)

Fix the import of trezorlib.ecash if not supported.
Fix the use of the non-existent capabability in trezorlib.

Harbormaster completed remote builds in B32391: Diff 52631.Feb 13 2025, 09:20

PiRK accepted this revision.Feb 13 2025, 09:59

This revision is now accepted and ready to land.Feb 13 2025, 09:59

Closed by commit rABC7ca27c8ac51b: [electrum] Sign stakes using Trezor devices (authored by Fabien). · Explain WhyFeb 13 2025, 12:25

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABC7ca27c8ac51b: [electrum] Sign stakes using Trezor devices.

[electrum] Sign stakes using Trezor devices
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 52637

electrum/electrumabc/avalanche/proof.py

electrum/electrumabc/keystore.py

electrum/electrumabc/tests/test_avalanche.py

electrum/electrumabc/wallet.py

electrum/electrumabc_gui/qt/avalanche/proof_editor.py

electrum/electrumabc_gui/qt/main_window.py

electrum/electrumabc_plugins/trezor/clientbase.py

electrum/electrumabc_plugins/trezor/trezor.py

[electrum] Sign stakes using Trezor devicesClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 52637

electrum/electrumabc/avalanche/proof.py

electrum/electrumabc/keystore.py

electrum/electrumabc/tests/test_avalanche.py

electrum/electrumabc/wallet.py

electrum/electrumabc_gui/qt/avalanche/proof_editor.py

electrum/electrumabc_gui/qt/main_window.py

electrum/electrumabc_plugins/trezor/clientbase.py

electrum/electrumabc_plugins/trezor/trezor.py

[electrum] Sign stakes using Trezor devices
ClosedPublic
Actions

Revision Contents
Changeset List