walletrpc: make it possible to actually RBF a transaction

Adds an RBF test plus associated utxo state tracking fixes needed to
make this possible.

Mostly, you *must* specify which inputs you're spending when you attempt
to RBF (makes sense) -- these inputs are allowed to be marked as 'spent'
(which means we've broadcast a tx spending them) but must not be
included in a block yet (which would be indicated by the spendheight)
being populated.

There's a few other places that 'spending' already spent tx's is
problematic, e.g. when we mark a utxo as spent we now allow it to be coming
from 'any' status -> spent.

We also allow psbtsign to pull up any 'already spent' utxos to be signed
again

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. "

tests/test_wallet.py

@@ -586,6 +586,59 @@ def test_reserveinputs(node_factory, bitcoind, chainparams):
     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 explodes
+    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):
     """
     Tests for the sign + send psbt RPCs

wallet/wallet.c

@@ -566,7 +566,7 @@ void wallet_confirm_utxos(struct wallet *w, const struct utxo **utxos)
 	for (size_t i = 0; i < tal_count(utxos); i++) {
 		if (!wallet_update_output_status(
 			w, &utxos[i]->txid, utxos[i]->outnum,
-			output_state_reserved, output_state_spent)) {
+			output_state_any, output_state_spent)) {
 			fatal("Unable to mark output as spent");
 		}
 	}
@@ -719,25 +719,52 @@ const struct utxo **wallet_select_specific(const tal_t *ctx, struct wallet *w,
 	const struct utxo **utxos = tal_arr(ctx, const struct utxo*, 0);
 	tal_add_destructor2(utxos, destroy_utxos, w);
 
-	available = wallet_get_utxos(ctx, w, output_state_available);
+	available = wallet_get_utxos(ctx, w, output_state_any);
 	for (i = 0; i < tal_count(txids); i++) {
 		for (j = 0; j < tal_count(available); j++) {
 
 			if (bitcoin_txid_eq(&available[j]->txid, txids[i])
 					&& available[j]->outnum == *outnums[i]) {
+				switch (available[j]->status) {
+				case output_state_available:
+					if (!wallet_update_output_status(
+						w, &available[j]->txid, available[j]->outnum,
+						output_state_available, output_state_reserved))
+						fatal("Unable to reserve output");
+					break;
+				case output_state_reserved:
+					if (!utxo_reservation_expired(w->ld, available[j]))
+						log_info(w->log, "Selecting reserved utxo %s:%u",
+							 type_to_string(tmpctx, struct bitcoin_txid, &available[j]->txid),
+							 available[j]->outnum);
+					break;
+				case output_state_spent:
+					/* We'll let you attempt to re-spend utxos that we've already
+					 * 'sent' now, since this is the only way to do it for RBF's*/
+					if (available[j]->spendheight) {
+						log_info(w->log, "Attempting to spend an already spent UTXO %s:%u"
+								 " (spent in block %u)",
+							 type_to_string(tmpctx, struct bitcoin_txid, &available[j]->txid),
+							 available[j]->outnum,
+							 *available[j]->spendheight);
+						goto fail;
+					}
+					break;
+				default:
+					fatal("Unable to reserve output");
+				}
+
 				struct utxo *u = tal_steal(utxos, available[j]);
 				tal_arr_expand(&utxos, u);
-
-				if (!wallet_update_output_status(
-					w, &available[j]->txid, available[j]->outnum,
-					output_state_available, output_state_reserved))
-					fatal("Unable to reserve output");
 			}
 		}
 	}
 	tal_free(available);
 
 	return utxos;
+fail:
+	tal_free(available);
+	return tal_free(utxos);
 }
 
 const struct utxo **wallet_select_all(const tal_t *ctx, struct wallet *w,

wallet/walletrpc.c

@@ -1384,24 +1384,25 @@ static struct command_result *match_psbt_inputs_to_utxos(struct command *cmd,
 
 		/* Oops we haven't reserved this utxo yet.
 		 * Let's just go ahead and reserve it now. */
-		if (utxo->status != output_state_reserved) {
-			if (utxo->status != output_state_available) {
-				return command_fail(cmd, LIGHTNINGD,
-						    "Aborting PSBT signing. UTXO %s:%u is already spent",
-						    type_to_string(tmpctx, struct bitcoin_txid,
-								   &utxo->txid),
-						    utxo->outnum);
-			}
-			/* Reserve this utxo before we spend it */
-			if (!wallet_update_output_status(cmd->ld->wallet,
-							 &txid, utxo->outnum,
-							 output_state_available,
-							 output_state_reserved))
-				return command_fail(cmd, LIGHTNINGD,
-						    "Aborting PSBT signing. Unable to reserve UTXO %s:%u",
-						    type_to_string(tmpctx, struct bitcoin_txid,
-								   &utxo->txid),
-						    utxo->outnum);
+		if (utxo->status == output_state_available &&
+			!wallet_update_output_status(cmd->ld->wallet,
+						     &txid, utxo->outnum,
+						     output_state_available,
+						     output_state_reserved)) {
+			return command_fail(cmd, LIGHTNINGD,
+					    "Aborting PSBT signing. Unable to reserve UTXO %s:%u",
+					    type_to_string(tmpctx, struct bitcoin_txid,
+							   &utxo->txid),
+					    utxo->outnum);
+		}
+		if (utxo->status == output_state_spent && utxo->spendheight) {
+			return command_fail(cmd, LIGHTNINGD,
+					    "Aborting PSBT signing. "
+					    "UTXO %s:%u is already spent",
+					    type_to_string(tmpctx,
+							   struct bitcoin_txid,
+							   &utxo->txid),
+					    utxo->outnum);
 		}
 		tal_arr_expand(utxos, utxo);
 	}