SeedSigner

Merge pull request #722 from Chaitanya-Keyal/psbt-huge-fee-warning

[Feature] Warning Screen for High Tx Fees

Settle is_high_fee once in parse()

Both PSBTOverviewView and PSBTMathView asked the parser whether the fee was
high, and the overview asked twice (once for the screen, once for routing).
Each call re-summed the outputs through psbt.tx, which rebuilds the whole
transaction on every access. The totals it depends on are all fixed by the end
of parse(), so compute it there once and store it as is_high_fee, the same way
the other totals are exposed. has_high_fee() stays as the computation itself.

Merge pull request #1032 from kdmukai/psbt_output_ownership

[security] Verify that change outputs actually pay this seed

Count an unresolved output cosigner set as a mismatch

The cosigner comparison skipped any output whose cosigners failed to
resolve, so the different-quorum case it exists for never fired on a
real psbt: coordinators write only the spending wallet's xpubs, and a
foreign cosigner's key then resolves against nothing. Once the inputs'
cosigners resolve, an output that differs or fails to resolve is
counted as a spend.

Keys now match xpubs by derivation alone rather than requiring the
claimed fingerprint to match first. The fingerprint is a label the
coordinator wrote; requiring it let one mislabeled cosigner turn
genuine change into a spend, and a key an xpub really derives is
proof enough on its own.

The different-quorum test now uses the wallet's own xpubs, the shape
every xpub-writing coordinator produces, and a second test keeps the
four-xpub variant.

Compare cosigners before presuming multisig change

A multisig output whose committed script holds this seed's key can
still belong to a different wallet: a 2-of-3 over {p1,p2,p4} when the
inputs spend {p1,p2,p3}. When the psbt supplies global xpubs and fully
annotates the output, _get_cosigners resolves a cosigner list for the
inputs and one for the output, and lists that differ mean the output
pays a quorum other than the one being spent from. That output is now
counted as an external spend. It stays presumed change when the lists
match or when either side's cosigners are unresolved.

This is hygiene, not a security control. The global xpubs are optional
and unauthenticated, so a coordinator that omits them skips the check,
and a mismatch demotes rather than raises because a transfer into a
second multisig this seed belongs to is legitimate. No surveyed
coordinator produces the shape the check fires on: every one except
Bitcoin Core annotates only its own wallet's outputs, and Core, which
annotates any output a descriptor in its wallet file can solve, writes
no global xpubs at all. Settling which wallet a multisig output pays
still needs the user's own descriptor.

The comparison sits after the ownership checks rather than at the
policy shape gate, so a psbt whose output cosigners fail to resolve
still faces every ownership check.

_get_cosigners' sorted() is what makes the two lists comparable: two of
the three multisig fixtures order the same wallet's keys differently on
the input and on its change output. Its docstring now separates what
is claim from what is checked and what the result does and does not
establish, its loop carries the procedure inline, and two early-outs
name the omitted-xpubs and omitted-derivations cases before the loop
(behaviour-neutral: both already raised from inside it). Tests pin the
sort, the different-quorum spend, and the no-xpubs presumed-change
fallback.

Rename is_change to is_presumed_change

The flag in _parse_outputs is set for two different strengths of
finding. For single sig our own key rebuilt the scriptPubKey, so the
output is provably ours. For multisig we have only shown that our key
sits in the committed script, and which wallet that script belongs to
is settled later, by the user's descriptor. Calling both "is_change"
gave the multisig case a certainty it does not have.

The process docstring is held to the same standard: it describes how
ownership is determined rather than how outputs are verified as change,
the verdict table reads "presumed change", and the multisig rebuild
bullet names the key-in-script step that the table row depends on.

No behavior change.

Match policy types exactly and refuse an unsupported one

The rebuild chain in _parse_outputs matched single sig and taproot by
substring ("pkh" in type, "p2tr" in type) and had no else. embit reports
a script type it does not recognize as None, and _get_policy passes that
through, so a psbt whose inputs and outputs were both unrecognized passed
the shape gate and died in the substring test with a bare TypeError. The
guard meant for an unhandled type sat inside the scriptPubKey match,
where an empty rebuild cannot arrive.

