multi: specify start and end height for ListTransactionDetails

Add start and end height parameters to the rpc and cli GetTransactions
endpoints. Default to returning all transactions from genesis to tip,
including unconfirmed transactions to maintain backwards compatibility.

lnwallet/btcwallet/btcwallet.go

@@ -26,6 +26,10 @@ import (
 
 const (
 	defaultAccount = uint32(waddrmgr.DefaultAccountNum)
+
+	// UnconfirmedHeight is the special case end height that is used to
+	// obtain unconfirmed transactions from ListTransactionDetails.
+	UnconfirmedHeight int32 = -1
 )
 
 var (
@@ -569,20 +573,22 @@ func unminedTransactionsToDetail(
 }
 
 // ListTransactionDetails returns a list of all transactions which are
-// relevant to the wallet.
+// relevant to the wallet. It takes inclusive start and end height to allow
+// paginated queries. Unconfirmed transactions can be included in the query
+// by providing endHeight = UnconfirmedHeight (= -1).
 //
 // This is a part of the WalletController interface.
-func (b *BtcWallet) ListTransactionDetails() ([]*lnwallet.TransactionDetail, error) {
+func (b *BtcWallet) ListTransactionDetails(startHeight,
+	endHeight int32) ([]*lnwallet.TransactionDetail, error) {
+
 	// Grab the best block the wallet knows of, we'll use this to calculate
 	// # of confirmations shortly below.
 	bestBlock := b.wallet.Manager.SyncedTo()
 	currentHeight := bestBlock.Height
 
-	// We'll attempt to find all unconfirmed transactions (height of -1),
-	// as well as all transactions that are known to have confirmed at this
-	// height.
-	start := base.NewBlockIdentifierFromHeight(0)
-	stop := base.NewBlockIdentifierFromHeight(-1)
+	// We'll attempt to find all transactions from start to end height.
+	start := base.NewBlockIdentifierFromHeight(startHeight)
+	stop := base.NewBlockIdentifierFromHeight(endHeight)
 	txns, err := b.wallet.GetTransactions(start, stop, nil)
 	if err != nil {
 		return nil, err

lnwallet/interface.go

@@ -199,8 +199,14 @@ type WalletController interface {
 	ListUnspentWitness(minconfirms, maxconfirms int32) ([]*Utxo, error)
 
 	// ListTransactionDetails returns a list of all transactions which are
-	// relevant to the wallet.
-	ListTransactionDetails() ([]*TransactionDetail, error)
+	// relevant to the wallet over [startHeight;endHeight]. If start height
+	// is greater than end height, the transactions will be retrieved in
+	// reverse order. To include unconfirmed transactions, endHeight should
+	// be set to the special value -1. This will return transactions from
+	// the tip of the chain until the start height (inclusive) and
+	// unconfirmed transactions.
+	ListTransactionDetails(startHeight,
+		endHeight int32) ([]*TransactionDetail, error)
 
 	// LockOutpoint marks an outpoint as locked meaning it will no longer
 	// be deemed as eligible for coin selection. Locking outputs are

lnwallet/interface_test.go

@@ -206,7 +206,7 @@ func assertTxInWallet(t *testing.T, w *lnwallet.LightningWallet,
 	// We'll fetch all of our transaction and go through each one until
 	// finding the expected transaction with its expected confirmation
 	// status.
-	txs, err := w.ListTransactionDetails()
+	txs, err := w.ListTransactionDetails(0, btcwallet.UnconfirmedHeight)
 	if err != nil {
 		t.Fatalf("unable to retrieve transactions: %v", err)
 	}
@@ -1101,6 +1101,12 @@ func testListTransactionDetails(miner *rpctest.Harness,
 		txids[*txid] = struct{}{}
 	}
 
+	// Get the miner's current best block height before we mine blocks.
+	_, startHeight, err := miner.Node.GetBestBlock()
+	if err != nil {
+		t.Fatalf("cannot get best block: %v", err)
+	}
+
 	// Generate 10 blocks to mine all the transactions created above.
 	const numBlocksMined = 10
 	blocks, err := miner.Node.Generate(numBlocksMined)
@@ -1108,12 +1114,22 @@ func testListTransactionDetails(miner *rpctest.Harness,
 		t.Fatalf("unable to mine blocks: %v", err)
 	}
 
-	// Next, fetch all the current transaction details.
+	// Our new best block height should be our start height + the number of
+	// blocks we just mined.
+	chainTip := startHeight + numBlocksMined
+
+	// Next, fetch all the current transaction details. We should find all
+	// of our transactions between our start height before we generated
+	// blocks, and our end height, which is the chain tip. This query does
+	// not include unconfirmed transactions, since all of our transactions
+	// should be confirmed.
 	err = waitForWalletSync(miner, alice)
 	if err != nil {
 		t.Fatalf("Couldn't sync Alice's wallet: %v", err)
 	}
-	txDetails, err := alice.ListTransactionDetails()
+	txDetails, err := alice.ListTransactionDetails(
+		startHeight, chainTip,
+	)
 	if err != nil {
 		t.Fatalf("unable to fetch tx details: %v", err)
 	}
@@ -1219,10 +1235,13 @@ func testListTransactionDetails(miner *rpctest.Harness,
 		t.Fatalf("Couldn't sync Alice's wallet: %v", err)
 	}
 
-	// We should be able to find the transaction above in the set of
-	// returned transactions, and it should have a confirmation of -1,
-	// indicating that it's not yet mined.
-	txDetails, err = alice.ListTransactionDetails()
+	// Query our wallet for transactions from the chain tip, including
+	// unconfirmed transactions. The transaction above should be included
+	// with a confirmation height of 0, indicating that it has not been
+	// mined yet.
+	txDetails, err = alice.ListTransactionDetails(
+		chainTip, btcwallet.UnconfirmedHeight,
+	)
 	if err != nil {
 		t.Fatalf("unable to fetch tx details: %v", err)
 	}
@@ -1259,18 +1278,27 @@ func testListTransactionDetails(miner *rpctest.Harness,
 		t.Fatalf("unable to find mempool tx in tx details!")
 	}
 
-	burnBlock, err := miner.Node.Generate(1)
+	// Generate one block for our transaction to confirm in.
+	var numBlocks int32 = 1
+	burnBlock, err := miner.Node.Generate(uint32(numBlocks))
 	if err != nil {
 		t.Fatalf("unable to mine block: %v", err)
 	}
 
+	// Progress our chain tip by the number of blocks we have just mined.
+	chainTip += numBlocks
+
 	// Fetch the transaction details again, the new transaction should be
-	// shown as debiting from the wallet's balance.
+	// shown as debiting from the wallet's balance. Start and end height
+	// are inclusive, so we use chainTip for both parameters to get only
+	// transactions from the last block.
 	err = waitForWalletSync(miner, alice)
 	if err != nil {
 		t.Fatalf("Couldn't sync Alice's wallet: %v", err)
 	}
-	txDetails, err = alice.ListTransactionDetails()
+	txDetails, err = alice.ListTransactionDetails(
+		chainTip, chainTip,
+	)
 	if err != nil {
 		t.Fatalf("unable to fetch tx details: %v", err)
 	}
@@ -1302,6 +1330,30 @@ func testListTransactionDetails(miner *rpctest.Harness,
 	if !burnTxFound {
 		t.Fatal("tx burning btc not found")
 	}
+
+	// Generate a block which has no wallet transactions in it.
+	chainTip += numBlocks
+	_, err = miner.Node.Generate(uint32(numBlocks))
+	if err != nil {
+		t.Fatalf("unable to mine block: %v", err)
+	}
+
+	err = waitForWalletSync(miner, alice)
+	if err != nil {
+		t.Fatalf("Couldn't sync Alice's wallet: %v", err)
+	}
+
+	// Query for transactions only in the latest block. We do not expect
+	// any transactions to be returned.
+	txDetails, err = alice.ListTransactionDetails(
+		chainTip, chainTip,
+	)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(txDetails) != 0 {
+		t.Fatalf("expected 0 transactions, got: %v", len(txDetails))
+	}
 }
 
 func testTransactionSubscriptions(miner *rpctest.Harness,