lightningd: keep indexes updated for channel_htlcs table (aka listhtlcs).

Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

lightningd/channel.c

@@ -94,7 +94,7 @@ void delete_channel(struct channel *channel STEALS)
 
 	struct peer *peer = channel->peer;
 	if (channel->dbid != 0)
-		wallet_channel_close(channel->peer->ld->wallet, channel->dbid);
+		wallet_channel_close(channel->peer->ld->wallet, channel);
 
 	/* Tell the hsm to forget the channel, needs to be after it's
 	 * been forgotten here */

lightningd/peer_htlcs.c

@@ -67,7 +67,13 @@ static bool htlc_in_update_state(struct channel *channel,
 			   max_unsigned(channel->next_index[LOCAL],
 					channel->next_index[REMOTE]),
 			   hin->badonion, hin->failonion, NULL,
-			   hin->we_filled);
+			   hin->we_filled,
+			   hin->key.id,
+			   hin->key.channel,
+			   REMOTE,
+			   &hin->payment_hash,
+			   hin->cltv_expiry,
+			   hin->msat);
 
 	hin->hstate = newstate;
 	return true;
@@ -87,7 +93,13 @@ static bool htlc_out_update_state(struct channel *channel,
 			   max_unsigned(channel->next_index[LOCAL],
 					channel->next_index[REMOTE]),
 			   0, hout->failonion,
-			   hout->failmsg, &we_filled);
+			   hout->failmsg, &we_filled,
+			   hout->key.id,
+			   hout->key.channel,
+			   LOCAL,
+			   &hout->payment_hash,
+			   hout->cltv_expiry,
+			   hout->msat);
 
 	hout->hstate = newstate;
 	return true;
@@ -234,7 +246,13 @@ static void fail_in_htlc(struct htlc_in *hin,
 			   max_unsigned(hin->key.channel->next_index[LOCAL],
 					hin->key.channel->next_index[REMOTE]),
 			   hin->badonion,
-			   hin->failonion, NULL, &we_filled);
+			   hin->failonion, NULL, &we_filled,
+			   hin->key.id,
+			   hin->key.channel,
+			   REMOTE,
+			   &hin->payment_hash,
+			   hin->cltv_expiry,
+			   hin->msat);
 
 	tell_channeld_htlc_failed(hin, failed_htlc);
 }
@@ -1500,7 +1518,14 @@ static void fulfill_our_htlc_out(struct channel *channel, struct htlc_out *hout,
 			   max_unsigned(channel->next_index[LOCAL],
 					channel->next_index[REMOTE]),
 			   0, hout->failonion,
-			   hout->failmsg, &we_filled);
+			   hout->failmsg, &we_filled,
+			   hout->key.id,
+			   hout->key.channel,
+			   LOCAL,
+			   &hout->payment_hash,
+			   hout->cltv_expiry,
+			   hout->msat);
+
 	/* Update channel stats */
 	channel_stats_incr_out_fulfilled(channel, hout->msat);
 
@@ -1761,7 +1786,13 @@ void onchain_failed_our_htlc(const struct channel *channel,
 			   max_unsigned(channel->next_index[LOCAL],
 					channel->next_index[REMOTE]),
 			   0, hout->failonion,
-			   hout->failmsg, &we_filled);
+			   hout->failmsg, &we_filled,
+			   hout->key.id,
+			   hout->key.channel,
+			   LOCAL,
+			   &hout->payment_hash,
+			   hout->cltv_expiry,
+			   hout->msat);
 
 	if (hout->am_origin) {
 		log_debug(channel->log, "HTLC id %"PRIu64" am origin",

tests/test_misc.py

@@ -3427,6 +3427,85 @@ def test_listforwards_wait(node_factory, executor):
                                     'status': 'failed'}}
 
 
