Skip to content

networks_from_xkeyversion rebuilds 21 version lists per address, and NETWORKS disagrees with itself about runtime networks #683

Description

@fametrano

Profiling the BIP44 address path after #643 left network.py as the
largest cost with no cryptography in it. Per address derived and
encoded, cProfile counts 21 calls into the two version-list
builders:

   calls  per addr   tottime  where
    6000       2.0     0.067  {secp256k1_ec_pubkey_tweak_add}
    3000       1.0     0.030  bech32.py:68(_polymod)                 <- #634
   12000       4.0     0.028  {secp256k1_ec_pubkey_parse}
    3000       1.0     0.016  base58.py:141(_b58decode_to_int)
    6000       2.0     0.009  bip32.py:219(assert_valid)
    6000       2.0     0.009  network.py:425(networks_from_xkeyversion)
   33000      11.0     0.008  network.py:388(xpubversions_from_network)   <- here
   30000      10.0     0.008  network.py:400(xprvversions_from_network)   <- here

networks_from_xkeyversion asks every network whether it carries the
prefix, and each question builds two fresh lists:

def networks_from_xkeyversion(xkeyversion: bytes) -> list[str]:
    """Return every network with the xkey version prefix, oldest first."""
    return [
        network
        for network in NETWORKS
        if xkeyversion in xprvversions_from_network(network)
        or xkeyversion in xpubversions_from_network(network)
    ]

Each builder does a .strip().lower(), a dict lookup and five attribute
reads, and the result is thrown away. A dict[bytes, list[str]] built
once answers the same question with one lookup: 1.40 us to 0.03 us,
and about 5% off an address end to end (p2wpkh from an account xpub 61.9
us against 65.2, from an account xprv 58.3 against 64.4, p2tr 82.1
against 87.2 — three alternating rounds, derive(xpub, "m/0/1") as a
control that did not move). The lookup is on seven modules' paths:
b58, b32, slip132, to_prv_key, to_pub_key, wallet and
bip44.

A prototype dict agreed with the scan on every version prefix the
built-in networks carry, and on an unknown one; the suite passes with it.

But it forces a decision, and the library has already half-made it

NETWORKS is a public, plain, mutable dict[str, Network], filled at
import from the json files. #207 records that callers build custom
networks today, and nothing stops one being registered:

NETWORKS["custom"] = Network(...)

Precomputing would freeze the answer at import, so a network registered
afterwards would stop being found. That inconsistency already exists,
pointing the other way
, and it looks like a defect independent of any
optimization:

networks_from_xkeyversion sees it:      ['custom']
XPRV_VERSIONS_ALL (frozen at import):   False

XPRV_VERSIONS_ALL and XPUB_VERSIONS_ALL are built at import from the
same two functions, so today network_from_xkeyversion will name a
runtime-registered network while bip32._assert_valid_key refuses its
keys with "unknown extended key version". One of the two is wrong about
what NETWORKS means.

So the question to settle first is not the microsecond one:

  • If NETWORKS is fixed at import, then the two lists are right, the
    scan is doing work for a case that cannot arise, and the dict is
    simply the third precomputed structure beside them. NETWORKS should
    then say so — a MappingProxyType, or a documented rule — because a
    plain public dict invites the mutation.
  • If callers may register a network, then the two frozen lists are
    the bug, and what this needs is one place that rebuilds all three when
    NETWORKS changes, not a fourth thing to go stale.

I have not touched it. Which of the two is the intent decides whether
this issue is a small optimization or a correctness fix with an
optimization falling out of it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions