lnwallet/test: convert the tx publish tests to sub-tests

lnwallet/test/test_interface.go

@@ -1721,139 +1721,128 @@ func testPublishTransaction(r *rpctest.Harness,
 	keyDesc, err := alice.DeriveNextKey(keychain.KeyFamilyMultiSig)
 	require.NoError(t, err, "unable to obtain public key")
 
-	// We will first check that publishing a transaction already in the
+	t.Run("no_error_on_duplicate_tx", func(t *testing.T) {
-	// mempool does NOT return an error. Create the tx.
+		// We will first check that publishing a transaction already in
+		// the mempool does NOT return an error. Create the tx.
 		tx1 := newTx(t, r, keyDesc.PubKey, alice, false)
 
 		// Publish the transaction.
 		err = alice.PublishTransaction(tx1, labels.External)
-	require.NoError(t, err, "unable to publish")
+		require.NoError(t, err)
 
 		txid1 := tx1.TxHash()
 		err = waitForMempoolTx(r, &txid1)
 		require.NoError(t, err, "tx not relayed to miner")
 
-	// Publish the exact same transaction again. This should not return an
+		// Publish the exact same transaction again. This should not
-	// error, even though the transaction is already in the mempool.
+		// return an error, even though the transaction is already in
+		// the mempool.
 		err = alice.PublishTransaction(tx1, labels.External)
-	require.NoError(t, err, "unable to publish")
+		require.NoError(t, err)
 
 		// Mine the transaction.
-	if _, err := r.Client.Generate(1); err != nil {
+		_, err := r.Client.Generate(1)
-		t.Fatalf("unable to generate block: %v", err)
+		require.NoError(t, err)
-	}
+	})
 
-	// We'll now test that we don't get an error if we try to publish a
+	t.Run("no_error_on_minged_tx", func(t *testing.T) {
-	// transaction that is already mined.
+		// We'll now test that we don't get an error if we try to
+		// publish a transaction that is already mined.
 		//
-	// Create a new transaction. We must do this to properly test the
+		// Create a new transaction. We must do this to properly test
-	// reject messages from our peers. They might only send us a reject
+		// the reject messages from our peers. They might only send us
-	// message for a given tx once, so we create a new to make sure it is
+		// a reject message for a given tx once, so we create a new to
-	// not just immediately rejected.
+		// make sure it is not just immediately rejected.
 		tx2 := newTx(t, r, keyDesc.PubKey, alice, false)
 
 		// Publish this tx.
 		err = alice.PublishTransaction(tx2, labels.External)
-	require.NoError(t, err, "unable to publish")
+		require.NoError(t, err)
 
 		// Mine the transaction.
-	if err := mineAndAssert(r, tx2); err != nil {
+		err := mineAndAssert(r, tx2)
-		t.Fatalf("unable to mine tx: %v", err)
+		require.NoError(t, err)
-	}
 
-	// Publish the transaction again. It is already mined, and we don't
+		// Publish the transaction again. It is already mined, and we
-	// expect this to return an error.
+		// don't expect this to return an error.
 		err = alice.PublishTransaction(tx2, labels.External)
-	require.NoError(t, err, "unable to publish")
+		require.NoError(t, err)
+	})
 
-	// We'll do the next mempool check on both RBF and non-RBF enabled
+	// We'll do the next mempool check on both RBF and non-RBF
-	// transactions.
+	// enabled transactions.
 	var (
-		txFee         = btcutil.Amount(0.005 * btcutil.SatoshiPerBitcoin)
+		txFee = btcutil.Amount(
+			0.005 * btcutil.SatoshiPerBitcoin,
+		)
 		tx3, tx3Spend *wire.MsgTx
 	)
-
+	t.Run("rbf_tests", func(t *testing.T) {
 		for _, rbf := range []bool{false, true} {
-		// Now we'll try to double spend an output with a different
+			// Now we'll try to double spend an output with a
-		// transaction. Create a new tx and publish it. This is the
+			// different transaction. Create a new tx and publish
-		// output we'll try to double spend.
+			// it. This is the output we'll try to double spend.
 			tx3 = newTx(t, r, keyDesc.PubKey, alice, false)
 			err := alice.PublishTransaction(tx3, labels.External)
-		if err != nil {
+			require.NoError(t, err)
-			t.Fatalf("unable to publish: %v", err)
-		}
 
 			// Mine the transaction.
-		if err := mineAndAssert(r, tx3); err != nil {
+			err = mineAndAssert(r, tx3)
-			t.Fatalf("unable to mine tx: %v", err)
+			require.NoError(t, err)
-		}
 
-		// Now we create a transaction that spends the output from the
+			// Now we create a transaction that spends the output
-		// tx just mined.
+			// from the tx just mined.
 			tx4, err := txFromOutput(
 				tx3, alice.Cfg.Signer, keyDesc.PubKey,
 				keyDesc.PubKey, txFee, rbf,
 			)
-		if err != nil {
+			require.NoError(t, err)
-			t.Fatal(err)
-		}
 
 			// This should be accepted into the mempool.
 			err = alice.PublishTransaction(tx4, labels.External)
-		if err != nil {
+			require.NoError(t, err)
-			t.Fatalf("unable to publish: %v", err)
-		}
 
-		// Keep track of the last successfully published tx to spend
+			// Keep track of the last successfully published tx to
-		// tx3.
+			// spend tx3.
 			tx3Spend = tx4
 
 			txid4 := tx4.TxHash()
 			err = waitForMempoolTx(r, &txid4)
-		if err != nil {
+			require.NoError(t, err, "tx not relayed to miner")
-			t.Fatalf("tx not relayed to miner: %v", err)
-		}
 
-		// Create a new key we'll pay to, to ensure we create a unique
+			// Create a new key we'll pay to, to ensure we create a
-		// transaction.
+			// unique transaction.
 			keyDesc2, err := alice.DeriveNextKey(
 				keychain.KeyFamilyMultiSig,
 			)
-		if err != nil {
+			require.NoError(t, err, "unable to obtain public key")
-			t.Fatalf("unable to obtain public key: %v", err)
-		}
 
-		// Create a new transaction that spends the output from tx3,
+			// Create a new transaction that spends the output from
-		// and that pays to a different address. We expect this to be
+			// tx3, and that pays to a different address. We expect
-		// rejected because it is a double spend.
+			// this to be rejected because it is a double spend.
 			tx5, err := txFromOutput(
 				tx3, alice.Cfg.Signer, keyDesc.PubKey,
 				keyDesc2.PubKey, txFee, rbf,
 			)
-		if err != nil {
+			require.NoError(t, err)
-			t.Fatal(err)
-		}
 
 			err = alice.PublishTransaction(tx5, labels.External)
-		if err != lnwallet.ErrDoubleSpend {
+			require.ErrorIs(t, err, lnwallet.ErrDoubleSpend)
-			t.Fatalf("expected ErrDoubleSpend, got: %v", err)
+
-		}
+			// Create another transaction that spends the same
+			// output, but has a higher fee. We expect also this tx
+			// to be rejected for non-RBF enabled transactions,
+			// while it should succeed otherwise.
+			pubKey3, err := alice.DeriveNextKey(
+				keychain.KeyFamilyMultiSig,
+			)
+			require.NoError(t, err, "unable to obtain public key")
 
-		// Create another transaction that spends the same output, but
-		// has a higher fee. We expect also this tx to be rejected for
-		// non-RBF enabled transactions, while it should succeed
-		// otherwise.
-		pubKey3, err := alice.DeriveNextKey(keychain.KeyFamilyMultiSig)
-		if err != nil {
-			t.Fatalf("unable to obtain public key: %v", err)
-		}
 			tx6, err := txFromOutput(
 				tx3, alice.Cfg.Signer, keyDesc.PubKey,
 				pubKey3.PubKey, 2*txFee, rbf,
 			)
-		if err != nil {
+			require.NoError(t, err)
-			t.Fatal(err)
-		}
 
 			// Expect rejection in non-RBF case.
 			expErr := lnwallet.ErrDoubleSpend
@@ -1863,45 +1852,40 @@ func testPublishTransaction(r *rpctest.Harness,
 				tx3Spend = tx6
 			}
 			err = alice.PublishTransaction(tx6, labels.External)
-		if err != expErr {
+			require.ErrorIs(t, err, expErr)
-			t.Fatalf("expected ErrDoubleSpend, got: %v", err)
-		}
 
 			// Mine the tx spending tx3.
-		if err := mineAndAssert(r, tx3Spend); err != nil {
+			err = mineAndAssert(r, tx3Spend)
-			t.Fatalf("unable to mine tx: %v", err)
+			require.NoError(t, err)
-		}
 		}
+	})
 
-	// At last we try to spend an output already spent by a confirmed
+	t.Run("tx_double_spend", func(t *testing.T) {
-	// transaction.
+		// At last we try to spend an output already spent by a
-	// TODO(halseth): we currently skip this test for neutrino, as the
+		// confirmed transaction.
-	// backing btcd node will consider the tx being an orphan, and will
+		//
-	// accept it. Should look into if this is the behavior also for
+		// TODO(halseth): we currently skip this test for neutrino, as
-	// bitcoind, and update test accordingly.
+		// the backing btcd node will consider the tx being an orphan,
+		// and will accept it. Should look into if this is the behavior
+		// also for bitcoind, and update test accordingly.
 		if alice.BackEnd() != "neutrino" {
 			// Create another tx spending tx3.
 			pubKey4, err := alice.DeriveNextKey(
 				keychain.KeyFamilyMultiSig,
 			)
-		if err != nil {
+			require.NoError(t, err, "unable to obtain public key")
-			t.Fatalf("unable to obtain public key: %v", err)
+
-		}
 			tx7, err := txFromOutput(
 				tx3, alice.Cfg.Signer, keyDesc.PubKey,
 				pubKey4.PubKey, txFee, false,
 			)
-
+			require.NoError(t, err)
-		if err != nil {
-			t.Fatal(err)
-		}
 
 			// Expect rejection.
 			err = alice.PublishTransaction(tx7, labels.External)
-		if err != lnwallet.ErrDoubleSpend {
+			require.ErrorIs(t, err, lnwallet.ErrDoubleSpend)
-			t.Fatalf("expected ErrDoubleSpend, got: %v", err)
-		}
 		}
+	})
 }
 
 func testSignOutputUsingTweaks(r *rpctest.Harness,