signpsbt: Require input selection by default

With the advent of dual-funding and splicing, the risk of users unwittingly signing away funds because their peers or other PSBT services slip malicious inputs in increases. `fundpsbt` now returns a `utxo_string` identifying the specific inputs it added and `signpsbt` now requires `utxo_string` (or the existing `sigonly` array) by default. The old behavior is moved to a flag called `unsafe_sign_all`. Changelog-Chaged: `signpsbt` now requires specifying which inputs to be signed. `fundpsbt` is also updated to provide a digest of inputs in `utxo_string` — a compatible format across both commands.
ElementsProject · Nov 10, 2023 · 184fd5d · 184fd5d
1 parent 02ca226
commit 184fd5d
Show file tree

Hide file tree

Showing 16 changed files with 238 additions and 237 deletions.
diff --git a/.msggen.json b/.msggen.json
@@ -745,7 +745,8 @@
             "FundPsbt.excess_msat": 4,
             "FundPsbt.feerate_per_kw": 2,
             "FundPsbt.psbt": 1,
-            "FundPsbt.reservations[]": 6
+            "FundPsbt.reservations[]": 6,
+            "FundPsbt.utxo_string": 7
         },
         "GetinfoAddress": {
             "Getinfo.address[].address": 3,
@@ -1541,7 +1542,8 @@
         },
         "SignpsbtRequest": {
             "SignPsbt.psbt": 1,
-            "SignPsbt.signonly[]": 2
+            "SignPsbt.signonly[]": 2,
+            "SignPsbt.utxo_string": 3
         },
         "SignpsbtResponse": {
             "SignPsbt.signed_psbt": 1
@@ -3009,6 +3011,10 @@
             "added": "pre-v0.10.1",
             "deprecated": false
         },
+        "FundPsbt.utxo_string": {
+            "added": "v23.11",
+            "deprecated": false
+        },
         "GetRoute": {
             "added": "pre-v0.10.1",
             "deprecated": null
@@ -5453,6 +5459,10 @@
             "added": "pre-v0.10.1",
             "deprecated": false
         },
+        "SignPsbt.utxo_string": {
+            "added": "v23.11",
+            "deprecated": false
+        },
         "StaticBackup": {
             "added": "pre-v0.10.1",
             "deprecated": null

diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py
@@ -1570,13 +1570,15 @@ def utxopsbt(self, satoshi, feerate, startweight, utxos, reserve=None, reservedo
         }
         return self.call("utxopsbt", payload)
 
-    def signpsbt(self, psbt, signonly=None):
+    def signpsbt(self, psbt, signonly=None, utxo_string=None, unsafe_sign_all=None):
         """
         Add internal wallet's signatures to PSBT
         """
         payload = {
             "psbt": psbt,
             "signonly": signonly,
+            "utxo_string": utxo_string,
+            "unsafe_sign_all": unsafe_sign_all,
         }
         return self.call("signpsbt", payload)
 

diff --git a/contrib/pyln-grpc-proto/pyln/grpc/node_pb2.py b/contrib/pyln-grpc-proto/pyln/grpc/node_pb2.py
diff --git a/contrib/pyln-testing/pyln/testing/grpc2py.py b/contrib/pyln-testing/pyln/testing/grpc2py.py
@@ -718,6 +718,7 @@ def fundpsbt2py(m):
         "excess_msat": amount2msat(m.excess_msat),  # PrimitiveField in generate_composite
         "change_outnum": m.change_outnum,  # PrimitiveField in generate_composite
         "reservations": [fundpsbt_reservations2py(i) for i in m.reservations],  # ArrayField[composite] in generate_composite
+        "utxo_string": m.utxo_string,  # PrimitiveField in generate_composite
     })
 
 

diff --git a/doc/lightning-fundpsbt.7.md b/doc/lightning-fundpsbt.7.md
@@ -81,6 +81,7 @@ On success, an object is returned, containing:
   - **was\_reserved** (boolean): Whether this output was previously reserved (always *false*)
   - **reserved** (boolean): Whether this output is now reserved (always *true*)
   - **reserved\_to\_block** (u32): The blockheight the reservation will expire
+- **utxo\_string** (string, optional): A command seperated list of utxos used to fund the psbt *(added v24.02)*
 
 [comment]: # (GENERATE-FROM-SCHEMA-END)
 
