Page MenuHomePhabricator
Differential D16675

[electrum] don't use ecdsa for basic bytes <-> int conversion
ClosedPublic
Actions

Authored by PiRK on Aug 23 2024, 11:52.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC1c69a840da62: [electrum] don't use ecdsa for basic bytes <-> int conversion
Summary

There are python builtin methods for that.

It is also a good opportunity to remove the misleading Python 2 era _to_string terminology when the output is actually bytes.

Partial backport of electrum#5947
https://github.com/spesmilo/electrum/pull/5947/commits/2cf213552822b0feda4775ed94719679bcb1efb4

Depends on D16665

Test Plan

python test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
electrum_crypto
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30072
Build 59678: Build Diffelectrum-tests
Build 59677: arc lint + arc unit

Event Timeline

PiRK created this revision.Aug 23 2024, 11:52
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 23 2024, 11:52
PiRK requested review of this revision.Aug 23 2024, 11:52
PiRK edited the summary of this revision. (Show Details)Aug 23 2024, 11:52
PiRK edited the summary of this revision. (Show Details)Aug 23 2024, 11:55
Harbormaster completed remote builds in B30072: Diff 49335.Aug 23 2024, 12:07
PiRK added a child revision: D16676: [electrum] refactor private key class to better abstract ecdsa library usage.Aug 23 2024, 16:14
PiRK added a child revision: D16677: [electrum] move .encrypt method to ECPubkey class.Aug 23 2024, 19:18
PiRK removed a child revision: D16676: [electrum] refactor private key class to better abstract ecdsa library usage.Aug 23 2024, 19:54
bytesofman added a subscriber: bytesofman.Aug 26 2024, 23:18
bytesofman added inline comments.
electrum/electrumabc/tests/test_ecc.py
34

do you think it makes sense to keep this 32 as a defined constant somewhere, e.g. PRIVATE_KEY_BYTECOUNT ?

repeated 4 times in this diff, isn't (widely) intuitive why this should be 32.

PiRK added inline comments.Aug 27 2024, 06:54
electrum/electrumabc/tests/test_ecc.py
34

You're right, that would improve things quite a bit.

PiRK updated this revision to Diff 49364.Aug 27 2024, 06:55

replace the magic 32 number with a constant

Harbormaster completed remote builds in B30094: Diff 49364.Aug 27 2024, 06:57
bytesofman accepted this revision.Aug 27 2024, 19:08
This revision is now accepted and ready to land.Aug 27 2024, 19:08
Closed by commit rABC1c69a840da62: [electrum] don't use ecdsa for basic bytes <-> int conversion (authored by PiRK). · Explain WhyAug 28 2024, 06:26
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC1c69a840da62: [electrum] don't use ecdsa for basic bytes <-> int conversion.

Revision Contents
Changeset List

PathSize
electrum/
electrumabc/
11 lines
19 lines
12 lines
tests/
3 lines
6 lines

Diff 49335

View Options

electrum/electrumabc/bip32.py

Loading...
View Options

electrum/electrumabc/ecc.py

Loading...
View Options

electrum/electrumabc/keystore.py

Loading...
View Options

electrum/electrumabc/tests/test_ecc.py

Loading...
View Options

electrum/electrumabc/x509.py

Loading...