Require the cache argument on the internal helpers
What the developer wrote
Authored by kdmukai
Require the cache argument on the internal helpers
Defaulting it to None means a call site that forgets it silently
derives without a cache. No caller omitted it; now none can.
_derive_with_cache keeps its optional cache, since that branch is the
plain embit derive() the equivalence test compares against.
What changed, and why it matters
This commit tightens up internal helper functions in a Bitcoin wallet's PSBT parser so that callers must explicitly pass a cache object (or None). Previously, the cache argument defaulted to None, meaning a future coding mistake could silently disable the derivation cache. The change is defensive: it makes accidental omissions impossible without breaking tests. There is no evidence of an exploitable vulnerability in the shipped code, because the commit message itself says no caller actually omitted the argument.
No immediate action required. Treat as routine code-quality hardening. Reviewers should verify that all current and future call sites of `_get_policy` and `_get_cosigners` pass the cache argument explicitly, and that the equivalence test in `test_psbt_parser.py` continues to exercise the uncached path.
Security signals we found
Defensive API hardening: removes silent default-to-None for cache argument
Prevents accidental cache bypass in future code changes
No functional change to current callers (commit message states 'No caller omitted it')
Maintains equivalence test path that deliberately derives without cache
Evidence from the diff
The patch removes default values for the child_key_derivation_cache parameter on _get_policy and _get_cosigners, requiring call sites to supply either a dict or None explicitly. _derive_with_cache keeps an optional cache parameter because the equivalence test intentionally calls it without a cache. The change is purely a hardening measure against future developer error; the diff and commit message confirm no current caller was omitting the cache.
Changed components
src/seedsigner/models/psbt_parser.pytests/test_psbt_parser.pyInspect captured patch +8 / −8
### src/seedsigner/models/psbt_parser.py
@@ -303,7 +303,7 @@ def sig_count(tx):
@staticmethod
- def _get_policy(scope, scriptpubkey, xpubs, child_key_derivation_cache=None):
+ def _get_policy(scope, scriptpubkey, xpubs, child_key_derivation_cache: dict | None):
"""Parse scope and get policy"""
# we don't know the policy yet, let's parse it
script_type = scriptpubkey.script_type()
@@ -371,7 +371,7 @@ def _parse_multisig(sc):
@staticmethod
- def _derive_with_cache(parent_key: bip32.HDKey, derivation_path: List[int], cache: dict | None = None) -> bip32.HDKey:
+ def _derive_with_cache(parent_key: bip32.HDKey, derivation_path: List[int], child_key_derivation_cache: dict | None = None) -> bip32.HDKey:
"""
Derives the key that sits at the given derivation path below parent_key, reusing
any levels along the way that have already been derived during this parse.
@@ -397,7 +397,7 @@ def _derive_with_cache(parent_key: bip32.HDKey, derivation_path: List[int], cach
The cache stops accepting new levels at MAX_CACHED_DERIVATIONS.
"""
- if cache is None:
+ if child_key_derivation_cache is None:
return parent_key.derive(derivation_path)
derived_key = parent_key
@@ -407,22 +407,22 @@ def _derive_with_cache(parent_key: bip32.HDKey, derivation_path: List[int], cach
for index in derivation_path:
derivation_path_so_far += (index,)
cache_key = (id(parent_key), derivation_path_so_far)
- cached_entry = cache.get(cache_key)
+ cached_entry = child_key_derivation_cache.get(cache_key)
if cached_entry is None:
# First time deriving this level. Do the work to derive this level's child
# and store it in the cache.
already_derived = derived_key.child(index)
- if len(cache) < PSBTParser.MAX_CACHED_DERIVATIONS:
+ if len(child_key_derivation_cache) < PSBTParser.MAX_CACHED_DERIVATIONS:
# Parent must also be stored to keep its id() from being reused
- cache[cache_key] = (parent_key, already_derived)
+ child_key_derivation_cache[cache_key] = (parent_key, already_derived)
else:
cached_parent, already_derived = cached_entry
derived_key = already_derived
return derived_key
@staticmethod
- def _get_cosigners(pubkeys, derivations, xpubs, child_key_derivation_cache=None):
+ def _get_cosigners(pubkeys, derivations, xpubs, child_key_derivation_cache: dict | None):
"""Returns xpubs used to derive pubkeys using global xpub field from psbt"""
cosigners = []
for i, pubkey in enumerate(pubkeys):
### tests/test_psbt_parser.py
@@ -654,7 +654,7 @@ def test_get_cosigners_identical_with_and_without_cache(self):
inp = psbt.inputs[0]
pubkeys = list(inp.bip32_derivations.keys())
- uncached = PSBTParser._get_cosigners(pubkeys, inp.bip32_derivations, psbt.xpubs)
+ uncached = PSBTParser._get_cosigners(pubkeys, inp.bip32_derivations, psbt.xpubs, None)
child_key_derivation_cache = {}
cached = PSBTParser._get_cosigners(Why this scored 28/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.