v0.3 audit tail: address validation + Stratum tests + diagnostics#4
Merged
devAsmodeus merged 3 commits intomainfrom Apr 30, 2026
Merged
v0.3 audit tail: address validation + Stratum tests + diagnostics#4devAsmodeus merged 3 commits intomainfrom
devAsmodeus merged 3 commits intomainfrom
Conversation
Adds stdlib-only BTC address validation in src/hope_hash/address.py covering bech32 (BIP-173), bech32m (BIP-350), and Base58Check. Mainnet only — testnet addresses rejected with explicit error. Wired into cli.main() before pool connect: typos, mixed case, bad checksums, and unsupported prefixes are caught locally with a human-readable message instead of opaque pool authorize failures. 18 tests in tests/test_address.py covering: - bech32 v0 P2WPKH/P2WSH, uppercase, mixed-case rejection, bad checksum - bech32m v1 Taproot, v0/v1+ checksum mismatch - Base58Check P2PKH (genesis) / P2SH, bad checksum, invalid charset - empty/non-string/unknown-prefix/testnet rejection Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
StratumClient was the largest untested surface in v0.3 — protocol logic relied on offline reasoning + manual smoke runs. This adds an in-memory FakeSocket that replays scripted server messages and captures client output, plus push_line() for live-feeding reader_loop in tests. Coverage: - subscribe: extranonce parsing + out-of-order server messages - authorize: success path + ConnectionError on pool rejection - suggest_difficulty auto-sent after authorize - _handle_message: set_difficulty / set_extranonce (resets job) / notify (all fields) / submit response accepted/rejected / non-submit id ignored / req_id consumed once - submit: req_id tracking, parameter encoding - reader_loop: graceful stop via stop_event + close, survives bad JSON - close() idempotent and safe without prior connect() Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mop-up of remaining audit items from v0.3-deep session:
miner.py:
- supervisor_loop: logger.error -> logger.exception so unexpected
errors (KeyError/AttributeError/etc) surface with full traceback
instead of being masked by single-line log
- mine: extranonce2_counter overflow guard with wrap-around. Without
this, f"{counter:0{en2_size*2}x}" silently truncated high bits at
2^(en2_size*8) jobs (reachable for en2_size=2)
- mine: submit-failed log now includes job_id/nonce/hash so operators
can trace lost shares against SQLite (which already records them
with accepted=False). Comment documents why retransmit is unsafe
(stale shares -> potential pool ban)
- mine: -> None return type annotation
- supervisor_loop: -> None return type annotation
parallel.py:
- stop_pool: replace 0.2s magic-number drain window with
drain-until-queue.Empty + safety cap. Workers put before checking
stop_event, so by the time stop_pool runs all finds are already in
the queue or feeder buffer; time-based deadline was unjustified
- remove unused `import time` after refactor
stratum.py:
- Add return-type annotations: __init__, connect,
subscribe_and_authorize, _handle_message, reader_loop, close
- submit: typed parameters (str instead of untyped)
- __init__: stop_event Optional[threading.Event] (was buggy
`threading.Event = None` with mismatched declared type)
- _send: explicit OSError when sock is None instead of falling
through to AttributeError
demo.py:
- run_demo: returns bool (True = nonce found, False = exhausted)
to make outcome assertable
tests/test_demo.py (new):
- Spawn-process integration test: low-difficulty run finds nonce
within timeout, exits cleanly
README.md:
- Add v0.3 audit/UX section listing closed items
- Document --demo / --suggest-diff / address validation
- Full Prometheus metrics table (incl. accepted/rejected counters)
- Update file tree (address.py, demo.py) and per-file test counts
- Update headline test count to 97
Tests: 64 -> 98 (+34 across address/stratum/demo).
Skipped intentionally:
- mining.submit retransmit on reconnect: stale-share submit risks
temporary worker ban. Current behaviour (log + drop) is conservative
- notify_share_accepted timing test: would require refactoring the
closure inside mine() into a class. Logic covered indirectly by
test_storage.test_update_share_accepted_from_false_to_true
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes 8 of 10 remaining items from the v0.3 deep-audit session and adds
pre-flight BTC address validation. Brings the test suite from 64 → 98.
Three logical commits:
feat(address) — pure-stdlib bech32/bech32m/Base58Check validator
wired into
cli.main()before pool connect. 18 new tests coveringBIP-173/BIP-350 vectors, mixed-case rejection, bad checksums, testnet
prefixes. (
bc1q7h92eqxlp5lkl5ak43fkeccvrcf4f4t0fy9p2evalidated assmoke vector.)
test(stratum) — 15 new tests for
StratumClientviaFakeSocketfixture: subscribe/authorize/notify/set_difficulty/set_extranonce/
submit/reader_loop/close. Was the largest untested surface in v0.3.
chore: audit tail — supervisor
logger.exceptionfor tracebacks,parallel.stop_pooldrain-until-empty (kills 0.2s magic), extranonce2overflow guard with wrap, type annotations sweep across
stratum.py/miner.py, better diagnostics in submit-failed log (includesjob_id/nonce/hash),
demo.run_demo()returns bool + integration test,README sync (v0.3 audit/UX section, full Prometheus metrics table,
file tree, test counts).
What's deferred (and why)
mining.submitretransmit on reconnect: stale shares submittedafter job change can trigger temporary pool bans. Current behavior
(log + drop) is conservative correct.
notify_share_acceptedtiming test: logic lives in a closureinside
mine(). Unit-testing it requires extracting to a class.Indirectly covered by
test_storage.test_update_share_accepted_from_false_to_true.Test plan
py -3.11 -m unittest discover -s tests -v— 98/98 pass on localhope-hash bc1q7h92eqxlp5lkl5ak43fkeccvrcf4f4t0fy9p2eacceptshope-hash bc1qBADADDRrejects with mixed-case errorhope-hash tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsxrejects(testnet)
🤖 Generated with Claude Code