+def test_listhtlcs_wait(node_factory, bitcoind, executor):
+    l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True)
+
+    scid12 = first_scid(l1, l2)
+    scid23 = first_scid(l2, l3)
+    waitres = l1.rpc.wait(subsystem='htlcs', indexname='created', nextvalue=0)
+    assert waitres == {'subsystem': 'htlcs',
+                       'created': 0}
+
+    # Now ask for 1.
+    waitcreate = executor.submit(l2.rpc.wait, subsystem='htlcs', indexname='created', nextvalue=1)
+    waitupdate = executor.submit(l2.rpc.wait, subsystem='htlcs', indexname='updated', nextvalue=1)
+    time.sleep(1)
+
+    amt1 = 1000
+    inv1 = l3.rpc.invoice(amt1, 'inv1', 'desc')
+    l1.rpc.pay(inv1['bolt11'])
+
+    waitres = waitcreate.result(TIMEOUT)
+    assert waitres == {'subsystem': 'htlcs',
+                       'created': 1,
+                       'htlcs': {'short_channel_id': scid12,
+                                 'cltv_expiry': 120,
+                                 'direction': 'in',
+                                 'htlc_id': 0,
+                                 'payment_hash': inv1['payment_hash'],
+                                 'amount_msat': amt1 + 1,
+                                 'state': 'RCVD_ADD_COMMIT'}}
+    waitres = waitupdate.result(TIMEOUT)
+    assert waitres == {'subsystem': 'htlcs',
+                       'updated': 1,
+                       'htlcs': {'short_channel_id': scid12,
+                                 'cltv_expiry': 120,
+                                 'direction': 'in',
+                                 'htlc_id': 0,
+                                 'payment_hash': inv1['payment_hash'],
+                                 'amount_msat': amt1 + 1,
+                                 'state': 'SENT_ADD_REVOCATION'}}
+
+    # There's a second new one too, for the outgoing, but we missed details
+    assert l2.rpc.wait(subsystem='htlcs', indexname='created', nextvalue=2) == {'created': 2, 'subsystem': 'htlcs'}
+
+    # Now check failure, and wait for OUTGOING.
+    amt2 = 42
+    inv2 = l3.rpc.invoice(amt2, 'inv2', 'invdesc2')
+    l3.rpc.delinvoice('inv2', 'unpaid')
+
+    waitcreate = executor.submit(l2.rpc.wait, subsystem='htlcs', indexname='created', nextvalue=4)
+    time.sleep(1)
+
+    with pytest.raises(RpcError, match="WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS"):
+        l1.rpc.pay(inv2['bolt11'])
+
+    waitres = waitcreate.result(TIMEOUT)
+    assert waitres == {'subsystem': 'htlcs',
+                       'created': 4,
+                       'htlcs': {'short_channel_id': scid23,
+                                 'cltv_expiry': 114,
+                                 'direction': 'out',
+                                 'htlc_id': 1,
+                                 'payment_hash': inv2['payment_hash'],
+                                 'amount_msat': amt2,
+                                 'state': 'SENT_ADD_HTLC'}}
+
+    # Finally, check deletion (only when channel finally forgotten)
+    l1.rpc.close(l2.info['id'])
+
+    waitfut = executor.submit(l2.rpc.wait, subsystem='htlcs', indexname='deleted', nextvalue=1)
+    time.sleep(1)
+
+    bitcoind.generate_block(100, wait_for_mempool=1)
+
+    waitres = waitfut.result(TIMEOUT)
+    # Both will be deleted at once!  We just get told the channel.
+    assert waitres == {'subsystem': 'htlcs',
+                       'deleted': 2,
+                       'htlcs': {'short_channel_id': scid12}}
+
+
 @unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "modifies database, which is assumed sqlite3")
 def test_listforwards_ancient(node_factory, bitcoind):
     """Test listforwards command with old records."""

wallet/test/run-db.c

