lntest+itest: strictly define the behavior of MineBlocks

This commit adds more assertion to `MineBlocks` so the caller won't
misuse it.

itest/lnd_hold_invoice_force_test.go

@@ -93,7 +93,7 @@ func testHoldInvoiceForceClose(ht *lntest.HarnessTest) {
 	// TODO(yy): fix block height asymmetry among all the subsystems.
 	//
 	// We first mine enough blocks to trigger an invoice cancelation.
-	ht.MineBlocks(blocksTillCancel)
+	ht.MineBlocks(int(blocksTillCancel))
 
 	// Wait for the nodes to be synced.
 	ht.WaitForBlockchainSync(alice)
@@ -133,7 +133,7 @@ func testHoldInvoiceForceClose(ht *lntest.HarnessTest) {
 	// happen in bitcoind backend, as Alice's CNCT was syncing way faster
 	// than Bob's INVC, causing the channel being force closed before the
 	// invoice cancelation message was received by Alice.
-	ht.MineBlocks(blocksTillForce - blocksTillCancel)
+	ht.MineBlocks(int(blocksTillForce - blocksTillCancel))
 
 	// Wait for the nodes to be synced.
 	ht.WaitForBlockchainSync(alice)

itest/lnd_multi-hop_test.go

@@ -243,7 +243,7 @@ func runMultiHopHtlcLocalTimeout(ht *lntest.HarnessTest,
 	numBlocks := padCLTV(
 		uint32(finalCltvDelta - lncfg.DefaultOutgoingBroadcastDelta),
 	)
-	ht.MineBlocks(numBlocks)
+	ht.MineBlocks(int(numBlocks))
 
 	// Bob's force close transaction should now be found in the mempool.
 	ht.Miner.AssertNumTxsInMempool(1)

itest/lnd_open_channel_test.go

@@ -58,7 +58,7 @@ func testOpenChannelAfterReorg(ht *lntest.HarnessTest) {
 	// and our new miner mine 15. This will also confirm our pending
 	// channel on the original miner's chain, which should be considered
 	// open.
-	block := ht.MineBlocks(10)[0]
+	block := ht.MineBlocksAndAssertNumTxes(10, 1)[0]
 	ht.Miner.AssertTxInBlock(block, fundingTxID)
 	_, err = tempMiner.Client.Generate(15)
 	require.NoError(ht, err, "unable to generate blocks")

itest/lnd_recovery_test.go

@@ -263,7 +263,7 @@ func testOnchainFundRecovery(ht *lntest.HarnessTest) {
 		txid := ht.Miner.AssertNumTxsInMempool(1)[0]
 		require.Equal(ht, txid.String(), resp.Txid)
 
-		block := ht.MineBlocks(1)[0]
+		block := ht.MineBlocksAndAssertNumTxes(1, 1)[0]
 		ht.Miner.AssertTxInBlock(block, txid)
 	}
 	restoreCheckBalance(finalBalance, 9, 20, promptChangeAddr)

itest/lnd_route_blinding_test.go

@@ -1021,7 +1021,7 @@ func testErrorHandlingOnChainFailure(ht *lntest.HarnessTest) {
 	// value.
 	info := ht.Bob.RPC.GetInfo()
 	target := carolHTLC.IncomingExpiry - info.BlockHeight
-	ht.MineBlocks(target)
+	ht.MineBlocks(int(target))
 
 	// Wait for Bob's timeout transaction in the mempool, since we've
 	// suspended Carol we don't need to account for her commitment output

itest/lnd_sweep_test.go

@@ -872,7 +872,7 @@ func testSweepHTLCs(ht *lntest.HarnessTest) {
 	numBlocks := padCLTV(uint32(
 		invoiceReqHold.CltvExpiry - lncfg.DefaultOutgoingBroadcastDelta,
 	))
-	ht.MineBlocks(numBlocks)
+	ht.MineBlocks(int(numBlocks))
 
 	// Before we mine empty blocks to check the RBF behavior, we need to be
 	// aware that Bob's incoming HTLC will expire before his outgoing HTLC
@@ -1804,7 +1804,7 @@ func createSimpleNetwork(ht *lntest.HarnessTest, nodeCfg []string,
 	}
 
 	// Mine 1 block to get the above coins confirmed.
-	ht.MineBlocks(1)
+	ht.MineBlocksAndAssertNumTxes(1, numNodes-1)
 
 	// Open channels in batch to save blocks mined.
 	reqs := make([]*lntest.OpenChannelRequest, 0, len(nodes)-1)

lntest/harness.go

@@ -1414,24 +1414,24 @@ func (h *HarnessTest) fundCoins(amt btcutil.Amount, target *node.HarnessNode,
 		pkScriptStr := utxos[0].PkScript
 		require.Equal(h, pkScriptStr, expPkScriptStr,
 			"pkscript mismatch")
+
+		expectedBalance := btcutil.Amount(
+			initialBalance.UnconfirmedBalance,
+		) + amt
+		h.WaitForBalanceUnconfirmed(target, expectedBalance)
 	}
 
 	// If the transaction should remain unconfirmed, then we'll wait until
 	// the target node's unconfirmed balance reflects the expected balance
 	// and exit.
-	if !confirmed && !h.IsNeutrinoBackend() {
-		expectedBalance := btcutil.Amount(
-			initialBalance.UnconfirmedBalance,
-		) + amt
-		h.WaitForBalanceUnconfirmed(target, expectedBalance)
-
+	if !confirmed {
 		return
 	}
 
 	// Otherwise, we'll generate 1 new blocks to ensure the output gains a
 	// sufficient number of confirmations and wait for the balance to
 	// reflect what's expected.
-	h.MineBlocks(1)
+	h.MineBlocksAndAssertNumTxes(1, 1)
 
 	expectedBalance := btcutil.Amount(initialBalance.ConfirmedBalance) + amt
 	h.WaitForBalanceConfirmed(target, expectedBalance)

lntest/harness_miner.go

@@ -3,6 +3,7 @@ package lntest
 import (
 	"fmt"
 
+	"github.com/btcsuite/btcd/blockchain"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/lightningnetwork/lnd/lntest/node"
 	"github.com/lightningnetwork/lnd/lntest/wait"
@@ -10,22 +11,46 @@ import (
 )
 
 // MineBlocks mines blocks and asserts all active nodes have synced to the
-// chain.
+// chain. It assumes no txns are expected in the blocks.
 //
-// NOTE: this differs from miner's `MineBlocks` as it requires the nodes to be
-// synced.
-func (h *HarnessTest) MineBlocks(num uint32) []*wire.MsgBlock {
-	require.Less(h, num, uint32(maxBlocksAllowed),
-		"too many blocks to mine")
+// NOTE: Use `MineBlocksAndAssertNumTxes` if you expect txns in the blocks. Use
+// `MineEmptyBlocks` if you want to make sure that txns stay unconfirmed.
+func (h *HarnessTest) MineBlocks(num int) {
+	require.Less(h, num, maxBlocksAllowed, "too many blocks to mine")
 
-	// Mining the blocks slow to give `lnd` more time to sync.
-	blocks := h.Miner.MineBlocksSlow(num)
+	// Mine num of blocks.
+	for i := 0; i < num; i++ {
+		block := h.Miner.MineBlocks(1)[0]
 
-	// Make sure all the active nodes are synced.
-	bestBlock := blocks[len(blocks)-1]
-	h.AssertActiveNodesSyncedTo(bestBlock)
+		// Check the block doesn't have any txns except the coinbase.
+		if len(block.Transactions) <= 1 {
+			// Make sure all the active nodes are synced.
+			h.AssertActiveNodesSyncedTo(block)
 
-	return blocks
+			// Mine the next block.
+			continue
+		}
+
+		// Create a detailed description.
+		desc := fmt.Sprintf("block %v has %d txns:\n",
+			block.BlockHash(), len(block.Transactions)-1)
+
+		// Print all the txns except the coinbase.
+		for _, tx := range block.Transactions {
+			if blockchain.IsCoinBaseTx(tx) {
+				continue
+			}
+
+			desc += fmt.Sprintf("%v\n", tx.TxHash())
+		}
+
+		desc += "Consider using `MineBlocksAndAssertNumTxes` if you " +
+			"expect txns, or `MineEmptyBlocks` if you want to " +
+			"keep the txns unconfirmed."
+
+		// Raise an error if the block has txns.
+		require.Fail(h, "MineBlocks", desc)
+	}
 }
 
 // MineEmptyBlocks mines a given number of empty blocks.
@@ -49,10 +74,6 @@ func (h *HarnessTest) MineEmptyBlocks(num int) []*wire.MsgBlock {
 //
 // NOTE: this differs from miner's `MineBlocks` as it requires the nodes to be
 // synced.
-//
-// TODO(yy): change the APIs to force callers to think about blocks and txns:
-// - MineBlocksAndAssertNumTxes -> MineBlocks
-// - add more APIs to mine a single tx.
 func (h *HarnessTest) MineBlocksAndAssertNumTxes(num uint32,
 	numTxs int) []*wire.MsgBlock {