Add a 'reservation expiration' mechanism

common/utxo.h

@@ -39,6 +39,9 @@ struct utxo {
 	/* NULL if not spent yet, otherwise, the block the spending transaction is in */
 	const u32 *spendheight;
 
+	/* Block this utxo becomes unreserved, if applicable */
+	const u32 *reserved_til;
+
 	/* The scriptPubkey if it is known */
 	u8 *scriptPubkey;
 

common/wallet_tx.c

@@ -91,11 +91,6 @@ struct command_result *param_utxos(struct command *cmd,
 	tal_free(txids);
 	tal_free(outnums);
 
-	if (!*utxos)
-		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
-							"Could not decode all of the outpoints. The utxos"
-							" should be specified as an array of "
-							" 'txid:output_index'.");
 	if (tal_count(*utxos) == 0)
 		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 							"No matching utxo was found from the wallet. "

common/withdraw_tx.c

@@ -12,17 +12,29 @@
 
 struct bitcoin_tx *withdraw_tx(const tal_t *ctx,
 			       const struct chainparams *chainparams,
+			       bool allow_rbf,
 			       const struct utxo **utxos,
 			       struct bitcoin_tx_output **outputs,
 			       const struct ext_key *bip32_base,
 			       u32 nlocktime)
 {
 	struct bitcoin_tx *tx;
 	int output_count;
+	/*
+	 * BIP-125: Opt-in Full Replace-by-Fee Signaling
+	 *     https://github.com/bitcoin/bips/blob/master/bip-0125.mediawiki
+	 * A transaction is considered to have opted in to
+	 * allowing replacement of itself if any of its
+	 * inputs have an nSequence number less than (0xffffffff - 1).
+	 */
+	/* A sequence of (0xffffffff - 1) signals to use locktime */
+	u32 sequence = BITCOIN_TX_DEFAULT_SEQUENCE - 1;
+	if (allow_rbf)
+		sequence--;
 
 	tx = tx_spending_utxos(ctx, chainparams, utxos, bip32_base,
 			       false, tal_count(outputs), nlocktime,
-			       BITCOIN_TX_DEFAULT_SEQUENCE - 1);
+			       sequence);
 
 	output_count = bitcoin_tx_add_multi_outputs(tx, outputs);
 	assert(output_count == tal_count(outputs));

common/withdraw_tx.h

@@ -19,13 +19,15 @@ struct utxo;
  *
  * @ctx: context to tal from.
  * @chainparams: (in) the params for the created transaction.
+ * @allow_rbf: (in) bool to signal whether to flag the sequence as RBF'able
  * @utxos: (in/out) tal_arr of UTXO pointers to spend (permuted to match)
  * @outputs: (in) tal_arr of bitcoin_tx_output, scriptPubKeys with amount to send to.
  * @bip32_base: (in) bip32 base for key derivation, or NULL.
  * @nlocktime: (in) the value to set as the transaction's nLockTime.
  */
 struct bitcoin_tx *withdraw_tx(const tal_t *ctx,
 			       const struct chainparams *chainparams,
+			       bool allow_rbf,
 			       const struct utxo **utxos,
 			       struct bitcoin_tx_output **outputs,
 			       const struct ext_key *bip32_base,

contrib/pyln-client/pyln/client/lightning.py

@@ -1107,7 +1107,7 @@ def txsend(self, txid):
         }
         return self.call("txsend", payload)
 
-    def reserveinputs(self, outputs, feerate=None, minconf=None, utxos=None):
+    def reserveinputs(self, outputs, feerate=None, minconf=None, utxos=None, expire_after=None, allow_rbf=None):
         """
         Reserve UTXOs and return a psbt for a 'stub' transaction that
         spends the reserved UTXOs.
@@ -1117,6 +1117,8 @@ def reserveinputs(self, outputs, feerate=None, minconf=None, utxos=None):
             "feerate": feerate,
             "minconf": minconf,
             "utxos": utxos,
+            "expire_after": expire_after,
+            "allow_rbf": allow_rbf,
         }
         return self.call("reserveinputs", payload)
 

doc/lightning-reserveinputs.7.md

@@ -4,7 +4,7 @@ lightning-reserveinputs -- Construct a transaction and reserve the UTXOs it spen
 SYNOPSIS
 --------
 
-**reserveinputs** *outputs* \[*feerate*\] \[*minconf*\] \[*utxos*\]
+**reserveinputs** *outputs* \[*feerate*\] \[*minconf*\] \[*utxos*\] \[*expire_after*\] \[*allow_rbf*\]
 
 DESCRIPTION
 -----------
@@ -45,6 +45,13 @@ should have. Default is 1.
 *utxos* specifies the utxos to be used to fund the transaction, as an array
 of "txid:vout". These must be drawn from the node's available UTXO set.
 
+*expire_after* specifies the number of blocks after which the UTXOs reserved
+by this command will be eligible for re-use. Defaults to 144 blocks.
+Can be disabled by passing in 0.
+
+*allow_rbf*, if flagged on, will set the sequence for each input to permit
+it to be RBF'd. Defaults to true.
+
 
 RETURN VALUE
 ------------

hsmd/hsmd.c

@@ -37,7 +37,6 @@
 #include <common/type_to_string.h>
 #include <common/utils.h>
 #include <common/version.h>
-#include <common/withdraw_tx.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <hsmd/capabilities.h>

tests/test_wallet.py

@@ -557,9 +557,86 @@ def test_reserveinputs(node_factory, bitcoind, chainparams):
     assert len([x for x in outputs if not x['reserved']]) == 0
     assert len([x for x in outputs if x['reserved']]) == 12
 
-    # FIXME: restart the node, nothing will remain reserved
+    # restart the node, things remain reserved
     l1.restart()
-    assert len(l1.rpc.listfunds()['outputs']) == 12
+    outputs = l1.rpc.listfunds()['outputs']
+    assert len([x for x in outputs if x['reserved']]) == 12
+
+    # move the blockchain forward 144 blocks (default reserved value)
+    l1.stop()
+    bitcoind.generate_block(144)
+    l1.start()
+    sync_blockheight(bitcoind, [l1])
+    # everything should be back to unreserved now
+    outputs = l1.rpc.listfunds()['outputs']
+    assert len([x for x in outputs if not x['reserved']]) == 12
+
+    # try setting the default reserved to 5 blocks
+    reserved = l1.rpc.reserveinputs([{addr: 'all'}], expire_after=5)
+    bitcoind.generate_block(5)
+    sync_blockheight(bitcoind, [l1])
+    outputs = l1.rpc.listfunds()['outputs']
+    assert len([x for x in outputs if not x['reserved']]) == 12
+
+    # try turning off the default reservation
+    reserved = l1.rpc.reserveinputs([{addr: 'all'}], expire_after=0)
+    bitcoind.generate_block(144)
+    sync_blockheight(bitcoind, [l1])
+    outputs = l1.rpc.listfunds()['outputs']
+    assert len([x for x in outputs if x['reserved']]) == 12
+
+
+def test_reserve_rbf(node_factory, bitcoind, chainparams):
+    """
+    We now default everything to RBF. I'm not really sure how to test this
+    """
+    amount = 1000000
+    total_outs = 12
+    coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py')
+    l1 = node_factory.get_node(options={'plugin': coin_mvt_plugin},
+                               feerates=(7500, 7500, 7500, 7500))
+    l2 = node_factory.get_node()
+    addr = chainparams['example_addr']
+
+    # Add a medley of funds to withdraw later, bech32 + p2sh-p2wpkh
+    for i in range(total_outs // 2):
+        bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['bech32'],
+                                   amount / 10**8)
+        bitcoind.rpc.sendtoaddress(l1.rpc.newaddr('p2sh-segwit')['p2sh-segwit'],
+                                   amount / 10**8)
+    bitcoind.generate_block(1)
+    wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == total_outs)
+
+    # Make a PSBT out of our inputs
+    reserved = l1.rpc.reserveinputs(outputs=[{addr: Millisatoshi(3 * amount * 1000)}])
+    assert len([x for x in l1.rpc.listfunds()['outputs'] if x['reserved']]) == 4
+
+    signed_psbt = l1.rpc.signpsbt(reserved['psbt'])['signed_psbt']
+    broadcast_tx = l1.rpc.sendpsbt(signed_psbt)
+    assert bitcoind.rpc.getmempoolinfo()['size'] == 1
+
+    # Try to do it again?
+    psbt = bitcoind.rpc.decodepsbt(reserved['psbt'])
+    utxos = []
+    unreserve_utxos = []
+    for vin in psbt['tx']['vin']:
+        utxos.append(vin['txid'] + ":" + str(vin['vout']))
+        unreserve_utxos.append({'txid': vin['txid'], 'vout': vin['vout'], 'sequence': vin['sequence']})
+    unreserve_psbt = bitcoind.rpc.createpsbt(unreserve_utxos, [])
+    unres = l1.rpc.unreserveinputs(unreserve_psbt)
+
+    # let's not set the fee high enough, should explode
+    reserved = l1.rpc.reserveinputs([{addr: Millisatoshi(amount * 0.5 * 1000)}], feerate='7000perkw', utxos=utxos)
+    signed_psbt = l1.rpc.signpsbt(reserved['psbt'])['signed_psbt']
+    with pytest.raises(RpcError, match=r'insufficient fee, rejecting replacement'):
+        l1.rpc.sendpsbt(signed_psbt)
+
+    # now let's do it for reals
+    unres = l1.rpc.unreserveinputs(unreserve_psbt)
+    reserved = l1.rpc.reserveinputs([{addr: Millisatoshi(amount * 0.5 * 1000)}], feerate='10000perkw', utxos=utxos)
+    signed_psbt = l1.rpc.signpsbt(reserved['psbt'])['signed_psbt']
+    l1.rpc.sendpsbt(signed_psbt)
+    assert bitcoind.rpc.getmempoolinfo()['size'] == 1
 
 
 def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):

wallet/db.c

@@ -617,6 +617,7 @@ static struct migration dbmigrations[] = {
     /* We track the counter for coin_moves, as a convenience for notification consumers */
     {SQL("INSERT INTO vars (name, intval) VALUES ('coin_moves_count', 0);"), NULL},
     {NULL, migrate_last_tx_to_psbt},
+    {SQL("ALTER TABLE outputs ADD reserved_til INTEGER DEFAULT NULL;"), NULL},
 };
 
 /* Leak tracking. */

wallet/test/run-wallet.c

@@ -902,6 +902,10 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx)
 	struct node_id id;
 	struct amount_sat fee_estimate, change_satoshis;
 	const struct utxo **utxos;
+	u32 expires_at = 30;
+
+	/* Set the chaintip height to 15 */
+	ld->topology->tip->height = 15;
 	CHECK(w);
 
 	memset(&u, 0, sizeof(u));
@@ -966,6 +970,61 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx)
 					      output_state_spent),
 		  "could not change output state ignoring oldstate");
 
+	/* Check for 'reserved' state changes */
+	utxos = wallet_select_coins(w, w, true, AMOUNT_SAT(1), 0, 21,
+				    0 /* no confirmations required */,
+				    &fee_estimate, &change_satoshis);
+	CHECK(utxos && tal_count(utxos) == 1);
+
+	u = *utxos[0];
+
+	/* Add expiration to reservation */
+	CHECK_MSG(wallet_output_reservation_update(w, &u, expires_at),
+		  "could not add high watermark for reserved output");
+
+	/* Usually we'd remove the reservation on free, but
+	 * we want to do some things with it, so check that
+	 * we can short-circuit this by calling persist */
+	wallet_persist_utxo_reservation(w, utxos);
+	tal_free(utxos);
+
+	/* Now should be unable to select 1 sat amount */
+	CHECK_MSG(!wallet_select_coins(w, w, true, AMOUNT_SAT(1), 0, 21,
+				       0 /* no confirmations required */,
+				       &fee_estimate, &change_satoshis),
+	          "too many utxos available");
+
+	/* Get reserved utxos */
+	utxos = (const struct utxo **)wallet_get_utxos(w, w, output_state_reserved);
+	CHECK(utxos && tal_count(utxos) == 1);
+
+	/* Check that reserved_til is set */
+	CHECK(*utxos[0]->reserved_til == expires_at);
+
+	/* Expire the lease! */
+	ld->topology->tip->height = expires_at;
+
+	utxos = wallet_select_coins(w, w, true, AMOUNT_SAT(1), 0, 21,
+				    0 /* no confirmations required */,
+				    &fee_estimate, &change_satoshis);
+	CHECK_MSG(utxos && tal_count(utxos) == 1, "utxo count is incorrect");
+
+	/* Check freeing them resets them to available */
+	tal_free(utxos);
+
+	/* There should be no reserved utxos */
+	utxos = (const struct utxo **)wallet_get_utxos(w, w, output_state_reserved);
+	CHECK(tal_count(utxos) == 0);
+
+	/* check that we can mark as spent */
+	utxos = wallet_select_coins(w, w, true, AMOUNT_SAT(1), 0, 21,
+				    0 /* no confirmations required */,
+				    &fee_estimate, &change_satoshis);
+	CHECK_MSG(utxos && tal_count(utxos) == 1, "utxo count is incorrect");
+	wallet_confirm_utxos(w, utxos);
+
+	tal_free(utxos);
+
 	/* Attempt to save an UTXO with close_info set, no commitment_point */
 	memset(&u.txid, 2, sizeof(u.txid));
 	u.amount = AMOUNT_SAT(5);
@@ -980,9 +1039,9 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx)
 	utxos = wallet_select_coins(w, w, true, AMOUNT_SAT(5), 0, 21,
 				    0 /* no confirmations required */,
 				    &fee_estimate, &change_satoshis);
-	CHECK(utxos && tal_count(utxos) == 2);
+	CHECK(utxos && tal_count(utxos) == 1);
 
-	u = *utxos[1];
+	u = *utxos[0];
 	CHECK(u.close_info->channel_id == 42 &&
 	      u.close_info->commitment_point == NULL &&
 	      node_id_eq(&u.close_info->peer_id, &id));
@@ -1481,6 +1540,9 @@ int main(int argc, const char *argv[])
 
 	/* Only elements in ld we should access */
 	list_head_init(&ld->peers);
+	ld->topology = tal(ld, struct chain_topology);
+	ld->topology->tip = tal(ld, struct block);
+
 	node_id_from_hexstr("02a1633cafcc01ebfb6d78e39f687a1f0995c62fc95f51ead10a02ee0be551b5dc", 66, &ld->id);
 	/* Accessed in peer destructor sanity check */
 	htlc_in_map_init(&ld->htlcs_in);