@@ -118,6 +118,31 @@ struct htlc_in *htlc_in_check(const struct htlc_in *hin UNNEEDED, const char *ab
 /* Generated stub for htlc_out_connect_htlc_in */
 void htlc_out_connect_htlc_in(struct htlc_out *hout UNNEEDED, struct htlc_in *hin UNNEEDED)
 { fprintf(stderr, "htlc_out_connect_htlc_in called!\n"); abort(); }
+/* Generated stub for htlcs_index_created */
+u64 htlcs_index_created(struct lightningd *ld UNNEEDED,
+			u64 htlc_id UNNEEDED,
+			const struct channel *channel UNNEEDED,
+			const struct sha256 *payment_hash UNNEEDED,
+			enum side owner UNNEEDED,
+			u32 expiry UNNEEDED,
+			struct amount_msat amount UNNEEDED,
+			enum htlc_state hstate UNNEEDED)
+{ fprintf(stderr, "htlcs_index_created called!\n"); abort(); }
+/* Generated stub for htlcs_index_deleted */
+void htlcs_index_deleted(struct lightningd *ld UNNEEDED,
+			 const struct channel *channel UNNEEDED,
+			 u64 num_deleted UNNEEDED)
+{ fprintf(stderr, "htlcs_index_deleted called!\n"); abort(); }
+/* Generated stub for htlcs_index_update_status */
+u64 htlcs_index_update_status(struct lightningd *ld UNNEEDED,
+			      u64 htlc_id UNNEEDED,
+			      const struct channel *channel UNNEEDED,
+			      const struct sha256 *payment_hash UNNEEDED,
+			      enum side owner UNNEEDED,
+			      u32 expiry UNNEEDED,
+			      struct amount_msat amount UNNEEDED,
+			      enum htlc_state hstate UNNEEDED)
+{ fprintf(stderr, "htlcs_index_update_status called!\n"); abort(); }
 /* Generated stub for inflight_set_last_tx */
 void inflight_set_last_tx(struct channel_inflight *inflight UNNEEDED,
 		          struct bitcoin_tx *last_tx STEALS UNNEEDED,

wallet/test/run-wallet.c

@@ -2151,8 +2151,8 @@ static bool test_channel_config_crud(struct lightningd *ld, const tal_t *ctx)
 static bool test_htlc_crud(struct lightningd *ld, const tal_t *ctx)
 {
 	struct db_stmt *stmt;
-	struct htlc_in in, *hin;
-	struct htlc_out out, *hout;
+	struct htlc_in in, in2, *hin;
+	struct htlc_out out, out2, *hout;
 	struct preimage payment_key;
 	struct channel *chan = tal(ctx, struct channel);
 	struct peer *peer = talz(ctx, struct peer);
@@ -2174,55 +2174,61 @@ static bool test_htlc_crud(struct lightningd *ld, const tal_t *ctx)
 	chan->state = CHANNELD_NORMAL;
 	chan->peer = peer;
 	chan->next_index[LOCAL] = chan->next_index[REMOTE] = 1;
+	chan->scid = tal(chan, struct short_channel_id);
 
 	memset(&in, 0, sizeof(in));
 	memset(&out, 0, sizeof(out));
 	memset(&in.payment_hash, 'A', sizeof(struct sha256));
 	memset(&out.payment_hash, 'A', sizeof(struct sha256));
 	memset(&payment_key, 'B', sizeof(payment_key));
+	assert(mk_short_channel_id(chan->scid, 1, 2, 3));
 	in.key.id = 42;
 	in.key.channel = chan;
+	in.cltv_expiry = 42;
 	in.msat = AMOUNT_MSAT(42);
 
 	out.in = ∈
 	out.key.id = 1337;
 	out.key.channel = chan;
+	out.cltv_expiry = 41;
 	out.msat = AMOUNT_MSAT(41);
 
 	/* Store the htlc_in */
 	CHECK_MSG(transaction_wrap(w->db, wallet_htlc_save_in(w, chan, &in)),
 		  tal_fmt(ctx, "Save htlc_in failed: %s", wallet_err));
 	CHECK_MSG(in.dbid != 0, "HTLC DB ID was not set.");
-	/* Saving again should get us a collision */
-	CHECK_MSG(!transaction_wrap(w->db, wallet_htlc_save_in(w, chan, &in)),
+	in2 = in;
+	/* Saving again should get us a collision (but overwrites dbid!) */
+	CHECK_MSG(!transaction_wrap(w->db, wallet_htlc_save_in(w, chan, &in2)),
 		  "Saving two HTLCs with the same data must not succeed.");
 	CHECK(wallet_err);
 	wallet_err = tal_free(wallet_err);
 
 	/* Update */
-	CHECK_MSG(transaction_wrap(w->db, wallet_htlc_update(w, in.dbid, RCVD_ADD_HTLC, NULL, 0, 0, NULL, NULL, &we_filled)),
+	CHECK_MSG(transaction_wrap(w->db, wallet_htlc_update(w, in.dbid, RCVD_ADD_HTLC, NULL, 0, 0, NULL, NULL, &we_filled, in.key.id, in.key.channel, REMOTE, &in.payment_hash, in.cltv_expiry, in.msat)),
 		  "Update HTLC with null payment_key failed");
 	CHECK_MSG(
-		transaction_wrap(w->db, wallet_htlc_update(w, in.dbid, SENT_REMOVE_HTLC, &payment_key, 0, 0, NULL, NULL, &we_filled)),
+		transaction_wrap(w->db, wallet_htlc_update(w, in.dbid, SENT_REMOVE_HTLC, &payment_key, 0, 0, NULL, NULL, &we_filled, in.key.id, in.key.channel, REMOTE, &in.payment_hash, in.cltv_expiry, in.msat)),
 	    "Update HTLC with payment_key failed");
 	onionreply = new_onionreply(tmpctx, tal_arrz(tmpctx, u8, 100));
 	CHECK_MSG(
-		transaction_wrap(w->db, wallet_htlc_update(w, in.dbid, SENT_REMOVE_HTLC, NULL, 0, 0, onionreply, NULL, &we_filled)),
+		transaction_wrap(w->db, wallet_htlc_update(w, in.dbid, SENT_REMOVE_HTLC, NULL, 0, 0, onionreply, NULL, &we_filled, in.key.id, in.key.channel, REMOTE, &in.payment_hash, in.cltv_expiry, in.msat)),
 	    "Update HTLC with failonion failed");
 	CHECK_MSG(
-		transaction_wrap(w->db, wallet_htlc_update(w, in.dbid, SENT_REMOVE_HTLC, NULL, 0, WIRE_INVALID_ONION_VERSION, NULL, NULL, &we_filled)),
+		transaction_wrap(w->db, wallet_htlc_update(w, in.dbid, SENT_REMOVE_HTLC, NULL, 0, WIRE_INVALID_ONION_VERSION, NULL, NULL, &we_filled, in.key.id, in.key.channel, REMOTE, &in.payment_hash, in.cltv_expiry, in.msat)),
 	    "Update HTLC with failcode failed");
 
 	CHECK_MSG(transaction_wrap(w->db, wallet_htlc_save_out(w, chan, &out)),
 		  tal_fmt(ctx, "Save htlc_out failed: %s", wallet_err));
 	CHECK_MSG(out.dbid != 0, "HTLC DB ID was not set.");
 
-	CHECK_MSG(!transaction_wrap(w->db, wallet_htlc_save_out(w, chan, &out)),
+	out2 = out;
+	CHECK_MSG(!transaction_wrap(w->db, wallet_htlc_save_out(w, chan, &out2)),
 		  "Saving two HTLCs with the same data must not succeed.");
 	CHECK(wallet_err);
 	wallet_err = tal_free(wallet_err);
 	CHECK_MSG(
-		transaction_wrap(w->db, wallet_htlc_update(w, out.dbid, SENT_ADD_ACK_REVOCATION, NULL, 0, 0, NULL, tal_arrz(tmpctx, u8, 100), &we_filled)),
+		transaction_wrap(w->db, wallet_htlc_update(w, out.dbid, SENT_ADD_ACK_REVOCATION, NULL, 0, 0, NULL, tal_arrz(tmpctx, u8, 100), &we_filled, out.key.id, out.key.channel, REMOTE, &out.payment_hash, out.cltv_expiry, out.msat)),
 	    "Update outgoing HTLC with failmsg failed");
 
 	/* Attempt to load them from the DB again */
@@ -2340,6 +2346,8 @@ int main(int argc, const char *argv[])
 	ld = tal(tmpctx, struct lightningd);
 	ld->config = test_config;
 	ld->hsm_capabilities = NULL;
+	memset(&ld->indexes[WAIT_SUBSYSTEM_HTLCS], 0,
+	       sizeof(ld->indexes[WAIT_SUBSYSTEM_HTLCS]));
 
 	/* Only elements in ld we should access */
 	ld->peers = tal(ld, struct peer_node_id_map);