Skip to content

Refuse the type at the coercion, not three modules later (#776) - #809

Merged
fametrano merged 1 commit into
mainfrom
iss776-coercions-refuse-wrong-types
Aug 14, 2026
Merged

Refuse the type at the coercion, not three modules later (#776)#809
fametrano merged 1 commit into
mainfrom
iss776-coercions-refuse-wrong-types

Conversation

@fametrano

@fametrano fametrano commented Aug 13, 2026

Copy link
Copy Markdown
Member

Closes fifty-eight of the fifty-nine entries the input-validation gate
holds open in #776. The five
roots of §5, the seven one-offs of §6 and two of the three of §7.

What was wrong

Each of the five is a coercion that takes "anything convertible",
handles the str spelling and passes everything else through
untouched, trusting the annotation. The value then fails later and
somewhere else, as a native TypeError or AttributeError about a
builtin, out of a module the caller never called. It is the bug
#766 fixed in
bytes_from_octets, five times over.

  • base58.decode reached len(v) with whatever it was handed. Every
    key and address converter in the library decodes there, which is why
    one line accounted for more than half the list
  • utils.bytesio_from_binarydata did not raise at all: it returned its
    argument unchanged, so a None came back a None and the six
    parse functions above it failed on .read or on .getbuffer
  • bip32.der_path's _indexes_from_der_path handed a float to list()
  • b32.has_segwit_prefix assumed bytes in the else of its one line
  • curves.curve_group.is_on_curve asked len of what is not sized

What the fix reuses

utils.str_from_string is the String half of bytes_from_octets,
and the four places that spelled that coercion out by hand now call it:
bech32._decode, b32.has_segwit_prefix, b32.witness_from_address
and silent_payments.keys_from_address. Two of them needed it before
their length bound rather than at the codec — len of a float is a
complaint about a builtin, where the codec below would have named the
argument — and it carries to all four the non-ascii refusal only
bech32 had.

What is left, and why it is a decision rather than a fix

ecc.dleq.verify_proof stays in _OPEN, and it is not a coercion.
What answers False is to_pub_key refusing anything that is not a
public key as a BTClibValueError whatever was wrong with it, and
folding the type in is what keeps a boolean verification total
which is the position #745
settled on, and which
#143 test pins:
dsa.verify answers False for a private key passed as a public one.

I did try the other way, and the suite refused it: making
point_from_pub_key raise BTClibTypeError broke
test_prv_key_is_not_a_pub_key and test_from_pub_key. So closing
that entry reverses #745 rather than plugging a hole, and it waits for
that call. The comment above _OPEN says so, in place of the line
being deleted on a reading of the code.

§8 of the issue — raising the ceiling above the 138 functions the walk
can drive — is untouched and belongs to a separate branch.

Note on the gate itself

_classify is the gate's verdict as a function of its own. No function
under btclib/ leaks a native exception any more, so the branch that
names one went uncovered by the 100% ratchet, and an unrun branch is a
poor thing to be relying on the day something does leak.
test_the_walk_names_what_escapes provokes all three answers.

Gates

  • uv run pytest — 26722 passed, coverage 100.00%
  • uv run pre-commit run --all-files — exit 0
  • sphinx-build -W --keep-going — exit 0

🤖 Generated with Claude Code

Summary by Sourcery

Tighten input validation across key/address codecs and curve utilities so invalid argument types fail early with BTClib-specific exceptions instead of leaking native errors.

New Features:

  • Introduce a shared str_from_string helper to coerce String aliases (text or ascii bytes) into text for address and bech32-related functions while rejecting non-ascii and wrong types.

Bug Fixes:

  • Ensure base58.decode and related key/address converters reject non-string/non-buffer inputs with BTClibTypeError instead of failing later on len or other builtins.
  • Fix bytesio_from_binarydata to return a BytesIO over validated octets (or the original BytesIO) so parse helpers no longer silently propagate unsupported types like None.
  • Harden tx_or_psbt_from_any input handling by routing non-str inputs through bytes_from_octets and only interpreting ascii text when appropriate, avoiding native exceptions from split/decode.
  • Validate derivation path types in _indexes_from_der_path, raising BTClibTypeError for non-iterable inputs instead of relying on list() to fail.
  • Coerce script_pub_key to octets before inspecting truthiness in script.script_pub_key.address so non-script inputs are refused instead of being treated as a nulldata script.
  • Reject non-list tapscripts in script.taproot.serialize with BTClibTypeError rather than leaking errors from list operations.
  • Enforce tuple-typed points in curves.curve_group.is_on_curve, raising BTClibTypeError for invalid point types before length and range checks.

Enhancements:

  • Refine the String alias documentation to reference str_from_string as the runtime enforcer for text vs hex semantics.
  • Update to_prv_key WIF/xkey converters to treat type errors from base58 decoding as "not a private key" format failures, preserving the caller-facing control flow.
  • Clarify exceptions module documentation to reference issue Five coercions still trust their annotation: the 59 the input-validation gate holds open #776 as the tracker for remaining native exceptions escaping the library.

Documentation:

  • Extend the changelog with detailed notes on tightened type coercion, error-class behavior, and the one intentional non-coercion in ecc.dleq.verify_proof.
  • Document the new input-validation gate behavior and its coverage via the changelog narrative.

Tests:

  • Refactor the input-validation gate’s verdict logic into a dedicated _classify helper and add test_the_walk_names_what_escapes to exercise its three possible outcomes.
  • Adjust curve tests to expect BTClibTypeError for invalid point types and BTClibValueError for malformed tuples, matching the new is_on_curve behavior.

Five coercions took "anything convertible", handled the str spelling
and passed everything else through untouched, trusting the annotation.
The value then failed elsewhere as a native TypeError or
AttributeError -- about a builtin, out of a module the caller never
called -- which is the bug #744 fixed in bytes_from_octets, five times
over. The input-validation gate held fifty-nine public functions open
on them, and one line of base58.decode accounted for more than half:
every key and address converter in the library decodes there.

bytesio_from_binarydata is the one that did not raise at all. It
returned its argument unchanged, so a None came back a None and the
six parse functions above it failed on .read or on .getbuffer; it now
wraps octets in a BytesIO and hands a BytesIO back as it came.
script_pub_key.address is the other: it answered "" for a None, which
is the answer a nulldata output has.

utils.str_from_string is the String half of bytes_from_octets, and the
four places that spelled that coercion out by hand call it. Two of
them needed it before their length bound rather than at the codec:
len of a float is a complaint about a builtin, where the codec would
have named the argument.

to_prv_key's WIF and xkey attempts catch a TypeError beside the
ValueError, as its two "it must be octets" fallbacks already did, so
an argument of the wrong type still ends as "not a private key".

What stays open is ecc.dleq.verify_proof, and it is not a coercion:
what answers False is to_pub_key refusing anything that is not a
public key as a BTClibValueError whatever was wrong with it, which is
what keeps a boolean verification total. That is #745's decision and
#143's test -- dsa.verify answers False for a private key passed as a
public one -- so closing the entry reverses those rather than plugging
a hole, and it waits for that call.

_classify is the gate's verdict as a function of its own: no function
under btclib/ leaks a native exception any more, so the branch that
names one went uncovered, and test_the_walk_names_what_escapes is what
provokes all three answers now.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @fametrano, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This PR tightens btclib’s input validation by moving type/encoding checks to the coercion boundaries, reusing a new str_from_string helper and bytes_from_octets, and updates the input-validation gate tests so native exceptions never leak from public APIs.

Sequence diagram for address decoding coercion with str_from_string

sequenceDiagram
    actor Caller
    participant keys_from_address
    participant str_from_string
    participant decode
    participant bech32__decode

    Caller->>keys_from_address: keys_from_address(address)
    keys_from_address->>str_from_string: str_from_string(address, "address")
    alt invalid_type_or_non_ascii
        str_from_string-->>keys_from_address: BTClibTypeError/BTClibValueError
        keys_from_address-->>Caller: exception
    else valid_ascii_string
        str_from_string-->>keys_from_address: addr
        keys_from_address->>decode: decode(addr, _BECH32_M_CONST)
        decode->>bech32__decode: _decode(bech)
        bech32__decode-->>decode: hrp, data
        decode-->>keys_from_address: hrp, data
        keys_from_address-->>Caller: Point, Point, NetworkType
    end
Loading

File-Level Changes

Change Details Files
Refactor input-validation gate to classify escape types via a dedicated helper and keep only the intentional DLEQ verify_proof entry open.
  • Introduce _classify helper that runs a callable and returns None, the native exception class name, or 'no exception'.
  • Refactor _leak to use functools.partial and _classify instead of inlined try/except.
  • Reduce _OPEN to only btclib.ecc.dleq.verify_proof and document why it remains.
  • Add test_the_walk_names_what_escapes to exercise all three _classify verdicts.
  • Update gate-related changelog section to describe this behaviour.
tests/input_validation_test.py
CHANGELOG.md
Introduce str_from_string as the canonical String coercion and reuse it across bech32, base32, and silent payments to reject non-text/non-ascii inputs early.
  • Add String alias import and str_from_string to btclib.utils, plus to __all__ and alias documentation.
  • Implement str_from_string to accept str or ascii bytes/bytearray/memoryview and raise BTClibTypeError/BTClibValueError otherwise.
  • Use str_from_string in bech32._decode instead of manual ascii decoding and update error messages to reference the decoded text.
  • Use str_from_string in b32.has_segwit_prefix and b32.witness_from_address before length checks and stripping.
  • Use str_from_string in silent_payments.keys_from_address before length checks and HRP decoding.
  • Update changelog to describe the new String coercion and its propagation.
btclib/utils.py
btclib/alias.py
btclib/bech32.py
btclib/b32.py
btclib/silent_payments.py
CHANGELOG.md
Harden binary/byte coercions so invalid types are rejected with BTClib*Error at the boundary instead of leaking native exceptions later.
  • Change bytesio_from_binarydata to return an existing BytesIO unchanged or wrap other inputs via bytes_from_octets, refusing non-octet types through that helper.
  • Update base58.decode to reject non-String/non-buffer types with BTClibTypeError and treat bytearray/memoryview like bytes; keep existing ascii encoding and alphabet checks.
  • Refactor _octets_from_any in tx_or_psbt.py to delegate non-str inputs to bytes_from_octets, then attempt ascii decoding and text parsing, falling back to raw bytes when parsing fails.
  • Make script.script_pub_key.address coerce script_pub_key with bytes_from_octets before truthiness checks so None is rejected rather than treated as an empty script.
  • Ensure script.taproot.serialize rejects non-list scripts with BTClibTypeError instead of leaking attribute/index errors.
  • Update curve_group.is_on_curve to enforce tuple type first (BTClibTypeError) before checking length/value, and adjust tests accordingly.
  • Extend to_prv_key WIF/xprv decoding to treat BTClibTypeError from base58 as "not this format" by catching both TypeError and ValueError.
  • Adjust changelog text and exception module docstring to reference issue Five coercions still trust their annotation: the 59 the input-validation gate holds open #776 and the tightened behaviour.
btclib/utils.py
btclib/base58.py
btclib/tx_or_psbt.py
btclib/script/script_pub_key.py
btclib/script/taproot.py
btclib/curves/curve_group.py
tests/curves/curve_test.py
btclib/to_prv_key.py
btclib/exceptions.py
CHANGELOG.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@fametrano
fametrano merged commit 01f62be into main Aug 14, 2026
24 checks passed
@fametrano
fametrano deleted the iss776-coercions-refuse-wrong-types branch August 14, 2026 06:03
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.

1 participant