wallet/walletrpc.c: `txprepare`d transactions now use current tip blockheight by default.

Changelog-Changed: `txprepare` now prepares transactions whose `nLockTime` is set to the tip blockheight, instead of using 0. `fundchannel` will use `nLockTime` set to the tip blockheight as well.

tests/test_wallet.py

@@ -1001,11 +1001,14 @@ def test_fundchannel_listtransaction(node_factory, bitcoind):
 
 def test_withdraw_nlocktime(node_factory):
     """
-    Test that we don't set the nLockTime to 0 for withdrawal transactions.
+    Test that we don't set the nLockTime to 0 for withdrawal and
+    txprepare transactions.
     """
     l1 = node_factory.get_node(1)
     l1.fundwallet(10**4)
+    l1.fundwallet(10**4)
 
+    # withdraw
     addr = l1.rpc.newaddr()["bech32"]
     tx = l1.rpc.withdraw(addr, 10**3)["tx"]
     nlocktime = node_factory.bitcoind.rpc.decoderawtransaction(tx)["locktime"]
@@ -1013,6 +1016,14 @@ def test_withdraw_nlocktime(node_factory):
 
     assert nlocktime > 0 and nlocktime <= tip
 
+    # txprepare
+    txid = l1.rpc.txprepare([{addr: 10**3}])["txid"]
+    tx = l1.rpc.txsend(txid)["tx"]
+    nlocktime = node_factory.bitcoind.rpc.decoderawtransaction(tx)["locktime"]
+    tip = node_factory.bitcoind.rpc.getblockcount()
+
+    assert nlocktime > 0 and nlocktime <= tip
+
 
 @flaky
 @unittest.skipIf(VALGRIND, "A big loop is used to check fuzz.")

wallet/walletrpc.c

@@ -181,7 +181,7 @@ static struct command_result *json_prepare_tx(struct command *cmd,
 	const u8 *destination = NULL;
 	size_t out_len, i;
 	const struct utxo **chosen_utxos = NULL;
-	u32 locktime = 0;
+	u32 locktime;
 
 	*utx = tal(cmd, struct unreleased_tx);
 	(*utx)->wtx = tal(*utx, struct wallet_tx);
@@ -318,22 +318,23 @@ static struct command_result *json_prepare_tx(struct command *cmd,
 			   p_opt_def("minconf", param_number, &minconf, 1),
 			   p_opt("utxos", param_utxos, &chosen_utxos),
 			   NULL))
-		return command_param_failed();
-		/* Setting the locktime to the next block to be mined has multiple
-		 * benefits:
-		 * - anti fee-snipping (even if not yet likely)
-		 * - less distinguishable transactions (with this we create
-		 *   general-purpose transactions which looks like bitcoind:
-		 *   native segwit, nlocktime set to tip, and sequence set to
-		 *   0xFFFFFFFE by default. Other wallets are likely to implement
-		 *   this too).
-		 */
-		locktime = cmd->ld->topology->tip->height;
-		/* Eventually fuzz it too. */
-		if (pseudorand(10) == 0)
-			locktime -= (u32)pseudorand(100);
+			return command_param_failed();
 	}
 
+	/* Setting the locktime to the next block to be mined has multiple
+	 * benefits:
+	 * - anti fee-snipping (even if not yet likely)
+	 * - less distinguishable transactions (with this we create
+	 *   general-purpose transactions which looks like bitcoind:
+	 *   native segwit, nlocktime set to tip, and sequence set to
+	 *   0xFFFFFFFE by default. Other wallets are likely to implement
+	 *   this too).
+	 */
+	locktime = cmd->ld->topology->tip->height;
+	/* Eventually fuzz it too. */
+	if (pseudorand(10) == 0)
+		locktime -= (u32)pseudorand(100);
+
 	if (!feerate_per_kw) {
 		/* We mainly use `txprepare` for opening transactions, and FEERATE_OPENING
 		 * is kind of the new FEERATE_NORMAL so it fits well `withdraw` too. */
@@ -455,6 +456,9 @@ create_tx:
 				 (*utx)->wtx->utxos,
 				 (*utx)->outputs,
 				 cmd->ld->wallet->bip32_base,
+				 /* FIXME: Should probably be
+				  * struct abs_locktime.
+				  */
 				 locktime);
 
 	bitcoin_txid((*utx)->tx, &(*utx)->txid);