wallet: set nLockTime to the tip for withdrawal transactions

This sets the nLockTime to the tip (and accordingly each input's nSequence to 0xfffffffe) for withdrawal transactions. Even if the anti fee-sniping argument might not be valid until some time yet, this makes our regular wallet transactions far less distinguishable from bitcoind's ones since it now defaults to using native Segwit transactions (like us). Moreover other wallets are likely to implement this (if they haven't already). Changelog-Added: wallet: withdrawal transactions now sets nlocktime to the current tip.
richur · Feb 3, 2020 · 273029f · 273029f
1 parent e4c6fd8
commit 273029f
Show file tree

Hide file tree

Showing 16 changed files with 50 additions and 28 deletions.
diff --git a/bitcoin/tx.c b/bitcoin/tx.c
@@ -380,7 +380,8 @@ static void bitcoin_tx_destroy(struct bitcoin_tx *tx)
 
 struct bitcoin_tx *bitcoin_tx(const tal_t *ctx,
 			      const struct chainparams *chainparams,
-			      varint_t input_count, varint_t output_count)
+			      varint_t input_count, varint_t output_count,
+			      u32 nlocktime)
 {
 	struct bitcoin_tx *tx = tal(ctx, struct bitcoin_tx);
 	assert(chainparams);
@@ -396,7 +397,7 @@ struct bitcoin_tx *bitcoin_tx(const tal_t *ctx,
 	tal_add_destructor(tx, bitcoin_tx_destroy);
 
 	tx->input_amounts = tal_arrz(tx, struct amount_sat*, input_count);
-	tx->wtx->locktime = 0;
+	tx->wtx->locktime = nlocktime;
 	tx->wtx->version = 2;
 	tx->chainparams = chainparams;
 	return tx;

diff --git a/bitcoin/tx.h b/bitcoin/tx.h
@@ -57,7 +57,8 @@ size_t bitcoin_tx_weight(const struct bitcoin_tx *tx);
  * zeroed with inputs' sequence_number set to FFFFFFFF) */
 struct bitcoin_tx *bitcoin_tx(const tal_t *ctx,
 			      const struct chainparams *chainparams,
-			      varint_t input_count, varint_t output_count);
+			      varint_t input_count, varint_t output_count,
+			      u32 nlocktime);
 
 /* This takes a raw bitcoin tx in hex. */
 struct bitcoin_tx *bitcoin_tx_from_hex(const tal_t *ctx, const char *hex,

diff --git a/channeld/commit_tx.c b/channeld/commit_tx.c
@@ -146,7 +146,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx,
 #endif
 
 	/* Worst-case sizing: both to-local and to-remote outputs. */
-	tx = bitcoin_tx(ctx, chainparams, 1, untrimmed + 2);
+	tx = bitcoin_tx(ctx, chainparams, 1, untrimmed + 2, 0);
 
 	/* We keep track of which outputs have which HTLCs */
 	*htlcmap = tal_arr(tx, const struct htlc *, tx->wtx->outputs_allocation_len);

diff --git a/common/close_tx.c b/common/close_tx.c
@@ -34,7 +34,7 @@ struct bitcoin_tx *create_close_tx(const tal_t *ctx,
 	 * * txin count: 1
 	 */
 	/* Now create close tx: one input, two outputs. */
-	tx = bitcoin_tx(ctx, chainparams, 1, 2);
+	tx = bitcoin_tx(ctx, chainparams, 1, 2, 0);
 
 	/* Our input spends the anchor tx output. */
 	bitcoin_tx_add_input(tx, anchor_txid, anchor_index,

diff --git a/common/funding_tx.c b/common/funding_tx.c
@@ -26,7 +26,8 @@ struct bitcoin_tx *funding_tx(const tal_t *ctx,
 	struct bitcoin_tx *tx;
 	bool has_change = !amount_sat_eq(change, AMOUNT_SAT(0));
 
-	tx = tx_spending_utxos(ctx, chainparams, utxomap, bip32_base, has_change, 1);
+	tx = tx_spending_utxos(ctx, chainparams, utxomap, bip32_base,
+			       has_change, 1, 0, BITCOIN_TX_DEFAULT_SEQUENCE);
 
 
 	wscript = bitcoin_redeem_2of2(tx, local_fundingkey, remote_fundingkey);

diff --git a/common/htlc_tx.c b/common/htlc_tx.c
@@ -15,7 +15,10 @@ static struct bitcoin_tx *htlc_tx(const tal_t *ctx,
 				  struct amount_sat htlc_fee,
 				  u32 locktime)
 {
-	struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, 1, 1);
+	/* BOLT #3:
+	 * * locktime: `0` for HTLC-success, `cltv_expiry` for HTLC-timeout
+	 */
+	struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, 1, 1, locktime);
 	u8 *wscript;
 	struct amount_sat amount;
 
@@ -34,11 +37,6 @@ static struct bitcoin_tx *htlc_tx(const tal_t *ctx,
 	 */
 	assert(tx->wtx->version == 2);
 
-	/* BOLT #3:
-	 * * locktime: `0` for HTLC-success, `cltv_expiry` for HTLC-timeout
-	 */
-	tx->wtx->locktime = locktime;
-
 	/* BOLT #3:
 	 * * txin count: 1
 	 *    * `txin[0]` outpoint: `txid` of the commitment transaction and

diff --git a/common/initial_commit_tx.c b/common/initial_commit_tx.c
@@ -145,7 +145,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx,
 
 
 	/* Worst-case sizing: both to-local and to-remote outputs. */
-	tx = bitcoin_tx(ctx, chainparams, 1, untrimmed + 2);
+	tx = bitcoin_tx(ctx, chainparams, 1, untrimmed + 2, 0);
 
 	/* This could be done in a single loop, but we follow the BOLT
 	 * literally to make comments in test vectors clearer. */

diff --git a/common/utxo.c b/common/utxo.c
@@ -62,14 +62,17 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx,
 				     const struct utxo **utxos,
 				     const struct ext_key *bip32_base,
 				     bool add_change_output,
-				     size_t num_output)
+				     size_t num_output,
+				     u32 nlocktime,
+				     u32 nsequence)
 {
 	struct pubkey key;
 	u8 *script;
 
 	assert(num_output);
 	size_t outcount = add_change_output ? 1 + num_output : num_output;
-	struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, tal_count(utxos), outcount);
+	struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, tal_count(utxos),
+					   outcount, nlocktime);
 
 	for (size_t i = 0; i < tal_count(utxos); i++) {
 		if (utxos[i]->is_p2sh && bip32_base) {
@@ -80,8 +83,7 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx,
 		}
 
 		bitcoin_tx_add_input(tx, &utxos[i]->txid, utxos[i]->outnum,
-				     BITCOIN_TX_DEFAULT_SEQUENCE,
-		 		     utxos[i]->amount, script);
+				     nsequence, utxos[i]->amount, script);
 	}
 
 	return tx;

diff --git a/common/utxo.h b/common/utxo.h
@@ -52,6 +52,8 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx,
 				     const struct utxo **utxos,
 				     const struct ext_key *bip32_base,
 				     bool add_change_output,
-				     size_t num_output);
+				     size_t num_output,
+				     u32 nlocktime,
+				     u32 nsequence);
 
 #endif /* LIGHTNING_COMMON_UTXO_H */
