Ask a value what it is before asking what it holds

CHANGELOG.md

@@ -1658,6 +1658,69 @@ documented at release-notes length in the first place, and are still in
 
 ### The public API and the module layout
 
+- **`bytes_from_octets` is the whole of what `Octets` means, and it is
+  total** (issue #744). It is the coercion every `Octets` parameter of
+  the library runs through, and it had two holes: a hex string that is
+  not one left through `bytes.fromhex`'s bare `ValueError`, and anything
+  that was not a `str` went through *untouched* -- so `len` of a tuple
+  of 33 ints was 33, which is how
+  `taproot.assert_valid_control_block` accepted one as a control block
+  size. Both are refused now, as `BTClibValueError` and
+  `BTClibTypeError`; every buffer is still taken, and returned as it
+  came, a read being no place to rewrite the field it reads.
+
+  The message is `bytes.fromhex`'s own, which names a position and never
+  the string. That is deliberate: an `Octets` parameter is candidate key
+  material as often as not, and `to_prv_key` puts this very message
+  inside its own "not a private key" (issue #137).
+
+  `int_from_integer` gets both through it, and `to_prv_key`'s two "it
+  must be octets" fallbacks now catch a `TypeError` beside the
+  `ValueError`, as `to_pub_key` already did: what is neither octets nor
+  a spelling of them means the same thing a wrong size does.
+
+  `mnemonic.entropy`'s five `int(x, 2)` parses and its two `int(x, 16)`
+  and `int(x)` ones are `BTClibValueError` too, and say a base rather
+  than the digits -- raw entropy being seed material, as every other
+  message in that module already assumed. `borromean.sign` checks that
+  its rings, signing indexes and nonces are of one length rather than
+  leaving it to `zip(strict=True)`, whose message named "argument 3" and
+  no parameter of the function; `strict=True` stays, as the assertion
+  that the check and the loops cannot drift apart.
+
+  **What moves for a caller**: the class is narrower and the control
+  flow identical, `BTClibValueError` being a `ValueError` and
+  `BTClibTypeError` a `TypeError`. A test or a caller matching on
+  `bytes.fromhex`'s message still matches -- it is carried through --
+  and one matching the *class* now has a btclib one to match.
+  `docs/source/guide.rst` shows the new spelling.
+
+- **Ten places in `block/`, `hashes` and `utils` asked a value what it
+  is not** (issue #744): a comparison, an arithmetic operation or an
+  attribute lookup on an argument nothing had checked. `"5" <= 16`,
+  `"2015" + 1` and `"hard" <= 0` are bare `TypeError`s about operands,
+  raised from underneath the library and naming neither the parameter
+  nor the function; `.tzinfo` on a str is an `AttributeError`, which is
+  outside *both* halves of this library's exception contract, so nothing
+  a caller is told to catch would have caught it; and two datetimes
+  given as unix timestamps subtracted to an int, whose missing
+  `total_seconds` is an `AttributeError` again.
+
+  The guard is `var_int.serialize`'s, and the vocabulary is
+  `utils.is_integer`'s: a bool is not a number, because it would be the
+  height one, the block count one, the leaf index one.
+
+  `bip34_commitment` and, through it, `Block.assert_valid_coinbase_height`;
+  `BlockHeader.assert_valid`'s timestamp and `assert_valid_time`'s `now`
+  -- the default `check_validity=True` path being as exposed as the
+  explicit one; `mining.mine`'s `max_tries` *and* its header, which
+  `dataclasses.replace` used to complain about; `next_bits`'s two
+  datetimes; `retarget_first_height`; all three numbers of `hash_rate`,
+  where a difficulty and a timespan are `float` and an integer is one of
+  those; `hashes.merkle_root_from_branch`'s leaf index, which is what
+  `merkle_proof.assert_as_valid` and `merkle_proof.verify` reach it
+  through; and `utils.encode_num`.
+
 - **Every field a sig_hash preimage writes is checked as a width, and
   every `vin_i` as an index** (issue #744). `int.to_bytes` answers a
   field too wide for it with an `OverflowError`, which is an

HISTORY.md

@@ -33,6 +33,29 @@ full year, short month, short day (YYYY-M-D)
   spaces around it -- `"MAINNET"` to `b32.address_from_witness` -- is
   accepted now where it used to raise.
 
+- **a malformed argument raises a btclib error, not a native one.** The
+  rest of issue #744's census, and the same move the network name above
+  made: an out-of-range derivation index was an `OverflowError`, an
+  out-of-range `vin_i` an `IndexError`, a header timestamp that is no
+  datetime an `AttributeError`, a hex string that is not one a bare
+  `ValueError`. Each is a `BTClibValueError` or a `BTClibTypeError` now.
+  Code catching `ValueError` or `TypeError` keeps working, the two
+  deriving from those; code catching `OverflowError`, `IndexError` or
+  `AttributeError` around one of these has to catch `BTClibException`,
+  or one of the two builtins, instead.
+
+  Two of them are not a narrowing, and are what to act on. What is
+  neither `bytes` nor a hex `str` is a `BTClibTypeError` where
+  `bytes_from_octets` used to return it untouched -- a tuple of 33 ints
+  passed `taproot.assert_valid_control_block` as a control block size,
+  and `bin_str_entropy_from_entropy(())` was reported as zero bits. And
+  a dozen calls that answered a malformed argument with a *number*
+  refuse it: `sig_hash.taproot` with an `input_index` past the end of
+  the vin, `taproot.input_script_sig` with a negative leaf index,
+  `bech32.encode` with a negative digit, `mod_inv` with a float,
+  `int_from_json_number` with 1.5, `Psbt.weight_estimate` on an
+  incoherent psbt. CHANGELOG.md lists all twelve.
+
 - **the individual point multiplications are private.** `from
   btclib.curves.curve_group import mult_jac` is an `ImportError` now, and
   so is every other variant of `curve_group` and `curve_group_2`: the

btclib/block/block.py

@@ -22,7 +22,7 @@
     WITNESS_SCALE_FACTOR,
 )
 from btclib.block.proof_of_work import MAINNET_POW_LIMIT_BITS
-from btclib.exceptions import BTClibValueError
+from btclib.exceptions import BTClibTypeError, BTClibValueError
 from btclib.hashes import (
     hash256,
     merkle_root_and_mutated,
@@ -31,7 +31,12 @@
 from btclib.script.script import op_int
 from btclib.script.script import serialize as serialize_script
 from btclib.tx import Tx
-from btclib.utils import assert_no_trailing, bytesio_from_binarydata, decode_num
+from btclib.utils import (
+    assert_no_trailing,
+    bytesio_from_binarydata,
+    decode_num,
+    is_integer,
+)
 
 __all__ = [
     "Block",
@@ -86,6 +91,12 @@ def bip34_commitment(height: int) -> bytes:
     binds first and every node on such a chain compares against the op
     code.
     """
+    # before the comparison below, which is what a non-number reaches:
+    # `-1 <= "5"` is a bare TypeError about the operands rather than
+    # about the height, and `True` is the height one
+    if not is_integer(height):
+        raise BTClibTypeError(f"invalid height type: {type(height).__name__}")
+
     # Core's push_int64 has three branches -- OP_0 for zero, OP_1..OP_16
     # for 1 to 16, OP_1NEGATE for -1 -- and op_int names all three
     command: Command = op_int(height) if -1 <= height <= 16 else height

btclib/block/block_header.py

@@ -283,6 +283,11 @@ def assert_valid_time(self, now: datetime) -> None:
         one, and it stays out of reach: it is the median time past of
         eleven ancestors, i.e. the chain.
         """
+        # the type before the time zone: `.tzinfo` on anything else is an
+        # AttributeError, which is neither a ValueError nor a TypeError
+        # and so is caught by nothing this library tells a caller to catch
+        if not isinstance(now, datetime):
+            raise BTClibTypeError(f"invalid current time type: {type(now).__name__}")
         if now.tzinfo is None or now.tzinfo.utcoffset(now) is None:
             raise BTClibValueError(f"naive current time (no time zone): {now}")
 
@@ -296,6 +301,35 @@ def assert_valid_time(self, now: datetime) -> None:
             err_msg += f" > {now} + {MAX_FUTURE_BLOCK_TIME} seconds"
             raise BTClibValueError(err_msg)
 
+    def _assert_valid_types(self) -> None:
+        """Refuse a field the eighty bytes could not be built from.
+
+        The type check the bytes fields get from bytes() below, for the
+        three that get none. Not a coercion, which would repair the
+        mistake by rewriting the header being inspected (__init__
+        coerces, this reports); not dropped either, which would let a
+        float reach to_bytes and leave the library through an
+        AttributeError, and a str reach `.tzinfo` and leave through
+        another one -- neither of them a half of this library's
+        exception contract.
+
+        The timestamp is the one field __init__ coerces nothing into: a
+        moment has no single spelling to coerce from,
+        `datetime.fromtimestamp` needing a time zone the caller has not
+        given.
+        """
+        for key in ("version", "nonce"):
+            value = getattr(self, key)
+            if not is_integer(value):
+                err_msg = f"invalid {key} type: {type(value).__name__}"
+                raise BTClibTypeError(err_msg)
+
+        if not isinstance(self.time, datetime):
+            # unreachable to mypy, the field being annotated: the caller
+            # this is here for is the one mypy never sees
+            err_msg = f"invalid timestamp type: {type(self.time).__name__}"  # type: ignore[unreachable]
+            raise BTClibTypeError(err_msg)
+
     def assert_valid(self) -> None:
         """Refuse a header the eighty bytes could not hold.
 
@@ -305,16 +339,7 @@ def assert_valid(self) -> None:
         are assert_valid_time and assert_valid_pow, whose docstrings
         say why they are separate.
         """
-        # the type check the bytes fields get from bytes() below, for the
-        # two int ones. Not a coercion, which would repair the mistake by
-        # rewriting the header being inspected (__init__ coerces, this
-        # reports); not dropped either, which would let a float reach
-        # to_bytes and leave the library through an AttributeError
-        for key in ("version", "nonce"):
-            value = getattr(self, key)
-            if not is_integer(value):
-                err_msg = f"invalid {key} type: {type(value).__name__}"
-                raise BTClibTypeError(err_msg)
+        self._assert_valid_types()
 
         # must be a 4-bytes _signed_ integer
         if not 0 < self.version <= 0x7FFFFFFF:

btclib/block/mining.py

@@ -28,8 +28,9 @@
 from btclib.alias import Octets
 from btclib.block.block import merkle_root_and_mutated_from_transactions
 from btclib.block.block_header import BlockHeader
-from btclib.exceptions import BTClibValueError
+from btclib.exceptions import BTClibTypeError, BTClibValueError
 from btclib.tx import Tx
+from btclib.utils import is_integer
 
 __all__ = [
     "NONCE_SPACE",
@@ -101,6 +102,14 @@ def mine(header: BlockHeader, max_tries: int = 1 << 20) -> BlockHeader | None:
 
     The caller's header is left alone: what comes back is a copy.
     """
+    # `replace()` says "should be called on dataclass instances" for
+    # anything else, from the standard library and about a call the caller
+    # never made; `max_tries < 1` is a bare TypeError about the operands,
+    # and a float passes it to fail at `range` a few lines down
+    if not isinstance(header, BlockHeader):
+        raise BTClibTypeError(f"invalid header type: {type(header).__name__}")
+    if not is_integer(max_tries):
+        raise BTClibTypeError(f"invalid max_tries type: {type(max_tries).__name__}")
     if max_tries < 1:
         raise BTClibValueError(f"invalid max_tries: {max_tries}")
 

btclib/block/proof_of_work.py

@@ -31,8 +31,8 @@
 from datetime import datetime
 
 from btclib.alias import Octets
-from btclib.exceptions import BTClibValueError
-from btclib.utils import bytes_from_octets
+from btclib.exceptions import BTClibTypeError, BTClibValueError
+from btclib.utils import bytes_from_octets, is_integer
 
 __all__ = [
     "DIFFICULTY_ADJUSTMENT_INTERVAL",
@@ -213,6 +213,11 @@ def retarget_first_height(last_height: int) -> int:
     error, so `GetNextWorkRequired` still reads
     `nHeight - (DifficultyAdjustmentInterval() - 1)`.
     """
+    # a height, before the arithmetic: `"2015" + 1` is a bare TypeError
+    # about concatenating a str, and `True + 1` is the height two
+    if not is_integer(last_height):
+        raise BTClibTypeError(f"invalid height type: {type(last_height).__name__}")
+
     # Core only retargets when the *next* height is a multiple of 2016,
     # so the last block of a period is the one 2015 blocks after its
     # first. Refused rather than answered for any other height: the
@@ -251,6 +256,17 @@ def next_bits(
 
     Bitcoin Core spells this `CalculateNextWorkRequired`.
     """
+    # both are datetimes, checked before the subtraction: two ints
+    # subtract to an int and answer `AttributeError: 'int' object has no
+    # attribute 'total_seconds'`, which is neither half of this library's
+    # exception contract, and a str answers a TypeError about the operands
+    for name, value in (
+        ("first block time", first_block_time),
+        ("last block time", last_block_time),
+    ):
+        if not isinstance(value, datetime):
+            raise BTClibTypeError(f"invalid {name} type: {type(value).__name__}")
+
     # the difference of two datetimes, not two timestamp() calls: aware
     # or naive, the subtraction is the same number of seconds, so the
     # answer does not depend on the machine's time zone for the naive
@@ -344,6 +360,18 @@ def hash_rate(difficulty: float, timespan: float, block_count: int = 1) -> float
     with to within the 1/65536 by which the genesis target falls short
     of 2^224.
     """
+    # the types before the three comparisons, each of which is a bare
+    # TypeError about the operands for anything that is not a number. A
+    # difficulty and a timespan are `float` here and an integer is one of
+    # those, `1.0` and `1` being the same rate; a block count is a count,
+    # so it is the narrower question -- and a bool is neither, `True`
+    # being one block, one second and difficulty one
+    for name, value in (("timespan", timespan), ("difficulty", difficulty)):
+        if isinstance(value, bool) or not isinstance(value, (int, float)):
+            raise BTClibTypeError(f"invalid {name} type: {type(value).__name__}")
+    if not is_integer(block_count):
+        raise BTClibTypeError(f"invalid block count type: {type(block_count).__name__}")
+
     if timespan <= 0:
         raise BTClibValueError(f"invalid timespan: {timespan}")
     if block_count < 1:

btclib/ecc/borromean.py

@@ -21,7 +21,7 @@
 
 from btclib.alias import HashF, Octets, Point
 from btclib.curves import Curve, bytes_from_point, double_mult, mult, secp256k1
-from btclib.exceptions import BTClibRuntimeError
+from btclib.exceptions import BTClibRuntimeError, BTClibValueError
 from btclib.utils import bytes_from_octets, int_from_bits
 
 __all__ = [
@@ -107,15 +107,20 @@ def sign(
         for pubk_ring in pubk_rings
     ]
 
+    # one entry per ring in each of the three, checked here rather than
+    # left to the `strict=True` of the two loops below: a short ks would
+    # truncate them silently and sign a subset of the rings -- a signature
+    # over fewer rings than the caller asked for, which is the one thing a
+    # ring signature must not do quietly. zip's own message is a
+    # BTClibValueError's class with none of its content, naming "argument
+    # 3" and no parameter of this function; strict=True stays, as the
+    # assertion that this check and those loops cannot drift apart
+    if not len(pubk_rings) == len(sign_key_idx) == len(ks):
+        err_msg = f"{len(pubk_rings)} rings, {len(sign_key_idx)} signing indexes"
+        err_msg += f" and {len(ks)} nonces"
+        raise BTClibValueError(err_msg)
+
     # step 1
-    # strict=True, and it is the one zip in this package that changes what
-    # an argument does rather than documenting an invariant already checked:
-    # nothing validates that ks, sign_key_idx and pubk_rings have one entry
-    # per ring, so a short ks would truncate the loop silently and sign a
-    # subset of the rings -- a signature over fewer rings than the caller
-    # asked for, which is the one thing a ring signature must not do
-    # quietly. ValueError, and BTClibValueError is a ValueError, so a caller
-    # already catching this package's errors catches it
     for i, (pubk_ring, j_star, k) in enumerate(
         zip(pubk_rings, sign_key_idx, ks, strict=True)
     ):

btclib/hashes.py

@@ -18,7 +18,7 @@
 from btclib._ripemd160 import ripemd160 as pure_python_ripemd160
 from btclib.alias import HashDigestF, HashF, Octets
 from btclib.exceptions import BTClibTypeError, BTClibValueError
-from btclib.utils import bytes_from_octets
+from btclib.utils import bytes_from_octets, is_integer
 
 __all__ = [
     "hash160",
@@ -286,6 +286,11 @@ def merkle_root_from_branch(
     means knowing what a transaction looks like -- a layer this module
     sits below, and must not import.
     """
+    # the type before the sign, `"0" < 0` being a bare TypeError about
+    # the operands: an index is a position and a bool is not one, `True`
+    # naming the second leaf of every tree it is passed to
+    if not is_integer(index):
+        raise BTClibTypeError(f"invalid leaf index type: {type(index).__name__}")
     if index < 0:
         raise BTClibValueError(f"negative leaf index: {index}")