chaintopology: allow minblock for broadcast_tx.

Fun story.  We're changing onchaind to hand txs to us, and we will
construct them and do the broadcast for it.  lightningd tells onchaind
the witness it used (with flags to indicate which fields were
signatures so should be ignored) so onchaind can recognize the tx
when/if it is mined.

And when onchaind was waiting for a CLTV delay, it wouldn't tell
lightningd yet, but wait until the parent was sufficiently deep

But this caused bugs!

In particular, on replay, onchaind would see transactions which it
hasn't sent yet.  This was not a problem before, as onchaind had
created the tx, even if it hadn't told lightningd to broadcast it, so
recognized the variant when it came in.  When we're relying on
lightningd to tell us what the tx will look like, this doesn't work
any more.

The cause of this is that we fire off txowatches ("this output was
spent!") while we process blocks, and only fire off txwatches ("this
tx increased depth") once all the current blocks are processed.  Often
this didn't matter, since we replay messages to onchaind from the
database, *but* we trim the last few blocks on restart (or, if there's
a small reorg while we're stopped), and we can hit this misordering.

Changing our topology code to only ever process one block at a time
would be a solution, but slows down catchup (and tests, where we often
mine a run of blocks).

So, this seems like a premature optimization, but it's really
required!  And in future, lightningd can use this knowledge of pending
transactions to combine them in more clever ways.

Note that if a tx is valid at block N, we broadcast it once we see
block N-1, to get it in the mempool for block N.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/chaintopology.c

@@ -167,6 +167,11 @@ static void rebroadcast_txs(struct chain_topology *topo)
 		if (wallet_transaction_height(topo->ld->wallet, &otx->txid))
 			continue;
 
+		/* Don't send ones which aren't ready yet.  Note that if the
+		 * minimum block is N, we broadcast it when we have block N-1! */
+		if (get_block_height(topo) + 1 < otx->minblock)
+			continue;
+
 		/* Don't free from txmap inside loop! */
 		if (otx->refresh
 		    && !otx->refresh(otx->channel, &otx->tx, otx->refresh_arg)) {
@@ -230,7 +235,7 @@ static void broadcast_done(struct bitcoind *bitcoind,
 
 void broadcast_tx_(struct chain_topology *topo,
 		   struct channel *channel, const struct bitcoin_tx *tx,
-		   const char *cmd_id, bool allowhighfees,
+		   const char *cmd_id, bool allowhighfees, u32 minblock,
 		   void (*finished)(struct channel *channel,
 				    bool success,
 				    const char *err),
@@ -245,6 +250,7 @@ void broadcast_tx_(struct chain_topology *topo,
 	otx->channel = channel;
 	bitcoin_txid(tx, &otx->txid);
 	otx->tx = clone_bitcoin_tx(otx, tx);
+	otx->minblock = minblock;
 	otx->finished = finished;
 	otx->refresh = refresh;
 	otx->refresh_arg = refresh_arg;
@@ -254,8 +260,22 @@ void broadcast_tx_(struct chain_topology *topo,
 		otx->cmd_id = tal_strdup(otx, cmd_id);
 	else
 		otx->cmd_id = NULL;
-	tal_add_destructor2(channel, clear_otx_channel, otx);
 
+	/* Note that if the minimum block is N, we broadcast it when
+	 * we have block N-1! */
+	if (get_block_height(topo) + 1 < otx->minblock) {
+		log_debug(topo->log, "Deferring broadcast of txid %s until block %u",
+			  type_to_string(tmpctx, struct bitcoin_txid, &otx->txid),
+			  otx->minblock - 1);
+
+		/* For continual rebroadcasting, until channel freed. */
+		tal_steal(otx->channel, otx);
+		outgoing_tx_map_add(topo->outgoing_txs, otx);
+		tal_add_destructor2(otx, destroy_outgoing_tx, topo);
+		return;
+	}
+
+	tal_add_destructor2(channel, clear_otx_channel, otx);
 	log_debug(topo->log, "Broadcasting txid %s%s%s",
 		  type_to_string(tmpctx, struct bitcoin_txid, &otx->txid),
 		  cmd_id ? " for " : "", cmd_id ? cmd_id : "");
@@ -372,7 +392,7 @@ static void update_feerates(struct bitcoind *bitcoind,
 
 		/* Initial smoothed feerate is the polled feerate */
 		if (!old_feerates[i]) {
-            notify_feerate_changed = true;
+			notify_feerate_changed = true;
 			old_feerates[i] = feerate;
 			init_feerate_history(topo, i, feerate);
 

lightningd/chaintopology.h

@@ -21,6 +21,7 @@ struct outgoing_tx {
 	struct channel *channel;
 	const struct bitcoin_tx *tx;
 	struct bitcoin_txid txid;
+	u32 minblock;
 	const char *cmd_id;
 	void (*finished)(struct channel *channel, bool success, const char *err);
 	bool (*refresh)(struct channel *, const struct bitcoin_tx **, void *arg);
@@ -180,15 +181,16 @@ u32 penalty_feerate(struct chain_topology *topo);
  * @tx: the transaction
  * @cmd_id: the JSON command id which triggered this (or NULL).
  * @allowhighfees: set to true to override the high-fee checks in the backend.
+ * @minblock: minimum block we can send it at (or 0).
  * @finished: if non-NULL, call that and don't rebroadcast.
  * @refresh: if non-NULL, callback before re-broadcasting (can replace tx):
  *           if returns false, delete.
  * @refresh_arg: argument for @refresh
  */
 #define broadcast_tx(topo, channel, tx, cmd_id, allowhighfees,		\
-		     finished, refresh, refresh_arg)			\
+		     minblock, finished, refresh, refresh_arg)		\
 	broadcast_tx_((topo), (channel), (tx), (cmd_id), (allowhighfees), \
-		      (finished),					\
+		      (minblock), (finished),				\
 		      typesafe_cb_preargs(bool, void *,			\
 					  (refresh), (refresh_arg),	\
 					  struct channel *,		\
@@ -198,7 +200,7 @@ u32 penalty_feerate(struct chain_topology *topo);
 void broadcast_tx_(struct chain_topology *topo,
 		   struct channel *channel,
 		   const struct bitcoin_tx *tx TAKES,
-		   const char *cmd_id, bool allowhighfees,
+		   const char *cmd_id, bool allowhighfees, u32 minblock,
 		   void (*finished)(struct channel *,
 				    bool success,
 				    const char *err),

lightningd/onchain_control.c

@@ -345,7 +345,7 @@ static void handle_onchain_broadcast_tx(struct channel *channel,
 	 * set allowhighfees, as the transaction may be RBFed into
 	 * high feerates as protection against the MAD-HTLC attack.  */
 	broadcast_tx(channel->peer->ld->topology, channel,
-		     tx, NULL, is_rbf,
+		     tx, NULL, is_rbf, 0,
 		     is_rbf ? &handle_onchain_broadcast_rbf_tx_cb : NULL,
 		     NULL, NULL);
 }

lightningd/peer_control.c

@@ -284,7 +284,7 @@ static void sign_and_send_last(struct lightningd *ld,
 
 	/* Keep broadcasting until we say stop (can fail due to dup,
 	 * if they beat us to the broadcast). */
-	broadcast_tx(ld->topology, channel, last_tx, cmd_id, false, NULL,
+	broadcast_tx(ld->topology, channel, last_tx, cmd_id, false, 0, NULL,
 		     NULL, NULL);
 
 	remove_sig(last_tx);

lightningd/test/run-invoice-select-inchan.c

@@ -54,7 +54,7 @@ char *bolt11_encode_(const tal_t *ctx UNNEEDED,
 void broadcast_tx_(struct chain_topology *topo UNNEEDED,
 		   struct channel *channel UNNEEDED,
 		   const struct bitcoin_tx *tx TAKES UNNEEDED,
-		   const char *cmd_id UNNEEDED, bool allowhighfees UNNEEDED,
+		   const char *cmd_id UNNEEDED, bool allowhighfees UNNEEDED, u32 minblock UNNEEDED,
 		   void (*finished)(struct channel * UNNEEDED,
 				    bool success UNNEEDED,
 				    const char *err) UNNEEDED,

wallet/test/run-wallet.c

@@ -72,7 +72,7 @@ bool blinding_next_pubkey(const struct pubkey *pk UNNEEDED,
 void broadcast_tx_(struct chain_topology *topo UNNEEDED,
 		   struct channel *channel UNNEEDED,
 		   const struct bitcoin_tx *tx TAKES UNNEEDED,
-		   const char *cmd_id UNNEEDED, bool allowhighfees UNNEEDED,
+		   const char *cmd_id UNNEEDED, bool allowhighfees UNNEEDED, u32 minblock UNNEEDED,
 		   void (*finished)(struct channel * UNNEEDED,
 				    bool success UNNEEDED,
 				    const char *err) UNNEEDED,