diff --git a/common/withdraw_tx.c b/common/withdraw_tx.c
@@ -16,14 +16,15 @@ struct bitcoin_tx *withdraw_tx(const tal_t *ctx,
 			       const struct pubkey *changekey,
 			       struct amount_sat change,
 			       const struct ext_key *bip32_base,
-			       int *change_outnum)
+			       int *change_outnum, u32 nlocktime)
 {
 	struct bitcoin_tx *tx;
 	int output_count;
 
 	tx = tx_spending_utxos(ctx, chainparams, utxos, bip32_base,
 			       !amount_sat_eq(change, AMOUNT_SAT(0)),
-			       tal_count(outputs));
+			       tal_count(outputs), nlocktime,
+			       BITCOIN_TX_DEFAULT_SEQUENCE - 1);
 
 	output_count = bitcoin_tx_add_multi_outputs(tx, outputs);
 	assert(output_count == tal_count(outputs));

diff --git a/common/withdraw_tx.h b/common/withdraw_tx.h
@@ -25,6 +25,7 @@ struct utxo;
  * @change: (in) amount to send as change.
  * @bip32_base: (in) bip32 base for key derivation, or NULL.
  * @change_outnum: (out) set to output index of change output or -1 if none, unless 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,
@@ -33,6 +34,6 @@ struct bitcoin_tx *withdraw_tx(const tal_t *ctx,
 			       const struct pubkey *changekey,
 			       struct amount_sat change,
 			       const struct ext_key *bip32_base,
-			       int *change_outnum);
+			       int *change_outnum, u32 nlocktime);
 
 #endif /* LIGHTNING_COMMON_WITHDRAW_TX_H */
