Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC7ca27c8ac51b: [electrum] Sign stakes using Trezor devices

Summary

This makes it possible to sign stakes with a Trezor device running a compatible firmware.
For now such a firmware doesn't exist unless it's built manually (so it's not official).

Test Plan

Sign a proof with a Trezor.
Run the tests.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

electrum_sign_stake_trezor

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 32386
Build 64261: Build Diff	electrum-tests
Build 64260: arc lint + arc unit

Event Timeline

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

Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Jan 21, 14:22

Harbormaster completed remote builds in B32177: Diff 52327.Tue, Jan 21, 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.Wed, Jan 22, 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.Wed, Jan 22, 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.Wed, Jan 29, 15:02

PiRK added a subscriber: PiRK.Fri, Jan 31, 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.Fri, Jan 31, 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.Fri, Jan 31, 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)Tue, Feb 11, 19:34

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

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

Fabien marked 2 inline comments as done.Tue, Feb 11, 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.Tue, Feb 11, 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.Wed, Feb 12, 09:46

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

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

PiRK added inline comments.Wed, Feb 12, 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.Wed, Feb 12, 10:39

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

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

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

Remove uneeded optional

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

Fix signing with a password protected wallet

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

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

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

PiRK requested changes to this revision.Thu, Feb 13, 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.Thu, Feb 13, 07:56

PiRK added inline comments.Thu, Feb 13, 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.Thu, Feb 13, 09:20

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

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

Closed by commit rABC7ca27c8ac51b: [electrum] Sign stakes using Trezor devices (authored by Fabien). · Explain WhyThu, Feb 13, 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 52625

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 52625

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