[electrum] cleanup ser_to_point
ClosedPublic
Actions

Authored by PiRK on Jun 19 2024, 12:24.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC1e5fc68f200a: [electrum] cleanup ser_to_point

Summary

Use better variable names, split the function from ser_to_point(bytes) -> Point into ser_to_coordinates(bytes) -> (int, int)] and ser_to_point(bytes) -> Point.

This will allow us in future commits to remove some ecdsa import in other modules, to keep implementation details contained in ecc.py.

Depends on D16347

This is split out of D16301

Test Plan

python test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Jun 19 2024, 12:24

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 19 2024, 12:24

PiRK requested review of this revision.Jun 19 2024, 12:24

Harbormaster completed remote builds in B29339: Diff 48296.Jun 19 2024, 12:26

PiRK added a child revision: D16349: [electrum] introduce a new ECPubkey class.Jun 19 2024, 12:28

PiRK edited the test plan for this revision. (Show Details)Jun 19 2024, 15:47

Fabien requested changes to this revision.Jun 19 2024, 19:45

Fabien added a subscriber: Fabien.

Fabien added inline comments.

electrum/electrumabc/ecc.py
167 ↗	(On Diff #48296)	It' no longer returning a point but coordinates => ser_to_coordinates is a better name
177 ↗	(On Diff #48296)	I understand why you named it like so but it puzzled me at first, as ecdsa is not a curve but an algorithm so it has no point. Not sure if the "ecdsa python module format" idea can be conveyed with a better name though, so I leave that up to you

This revision now requires changes to proceed.Jun 19 2024, 19:45

PiRK added inline comments.Jun 19 2024, 21:07

electrum/electrumabc/ecc.py
177 ↗	(On Diff #48296)	I agree that the name is pretty bad. I kept it just to make future backports easier, but if we are going to rename the other one this one can get a better name too.

rename ser_to_point -> ser_to_coordinates and ser_to_ecdsa_python_point -> ser_to_point
Use a NamedTuple for the output of ser_to_point (better type-safety than a generic 2-tuple)

This makes this diff much smaller

Harbormaster completed remote builds in B29353: Diff 48315.Jun 20 2024, 06:51

Fabien accepted this revision.Jun 20 2024, 07:13

Fabien added inline comments.