diff --git a/devtools/mkclose.c b/devtools/mkclose.c
@@ -128,7 +128,7 @@ int main(int argc, char *argv[])
 	    || !pubkey_from_privkey(&funding_privkey[REMOTE], &funding_pubkey[REMOTE]))
 		errx(1, "Bad deriving funding pubkeys");
 
-	tx = bitcoin_tx(NULL, chainparams, 1, 2);
+	tx = bitcoin_tx(NULL, chainparams, 1, 2, 0);
 
 	/* Our input spends the anchor tx output. */
 	bitcoin_tx_add_input(tx, &funding_txid, funding_outnum,

diff --git a/hsmd/hsm_wire.csv b/hsmd/hsm_wire.csv
@@ -74,6 +74,7 @@ msgdata,hsm_sign_withdrawal,num_outputs,u16,
 msgdata,hsm_sign_withdrawal,outputs,bitcoin_tx_output,num_outputs
 msgdata,hsm_sign_withdrawal,num_inputs,u16,
 msgdata,hsm_sign_withdrawal,inputs,utxo,num_inputs
+msgdata,hsm_sign_withdrawal,nlocktime,u32,
 
 msgtype,hsm_sign_withdrawal_reply,107
 msgdata,hsm_sign_withdrawal_reply,tx,bitcoin_tx,

diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c
@@ -1643,10 +1643,11 @@ static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn,
 	struct bitcoin_tx *tx;
 	struct pubkey changekey;
 	struct bitcoin_tx_output **outputs;
+	u32 nlocktime;
 
 	if (!fromwire_hsm_sign_withdrawal(tmpctx, msg_in, &satoshi_out,
 					  &change_out, &change_keyindex,
-					  &outputs, &utxos))
+					  &outputs, &utxos, &nlocktime))
 		return bad_req(conn, c, msg_in);
 
 	if (!bip32_pubkey(&secretstuff.bip32, &changekey, change_keyindex))
@@ -1655,7 +1656,7 @@ static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn,
 
 	tx = withdraw_tx(tmpctx, c->chainparams,
 			 cast_const2(const struct utxo **, utxos), outputs,
-			 &changekey, change_out, NULL, NULL);
+			 &changekey, change_out, NULL, NULL, nlocktime);
 
 	sign_all_inputs(tx, utxos);
 

diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c
@@ -330,8 +330,7 @@ static struct bitcoin_tx *tx_to_us(const tal_t *ctx,
 	u8 *msg;
 	u8 **witness;
 
-	tx = bitcoin_tx(ctx, out->chainparams, 1, 1);
-	tx->wtx->locktime = locktime;
+	tx = bitcoin_tx(ctx, out->chainparams, 1, 1, locktime);
 	bitcoin_tx_add_input(tx, &out->txid, out->outnum, to_self_delay,
 			     out->sat, NULL);
 

diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c
@@ -89,7 +89,8 @@ static struct command_result *broadcast_and_wait(struct command *cmd,
 					     utx->wtx->change_key_index,
 					     cast_const2(const struct bitcoin_tx_output **,
 							 utx->outputs),
-					     utx->wtx->utxos);
+					     utx->wtx->utxos,
+					     utx->tx->wtx->locktime);
 
 	if (!wire_sync_write(cmd->ld->hsm_fd, take(msg)))
 		fatal("Could not write sign_withdrawal to HSM: %s",
@@ -141,6 +142,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;
 
 	*utx = tal(cmd, struct unreleased_tx);
 	(*utx)->wtx = tal(*utx, struct wallet_tx);
@@ -278,6 +280,17 @@ static struct command_result *json_prepare_tx(struct command *cmd,
 			   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).
+		 * FIXME: Do we want to also fuzz this like bitcoind does ?
+		 */
+		locktime = cmd->ld->topology->tip->height;
 	}
 
 	if (!feerate_per_kw) {
@@ -397,7 +410,8 @@ static struct command_result *json_prepare_tx(struct command *cmd,
 				 (*utx)->wtx->utxos, (*utx)->outputs,
 				 changekey, (*utx)->wtx->change,
 				 cmd->ld->wallet->bip32_base,
-				 &(*utx)->change_outnum);
+				 &(*utx)->change_outnum,
+				 locktime);
 	bitcoin_txid((*utx)->tx, &(*utx)->txid);
 
 	return NULL;