Repository navigation
electrum-ecc: assorted typing and CI hygiene gaps #668
Description
Activity
Re-verified 2026-08-14
Still
6314eb3f(0.0.7). Each item, checked:- Implicit-Optional:
grind_r_value: bool = None(keys.py:394),
aux_rand32: bytes = None(keys.py:440). Thebip32.py:427-428and
descriptor.py:361,369,393entries are in the parentspesmilo/electrum
repository, not this one — the body says so, but the report must not,
or it will be answered with "wrong repo".descriptor.py's three are at
L361, L369 and L393 at electrum master today. - No
py.typed:git ls-filesshowsMANIFEST.in,pyproject.toml,
setup.pyand no marker file. Confirmed. - CI:
.github/workflows/ci.ymlusesactions/checkout@v6(L27, L58)
andactions/setup-python@v6(L32, L62), by tag. It does set
timeout-minutes: 15(L20) — the body did not claim otherwise, but the
report should not either. There is nopermissions:block anywhere in
the file. ENABLE_ECDSA_R_VALUE_GRINDING:__init__.py:12, a module-level
True, read atkeys.py:407-408on each call.
The tracker has no open issues and one open PR
(#17, MuSig2 bindings),
so nothing here is a duplicate.What to send: three PRs, no issue
Each is small and self-evident; an issue asking permission for a
py.typed
file would be noise. Order matters — send the first alone, and let its
reception decide whether the other two are worth the maintainers' time.PR 1 —
add py.typed. One empty file plus the packaging line that ships
it (setup.py/MANIFEST.in, whichever their build uses). The body is two
sentences: the package is annotated, and without the marker PEP 561 makes
those annotations invisible to every downstream mypy run, Electrum's own
included. Highest value per byte of the three.PR 2 — implicit-Optional.
grind_r_value: Optional[bool] = Noneand
aux_rand32: Optional[bytes] = None. Two lines. Worth pairing with PR 1 in
the same PR if the maintainers prefer fewer PRs — mention the option in the
body, since withpy.typedshipped these two annotations become other
people's mypy errors, which is the argument for fixing them at all.PR 3 — CI. SHA-pin the two actions with the tag in a trailing comment,
addpermissions: contents: read. Say plainly that this is hygiene matching
what the parentspesmilo/electrumrepository'stests.ymlalready does —
the child being looser than the parent that depends on it is the whole
argument, and it is a good one.ENABLE_ECDSA_R_VALUE_GRINDING: report, do not patch. A module-level
mutable switch that changes signing behaviour process-wide is a design
choice with a stated reason ("Some unit tests need..."). The useful thing is
a question in PR 3's description or a separate short issue: would a
context manager, or a per-call parameter, be preferred? Sending a patch that
changes it unasked is how a small hygiene PR turns into an argument.Verified against btclib
d7c4b3d5.- Implicit-Optional:
The py.typed item is filed upstream as spesmilo/electrum-ecc#22. The other three items (implicit-Optional annotations, CI SHA-pinning/permissions, the grinding global) remain; keeping this open for them.
Closing without further follow-up. The py.typed item — the one with clear downstream value — is filed upstream as spesmilo/electrum-ecc#22. The remaining three (implicit-Optional annotations, CI SHA-pinning/permissions, the ENABLE_ECDSA_R_VALUE_GRINDING global) are project-configuration preferences a maintainer can reasonably decline, the same bar that closed #659; not worth an upstream report.
What is measured
spesmilo/electrum-ecc, pinned at6314eb3f(tag 0.0.7, 2026-05-13). Small items grouped into one issue, matching how
btclib's own #149 groups this class of finding:
grind_r_value: bool = None(keys.py:394),aux_rand32: bytes = None(keys.py:440),derivation_prefix: str = None, root_fingerprint: str = None(
bip32.py:427-428),sigdata: Mapping[bytes, bytes] = None(descriptor.py:361,369,393,in the parent
electrumrepo's fork of this pattern).py.typedin the publishedelectrum-eccpackage — itsannotations (implicit-Optional issues aside) are invisible to a
downstream mypy user.
permissions:block(
.github/workflows/ci.yml) — inconsistent with the parentelectrumrepository's own
tests.yml, which already SHA-pins with tag commentsand sets
permissions: contents: read. The child package is looserthan the parent that depends on it.
ENABLE_ECDSA_R_VALUE_GRINDINGis a mutable module-level global in__init__.py, with a comment "Some unit tests need..." — atest-configuration switch left reachable from any importer.
Before sending upstream
Draft only. All four are small and independent;
py.typedalone is wortha standalone one-line PR given its outsized downstream value, the rest
can go together or separately at the reporter's discretion.