Test gaps: PSBTParser self-output classification and QR type detection

[kdmukai]

After reviewing a number of PRs that want to incorporate hypothesis testing (full disclosure: I did not look deeply into any of them), I used Claude to determine what we'd actually gain from this approach.

In the end, it seemed simpler to me to just create new targeted test cases that directly address what "property-based testing" (to the extent I understand it) would be aiming for.

Below is Claude's list of specific test case additions.

---

Recommended SeedSigner test additions

These all belong in tests/test_psbt_parser.py or tests/test_decodepsbtqr.py
and use the existing pytest harness — no new dependencies.

1. Multisig PSBT, no descriptor registered

Construct a multisig PSBT (any of the three existing fixture types).
Instantiate PSBTParser without loading a descriptor. Assert:

• num_change_outputs == 0 (no output trusted as self/change)
• spend_amount == sum of all outputs
• verify_multisig_output(...) is not called or returns False on every
  output

2. Lying-coordinator (single-sig): forged BIP_DERIVATION on output

For each of the four single-sig types, take a fixture and modify one
output's PSBT_OUT_BIP32_DERIVATION to claim a path that does NOT
derive the output's actual scriptPubKey from the on-device seed.
Assert that output is NOT classified as change/self.

3. Lying-coordinator (multisig): forged BIP_DERIVATION with correct

descriptor loaded
Same construction as #2 but for multisig with the correct descriptor
loaded. Assert verify_multisig_output(...) returns False.

4. Multi-output consolidation across both branches