The arms now name their types exactly, matching _build_singlesig_script,
and the chain ends in a catch-all else that raises RuntimeError with the
unsupported type in the message. The unreachable inner guard is removed.
A test drives a p2pk psbt into the catch-all.

Set is_change only after each branch's own checks

In _parse_outputs the flag was set True as soon as the rebuilt
scriptPubKey matched, and two multisig branches then turned it back
off. That made "change" the default inside the matched arm, so a branch
that forgot the flag would record change with nothing verified behind
it. Now each branch sets the flag only once its own checks have passed,
and an output that satisfies none of them stays an external spend.

The two sites where the flag is set split by signature type, and each
carries the certainty it actually has: single sig is settled, because
our own key rebuilt the scriptPubKey; multisig is a presumption, because
our key in the committed script does not say which wallet that script
belongs to, and only the user's descriptor can. The reminder that
"change" means any output coming back to our seed moves to the line
where change_data is built, which is where the word takes that meaning.

No behavior change: the same outputs land in the same lists on every
fixture shape, and the suite is unchanged.

Verify that change outputs actually pay this seed

An output counted as change when its policy matched the inputs' and its
rebuilt scriptPubKey matched what the output committed to. Neither step
established that the key involved was ours. The policy comparison
included cosigners resolved from the coordinator's own global xpubs, so
one misannotated fingerprint made an output stop matching and skip
verification altogether, and multisig never checked our key against the
committed script at all.

Outputs now compare on script shape alone, and every candidate proves
ownership: single sig by rebuilding from the claimed derivation path,
multisig by finding this seed's key in the committed script. Where the
psbt's account of an output contradicts what the output commits to, the
parse refuses rather than quietly reclassifying. A claim set too
malformed to answer that question, one populating both derivation path
maps or claiming more keys than its script uses, is refused as well.

change_data now carries the verified derivation path in place of the
coordinator's claimed fingerprints and paths, so the views no longer
re-derive trust from strings the parse has already settled.

Taproot mismatches stay exempt. A script tree tweaks the internal key,
so honest taproot change fails the rebuild too, and embit leaves
PSBT_OUT_TAP_TREE unparsed, which is what would tell the two apart.

Merge pull request #1027 from kdmukai/psbt_parser_docstrings

Clarify psbt_parser.py docstrings and comments; minor cleanup

Drop "scope" from the two ownership-claim summary lines

"An output scope claims..." reads no better than "An output claims...".
The word is doing no work in a sentence about what the output says, and
the psbt-scope sense it borrows is not the one the reader needs there.

Clarify psbt_parser.py docstrings and comments

No behavior change. Corrects prose that had drifted from what the code
does, and addresses two local names for the same reason.

Docstrings and comments:
- The two ownership-claim exceptions state the condition each signals,
without speculating about how it arises.
- _get_seed_derivation_path drops the output-versus-input severity
split, which _verify_claimed_derivation_paths already states.
- The verified_*_derivation_paths comment says what an entry holds, and
no longer explains the None the positive statement already implies.
- parse() steps 1 and 2 name the per-key condition instead of "scopes".
- _parse_inputs gains a docstring covering the amount total and the
one-shared-policy requirement.
- The single-sig branch comment names the script types it covers.

Naming:
- _parse_multisig's `sc` parameter becomes `multisig_script`.
- A TODO marks _get_policy's local `script`, which shadows the embit
module imported at the top of the file.

Merge pull request #1013 from kdmukai/psbt_parser_ownership_scan

[security] Reject PSBTs that falsely claim this seed's ownership

Merge pull request #1005 from kdmukai/psbt_parser_trust_vocabulary

[refactor] PSBT parsing: rename `change_data`'s unverified fields to `claimed_*`

Improve comment accuracy

Pluralize change_data's claimed keys

claimed_fingerprints and claimed_derivation_paths each hold a list
(one entry per cosigner), so the singular key names misread at their
call sites. Also drop a commented-out debugging print.

Improve comment accuracy

Merge pull request #1002 from kdmukai/psbt_parser_derivation_cache

