Fix bkpr-listincome start_time and end_time for consolidated onchain fees, too.

plugins/bkpr/incomestmt.c

@@ -276,7 +276,7 @@ static struct onchain_fee **find_consolidated_fees(const tal_t *ctx,
 	struct onchain_fee **fee_sums
 		= tal_arr(ctx, struct onchain_fee *, 0);
 
-	sums = calculate_onchain_fee_sums(ctx, bkpr);
+	sums = calculate_onchain_fee_sums(ctx, bkpr, start_time, end_time);
 
 	for (size_t i = 0; i < tal_count(sums); i++) {
 		/* Find the last matching feerate's data */
@@ -290,11 +290,7 @@ static struct onchain_fee **find_consolidated_fees(const tal_t *ctx,
 		fee->debit = AMOUNT_MSAT(0);
 		fee->acct_name = tal_steal(fee, sums[i]->acct_name);
 		fee->txid = *sums[i]->txid;
-
-		fee->timestamp =
-			onchain_fee_last_timestamp(bkpr, sums[i]->acct_name,
-						   sums[i]->txid);
-
+		fee->timestamp = sums[i]->last_timestamp;
 		tal_arr_expand(&fee_sums, fee);
 	}
 

plugins/bkpr/onchain_fee.c

@@ -366,10 +366,13 @@ static struct fee_sum **fee_sums_by_txid_and_account(const tal_t *ctx,
 			sum->txid = tal_dup(sum, struct bitcoin_txid,
 					    &ofs[i]->txid);
 			credit = debit = AMOUNT_MSAT(0);
+			sum->last_timestamp = 0;
 		}
 		ok = amount_msat_accumulate(&credit, ofs[i]->credit);
 		assert(ok);
 		ok = amount_msat_accumulate(&debit, ofs[i]->debit);
+		if (ofs[i]->timestamp > sum->last_timestamp)
+			sum->last_timestamp = ofs[i]->timestamp;
 	}
 
 	/* Final, if any */
@@ -380,11 +383,13 @@ static struct fee_sum **fee_sums_by_txid_and_account(const tal_t *ctx,
 }
 
 struct fee_sum **calculate_onchain_fee_sums(const tal_t *ctx,
-					    const struct bkpr *bkpr)
+					    const struct bkpr *bkpr,
+					    u64 start_time,
+					    u64 end_time)
 {
 	struct onchain_fee **ofs;
 
-	ofs = list_chain_fees(tmpctx, bkpr);
+	ofs = list_chain_fees_timebox(tmpctx, bkpr, start_time, end_time);
 	return fee_sums_by_txid_and_account(ctx, ofs);
 }
 
@@ -679,24 +684,6 @@ struct fee_sum **find_account_onchain_fees(const tal_t *ctx,
 	return fee_sums_by_txid_and_account(ctx, ofs);
 }
 
-/* FIXME: Put this value into fee_sums! */
-u64 onchain_fee_last_timestamp(const struct bkpr *bkpr,
-			       const char *acct_name,
-			       const struct bitcoin_txid *txid)
-{
-	struct onchain_fee **ofs;
-	u64 timestamp = 0;
-
-	ofs = account_get_chain_fees(tmpctx, bkpr, acct_name);
-	for (size_t i = 0; i < tal_count(ofs); i++) {
-		if (!bitcoin_txid_eq(&ofs[i]->txid, txid))
-			continue;
-		if (ofs[i]->timestamp > timestamp)
-			timestamp = ofs[i]->timestamp;
-	}
-	return timestamp;
-}
-
 /* If we're freeing the entire hash table, remove destructors from
  * individual entries! */
 static void ofees_hash_destroy(struct ofees_hash *ofees_hash)

plugins/bkpr/onchain_fee.h

@@ -65,12 +65,9 @@ struct fee_sum **find_account_onchain_fees(const tal_t *ctx,
 
 /* Final all the onchain fees */
 struct fee_sum **calculate_onchain_fee_sums(const tal_t *ctx,
-					    const struct bkpr *bkpr);
-
-/* Find the last timestamp for the onchain fees for this txid + account */
-u64 onchain_fee_last_timestamp(const struct bkpr *bkpr,
-			       const char *acct_name,
-			       const struct bitcoin_txid *txid);
+					    const struct bkpr *bkpr,
+					    u64 start_time,
+					    u64 end_time);
 
 /* Update our onchain fees now? */
 char *maybe_update_onchain_fees(const tal_t *ctx,

plugins/bkpr/recorder.h

@@ -18,6 +18,7 @@ struct fee_sum {
 	const char *acct_name;
 	struct bitcoin_txid *txid;
 	struct amount_msat fees_paid;
+	u64 last_timestamp;
 };
 
 struct txo_pair {

tests/test_bookkeeper.py

@@ -1162,3 +1162,34 @@ def test_migration_no_bkpr(node_factory, bitcoind):
                           'is_rebalance': False,
                           'tag': 'journal_entry',
                           'type': 'channel'}]
+
+
+@unittest.skipIf(TEST_NETWORK != 'regtest', "External wallet support doesn't work with elements yet.")
+def test_listincome_timebox(node_factory, bitcoind):
+    l1 = node_factory.get_node()
+    addr = l1.rpc.newaddr()['bech32']
+
+    amount = 1111111
+    bitcoind.rpc.sendtoaddress(addr, amount / 10**8)
+
+    bitcoind.generate_block(1, wait_for_mempool=1)
+    wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 1)
+
+    waddr = bitcoind.rpc.getnewaddress()
+
+    # Ok, now we send some funds to an external address, get change.
+    l1.rpc.withdraw(waddr, amount // 2)
+    bitcoind.generate_block(1, wait_for_mempool=1)
+    wait_for(lambda: len(l1.rpc.listfunds(spent=True)['outputs']) == 2)
+
+    first_one = int(time.time())
+    time.sleep(2)
+
+    # Do another one, make sure we don't see it if we filter by timestamp.
+    bitcoind.rpc.sendtoaddress(addr, amount / 10**8)
+
+    bitcoind.generate_block(1, wait_for_mempool=1)
+    wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2)
+
+    incomes = l1.rpc.bkpr_listincome(end_time=first_one)['income_events']
+    assert [i for i in incomes if i['timestamp'] > first_one] == []