@@ -112,4 +113,4 @@ RESOURCES
 
 Main web site: <https://github.com/ElementsProject/lightning>
 
-[comment]: # ( SHA256STAMP:13e35920ba8810db082e3cca62d1141a67498a2756da2479a24eaa62567ff4fe)
+[comment]: # ( SHA256STAMP:8c942b9a2303a05e3cda8e3190b4c299ede6020849573ef4c4ca7ee4c4363fae)
diff --git a/doc/lightning-signpsbt.7.md b/doc/lightning-signpsbt.7.md
@@ -13,11 +13,18 @@ DESCRIPTION
 BIP-174.
 
 - *psbt*: A string that represents the PSBT value.
-- *signonly*: An optional array of input numbers to sign.
+- *signonly*: An an array of input numbers to sign.
+- *utxo\_string*: An an array of outputs to sign.
+- *unsafe\_sign\_all*: A flag to recklessly sign all reserved inputs.
 
-By default, all known inputs are signed, and others ignored: with
-*signonly*, only those inputs are signed, and an error is returned if
-one of them cannot be signed.
+Specify which inputs to sign by either an array of input indices to sign with
+*sigonly* or a *utxo\_string*.
+
+*utxo\_string* is in the format of a txid and outnum seperated by a colon.
+Multiple utxos can be added by putting a comma inbetween entries. As such:
+txid:outnum,txid:outnum... This is the same format returned by fundpsbt.
+
+An error is returned if one of them cannot be signed.
 
 Note that the command will fail if there are no inputs to sign, or
 if the inputs to be signed were not previously reserved.