[performance] PSBT parsing: remove redundant BIP32 derivations and Transaction rebuilds

Mark coordinator-supplied change data as claimed

Nothing in a psbt proves the fingerprints and derivation paths it
carries, but change_data's fingerprint and derivation_path keys read
like facts -- the change details view splits one into a wallet path and
hands it straight to get_xpub. Renaming them to claimed_fingerprint and
claimed_derivation_path, along with the locals that carry them, puts the
trust boundary in the data itself rather than in the logic around it.

The class docstring now states the convention behind the prefix:
claimed_ for coordinator-supplied metadata, verified_ for what this
device proved by re-deriving from the signing seed, and the invariant
that no verified_ value comes from a claimed_ one without a derivation
in between. It also records that change_data covers self-transfers, not
just change-branch outputs. No logic changes; the two dict keys are the
only behavioral difference.

Correct the cap comment's memory figure

The cache costs roughly 650KB at the cap, not 600KB. Drop the claim
that the cap guards against malicious memory exhaustion: the device is
airgapped and stateless, so a psbt that bloats the cache costs the user
a power cycle, not funds.

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.

Scope the derivation cache to parse()

Nothing outside parse() reads the cache, so it need not be an instance
attribute. As a local, "the cache does not outlive the parse" is a
property of scope rather than a teardown to maintain -- which also
retires the finally block, both initializations, and the test guarding
them.

Exercise the cache's read path in the equivalence test

Both psbts carried a single change output, so every derivation was a
first-time miss and the cached run only ever wrote to the cache. A
wrong value returned on a hit would have gone unnoticed. Carrying the
change output twice makes the second traversal read back what the
first one cached.

Trim the cache tests down to what catches a regression

"Explain how MAX_CACHED_DERIVATIONS was chosen and test the cap" left
test_cache_does_not_change_parse_output hand-rolling its own comparison
as a tuple of field reprs, and promised to move it onto the new
assert_same_parse_result helper. Do that, and put it on patch.object
like the rest of the suite instead of the monkeypatch fixture.

Then measure what the tests around it are worth, by breaking the cache
eleven different ways and recording which tests notice. Two results
stood out.

The equivalence test is differential: it parses once with the cache and
once without and requires the two to agree, so it can only catch a
break that makes those two runs disagree. Anything in logic they share
passes it. Breaking legacy p2sh change detection and breaking taproot
change detection both went unnoticed here; TestPSBTParser is what
catches those. Running it across four wallet types and both fingerprint
states was therefore buying nothing, and all eight cases detect exactly
what two of them do. Narrow it to one single-sig and one multisig psbt,
the two routes into the cache, and drop the fingerprint-zeroing
variants: test_zero_fingerprint_fill_over_many_inputs is what actually
catches a break in the fill path.

test_derive_with_cache_matches_plain_derive caught nothing that
test_get_cosigners_identical_with_and_without_cache does not also
catch, so remove it.

Nothing covered the parse dropping its cache when it ends. Deleting
that clear leaves the whole suite green, and the cache holds keys
derived from the signing seed, so add a test that asserts the parse
filled the cache it was handed and that the parser is no longer holding
it afterward.

Hold the parent key in each cache entry

The cache keys on id(parent_key), which is the parent's address in
memory. Python is free to hand that address to a new object once the
original is released, so an entry outliving its parent could be matched
by an unrelated key that landed on the same address, and
_derive_with_cache would return that entry's child instead of deriving
the right one. It fails silently: a recycled address is
indistinguishable from a hit.

Nothing triggers this today, because every parent -- the signing seed's
root and the cosigner xpubs from psbt.xpubs -- is held for the whole
parse. But that is a property of the call sites rather than of the
function, and the failure would be a wrong key in the middle of change
detection.

Storing the parent alongside its derived child keeps that address
occupied for as long as the entry depending on it is alive. The entry
holds a reference to a key the parser already owns, so nothing new is
retained and nothing is copied. Costs 56 bytes per entry, so the cap now
holds the cache to about 600 KB; the comment is updated to match.

Reported by newtonick in review.