mirror of
https://github.com/SeedSigner/seedsigner.git
synced 2026-10-05 15:08:25 +00:00
Narrow bare p2sh candidacy to what the rebuild cannot decide
The nested single sig outlier in change candidacy also required the output's one derivation path entry to claim this seed's fingerprint. That kept an output paying our key under another seed's key and fingerprint out of the rebuild, so it was shown as a spend to the user's own address, while the same output with its redeem script present is refused as a contradiction. The rebuild derives our key at the entry's path whatever fingerprint it lists, so the outlier drops the condition and _is_change_candidate drops the verified derivation paths it read for it. The outlier also required exactly one derivation path entry. It only kept out bare p2sh outputs annotated with several keys while omitting their scripts, which no surveyed coordinator emits, and the single sig surplus check now refuses those. The m-of-n exclusion becomes a check that the output omits its redeem script. For an output parsed as plain p2sh the two admit the same outputs, since a supplied multisig redeem script is the only source of an m-of-n there, and the new check states what the outlier is for. New tests refuse an output that pays this seed but lists another key, and pin each of the three remaining conditions: a payment to another wallet of this seed, annotated with our key, stays a spend.
This commit is contained in:
@@ -411,7 +411,7 @@ class PSBTParser():
|
||||
# Is this output change? If this output's policy is superficially similar to
|
||||
# the spending wallet's policy (e.g. they're both 2-of-3 p2wsh), then it's a
|
||||
# candidate for being change.
|
||||
if self._is_change_candidate(out, out_policy, self.verified_output_derivation_paths[i]):
|
||||
if self._is_change_candidate(out, out_policy):
|
||||
# Begin the extensive work to fully verify whether this output is indeed
|
||||
# change.
|
||||
|
||||
@@ -708,7 +708,7 @@ class PSBTParser():
|
||||
return policy
|
||||
|
||||
|
||||
def _is_change_candidate(self, out: OutputScope, out_policy: dict, verified_derivation_paths: List[DerivationPath]) -> bool:
|
||||
def _is_change_candidate(self, out: OutputScope, out_policy: dict) -> bool:
|
||||
"""
|
||||
Determines whether an output is worth the full ownership check in _parse_outputs.
|
||||
|
||||
@@ -732,15 +732,13 @@ class PSBTParser():
|
||||
"""
|
||||
# The outlier: a single sig p2sh output when the inputs are p2sh-p2wpkh.
|
||||
if (
|
||||
self.policy["type"] == "p2sh-p2wpkh" # Input policy criteria
|
||||
and out_policy["type"] == "p2sh" # Output policy criteria
|
||||
and "m" not in out_policy # Exclude multisig
|
||||
and len(out.bip32_derivations) == 1 # Nested single sig pays just one key
|
||||
and len(verified_derivation_paths) == 1 # And that one key must be ours
|
||||
self.policy["type"] == "p2sh-p2wpkh" # Input is nested single sig
|
||||
and out_policy["type"] == "p2sh" # Output parses as plain p2sh
|
||||
and out.redeem_script is None # Output omits its redeem script
|
||||
):
|
||||
return True
|
||||
|
||||
# The usual test: the output's policy has the same shape as the inputs' policy.
|
||||
# All other outputs must have the same policy shape as the inputs
|
||||
for field in ("type", "m", "n"):
|
||||
if out_policy.get(field) != self.policy.get(field):
|
||||
return False
|
||||
|
||||
@@ -2105,6 +2105,70 @@ class TestPSBTParserOutputOwnership(PSBTParserOwnershipTestBase):
|
||||
self._parse(psbt)
|
||||
|
||||
|
||||
def test__parse__rejects_a_bare_p2sh_output_that_pays_this_seed_but_lists_another_key(self):
|
||||
"""
|
||||
Another variation: the redeem script is omitted and the output still pays our key,
|
||||
but its one derivation path entry lists another seed's key and fingerprint at the
|
||||
same path.
|
||||
|
||||
The parser rebuilds p2sh(p2wpkh(K)) from our own key at that path and ignores the
|
||||
fingerprint the entry lists. The result based on our key matches what the output
|
||||
commits to, so it actually is our output even though the psbt claimed that it paid
|
||||
to a different key. The psbt should be refused with
|
||||
PSBTOutputOwnershipContradictionError.
|
||||
"""
|
||||
psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_CHANGE)
|
||||
psbt.outputs[0].redeem_script = None
|
||||
|
||||
# Swap the output's entry for another seed's key at the same path. The
|
||||
# scriptPubKey still pays our key there.
|
||||
derivation_path = list(psbt.outputs[0].bip32_derivations.values())[0]
|
||||
path = bip32.path_to_str(derivation_path.derivation)
|
||||
psbt.outputs[0].bip32_derivations.clear()
|
||||
claim_seed_owns_key(psbt.outputs[0], path, foreign_public_key(path), seed=PSBTTestData.recipient_seed)
|
||||
|
||||
with pytest.raises(PSBTOutputOwnershipContradictionError):
|
||||
self._parse(psbt)
|
||||
|
||||
|
||||
def test__parse__counts_a_payment_to_another_wallet_of_this_seed_as_a_spend(self):
|
||||
"""
|
||||
A psbt can pay another wallet of this seed, such as its native segwit account or a
|
||||
multisig it belongs to, and annotate that output with our key. Each case below is
|
||||
such a payment. Each should parse and be counted as a spend.
|
||||
|
||||
The parser makes an exception for nested single sig change that omits its redeem
|
||||
script. An output must meet three criteria to qualify for the exception:
|
||||
* inputs: the inputs are nested single sig
|
||||
* output type: the output is parsed as plain p2sh
|
||||
* redeem script: the output omits its redeem script
|
||||
|
||||
These outputs should be categorized as external spends if any of the three
|
||||
criteria are not met. Each scenario in this test sets up one criterion to fail
|
||||
while the other two are met.
|
||||
"""
|
||||
# Fails the inputs condition: a native segwit input instead of nested single sig
|
||||
psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_1_INPUT, PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_CHANGE)
|
||||
psbt.outputs[0].redeem_script = None
|
||||
psbt_parser = self._parse(psbt)
|
||||
assert psbt_parser.change_amount == 0
|
||||
assert psbt_parser.spend_amount == 10_000
|
||||
|
||||
# Fails the output type condition: p2wpkh is not parsed as a plain p2sh output
|
||||
psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.SINGLE_SIG_NATIVE_SEGWIT_CHANGE)
|
||||
psbt_parser = self._parse(psbt)
|
||||
assert psbt_parser.change_amount == 0
|
||||
assert psbt_parser.spend_amount == 10_000
|
||||
|
||||
# Fails the redeem script condition: legacy multisig is parsed as plain p2sh even
|
||||
# when it supplies its redeem script.
|
||||
psbt = self._psbt_with_change(PSBTTestData.SINGLE_SIG_NESTED_SEGWIT_1_INPUT, PSBTTestData.MULTISIG_LEGACY_P2SH_CHANGE)
|
||||
assert psbt.outputs[0].redeem_script is not None
|
||||
psbt_parser = self._parse(psbt)
|
||||
assert psbt_parser.change_amount == 0
|
||||
assert psbt_parser.spend_amount == 10_000
|
||||
|
||||
|
||||
def test_get_cosigners_returns_a_sorted_list(self):
|
||||
"""
|
||||
Two multisig scripts can list the same wallet's keys in different orders, so the
|
||||
|
||||
Reference in New Issue
Block a user