@@ -30,7 +37,8 @@ EXAMPLE JSON REQUEST
   "id": 82,
   "method": "signpsbt",
   "params": {
-    "psbt": "some_psbt"
+    "psbt": "some_psbt",
+    "utxo_string": "txid:outnum,txid:outnum"
   }
 }
 ```

diff --git a/doc/lightning-splice_init.7.md b/doc/lightning-splice_init.7.md
@@ -46,6 +46,7 @@ echo $RESULT;
 
 RESULT=$(lightning-cli fundpsbt -k satoshi=100000sat feerate=urgent startweight=800 excess_as_change=true);
 INITIALPSBT=$(echo $RESULT | jq -r ".psbt");
+UTXO_STRING=$(echo $RESULT | jq -r ".utxo_string");
 echo $RESULT;
 
 RESULT=$(lightning-cli splice_init $CHANNEL_ID 100000 $INITIALPSBT);
@@ -56,7 +57,7 @@ RESULT=$(lightning-cli splice_update $CHANNEL_ID $PSBT);
 PSBT=$(echo $RESULT | jq -r ".psbt");
 echo $RESULT;
 
-RESULT=$(lightning-cli signpsbt -k psbt="$PSBT");
+RESULT=$(lightning-cli signpsbt -k psbt="$PSBT" utxo_string="$UTXO_STRING");
 PSBT=$(echo $RESULT | jq -r ".signed_psbt");
 echo $RESULT;
 

diff --git a/doc/schemas/fundpsbt.schema.json b/doc/schemas/fundpsbt.schema.json
@@ -71,6 +71,11 @@
           }
         }
       }
+    },
+    "utxo_string": {
+      "type": "string",
+      "description": "A command seperated list of utxos used to fund the psbt",
+      "added": "v23.11"
     }
   }
 }
diff --git a/doc/schemas/signpsbt.request.json b/doc/schemas/signpsbt.request.json
@@ -14,6 +14,14 @@
       "items": {
         "type": "u32"
       }
+    },
+    "utxo_string": {
+      "type": "string",
+      "added": "v23.11"
+    },
+    "unsafe_sign_all": {
+      "type": "bool",
+      "added": "v23.11"
     }
   }
 }
diff --git a/external/lnprototest b/external/lnprototest
diff --git a/tests/test_closing.py b/tests/test_closing.py
@@ -3717,13 +3717,15 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind):
 
     # We reduce l1's UTXOs so it's forced to use more than one UTXO to push.
     fundsats = int(only_one(l1.rpc.listfunds()['outputs'])['amount_msat'].to_satoshi())
-    psbt = l1.rpc.fundpsbt("all", "1000perkw", 1000)['psbt']
+    result = l1.rpc.fundpsbt("all", "1000perkw", 1000)
+    psbt = result['psbt']
+    utxo_string = result['utxo_string']
     # Pay 5k sats in fees, send most to l2
     psbt = l1.rpc.addpsbtoutput(fundsats - 20000 - 5000, psbt, destination=l2.rpc.newaddr()['bech32'])['psbt']
     # 10x2000 sat outputs for l1 to use.
     for i in range(10):
         psbt = l1.rpc.addpsbtoutput(2000, psbt)['psbt']
-    l1.rpc.sendpsbt(l1.rpc.signpsbt(psbt)['signed_psbt'])
+    l1.rpc.sendpsbt(l1.rpc.signpsbt(psbt, utxo_string=utxo_string)['signed_psbt'])
     bitcoind.generate_block(1, wait_for_mempool=1)
     sync_blockheight(bitcoind, [l1])
 
@@ -3912,12 +3914,14 @@ def test_peer_anchor_push(node_factory, bitcoind, executor, chainparams):
     fundsats = int(only_one(l2.rpc.listfunds()['outputs'])['amount_msat'].to_satoshi())
     OUTPUT_SAT = 10000
     NUM_OUTPUTS = 10
-    psbt = l2.rpc.fundpsbt("all", "1000perkw", 1000)['psbt']
+    result = l2.rpc.fundpsbt("all", "1000perkw", 1000)
+    psbt = result['psbt']
+    utxo_string = result['utxo_string']
     # Pay 5k sats in fees.
     psbt = l2.rpc.addpsbtoutput(fundsats - OUTPUT_SAT * NUM_OUTPUTS - 5000, psbt, destination=l3.rpc.newaddr()['bech32'])['psbt']
     for _ in range(NUM_OUTPUTS):
         psbt = l2.rpc.addpsbtoutput(OUTPUT_SAT, psbt)['psbt']
-    l2.rpc.sendpsbt(l2.rpc.signpsbt(psbt)['signed_psbt'])
+    l2.rpc.sendpsbt(l2.rpc.signpsbt(psbt, utxo_string=utxo_string)['signed_psbt'])
     bitcoind.generate_block(1, wait_for_mempool=1)
     sync_blockheight(bitcoind, [l2])
 

diff --git a/tests/test_opening.py b/tests/test_opening.py
@@ -386,7 +386,7 @@ def test_v2_rbf_single(node_factory, bitcoind, chainparams):
     assert int(info_2['next_feerate'][:-5]) == rate * 65 // 64
 
     # Sign our inputs, and continue
-    signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+    signed_psbt = l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
 
     # Fails because we didn't put enough feerate in.
     with pytest.raises(RpcError, match=r'insufficient fee'):
@@ -412,7 +412,7 @@ def test_v2_rbf_single(node_factory, bitcoind, chainparams):
                                    funding_feerate=next_rate)
     update = l1.rpc.openchannel_update(chan_id, bump['psbt'])
     assert update['commitments_secured']
-    signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+    signed_psbt = l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
     l1.rpc.openchannel_signed(chan_id, signed_psbt)
 
     bitcoind.generate_block(1)
@@ -505,7 +505,7 @@ def test_v2_rbf_abort_retry(node_factory, bitcoind, chainparams):
                                    funding_feerate=next_rate)
 
     update = l1.rpc.openchannel_update(chan_id, bump['psbt'])
-    signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+    signed_psbt = l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
     l1.rpc.openchannel_signed(chan_id, signed_psbt)
 
     bitcoind.generate_block(1)
@@ -638,7 +638,7 @@ def test_v2_rbf_liquidity_ad(node_factory, bitcoind, chainparams):
     assert update['commitments_secured']
 
     # Sign our inputs, and continue
-    signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+    signed_psbt = l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
     l1.rpc.openchannel_signed(chan_id, signed_psbt)
 
     # There's data in the datastore now (l2 only)
@@ -731,7 +731,7 @@ def test_v2_rbf_multi(node_factory, bitcoind, chainparams):
     assert update['commitments_secured']
 
     # Sign our inputs, and continue
-    signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+    signed_psbt = l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
     l1.rpc.openchannel_signed(chan_id, signed_psbt)
 
     # We 2x the feerate to beat the min-relay fee
@@ -754,7 +754,7 @@ def test_v2_rbf_multi(node_factory, bitcoind, chainparams):
     assert update['commitments_secured']
 
     # Sign our inputs, and continue
-    signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+    signed_psbt = l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
     l1.rpc.openchannel_signed(chan_id, signed_psbt)
 
     bitcoind.generate_block(1)
@@ -969,7 +969,7 @@ def test_rbf_reconnect_tx_construct(node_factory, bitcoind, chainparams):
     # We can call update again! It should short-circuit this time :)
     update = l1.rpc.openchannel_update(chan_id, bump['psbt'])
     assert update['commitments_secured']
-    signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+    signed_psbt = l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
     l1.rpc.openchannel_signed(chan_id, signed_psbt)
 
     l2.daemon.wait_for_log('Broadcasting funding tx')
@@ -1038,7 +1038,7 @@ def test_rbf_reconnect_tx_sigs(node_factory, bitcoind, chainparams):
     update = l1.rpc.openchannel_update(chan_id, bump['psbt'])
 
     # Sign our inputs, and continue
-    signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+    signed_psbt = l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
 
     # First time we error when we send our sigs
     with pytest.raises(RpcError):
@@ -1210,7 +1210,7 @@ def run_retry():
         update = l1.rpc.openchannel_update(chan_id, bump['psbt'])
         assert update['commitments_secured']
 
-        return l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+        return l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
 
     signed_psbt = run_retry()
     l1.rpc.openchannel_signed(chan_id, signed_psbt)
@@ -1295,7 +1295,7 @@ def run_retry():
         update = l1.rpc.openchannel_update(chan_id, bump['psbt'])
         assert update['commitments_secured']
 
-        return l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+        return l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
 
     signed_psbt = run_retry()
     l1.rpc.openchannel_signed(chan_id, signed_psbt)
@@ -1363,7 +1363,7 @@ def run_retry():
         update = l1.rpc.openchannel_update(chan_id, bump['psbt'])
         assert update['commitments_secured']
 
-        return l1.rpc.signpsbt(update['psbt'])['signed_psbt']
+        return l1.rpc.signpsbt(update['psbt'], unsafe_sign_all=True)['signed_psbt']
 
     # Make a second inflight
     signed_psbt = run_retry()

diff --git a/tests/test_splicing.py b/tests/test_splicing.py
@@ -23,7 +23,7 @@ def test_splice(node_factory, bitcoind):
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
-    result = l1.rpc.signpsbt(result['psbt'])
+    result = l1.rpc.signpsbt(result['psbt'], utxo_string=funds_result['utxo_string'])
     result = l1.rpc.splice_signed(chan_id, result['signed_psbt'])
 
     l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')
@@ -61,7 +61,7 @@ def test_splice_gossip(node_factory, bitcoind):
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
-    result = l1.rpc.signpsbt(result['psbt'])
+    result = l1.rpc.signpsbt(result['psbt'], utxo_string=funds_result['utxo_string'])
     result = l1.rpc.splice_signed(chan_id, result['signed_psbt'])
 
     wait_for(lambda: only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['state'] == 'CHANNELD_AWAITING_SPLICE')
@@ -119,7 +119,7 @@ def test_splice_listnodes(node_factory, bitcoind):
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
-    result = l1.rpc.signpsbt(result['psbt'])
+    result = l1.rpc.signpsbt(result['psbt'], utxo_string=funds_result['utxo_string'])
     result = l1.rpc.splice_signed(chan_id, result['signed_psbt'])
 
     l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')
@@ -208,7 +208,7 @@ def test_invalid_splice(node_factory, bitcoind):
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
-    result = l1.rpc.signpsbt(result['psbt'])
+    result = l1.rpc.signpsbt(result['psbt'], utxo_string=funds_result['utxo_string'])
     result = l1.rpc.splice_signed(chan_id, result['signed_psbt'])
 
     l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')
@@ -307,7 +307,7 @@ def test_splice_stuck_htlc(node_factory, bitcoind, executor):
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
-    result = l1.rpc.signpsbt(result['psbt'])
+    result = l1.rpc.signpsbt(result['psbt'], utxo_string=funds_result['utxo_string'])
     result = l1.rpc.splice_signed(chan_id, result['signed_psbt'])
 
     l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')