[security] Store multiple verified derivation path entries per input/output; reject all decoy keys

[kdmukai]

High-level tldr:

On a multisig output we check for any "decoy" keys: a key that DOES derive from our seed but is NOT referenced/committed to in the output's actual script.

When populating PSBTParser.verified_[input|output]_derivation_paths, the parser was only storing the derivation path for the FIRST key that we verified to be ours on a given input or output.

If that first key turned out to be a decoy, the parser would raise a dire warning: PSBTOutputOwnershipContradictionError.

However, if the first key was legit but a subsequent key for that output was a decoy, it would either go unnoticed (if the total number of keys provided matched the number of keys in the multisig script) or it would raise a lesser warning (PSBTSurplusDerivationPathsError).

The presence of ANY decoy, regardless of its position in the psbt, should raise the same dire warning.

---

The fix

So this PR is pretty trivial. It's just modifying PSBTParser.verified_[input|output]_derivation_paths:

• Before: a list storing ONE derivation path entry per input or output.
• After: a list storing potentially MANY derivation path entries per input or output.

So now the parser can check EVERY key provided for each output that is derived from our seed and raise the alarm if it finds a decoy.

Minor implementation detail

The verified derivation path was originally stored as a List[int] (the individual indices of the derivation path).

But allowing for multiple derivation paths per input or output would have meant: List[List[int]]. And the verified_* vars are themselves a List encompassing all inputs or all outputs. Altogether, we would have had: List[List[List[int]]].

So instead I refactored to just reuse the embit.DerivationPath entries straight from the psbt:

self.verified_output_derivation_paths: List[List[DerivationPath]] = []

---

Testing

I have updated testqrs.com to include this PR's test scenario txs.

---

Description written by Claude:

Problem or Issue being addressed

A multisig output lists one derivation path entry per cosigner. #1032 checks that the key at the
first entry claiming this seed is in the output's script. A second claim of ours whose key the
script has no use for currently goes unreported when it is listed behind our real entry, or when
it is substituted for another cosigner's entry, which keeps the entry count at n and so passes the
surplus count too.

This is conservative (an unraised alarm, never a wrong number) and small.

---

Solution

Every claim of ours on a multisig output is held to its script

Every entry claiming this seed is held to the output's committed script, so a second claim of
ours whose key the script has no use for is refused as PSBTOutputOwnershipContradictionError
wherever the psbt lists it: ahead of our real entry, behind it, or in the place of another
cosigner's entry. The substituted placement keeps the entry count at n, which is why the surplus
count alone could not catch it.

The ownership scan already re-derives every claim of ours and refuses a false one; it now keeps
every entry it proved instead of the first. verified_output_derivation_paths[i] (and the input
list) holds a list of the psbt's own DerivationPath entries per output, empty for an output
claiming none of our keys, and the multisig check iterates that list. The single-sig rebuild
requires that list to be exactly the one entry it rebuilt from. The contradiction messages and
the change data the views read take the first entry's path, which is the same path they used
before.

_derive_with_cache is renamed _derive_with_cache_via_indices, and
_derive_with_cache_via_derivation_path is added beside it for callers holding a psbt entry.
The two names say which shape a caller has; the one caller deriving below a cosigner xpub keeps
the index form, since the two levels below an xpub are not a full path from a master.

---

Design Consideration

Why an appended decoy now reports as a contradiction

#1032 refuses a decoy entry of ours listed after our real entry on a multisig output by the
surplus count, on a plain Warning, and the same decoy listed first as a contradiction, on a Dire
Warning. Identical content, two screens. Holding every claim to the script puts every placement
on the contradiction, which is what the psbt actually did: it claimed our key on an output whose
script has no use for it. The surplus count still stands behind it, for an output padded with a
stranger's entry, which redirects nothing of ours.

Why the scan keeps every verified path

The multisig check needs every path of ours on the output. Reading them back off the output's
bip32_derivations would have worked, since the scan had proved each one, but only by relying
on call order: a reader would have a claim-named source standing in for a verified one, and a
future caller ahead of the scan would get unproven paths with nothing to say so. Recording every
verified path where the scan proves it keeps the split visible at each use: the surplus counts
read the psbt's entries (a stranger's entry counts the same as ours there), and ownership reads
the verified list.

---

Screenshots

No screen changes. The refusal reaches PSBTOutputOwnershipContradictionView, which #1032 added.

---

Testing

pytest passes at 257. Two new parser tests (the padded-output case, and a unit test that the
entry-taking derive helper ignores the entry's fingerprint), plus the multisig decoy test
reshaped to cover three placements (first, last, substituted) and one verdict.

I mutation-checked the change: checking only the first verified path fails the decoy test,
removing the surplus count fails the padded-output test, and a fingerprint check inside the
derive helper fails its unit test.

I have not tested this on hardware.

---

Other Notes

• The multisig surplus count is now reached only by an output padded with a stranger's entry,
  since a decoy of ours is refused as a contradiction before the count runs.
  test__parse__rejects_a_multisig_output_padded_with_a_strangers_entry covers it.
• verified_input_derivation_paths and verified_output_derivation_paths change shape from
  one index list or None per input/output to a list of DerivationPath entries per
  input/output. Nothing outside the parser reads them; the views read
  change_data["verified_derivation_path"], which stays an index list. The existing assertions
  on the old shape are updated, and two tests are renamed from ..._none_for_not_owned_... to
  ..._empty_for_not_owned_....
• The _derive_with_cache rename is its own commit, so the check's commit reads without it.
• _derive_with_cache_via_derivation_path reads only the entry's path. The fingerprint-blind
  checks in _parse_outputs (the single-sig rebuild and the multisig fallback) rely on that, so
  a fingerprint check does not belong in it.
• The honest version of the multi-entry case, a 2-of-3 that uses this seed for two of its
  keys, is written up as a commented-out test beside the decoy test rather than built: the
  fixture work is out of proportion to a wallet nobody would set up.
• Four existing assertions on the verified-path lists that used any()/all() over a
  generator are rewritten as plain loops, since this PR rewrites those lines anyway.
• A TODO on the ownership tests' _parse helper: its name reads as a twin of PSBT.parse()
  in tests that call both. This PR's new tests construct PSBTParser directly.
• One em dash in an unrelated test docstring (test_cache_does_not_change_parse_output) is
  rewritten in passing, per the file-wide style rule.

---

This pull request is categorized as a:

• Other: security hardening improvement

---

Checklist

I ran pytest locally

• All tests passed before submitting the PR

---

I included screenshots of any new or modified screens

Should be part of the PR description above.

• N/A

---

I added or updated tests

Any new or altered functionality should be covered in a unit test. Any new or updated sequences require FlowTests.

• Yes

---

I tested this PR hands-on on the following platform(s):

• SeedSigner OS on a Pi0/Pi0W board

---

I have reviewed these notes:

• Keep your changes limited in scope.
• If you uncover other issues or improvements along the way, ideally submit those as a separate PR.
• The more complicated the PR, the harder it is to review, test, and merge.
• We appreciate your efforts, but we're a small team of volunteers so PR review can be a very slow process.
• Please only "@" mention a contributor if their input is truly needed to enable further progress.

• I understand

---

Thank you! Please join our Devs' Telegram group to get more involved.

[Chaitanya-Keyal]

ACK at e1cd05d. Only a few minor nits.

[Chaitanya-Keyal]

Suggested change

	for verified_derivation_paths in self.verified_input_derivation_paths:
	if verified_derivation_paths != []:
	return
	if any(self.verified_input_derivation_paths):
	return

I know explicit code is usually the preference here, but in this case any reads much more cleanly than a loop with a negative comparison. The code this replaces already used any, in a more complicated form.

[kdmukai]

I'm still personally allergic to any() and all() even though they really are perfectly readable.

I did make one tiny change to make the if check read more affirmatively in 34fb174.

[Chaitanya-Keyal]

Suggested change

	# Prep the our modified psbt in embit
	# Prep the modified psbt in embit

[kdmukai]

Fixed in 74cf692.

[Chaitanya-Keyal]

I agree this isn't worth the effort, but maybe we could leave a TODO comment instead of the commented-out block so someone might pick it up later.

[kdmukai]

The original Claude-generated test was ~75 lines, most of which was more complex / difficult to review than the rest of the tests.

I wanted to document the intentional test coverage gap, but I'm not sure I'd ever actually want the test to be added.

[newtonick]

ACK e1cd05d

[alvroble]

ACK as of e1cd05d

[newtonick]

ACK 34fb174 LGTM