Construct a PSBT with no external outputs — multiple outputs to the
device's own addresses, mixing the change branch (1/*) and receive
branch (0/*). Assert:

• spend_amount == 0
• change_amount == input_amount - fee_amount
• All outputs classified as self

5. Multisig descriptor that doesn't contain the device's xpub

Load a descriptor whose key set does not include the device's xpub at
all (different signing quorum entirely). Construct any multisig PSBT.
Assert verify_multisig_output(...) returns False on all outputs.

6. Self-output at non-standard derivation path

Construct PSBTs with self-outputs at:

• A high index (e.g. 0/9999)
• A non-standard branch (e.g. m/.../2/*)
  Assert correct self-classification (high index) and correct rejection
  or non-classification (non-standard branch — pin whatever the policy is).

7. Regression test for the known duplicate-branch bug

Pin the current behavior of the bug referenced in PR #929 / the
proposal. Include a docstring explaining the bug, expected behavior
once fixed, and link to the issue.

8. QR type-detection boundary cases (DecodeQR)

Feed DecodeQR.add_data() inputs that could be ambiguous between
detected types:

• A base64 string that is also a valid prefix of another format
• A UR-prefixed payload with a non-PSBT type
• Inputs at the boundary between PSBT formats (UR2 vs Specter vs BBQR)
• An input that could parse as both seed-type and wallet-json-type
  For each, assert the chosen qr_type matches the documented priority
  order. Pins the detection policy against silent regressions.

---

And then a separate section for embit test gaps:

Recommended embit upstream contributions

The following gaps exist in embit's test_psbt.py negative coverage
and would be appropriate as upstream PRs to
github.com/diybitcoinhardware/embit, not as SeedSigner tests.

Byte-level robustness

• Length-prefix mismatch: declared length exceeds remaining bytes
• Length-prefix mismatch: declared length is zero where data is expected
• Truncated input mid-key, mid-value, mid-section
• Integer overflow on amount fields (values near 2^64 - 1)

Each should assert the parser raises a specific exception cleanly,
without leaking a generic Python exception or silently truncating.

embit already covers BIP-174 invalid-vector compliance, duplicate keys,
invalid typed keys at all levels, malformed descriptors, and
sortedmulti vs multi key ordering — so those don't need duplication
upstream or in SeedSigner.

[nkatha23]

Hi @kdmukai I'd like to take on these test cases.

2. Lying-coordinator (single-sig): forged BIP_DERIVATION on output

For each of the four single-sig types, take a fixture and modify one output's PSBT_OUT_BIP32_DERIVATION to claim a path that does NOT derive the output's actual scriptPubKey from the on-device seed. Assert that output is NOT classified as change/self.

3. Lying-coordinator (multisig): forged BIP_DERIVATION with correct

descriptor loaded Same construction as #2 but for multisig with the correct descriptor loaded. Assert verify_multisig_output(...) returns False.

For this, I'll need to modify PSBT_OUT_BIP32_DERIVATION on existing fixtures to point to a derivation path that doesn't derive the output's actual scriptPubKey. I can do this using embit's DerivationPath directly, similar to what test_missing_fingerprint_handling already does when zeroing out fingerprints.

7. Regression test for the known duplicate-branch bug

Pin the current behavior of the bug referenced in PR #929 / the proposal. Include a docstring explaining the bug, expected behavior once fixed, and link to the issue.

I found this bug while reading _parse_outputs(), the bare if sc.data == self.psbt.tx.vout[i].script_pubkey.data after the
elif "p2tr" branch. I can write the regression test and document the exact condition under which it fires.

8. QR type-detection boundary cases (DecodeQR)

Feed DecodeQR.add_data() inputs that could be ambiguous between detected types:

• A base64 string that is also a valid prefix of another format
• A UR-prefixed payload with a non-PSBT type
• Inputs at the boundary between PSBT formats (UR2 vs Specter vs BBQR)
• An input that could parse as both seed-type and wallet-json-type
  For each, assert the chosen qr_type matches the documented priority
  order. Pins the detection policy against silent regressions.

For this one, I'll need to map the documented priority order for QR type detection before writing the boundary cases. is that written down somewhere or should I derive it from the source?

I'll start with 1, 4, and 7 as they're the most self-contained, then move to the lying-coordinator cases. Happy to take feedback as I go.

[kdmukai]

For this one, I'll need to map the documented priority order for QR type detection before writing the boundary cases. is that written down somewhere or should I derive it from the source?

Derive from source. But also note that since the suggestions were created by Claude, it may not have had full context of real-world use cases. I can't quite think through if the bullet points provided are actual edge cases or just AI guesses.

So I'd suggest doing some deeper research on the various formats before taking Claude at face value here.

[nkatha23]

For this one, I'll need to map the documented priority order for QR type detection before writing the boundary cases. is that written down somewhere or should I derive it from the source?

Derive from source. But also note that since the suggestions were created by Claude, it may not have had full context of real-world use cases. I can't quite think through if the bullet points provided are actual edge cases or just AI guesses.

So I'd suggest doing some deeper research on the various formats before taking Claude at face value here.

Noted on the claude :)
I've looked at these files:

src/seedsigner/models/decode_qr.py
tests/test_decodepsbtqr.py
tests/test_encodepsbtqr.py
https://github.com/BlockchainCommons/Research/blob/master/papers/bcr-2020-006-urtypes.md
https://github.com/coinkite/BBQr/blob/master/BBQr.md
https://github.com/SeedSigner/seedsigner/blob/dev/docs/qr_formats.md

The detection priority order from detect_segment_type():
UR:CRYPTO-PSBT/ - UR:CRYPTO-OUTPUT/ - UR:CRYPTO-ACCOUNT/ - pNofN {base64only} - UR:BYTES/ - is_base64_psbt() - B$[2HZ]P - pNofN (not base64) - wallet JSON - multisig setup file - sortedmulti - \d{48,96} - bitcoin address - signmessage - settings:: - BIP-39 mnemonic - 4-letter mnemonic - is_base43_psbt() - 16/32 bytes - INVALID

On your point 8 (QR type-detection boundary cases),sub-bullets listed:

Bullet 1 (base64 prefix overlap): Mostly handled — $ is not valid base64 so BBQR strings don't pass is_base64(). One real case: pNofN {non-base64-data} misses the Specter PSBT regex (which requires base64 chars only anchored to end of string) and falls through to the wallet Specter check, returning WALLET__SPECTER instead of INVALID. Pinnable.

Bullet 2 (UR-prefixed non-PSBT type): Real. Only four UR types are handled explicitly. UR:SEED/, UR:HDKEY/, UR:OUTPUT-DESCRIPTOR/, and the newer ur:psbt tag from BCR-2020-006 all fall through to INVALID. Worth pinning as documented behavior. Also — SEED__UR2 is defined in qr_type.py, listed in the is_seed property, and has a decoder path in add_data(), but detect_segment_type() never returns it. UR:SEED/ strings currently return INVALID. Is that a missing case or intentional?

Bullet 3 (UR2 vs Specter vs BBQR boundary): The format discriminators are distinct enough that real overlap doesn't happen in practice. The one boundary case worth testing is already covered by bullet 1.

Bullet 4 (seed vs wallet-json): No real ambiguity. The wallet discriminators ({, sortedmulti, multisig setup file) can't match any BIP-39 mnemonic string.
One additional case not in the original list: the BBQR regex has no re.IGNORECASE, so lowercase b$hp0100... returns INVALID. Consistent with the BBQr spec (uppercase required) but worth a pinned test.

If this kind of research is what you're looking for, I'd love to rewrite my proposal around this issue and work on it over the summer, still under SoB if that's possible :)