Accept XFP==0 as special case

releases/ChangeLog.md

@@ -1,3 +1,10 @@
+## 4.1.4 - Sep ??, 2021
+
+- Enhancement: if an XFP of zero is seen in a PSBT file, assume that should be replaced by
+  our current XFP value and try to sign the input (same for change outputs and change-fraud
+  checks).  This makes building a workable PSBT file easier and could be used to preserve
+  privacy of XFP value itself. A warning is shown when this happens.
+
 ## 4.1.3 - Sep 2, 2021
 
 - Enhancement: support "importdescriptors" command in Bitcoin Core 0.21 so that

shared/psbt.py

@@ -228,7 +228,7 @@ class psbtProxy:
         self.fd.seek(pos)
         return self.fd.read(ll)
 
-    def parse_subpaths(self, my_xfp):
+    def parse_subpaths(self, my_xfp, warnings):
         # Reformat self.subpaths into a more useful form for us; return # of them
         # that are ours (and track that as self.num_our_keys)
         # - works in-place, on self.subpaths
@@ -259,6 +259,15 @@ class psbtProxy:
             v = self.get(self.subpaths[pk])
             here = list(unpack_from('<%dI' % (vl//4), v))
 
+            # Tricky & Useful: if xfp of zero is observed in file, assume that's a 
+            # placeholder for my XFP value. Replace on the fly. Great when master
+            # XFP is unknown because PSBT built from derived XPUB only. Also privacy.
+            if here[0] == 0:
+                here[0] = my_xfp
+                if not any(True for k,_ in warnings if 'XFP' in k):
+                    warnings.append(('Zero XFP',
+                            'Assuming XFP of zero should be replaced by correct XFP'))
+
             # update in place
             self.subpaths[pk] = here
 
@@ -327,7 +336,7 @@ class psbtOutputProxy(psbtProxy):
             for k in self.unknown:
                 wr(k[0], self.unknown[k], k[1:])
 
-    def validate(self, out_idx, txo, my_xfp, active_multisig):
+    def validate(self, out_idx, txo, my_xfp, active_multisig, parent):
         # Do things make sense for this output?
     
         # NOTE: We might think it's a change output just because the PSBT
@@ -339,7 +348,7 @@ class psbtOutputProxy(psbtProxy):
         # - we raise fraud alarms, since these are not innocent errors
         #
 
-        num_ours = self.parse_subpaths(my_xfp)
+        num_ours = self.parse_subpaths(my_xfp, parent.warnings)
 
         if num_ours == 0:
             # - not considered fraud because other signers looking at PSBT may have them
@@ -513,7 +522,7 @@ class psbtInputProxy(psbtProxy):
 
         self.parse(fd)
 
-    def validate(self, idx, txin, my_xfp):
+    def validate(self, idx, txin, my_xfp, parent):
         # Validate this txn input: given deserialized CTxIn and maybe witness
 
         # TODO: tighten these
@@ -525,7 +534,7 @@ class psbtInputProxy(psbtProxy):
         # require path for each addr, check some are ours
 
         # rework the pubkey => subpath mapping
-        self.parse_subpaths(my_xfp)
+        self.parse_subpaths(my_xfp, parent.warnings)
 
         # sighash, but we're probably going to ignore anyway.
         self.sighash = SIGHASH_ALL if self.sighash is None else self.sighash
@@ -1109,7 +1118,7 @@ class psbtObject(psbtProxy):
 
         # this parses the input TXN in-place
         for idx, txin in self.input_iter():
-            self.inputs[idx].validate(idx, txin, self.my_xfp)
+            self.inputs[idx].validate(idx, txin, self.my_xfp, self)
 
         assert len(self.inputs) == self.num_inputs, 'ni mismatch'
 
@@ -1132,7 +1141,7 @@ class psbtObject(psbtProxy):
         # - mark change outputs, so perhaps we don't show them to users
 
         for idx, txo in self.output_iter():
-            self.outputs[idx].validate(idx, txo, self.my_xfp, self.active_multisig)
+            self.outputs[idx].validate(idx, txo, self.my_xfp, self.active_multisig, self)
 
         if self.total_value_out is None:
             # this happens, but would expect this to have done already?

testing/test_sign.py

@@ -1510,7 +1510,7 @@ def test_wrong_pubkey(dev, try_sign, fake_txn):
 def test_incomplete_signing(dev, try_sign, fake_txn, cap_story):
     # psbt where we only sign one input
     # - must not allow finalization
-    psbt = fake_txn(2, 1, dev.master_xpub, segwit_in=False)
+    psbt = fake_txn(3, 1, dev.master_xpub, segwit_in=False)
 
     oo = BasicPSBT().parse(psbt)
     oo.inputs[1].bip32_paths = { k: b'\x01\x02\x03\x04'+v[4:] 
@@ -1528,4 +1528,30 @@ def test_incomplete_signing(dev, try_sign, fake_txn, cap_story):
     title, story = cap_story()
     assert 'No signature on input' in story
 
+def test_zero_xfp(dev, start_sign, end_sign, fake_txn, cap_story):
+    # will sign PSBT with zero values for XFP in ins and outs
+    psbt = fake_txn(2, 3, dev.master_xpub, segwit_in=False, change_outputs=[1,2])
+
+    oo = BasicPSBT().parse(psbt)
+    for i in oo.inputs:
+        i.bip32_paths = { k: b'\x00\x00\x00\x00'+v[4:] for k,v in i.bip32_paths.items() }
+    for o in oo.outputs:
+        o.bip32_paths = { k: b'\x00\x00\x00\x00'+v[4:] for k,v in o.bip32_paths.items() }
+
+    with BytesIO() as fd:
+        oo.serialize(fd)
+        mod_psbt = fd.getvalue()
+
+    # should work, with a warning
+    start_sign(mod_psbt, finalize=True)
+    time.sleep(.1)
+    _, story = cap_story()
+
+    assert '(1 warning below)' in story
+    assert 'Zero XFP' in story
+
+    # and then signing should work.
+    signed = end_sign(True, finalize=True)
+
+
 # EOF