Skip to content

Fix two broken key-generation code paths - #20

Draft
ZenulAbidin wants to merge 2 commits into
masterfrom
cursor/fix-key-generation-2b83
Draft

ZenulAbidin wants to merge 2 commits into
masterfrom
cursor/fix-key-generation-2b83

Conversation

@ZenulAbidin

Copy link
Copy Markdown
Owner

Summary

Fixes two deterministic, reproducible bugs in the key-generation API. Both are exercised by the public surface (create_keypair is exported from zpywallet/__init__.py), both fail 100% of the time, and neither was covered by tests.

Bug 1 — PrivateKey.from_random() always returns the zero scalar

i = 0
while i < 0 or i >= secp256k1.N:   # 0 is not < 0 and not >= N, so the loop body never runs
    i = int.from_bytes(Random.new().read(32), byteorder="big")
return PrivateKey.from_int(i, network)   # from_int(0) -> coincurve rejects it

i starts at 0; the guard never rejects 0, so the loop body never executes and from_int(0) raises:

ValueError: Secret scalar must be greater than 0 and less than 115792089237316195423570985008687907852837564279074904382605163141518161494337.

Even if the loop had run, 0 would still have been accepted. Fix: reject 0 as well (while i <= 0 or i >= secp256k1.N).

Bug 2 — create_keypair() passes raw bytes to PrivateKey()

random_bytes = urandom(32)
prv = PrivateKey(random_bytes, network=network)   # __init__ calls ckey.to_hex()

PrivateKey.__init__ expects a coincurve.PrivateKey (it calls ckey.to_hex()), but raw bytes were passed, so every call raised:

AttributeError: 'bytes' object has no attribute 'to_hex'

Fix: build the key via the (now-correct) PrivateKey.from_random(network), which also guarantees a valid in-range scalar.

Reproduction (before)

[FAIL] create_keypair(BTC) AttributeError: 'bytes' object has no attribute 'to_hex'
[FAIL] create_keypair(ETH) AttributeError: 'bytes' object has no attribute 'to_hex'
[FAIL] PrivateKey.from_random ValueError: Secret scalar must be greater than 0 ...

Tests

  • tests/test_12_keys.py::test_008_from_random_returns_valid_scalar — asserts from_random yields a scalar in [1, N-1] and a usable address, across repeated draws.
  • tests/test_05_zpywallet.py::test_010_create_keypair — asserts create_keypair returns a consistent (PrivateKey, PublicKey) pair for BTC (base58 address) and ETH (0x hex address).

Validation

Check Result
Full suite (pytest tests) 166 passed, 7 skipped (was 164 + 2 new)
flake8 (repo config) Passed
Repro probe, post-fix, 5x create_keypair + from_random all OK

No behavioral change to any already-working path; the diff to library code is 2 lines.

Open in Web Open in Cursor 

cursoragent and others added 2 commits September 13, 2026 12:55
Co-authored-by: Ali Sherief <ali@zenulabidin.com>
Co-authored-by: Ali Sherief <ali@zenulabidin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants