coin moves: actually record the blockheight for all chain moves

Previously we were annotating every movement with the blockheight of
lightningd at notification time. Which is lossy in terms of info, and
won't be helpful for reorg reconciliation. Here we switch over to
logging chain moves iff they've been confirmed.

Next PR will fix this up for withdrawals, which are currently tagged
with a blockheight of zero, since we log on successful send.

common/coin_mvt.c

@@ -69,6 +69,7 @@ struct chain_coin_mvt *new_chain_coin_mvt(const tal_t *ctx,
 					  const struct bitcoin_txid *output_txid,
 					  u32 vout,
 					  struct sha256 *payment_hash,
+					  u32 blockheight,
 					  enum mvt_tag tag,
 					  struct amount_msat amount,
 					  bool is_credit,
@@ -89,6 +90,7 @@ struct chain_coin_mvt *new_chain_coin_mvt(const tal_t *ctx,
 	/* for htlc's that are filled onchain, we also have a
 	 * preimage, NULL otherwise */
 	mvt->payment_hash = payment_hash;
+	mvt->blockheight = blockheight;
 
 	mvt->tag = tag;
 	if (is_credit) {
@@ -109,6 +111,7 @@ struct chain_coin_mvt *new_chain_coin_mvt_sat(const tal_t *ctx,
 					      const struct bitcoin_txid *output_txid,
 					      u32 vout,
 					      struct sha256 *payment_hash,
+					      u32 blockheight,
 					      enum mvt_tag tag,
 					      struct amount_sat amt_sat,
 					      bool is_credit,
@@ -120,14 +123,13 @@ struct chain_coin_mvt *new_chain_coin_mvt_sat(const tal_t *ctx,
 
 	return new_chain_coin_mvt(ctx, account_name, tx_txid,
 				  output_txid, vout, payment_hash,
-				  tag, amt_msat, is_credit,
+				  blockheight, tag, amt_msat, is_credit,
 				  unit);
 }
 
 struct coin_mvt *finalize_chain_mvt(const tal_t *ctx,
 				    const struct chain_coin_mvt *chain_mvt,
 				    u32 timestamp,
-				    u32 blockheight,
 				    struct node_id *node_id)
 {
 	struct coin_mvt *mvt = tal(ctx, struct coin_mvt);
@@ -145,7 +147,7 @@ struct coin_mvt *finalize_chain_mvt(const tal_t *ctx,
 	mvt->debit = chain_mvt->debit;
 	mvt->unit = chain_mvt->unit;
 	mvt->timestamp = timestamp;
-	mvt->blockheight = blockheight;
+	mvt->blockheight = chain_mvt->blockheight;
 	mvt->version = COIN_MVT_VERSION;
 	mvt->node_id = node_id;
 	mvt->counter = mvt_count++;
@@ -155,8 +157,7 @@ struct coin_mvt *finalize_chain_mvt(const tal_t *ctx,
 
 struct coin_mvt *finalize_channel_mvt(const tal_t *ctx,
 				      const struct channel_coin_mvt *chan_mvt,
-				      u32 timestamp, u32 blockheight,
-				      struct node_id *node_id)
+				      u32 timestamp, struct node_id *node_id)
 {
 	struct coin_mvt *mvt = tal(ctx, struct coin_mvt);
 
@@ -173,7 +174,8 @@ struct coin_mvt *finalize_channel_mvt(const tal_t *ctx,
 	mvt->debit = chan_mvt->debit;
 	mvt->unit = chan_mvt->unit;
 	mvt->timestamp = timestamp;
-	mvt->blockheight = blockheight;
+	/* channel movements don't have a blockheight */
+	mvt->blockheight = 0;
 	mvt->version = COIN_MVT_VERSION;
 	mvt->node_id = node_id;
 	mvt->counter = mvt_count++;
@@ -201,6 +203,7 @@ void towire_chain_coin_mvt(u8 **pptr, const struct chain_coin_mvt *mvt)
 		towire_sha256(pptr, mvt->payment_hash);
 	} else
 		towire_bool(pptr, false);
+	towire_u32(pptr, mvt->blockheight);
 	towire_u8(pptr, mvt->tag);
 	towire_amount_msat(pptr, mvt->credit);
 	towire_amount_msat(pptr, mvt->debit);
@@ -233,6 +236,7 @@ void fromwire_chain_coin_mvt(const u8 **cursor, size_t *max, struct chain_coin_m
 		fromwire_sha256(cursor, max, mvt->payment_hash);
 	} else
 		mvt->payment_hash = NULL;
+	mvt->blockheight = fromwire_u32(cursor, max);
 	mvt->tag = fromwire_u8(cursor, max);
 	mvt->credit = fromwire_amount_msat(cursor, max);
 	mvt->debit = fromwire_amount_msat(cursor, max);

common/coin_mvt.h

@@ -70,6 +70,10 @@ struct chain_coin_mvt {
 	/* label / tag */
 	enum mvt_tag tag;
 
+	/* block this transaction is confirmed in
+	 * zero means it's unknown/unconfirmed */
+	u32 blockheight;
+
 	/* only one or the other */
 	struct amount_msat credit;
 	struct amount_msat debit;
@@ -137,6 +141,7 @@ struct chain_coin_mvt *new_chain_coin_mvt(const tal_t *ctx,
 					  const struct bitcoin_txid *output_txid,
 					  u32 vout,
 					  struct sha256 *payment_hash,
+					  u32 blockheight,
 					  enum mvt_tag tag,
 					  struct amount_msat amount,
 					  bool is_credit,
@@ -147,6 +152,7 @@ struct chain_coin_mvt *new_chain_coin_mvt_sat(const tal_t *ctx,
 					      const struct bitcoin_txid *output_txid,
 					      u32 vout,
 					      struct sha256 *payment_hash,
+					      u32 blockheight,
 					      enum mvt_tag tag,
 					      struct amount_sat amt_sat,
 					      bool is_credit,
@@ -154,12 +160,10 @@ struct chain_coin_mvt *new_chain_coin_mvt_sat(const tal_t *ctx,
 struct coin_mvt *finalize_chain_mvt(const tal_t *ctx,
 				    const struct chain_coin_mvt *chain_mvt,
 				    u32 timestamp,
-				    u32 blockheight,
 				    struct node_id *node_id);
 struct coin_mvt *finalize_channel_mvt(const tal_t *ctx,
 				      const struct channel_coin_mvt *chan_mvt,
-				      u32 timestamp, u32 blockheight,
-				      struct node_id *node_id);
+				      u32 timestamp, struct node_id *node_id);
 
 const char *mvt_type_str(enum mvt_type type);
 const char *mvt_tag_str(enum mvt_tag tag);

lightningd/channel_control.c

@@ -81,24 +81,32 @@ static void record_channel_open(struct channel *channel)
 	struct channel_id channel_id;
 	struct chain_coin_mvt *mvt;
 	struct amount_msat channel_open_amt;
+	u32 blockheight;
+
 	u8 *ctx = tal(NULL, u8);
 
 	/* figure out the 'account name' */
 	derive_channel_id(&channel_id, &channel->funding_txid,
 			  channel->funding_outnum);
 
+	blockheight = short_channel_id_blocknum(channel->scid);
+
 	/* FIXME: logic here will change for dual funded channels */
 	if (channel->opener == LOCAL) {
 		if (!amount_sat_to_msat(&channel_open_amt, channel->funding))
 			fatal("Unable to convert funding %s to msat",
-			      type_to_string(tmpctx, struct amount_sat, &channel->funding));
+			      type_to_string(tmpctx, struct amount_sat,
+					     &channel->funding));
 
 		/* if we pushed sats, we should decrement that from the channel balance */
 		if (amount_msat_greater(channel->push, AMOUNT_MSAT(0))) {
 			mvt = new_chain_coin_mvt(ctx,
-						 type_to_string(tmpctx, struct channel_id, &channel_id),
+						 type_to_string(tmpctx,
+								struct channel_id,
+								&channel_id),
 						 &channel->funding_txid,
 						 NULL, 0, NULL,
+						 blockheight,
 						 PUSHED, channel->push,
 						 false, BTC);
 			notify_chain_mvt(channel->peer->ld, mvt);
@@ -111,11 +119,13 @@ static void record_channel_open(struct channel *channel)
 	}
 
 	mvt = new_chain_coin_mvt(ctx,
-				 type_to_string(tmpctx, struct channel_id, &channel_id),
+				 type_to_string(tmpctx, struct channel_id,
+						&channel_id),
 				 &channel->funding_txid,
 				 &channel->funding_txid,
 				 channel->funding_outnum,
-				 NULL, DEPOSIT, channel_open_amt,
+				 NULL, blockheight,
+				 DEPOSIT, channel_open_amt,
 				 true, BTC);
 	notify_chain_mvt(channel->peer->ld, mvt);
 	tal_free(ctx);

lightningd/coin_mvts.c

@@ -8,7 +8,6 @@ void notify_channel_mvt(struct lightningd *ld, const struct channel_coin_mvt *mv
 
 	timestamp = time_now().ts.tv_sec;
 	cm = finalize_channel_mvt(mvt, mvt, timestamp,
-				  get_block_height(ld->topology),
 				  &ld->id);
 	notify_coin_mvt(ld, cm);
 }
@@ -20,7 +19,6 @@ void notify_chain_mvt(struct lightningd *ld, const struct chain_coin_mvt *mvt)
 
 	timestamp = time_now().ts.tv_sec;
 	cm = finalize_chain_mvt(mvt, mvt, timestamp,
-				get_block_height(ld->topology),
 				&ld->id);
 	notify_coin_mvt(ld, cm);
 }

lightningd/notification.c

@@ -392,7 +392,15 @@ static void coin_movement_notification_serialize(struct json_stream *stream,
 	json_add_amount_msat_only(stream, "credit", mvt->credit);
 	json_add_amount_msat_only(stream, "debit", mvt->debit);
 	json_add_string(stream, "tag", mvt_tag_str(mvt->tag));
-	json_add_u32(stream, "blockheight", mvt->blockheight);
+
+	/* Only chain movements have blockheights. A blockheight
+	 * of 'zero' means we haven't seen this tx confirmed yet. */
+	if (mvt->type == CHAIN_MVT) {
+		if (mvt->blockheight)
+			json_add_u32(stream, "blockheight", mvt->blockheight);
+		else
+			json_add_null(stream, "blockheight");
+	}
 	json_add_u32(stream, "timestamp", mvt->timestamp);
 	json_add_string(stream, "unit_of_account", mvt_unit_str(mvt->unit));
 

lightningd/onchain_control.c

@@ -341,8 +341,9 @@ static void onchain_add_utxo(struct channel *channel, const u8 *msg)
 	outpointfilter_add(channel->peer->ld->wallet->owned_outpoints, &u->txid, u->outnum);
 	wallet_add_utxo(channel->peer->ld->wallet, u, p2wpkh);
 
-	mvt = new_chain_coin_mvt_sat(msg, "wallet", &u->txid, &u->txid, u->outnum,
-				     NULL, DEPOSIT, u->amount, true, BTC);
+	mvt = new_chain_coin_mvt_sat(msg, "wallet", &u->txid, &u->txid,
+				     u->outnum, NULL, blockheight,
+				     DEPOSIT, u->amount, true, BTC);
 	notify_chain_mvt(channel->peer->ld, mvt);
 }
 

onchaind/onchaind.c

@@ -136,6 +136,7 @@ static void send_coin_mvt(struct chain_coin_mvt *mvt TAKES)
 }
 
 static void record_their_successful_cheat(const struct bitcoin_txid *txid,
+					  u32 blockheight,
 					  struct tracked_output *out)
 {
 	struct chain_coin_mvt *mvt;
@@ -144,7 +145,7 @@ static void record_their_successful_cheat(const struct bitcoin_txid *txid,
 	mvt = new_chain_coin_mvt_sat(NULL, NULL,
 				     txid, &out->txid,
 				     out->outnum, NULL,
-				     PENALTY,
+				     blockheight, PENALTY,
 				     out->sat, false,
 				     BTC);
 
@@ -153,6 +154,7 @@ static void record_their_successful_cheat(const struct bitcoin_txid *txid,
 
 static void record_htlc_fulfilled(const struct bitcoin_txid *txid,
 				  struct tracked_output *out,
+				  u32 blockheight,
 				  bool we_fulfilled)
 {
 	struct chain_coin_mvt *mvt;
@@ -166,6 +168,7 @@ static void record_htlc_fulfilled(const struct bitcoin_txid *txid,
 				     txid, &out->txid,
 				     out->outnum,
 				     out->payment_hash,
+				     blockheight,
 				     ONCHAIN_HTLC,
 				     out->sat, we_fulfilled,
 				     BTC);
@@ -174,11 +177,13 @@ static void record_htlc_fulfilled(const struct bitcoin_txid *txid,
 }
 
 static void update_ledger_chain_fees_msat(const struct bitcoin_txid *txid,
+					  u32 blockheight,
 					  struct amount_msat fees)
 {
 	struct chain_coin_mvt *mvt;
 	mvt = new_chain_coin_mvt(NULL, NULL,
 				 txid, NULL, 0, NULL,
+				 blockheight,
 				 CHAIN_FEES, fees,
 				 false, BTC);
 
@@ -186,11 +191,13 @@ static void update_ledger_chain_fees_msat(const struct bitcoin_txid *txid,
 }
 
 static void update_ledger_chain_fees(const struct bitcoin_txid *txid,
+				     u32 blockheight,
 				     struct amount_sat fees)
 {
 	struct chain_coin_mvt *mvt;
 	mvt = new_chain_coin_mvt_sat(NULL, NULL,
 				     txid, NULL, 0, NULL,
+				     blockheight,
 				     CHAIN_FEES, fees,
 				     false, BTC);
 
@@ -206,13 +213,14 @@ static void update_ledger_chain_fees(const struct bitcoin_txid *txid,
  * you *cannot* pass a chaintopology-originated tx to this method,
  * as they don't have the input_amounts populated */
 static struct amount_sat record_chain_fees_tx(const struct bitcoin_txid *txid,
-					      const struct bitcoin_tx *tx)
+					      const struct bitcoin_tx *tx,
+					      u32 blockheight)
 {
 	struct amount_sat fees;
 	fees = bitcoin_tx_compute_fee(tx);
 	status_debug("recording chain fees for tx %s",
 		     type_to_string(tmpctx, struct bitcoin_txid, txid));
-	update_ledger_chain_fees(txid, fees);
+	update_ledger_chain_fees(txid, blockheight, fees);
 	return fees;
 }
 
@@ -228,6 +236,7 @@ static void add_amt(struct amount_sat *sum, struct amount_sat amt)
 }
 
 static void record_mutual_closure(const struct bitcoin_txid *txid,
+				  u32 blockheight,
 				  struct amount_sat our_out,
 				  int output_num)
 {
@@ -254,7 +263,7 @@ static void record_mutual_closure(const struct bitcoin_txid *txid,
 					     &our_msat));
 
 	if (!amount_msat_eq(AMOUNT_MSAT(0), chain_fees))
-		update_ledger_chain_fees_msat(txid, chain_fees);
+		update_ledger_chain_fees_msat(txid, blockheight, chain_fees);
 
 	/* If we have no output, we exit early */
 	if (amount_msat_eq(AMOUNT_MSAT(0), output_msat))
@@ -264,6 +273,7 @@ static void record_mutual_closure(const struct bitcoin_txid *txid,
 	/* Otherwise, we record the channel withdrawal */
 	mvt = new_chain_coin_mvt(NULL, NULL, txid,
 				 txid, output_num, NULL,
+				 blockheight,
 				 WITHDRAWAL, output_msat,
 				 false, BTC);
 
@@ -271,6 +281,7 @@ static void record_mutual_closure(const struct bitcoin_txid *txid,
 }
 
 static void record_chain_fees_unilateral(const struct bitcoin_txid *txid,
+					 u32 blockheight,
 					 struct amount_sat funding,
 					 struct amount_sat their_outs,
 					 struct amount_sat our_outs)
@@ -296,10 +307,11 @@ static void record_chain_fees_unilateral(const struct bitcoin_txid *txid,
 
 	status_debug("logging 'chain fees' for unilateral (trimmed) %s",
 		     type_to_string(tmpctx, struct amount_msat, &trimmed));
-	update_ledger_chain_fees_msat(txid, trimmed);
+	update_ledger_chain_fees_msat(txid, blockheight, trimmed);
 }
 
 static void record_coin_loss(const struct bitcoin_txid *txid,
+			     u32 blockheight,
 			     struct tracked_output *out)
 {
 	struct chain_coin_mvt *mvt;
@@ -308,6 +320,7 @@ static void record_coin_loss(const struct bitcoin_txid *txid,
 	mvt = new_chain_coin_mvt_sat(NULL, NULL,
 				     txid, &out->txid,
 				     out->outnum, NULL,
+				     blockheight,
 				     PENALTY, out->sat, false,
 				     BTC);
 
@@ -322,6 +335,7 @@ static void record_coin_loss(const struct bitcoin_txid *txid,
 
 static void record_channel_withdrawal_minus_fees(const struct bitcoin_txid *tx_txid,
 				      		 struct tracked_output *out,
+						 u32 blockheight,
 						 struct amount_sat fees)
 {
 	struct chain_coin_mvt *mvt;
@@ -337,7 +351,8 @@ static void record_channel_withdrawal_minus_fees(const struct bitcoin_txid *tx_t
 
 	mvt = new_chain_coin_mvt_sat(NULL, NULL,
 				     tx_txid, &out->txid,
-				     out->outnum, NULL, WITHDRAWAL,
+				     out->outnum, NULL,
+				     blockheight, WITHDRAWAL,
 				     emitted_amt, false,
 				     BTC);
 
@@ -352,9 +367,10 @@ static void record_channel_withdrawal_minus_fees(const struct bitcoin_txid *tx_t
 
 
 static void record_channel_withdrawal(const struct bitcoin_txid *tx_txid,
+				      u32 blockheight,
 				      struct tracked_output *out)
 {
-	record_channel_withdrawal_minus_fees(tx_txid, out, AMOUNT_SAT(0));
+	record_channel_withdrawal_minus_fees(tx_txid, out, blockheight, AMOUNT_SAT(0));
 }
 
 static bool is_our_htlc_tx(struct tracked_output *out)
@@ -372,6 +388,7 @@ static bool is_channel_deposit(struct tracked_output *out)
 }
 
 static void record_coin_movements(struct tracked_output *out,
+				  u32 blockheight,
 				  const struct bitcoin_tx *tx,
 				  const struct bitcoin_txid *txid)
 {
@@ -379,19 +396,19 @@ static void record_coin_movements(struct tracked_output *out,
 	/* there is a case where we've fulfilled an htlc onchain,
 	 * in which case we log a deposit to the channel */
 	if (is_channel_deposit(out))
-		record_htlc_fulfilled(txid, out, true);
+		record_htlc_fulfilled(txid, out, blockheight, true);
 
 
 	/* record fees paid for the tx here */
 	/* FIXME: for now, every resolution generates its own tx,
 	 * this will need to be updated if we switch to batching */
-	fees = record_chain_fees_tx(txid, tx);
+	fees = record_chain_fees_tx(txid, tx, blockheight);
 
 	/* we don't record a channel withdrawal until we get to
 	 * the 'exit' utxo, which for local commitment htlc txs
 	 * is the child htlc_tx's output */
 	if (!is_our_htlc_tx(out))
-		record_channel_withdrawal_minus_fees(txid, out, fees);
+		record_channel_withdrawal_minus_fees(txid, out, blockheight, fees);
 }
 
 /* We vary feerate until signature they offered matches. */
@@ -822,8 +839,8 @@ static void proposal_meets_depth(struct tracked_output *out, bool is_replay)
 	wire_sync_write(
 	    REQ_FD,
 	    take(towire_onchain_broadcast_tx(
-		NULL, out->proposal->tx,
-		onchain_txtype_to_wallet_txtype(out->proposal->tx_type))));
+		 NULL, out->proposal->tx,
+		 onchain_txtype_to_wallet_txtype(out->proposal->tx_type))));
 
 	/* Don't wait for this if we're ignoring the tiny payment. */
 	if (out->proposal->tx_type == IGNORING_TINY_PAYMENT) {
@@ -836,8 +853,8 @@ static void proposal_meets_depth(struct tracked_output *out, bool is_replay)
 			/* log the coin movements here, since we're not
 			 * going to wait til we hear about it */
 			bitcoin_txid(out->proposal->tx, &txid);
-			fees = record_chain_fees_tx(&txid, out->proposal->tx);
-			record_channel_withdrawal_minus_fees(&txid, out, fees);
+			fees = record_chain_fees_tx(&txid, out->proposal->tx, 0);
+			record_channel_withdrawal_minus_fees(&txid, out, 0, fees);
 		}
 	}
 
@@ -1372,7 +1389,7 @@ static void steal_htlc_tx(const struct chainparams *chainparams,
 		     type_to_string(tmpctx, struct amount_sat, &fees));
 
 	if (!is_replay)
-		update_ledger_chain_fees(htlc_txid, fees);
+		update_ledger_chain_fees(htlc_txid, htlc_tx_blockheight, fees);
 
 	/* annnd done! */
 	propose_resolution(htlc_out, tx, 0, tx_type, is_replay);
@@ -1426,7 +1443,8 @@ static void output_spent(const struct chainparams *chainparams,
 						tx_blockheight, is_replay);
 
 			if (!is_replay)
-				record_coin_movements(out, out->proposal->tx, &txid);
+				record_coin_movements(out, tx_blockheight,
+						      out->proposal->tx, &txid);
 			return;
 		}
 
@@ -1435,7 +1453,7 @@ static void output_spent(const struct chainparams *chainparams,
 		case DELAYED_OUTPUT_TO_US:
 			unknown_spend(out, tx);
 			if (!is_replay)
-				record_coin_loss(&txid, out);
+				record_coin_loss(&txid, tx_blockheight, out);
 			break;
 
 		case THEIR_HTLC:
@@ -1486,7 +1504,9 @@ static void output_spent(const struct chainparams *chainparams,
 				ignore_output(out);
 
 				if (!is_replay)
-					record_htlc_fulfilled(&txid, out, false);
+					record_htlc_fulfilled(&txid, out,
+							      tx_blockheight,
+							      false);
 				onchain_annotate_txout(
 				    &spendertxid, out->outnum,
 				    TX_CHANNEL_HTLC_SUCCESS | TX_THEIRS);
@@ -1503,7 +1523,8 @@ static void output_spent(const struct chainparams *chainparams,
 			/* They successfully spent a delayed revoked output */
 			resolved_by_other(out, &txid, THEIR_DELAYED_CHEAT);
 			if (!is_replay)
-				record_their_successful_cheat(&txid, out);
+				record_their_successful_cheat(&txid,
+							      tx_blockheight, out);
 			break;
 		/* Um, we don't track these! */
 		case OUTPUT_TO_THEM:
@@ -1730,7 +1751,7 @@ static void handle_preimage(const struct chainparams *chainparams,
 			if (!is_replay && tx_type == IGNORING_TINY_PAYMENT) {
 				struct bitcoin_txid txid;
 				bitcoin_txid(tx, &txid);
-				record_htlc_fulfilled(&txid, outs[i], true);
+				record_htlc_fulfilled(&txid, outs[i], 0, true);
 			}
 
 		}
@@ -1842,6 +1863,7 @@ static void handle_mutual_close(const struct chainparams *chainparams,
 				const struct bitcoin_txid *txid,
 				struct tracked_output **outs,
 				const struct bitcoin_tx *tx,
+				u32 tx_blockheight,
 				int our_outnum,
 				bool is_replay)
 {
@@ -1871,7 +1893,8 @@ static void handle_mutual_close(const struct chainparams *chainparams,
 		} else
 			our_out = AMOUNT_SAT(0);
 
-		record_mutual_closure(txid, our_out, our_outnum);
+		record_mutual_closure(txid, tx_blockheight,
+				      our_out, our_outnum);
 	}
 
 	wait_for_resolved(chainparams, outs);
@@ -2393,7 +2416,8 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx,
 	note_missing_htlcs(htlc_scripts, htlcs,
 			   tell_if_missing, tell_immediately);
 	if (!is_replay)
-		record_chain_fees_unilateral(txid, outs[0]->sat,
+		record_chain_fees_unilateral(txid, tx_blockheight,
+					     outs[0]->sat,
 					     their_outs, our_outs);
 
 	wait_for_resolved(tx->chainparams, outs);
@@ -2481,6 +2505,7 @@ static void tell_wallet_to_remote(const struct bitcoin_tx *tx,
  * will correctly mark the funds as a 'channel withdrawal'
  */
 static void update_ledger_cheat(const struct bitcoin_txid *txid,
+				u32 blockheight,
 				struct tracked_output *out)
 {
 	/* how much of a difference should we update the
@@ -2503,7 +2528,9 @@ static void update_ledger_cheat(const struct bitcoin_txid *txid,
 	/* FIXME: elements is not always btc? */
 	mvt = new_chain_coin_mvt(NULL, NULL,
 				 txid, &out->txid,
-				 out->outnum, NULL, JOURNAL, amt,
+				 out->outnum, NULL,
+				 blockheight,
+				 JOURNAL, amt,
 				 true, BTC);
 	send_coin_mvt(take(mvt));
 }
@@ -2539,8 +2566,9 @@ static void handle_their_cheat(const struct bitcoin_tx *tx,
 	init_reply("Tracking their illegal close: taking all funds");
 	onchain_annotate_txin(
 	    txid, 0, TX_CHANNEL_UNILATERAL | TX_CHANNEL_CHEAT | TX_THEIRS);
+
 	if (!is_replay)
-		update_ledger_cheat(txid, outs[0]);
+		update_ledger_cheat(txid, tx_blockheight, outs[0]);
 
 	/* BOLT #5:
 	 *
@@ -2688,7 +2716,7 @@ static void handle_their_cheat(const struct bitcoin_tx *tx,
 			ignore_output(out);
 
 			if (!is_replay)
-				record_channel_withdrawal(txid, out);
+				record_channel_withdrawal(txid, tx_blockheight, out);
 
 			tell_wallet_to_remote(tx, i, txid,
 					      tx_blockheight,
@@ -2779,7 +2807,7 @@ static void handle_their_cheat(const struct bitcoin_tx *tx,
 		     type_to_string(tmpctx, struct amount_sat, &fee_cost));
 
 	if (!is_replay)
-		update_ledger_chain_fees(txid, fee_cost);
+		update_ledger_chain_fees(txid, tx_blockheight, fee_cost);
 
 	wait_for_resolved(tx->chainparams, outs);
 }
@@ -2941,7 +2969,7 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx,
 			ignore_output(out);
 
 			if (!is_replay)
-				record_channel_withdrawal(txid, out);
+				record_channel_withdrawal(txid, tx_blockheight, out);
 
 			tell_wallet_to_remote(tx, i, txid,
 					      tx_blockheight,
@@ -3028,13 +3056,15 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx,
 			   tell_if_missing, tell_immediately);
 
 	if (!is_replay)
-		record_chain_fees_unilateral(txid, outs[0]->sat,
+		record_chain_fees_unilateral(txid, tx_blockheight,
+					     outs[0]->sat,
 					     their_outs, our_outs);
 
 	wait_for_resolved(tx->chainparams, outs);
 }
 
 static void update_ledger_unknown(const struct bitcoin_txid *txid,
+				  u32 blockheight,
 				  struct amount_sat amt_salvaged)
 {
 	/* ideally, we'd be able to capture the loss to fees (if we funded
@@ -3069,6 +3099,7 @@ static void update_ledger_unknown(const struct bitcoin_txid *txid,
 	/* FIXME: elements txs not in BTC ?? */
 	mvt = new_chain_coin_mvt(NULL, NULL,
 				 txid, NULL, 0, NULL,
+				 blockheight,
 				 JOURNAL, diff,
 				 is_credit, BTC);
 	send_coin_mvt(take(mvt));
@@ -3154,7 +3185,7 @@ static void handle_unknown_commitment(const struct bitcoin_tx *tx,
 			ignore_output(out);
 
 			if (!is_replay)
-				record_channel_withdrawal(txid, out);
+				record_channel_withdrawal(txid, tx_blockheight, out);
 
 			add_amt(&amt_salvaged, amt);
 
@@ -3183,7 +3214,7 @@ search_done:
 	/* update our accounting notions for this channel.
 	 * should result in a channel balance of zero */
 	if (!is_replay)
-		update_ledger_unknown(txid, amt_salvaged);
+		update_ledger_unknown(txid, tx_blockheight, amt_salvaged);
 
 	/* Tell master to give up on HTLCs immediately. */
 	for (size_t i = 0; i < tal_count(htlcs); i++) {
@@ -3315,7 +3346,8 @@ int main(int argc, char *argv[])
 	 * [BOLT #2: Channel Close](02-peer-protocol.md#channel-close)).
 	 */
 	if (is_mutual_close(tx, scriptpubkey[LOCAL], scriptpubkey[REMOTE], &mutual_outnum))
-		handle_mutual_close(tx->chainparams, &txid, outs, tx, mutual_outnum, open_is_replay);
+		handle_mutual_close(tx->chainparams, &txid, outs, tx,
+				    tx_blockheight, mutual_outnum, open_is_replay);
 	else {
 		/* BOLT #5:
 		 *

onchaind/test/run-grind_feerate-bug.c

@@ -104,6 +104,7 @@ struct chain_coin_mvt *new_chain_coin_mvt(const tal_t *ctx UNNEEDED,
 					  const struct bitcoin_txid *output_txid UNNEEDED,
 					  u32 vout UNNEEDED,
 					  struct sha256 *payment_hash UNNEEDED,
+					  u32 blockheight UNNEEDED,
 					  enum mvt_tag tag UNNEEDED,
 					  struct amount_msat amount UNNEEDED,
 					  bool is_credit UNNEEDED,
@@ -116,6 +117,7 @@ struct chain_coin_mvt *new_chain_coin_mvt_sat(const tal_t *ctx UNNEEDED,
 					      const struct bitcoin_txid *output_txid UNNEEDED,
 					      u32 vout UNNEEDED,
 					      struct sha256 *payment_hash UNNEEDED,
+					      u32 blockheight UNNEEDED,
 					      enum mvt_tag tag UNNEEDED,
 					      struct amount_sat amt_sat UNNEEDED,
 					      bool is_credit UNNEEDED,

onchaind/test/run-grind_feerate.c

@@ -116,6 +116,7 @@ struct chain_coin_mvt *new_chain_coin_mvt(const tal_t *ctx UNNEEDED,
 					  const struct bitcoin_txid *output_txid UNNEEDED,
 					  u32 vout UNNEEDED,
 					  struct sha256 *payment_hash UNNEEDED,
+					  u32 blockheight UNNEEDED,
 					  enum mvt_tag tag UNNEEDED,
 					  struct amount_msat amount UNNEEDED,
 					  bool is_credit UNNEEDED,
@@ -128,6 +129,7 @@ struct chain_coin_mvt *new_chain_coin_mvt_sat(const tal_t *ctx UNNEEDED,
 					      const struct bitcoin_txid *output_txid UNNEEDED,
 					      u32 vout UNNEEDED,
 					      struct sha256 *payment_hash UNNEEDED,
+					      u32 blockheight UNNEEDED,
 					      enum mvt_tag tag UNNEEDED,
 					      struct amount_sat amt_sat UNNEEDED,
 					      bool is_credit UNNEEDED,

tests/plugins/coin_movements.py

@@ -30,11 +30,10 @@ def notify_coin_movement(plugin, coin_movement, **kwargs):
     plugin.log("{} coins credit: {}".format(idx, coin_movement['credit']))
     plugin.log("{} coins debit: {}".format(idx, coin_movement['debit']))
     plugin.log("{} coins tag: {}".format(idx, coin_movement['tag']))
-    plugin.log("{} coins blockheight: {}".format(idx, coin_movement['blockheight']))
     plugin.log("{} coins timestamp: {}".format(idx, coin_movement['timestamp']))
     plugin.log("{} coins unit_of_account: {}".format(idx, coin_movement['unit_of_account']))
 
-    for f in ['payment_hash', 'utxo_txid', 'vout', 'txid', 'part_id']:
+    for f in ['payment_hash', 'utxo_txid', 'vout', 'txid', 'part_id', 'blockheight']:
         if f in coin_movement:
             plugin.log("{} coins {}: {}".format(idx, f, coin_movement[f]))
 

wallet/test/run-wallet.c

@@ -391,6 +391,7 @@ struct chain_coin_mvt *new_chain_coin_mvt_sat(const tal_t *ctx UNNEEDED,
 					      const struct bitcoin_txid *output_txid UNNEEDED,
 					      u32 vout UNNEEDED,
 					      struct sha256 *payment_hash UNNEEDED,
+					      u32 blockheight UNNEEDED,
 					      enum mvt_tag tag UNNEEDED,
 					      struct amount_sat amt_sat UNNEEDED,
 					      bool is_credit UNNEEDED,

wallet/wallet.c

@@ -1654,6 +1654,16 @@ int wallet_extract_owned_outputs(struct wallet *w, const struct bitcoin_tx *tx,
 			  type_to_string(tmpctx, struct bitcoin_txid,
 					 &utxo->txid), blockheight ? " CONFIRMED" : "");
 
+		/* We only record final ledger movements */
+		if (blockheight) {
+			mvt = new_chain_coin_mvt_sat(utxo, "wallet", &utxo->txid,
+						     &utxo->txid, utxo->outnum, NULL,
+						     blockheight ? *blockheight : 0,
+						     DEPOSIT, utxo->amount,
+						     true, BTC);
+			notify_chain_mvt(w->ld, mvt);
+		}
+
 		if (!wallet_add_utxo(w, utxo, is_p2sh ? p2sh_wpkh : our_change)) {
 			/* In case we already know the output, make
 			 * sure we actually track its
@@ -1666,13 +1676,6 @@ int wallet_extract_owned_outputs(struct wallet *w, const struct bitcoin_tx *tx,
 			continue;
 		}
 
-		/* add this to our wallet amount */
-		mvt = new_chain_coin_mvt_sat(utxo, "wallet", &utxo->txid,
-					     &utxo->txid, utxo->outnum,
-					     NULL, DEPOSIT, utxo->amount,
-					     true, BTC);
-		notify_chain_mvt(w->ld, mvt);
-
 		/* This is an unconfirmed change output, we should track it */
 		if (!is_p2sh && !blockheight)
 			txfilter_add_scriptpubkey(w->ld->owned_txfilter, script);

wallet/walletrpc.c

@@ -58,7 +58,6 @@ static struct amount_sat compute_fee(const struct bitcoin_tx *tx,
 
 	return bitcoin_tx_compute_fee_w_inputs(tx, input_sum);
 }
-
 static void record_coin_moves(struct lightningd *ld,
 			      struct unreleased_tx *utx)
 {
@@ -78,7 +77,7 @@ static void record_coin_moves(struct lightningd *ld,
 		}
 		sats = amount_asset_to_sat(&asset);
 		mvt = new_chain_coin_mvt_sat(utx, "wallet", &utx->txid,
-					     &utx->txid, i, NULL,
+					     &utx->txid, i, NULL, 0,
 					     WITHDRAWAL, sats,
 					     false, BTC);
 		if (!mvt)
@@ -98,7 +97,7 @@ static void record_coin_moves(struct lightningd *ld,
 	 * You can do this in post by accounting for any 'chain_fees' logged for
 	 * the funding txid when looking at a channel. */
 	mvt = new_chain_coin_mvt_sat(utx, "wallet", &utx->txid,
-				     NULL, 0, NULL, CHAIN_FEES,
+				     NULL, 0, NULL, 0, CHAIN_FEES,
 				     fees, false, BTC);
 
 	if (!mvt)