laantungir/seedsigner
1
0
Fork 0
mirror of https://github.com/SeedSigner/seedsigner.git synced 2026-10-05 15:08:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Compare cosigners before presuming multisig change

Browse Source 
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.
This commit is contained in:
kdmukai
2026-09-12 08:18:59 -05:00
parent 321bf14deb
commit ff13396691
2 changed files with 244 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/seedsigner/models/psbt_parser.py
+65 -6
View File
@@ -559,6 +559,19 @@ class PSBTParser():
# their "known-good" multisig descriptor.
is_presumed_change = True
# One thing we can rule out now: if the psbt supplied global
# xpubs (see _get_cosigners) AND it fully annotated this
# output, we can see if this output's cosigners differ from
# the inputs' cosigners. If so, then we can be sure this
# output is NOT our change. This sort of mismatch is a
# scenario that no known coordinator would produce, but
# there's no harm in checking this edge case.
input_cosigners = self.policy.get("cosigners")
output_cosigners = out_policy.get("cosigners")
cosigners_resolved = input_cosigners is not None and output_cosigners is not None
if cosigners_resolved and input_cosigners != output_cosigners:
is_presumed_change = False
elif verified_derivation_path is not None and self.policy["type"] != "p2tr":
# The psbt claims one of this seed's keys on this output, yet the
# output does NOT pay what that claim describes. We treat this
@@ -816,25 +829,71 @@ class PSBTParser():
@staticmethod
def _get_cosigners(pubkeys, derivations, xpubs, child_key_derivation_cache: dict | None):
"""Returns xpubs used to derive pubkeys using global xpub field from psbt"""
"""
Traces every key in a multisig script back to the global xpub it was derived
from, then returns the xpubs it found as a sorted list of base58 strings.
Args:
* pubkeys: The keys that actually appear in the script (the witness script for
segwit; the redeem script for legacy p2sh). Extracted by _get_policy(). One
per cosigner.
* derivations: (embit's bip32_derivations) Each pubkey's associated fingerprint
and full derivation path (e.g. m/48'/0'/0'/2'/1/5). A dict keyed on each
pubkey.
* xpubs: aka "global xpubs". The account-level xpub, with its associated
fingerprint and derivation path, but only down to the account level (e.g.
m/48'/0'/0'/2'). A dict keyed on each xpub.
The derivations and xpubs are unproven claims provided by the coordinator. So for
each pubkey we check whether the xpub the psbt points us to really derives it.
The resulting cosigners list consists of each xpub that provably derives each of
the script's keys. But that is ALL it proves. We have no way to verify who those
xpubs actually belong to; the coordinator can list any xpubs it likes.
The list is sorted so that two scripts holding the same wallet's keys in a
different order resolve to the same cosigners.
Note that the bip32_derivations and the global xpubs are both optional psbt
fields. If either is omitted or incomplete, this function raises rather than
return a partial list.
"""
# TODO: Improve error handling by providing custom exceptions.
# Early-out if the optional data is omitted. Not actually an error: raising is
# how this function reports that a complete cosigner list can't be built.
if not xpubs:
raise ValueError("No global xpubs supplied")
if not derivations:
raise ValueError("No derivation paths supplied")
cosigners = []
for i, pubkey in enumerate(pubkeys):
# For each pubkey, get the claimed fingerprint and full derivation path
if pubkey not in derivations:
raise ValueError("Missing derivation")
der = derivations[pubkey]
# Scan the xpubs for one whose fingerprint and derivation path match the
# claim (xpub path comparisons have to stop at the account level).
for xpub in xpubs:
origin_der = xpubs[xpub]
# check fingerprint
if origin_der.fingerprint == der.fingerprint:
# check derivation - last two indexes give pub from xpub
if origin_der.derivation == der.derivation[:-2]:
# check that it derives to pubkey actually
derived_key = PSBTParser._derive_with_cache(
xpub, der.derivation[-2:], child_key_derivation_cache)
# Then derive the actual child key (its full derivation path is
# two indices deeper than the xpub's stated derivation path)
derived_key = PSBTParser._derive_with_cache(xpub, der.derivation[-2:], child_key_derivation_cache)
# Finally, compare that key with the target pubkey
if derived_key.key == pubkey:
# append strings so they can be sorted and compared
cosigners.append(xpub.to_base58())
break
# Every key in the script has to trace back to an xpub for the result to mean
# anything.
if len(cosigners) != len(pubkeys):
raise RuntimeError("Can't get all cosigners")
return sorted(cosigners)
tests/test_psbt_parser.py
+179
View File
@@ -1889,6 +1889,185 @@ class TestPSBTParserOutputOwnership(PSBTParserOwnershipTestBase):
self._parse(psbt)
def test_get_cosigners_returns_a_sorted_list(self):
"""
Two multisig scripts can list the same wallet's keys in different orders, so the
cosigners resolved for one script and the cosigners resolved for another have to
be sorted before they can be compared. Regression test against _get_cosigners
ever dropping the sort logic.
"""
psbt = PSBT.parse(a2b_base64(PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT))
inp = psbt.inputs[0]
pubkeys = list(inp.bip32_derivations.keys())
cosigners = PSBTParser._get_cosigners(pubkeys, inp.bip32_derivations, psbt.xpubs, None)
assert cosigners == sorted(cosigners)
# Sanity check: the fixture's three cosigners are three different xpubs.
assert len(set(cosigners)) == 3
# The same keys, handed over in the opposite order
reordered = PSBTParser._get_cosigners(list(reversed(pubkeys)), inp.bip32_derivations, psbt.xpubs, None)
assert reordered == cosigners
def _repoint_at_a_different_quorum(self, psbt: PSBT):
"""
Helper function to rebuild the change output's script so it pays a 2-of-3 that
still holds this seed's key, but with one cosigner swapped for a different seed.
That swap makes the output's 2-of-3 a different wallet from the one the inputs
spend from.
"""
# Callers pass a psbt whose change output comes first
out = psbt.outputs[0]
seed_fingerprint = root_for_seed(self.seed).my_fingerprint
# The replacement cosigner will use the same account-level derivation path as the
# first xpub.
account_derivation_path = list(psbt.xpubs.values())[0].derivation
# Pick the first entry that isn't the current seed's.
for public_key, derivation_path_obj in out.bip32_derivations.items():
if derivation_path_obj.fingerprint != seed_fingerprint:
displaced_public_key = public_key
address_derivation_path = derivation_path_obj.derivation
break
# Build the outsider from a different seed: its account xpub at the shared
# account path, then the child key two levels down at the displaced entry's
# change/index.
outsider_root = root_for_seed(PSBTTestData.recipient_seed)
outsider_account = outsider_root.derive(account_derivation_path)
outsider_public_key = outsider_account.derive(address_derivation_path[-2:]).get_public_key()
# Add the outsider's account xpub to the psbt's global xpubs, which now hold
# four: the inputs' three cosigners plus the outsider.
psbt.xpubs[outsider_account.to_public()] = DerivationPath(outsider_root.my_fingerprint, account_derivation_path)
# Rebuild the multisig script with the outsider's key in the displaced key's
# slot.
original_script = out.witness_script if out.witness_script is not None else out.redeem_script
m, n, public_keys = PSBTParser._parse_multisig(original_script)
new_pubkey_list = []
for public_key in public_keys:
if public_key == displaced_public_key:
new_pubkey_list.append(outsider_public_key)
else:
new_pubkey_list.append(public_key)
rebuilt_script = script.multisig(m, new_pubkey_list)
# Swap the displaced cosigner's derivation path entry for the outsider's, so
# the psbt describes the rebuilt script truthfully.
del out.bip32_derivations[displaced_public_key]
out.bip32_derivations[outsider_public_key] = DerivationPath(outsider_root.my_fingerprint, address_derivation_path)
# Recommit the output to the rebuilt script, through the wrapping the fixture
# uses: p2wsh, p2sh-p2wsh, or bare p2sh.
if out.witness_script is not None:
out.witness_script = rebuilt_script
inner_script = script.p2wsh(rebuilt_script)
else:
inner_script = rebuilt_script
if out.redeem_script is not None:
out.redeem_script = inner_script
out.script_pubkey = script.p2sh(inner_script)
else:
out.script_pubkey = inner_script
def test__parse__counts_a_different_quorum_as_a_spend(self):
"""
An output paying a 2-of-3 that this seed is genuinely part of, but whose third
cosigner is a different seed. That makes the output's 2-of-3 a different wallet
from the one the inputs spend from.
Every ownership check passes: the output commits to a script holding this seed's
key and the psbt claims this seed there truthfully.
The psbt supplies global xpubs (the three from the input plus the outsider's that
is part of the output) and it fully annotates this output's derivation paths,
allowing for the parser to determine the output's cosigners accurately.
End result of this setup: The output's cosigner list differs from the inputs'
list. So the output is counted as a spend.
This sort of cosigner mismatch is a scenario that no known coordinator would
produce; normally an output to a different wallet would not be annotated nor have
its xpubs added to the global xpubs data.
"""
for input_base64, change_hex in [
(PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE),
(PSBTTestData.MULTISIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NESTED_SEGWIT_CHANGE),
(PSBTTestData.MULTISIG_LEGACY_P2SH_1_INPUT, PSBTTestData.MULTISIG_LEGACY_P2SH_CHANGE),
]:
# The wallet's own change output, for comparison
psbt_parser = self._parse(self._psbt_with_change(input_base64, change_hex))
assert psbt_parser.change_amount == 10_000
assert psbt_parser.spend_amount == 0
psbt = self._psbt_with_change(input_base64, change_hex)
self._repoint_at_a_different_quorum(psbt)
# The parse accepts the psbt: it described this output accurately.
psbt_parser = self._parse(psbt)
# This seed's key really is in the committed script and the psbt's claim of
# this seed verified.
assert psbt_parser.verified_output_derivation_paths[0] is not None
# But the output pays a different quorum than the inputs spend from, so it
# is counted as a spend.
assert psbt_parser.change_data == []
assert psbt_parser.change_amount == 0
assert psbt_parser.spend_amount == 10_000
def test__parse__counts_a_different_quorum_as_change_if_no_global_xpubs(self):
"""
Same setup as the previous test, but this time the psbt omits its global xpubs.
The global xpubs are needed in order to resolve cosigners. So without them, the
inputs' cosigner list and the output's cosigner list comparison is skipped. The
user's seed is part of the output wallet and the output's policy "shape"
superficially matches the input's (2-of-3, same script type), so the output is
counted as presumed change.
BIP-174 makes the global xpubs optional and honest coordinators do omit them. The
previous test notes that no known coordinator annotates external spend outputs so
this test scenario is unlikely to be seen in the real world. But this version of
the test has one notable exception: Bitcoin Core.
Core can hold the descriptors of several spending wallets. It will annotate an
output that belongs to ANY of its descriptors, regardless of whether it differs
from the input wallet. But Core does not write global xpubs at all, so it wasn't a
factor in the previous test (which required the global xpubs).
But a Core-built psbt can exactly match this test's shape: a fully annotated
foreign output and no global xpubs to compare against.
"""
# Test each multisig script type
for input_base64, change_hex in [
(PSBTTestData.MULTISIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NATIVE_SEGWIT_CHANGE),
(PSBTTestData.MULTISIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_NESTED_SEGWIT_CHANGE),
(PSBTTestData.MULTISIG_LEGACY_P2SH_1_INPUT, PSBTTestData.MULTISIG_LEGACY_P2SH_CHANGE),
]:
psbt = self._psbt_with_change(input_base64, change_hex)
self._repoint_at_a_different_quorum(psbt)
# The global xpubs must be omitted for this scenario
psbt.xpubs.clear()
psbt_parser = self._parse(psbt)
# Parser categorizes the output as presumed change.
assert psbt_parser.change_amount == 10_000
assert psbt_parser.spend_amount == 0
def test__parse__refuses_an_unsupported_script_type(self):
"""
Parsing should be aborted if a psbt has inputs and outputs that use a script type