lntemp+itest: refactor node restart process

This commit refactors how the node restart is done. It removes the usage
of `HasSeed` and replaces it with `SkipUnlock` for clarity.

lntemp/harness.go

@@ -486,13 +486,33 @@ func (h *HarnessTest) SuspendNode(node *node.HarnessNode) func() error {
 	}
 }
 
-// RestartNode restarts a given node and asserts.
-func (h *HarnessTest) RestartNode(hn *node.HarnessNode,
+// RestartNode restarts a given node, unlocks it and asserts it's successfully
+// started.
+func (h *HarnessTest) RestartNode(hn *node.HarnessNode) {
+	err := h.manager.restartNode(h.runCtx, hn, nil)
+	require.NoErrorf(h, err, "failed to restart node %s", hn.Name())
+
+	err = h.manager.unlockNode(hn)
+	require.NoErrorf(h, err, "failed to unlock node %s", hn.Name())
+
+	if !hn.Cfg.SkipUnlock {
+		// Give the node some time to catch up with the chain before we
+		// continue with the tests.
+		h.WaitForBlockchainSync(hn)
+	}
+}
+
+// RestartNodeWithChanBackups restarts a given node with the specified channel
+// backups.
+func (h *HarnessTest) RestartNodeWithChanBackups(hn *node.HarnessNode,
 	chanBackups ...*lnrpc.ChanBackupSnapshot) {
 
-	err := h.manager.restartNode(h.runCtx, hn, nil, chanBackups...)
+	err := h.manager.restartNode(h.runCtx, hn, nil)
 	require.NoErrorf(h, err, "failed to restart node %s", hn.Name())
 
+	err = h.manager.unlockNode(hn, chanBackups...)
+	require.NoErrorf(h, err, "failed to unlock node %s", hn.Name())
+
 	// Give the node some time to catch up with the chain before we
 	// continue with the tests.
 	h.WaitForBlockchainSync(hn)
@@ -503,7 +523,7 @@ func (h *HarnessTest) RestartNodeWithExtraArgs(hn *node.HarnessNode,
 	extraArgs []string) {
 
 	hn.SetExtraArgs(extraArgs)
-	h.RestartNode(hn, nil)
+	h.RestartNode(hn)
 }
 
 // NewNodeWithSeed fully initializes a new HarnessNode after creating a fresh
@@ -538,7 +558,7 @@ func (h *HarnessTest) newNodeWithSeed(name string,
 
 	// Start the node with seed only, which will only create the `State`
 	// and `WalletUnlocker` clients.
-	err = node.StartWithSeed(h.runCtx)
+	err = node.StartWithNoAuth(h.runCtx)
 	require.NoErrorf(h, err, "failed to start node %s", node.Name())
 
 	// Generate a new seed.
@@ -581,7 +601,7 @@ func (h *HarnessTest) RestoreNodeWithSeed(name string, extraArgs []string,
 
 	// Start the node with seed only, which will only create the `State`
 	// and `WalletUnlocker` clients.
-	err = node.StartWithSeed(h.runCtx)
+	err = node.StartWithNoAuth(h.runCtx)
 	require.NoErrorf(h, err, "failed to start node %s", node.Name())
 
 	// Create the wallet.
@@ -1344,6 +1364,9 @@ func (h *HarnessTest) RestartNodeAndRestoreDB(hn *node.HarnessNode) {
 	err := h.manager.restartNode(h.runCtx, hn, cb)
 	require.NoErrorf(h, err, "failed to restart node %s", hn.Name())
 
+	err = h.manager.unlockNode(hn)
+	require.NoErrorf(h, err, "failed to unlock node %s", hn.Name())
+
 	// Give the node some time to catch up with the chain before we
 	// continue with the tests.
 	h.WaitForBlockchainSync(hn)

lntemp/harness_node_manager.go

@@ -71,7 +71,7 @@ func (nm *nodeManager) nextNodeID() uint32 {
 // node can be used immediately. Otherwise, the node will require an additional
 // initialization phase where the wallet is either created or restored.
 func (nm *nodeManager) newNode(t *testing.T, name string, extraArgs []string,
-	password []byte, useSeed bool) (*node.HarnessNode, error) {
+	password []byte, noAuth bool) (*node.HarnessNode, error) {
 
 	cfg := &node.BaseNodeConfig{
 		Name:              name,
@@ -84,7 +84,7 @@ func (nm *nodeManager) newNode(t *testing.T, name string, extraArgs []string,
 		NodeID:            nm.nextNodeID(),
 		LndBinary:         nm.lndBinary,
 		NetParams:         harnessNetParams,
-		HasSeed:           useSeed,
+		SkipUnlock:        noAuth,
 	}
 
 	node, err := node.NewHarnessNode(t, cfg)
@@ -125,56 +125,11 @@ func (nm *nodeManager) shutdownNode(node *node.HarnessNode) error {
 // the connection attempt is successful. If the callback parameter is non-nil,
 // then the function will be executed after the node shuts down, but *before*
 // the process has been started up again.
-//
-// This method can be useful when testing edge cases such as a node broadcast
-// and invalidated prior state, or persistent state recovery, simulating node
-// crashes, etc. Additionally, each time the node is restarted, the caller can
-// pass a set of SCBs to pass in via the Unlock method allowing them to restore
-// channels during restart.
-func (nm *nodeManager) restartNode(ctxt context.Context, node *node.HarnessNode,
-	callback func() error, chanBackups ...*lnrpc.ChanBackupSnapshot) error {
+func (nm *nodeManager) restartNode(ctxt context.Context,
+	hn *node.HarnessNode, callback func() error) error {
 
-	err := nm.restartNodeNoUnlock(ctxt, node, callback)
-	if err != nil {
-		return err
-	}
-
-	// If the node doesn't have a password set, then we can exit here as we
-	// don't need to unlock it.
-	if len(node.Cfg.Password) == 0 {
-		return nil
-	}
-
-	// Otherwise, we'll unlock the wallet, then complete the final steps
-	// for the node initialization process.
-	unlockReq := &lnrpc.UnlockWalletRequest{
-		WalletPassword: node.Cfg.Password,
-	}
-	if len(chanBackups) != 0 {
-		unlockReq.ChannelBackups = chanBackups[0]
-		unlockReq.RecoveryWindow = 100
-	}
-
-	err = wait.NoError(func() error {
-		return node.Unlock(unlockReq)
-	}, DefaultTimeout)
-	if err != nil {
-		return fmt.Errorf("%s: failed to unlock: %w", node.Name(), err)
-	}
-
-	return nil
-}
-
-// restartNodeNoUnlock attempts to restart a lightning node by shutting it down
-// cleanly, then restarting the process. In case the node was setup with a
-// seed, it will be left in the unlocked state. This function is fully
-// blocking. If the callback parameter is non-nil, then the function will be
-// executed after the node shuts down, but *before* the process has been
-// started up again.
-func (nm *nodeManager) restartNodeNoUnlock(ctxt context.Context,
-	node *node.HarnessNode, callback func() error) error {
-
-	if err := node.Stop(); err != nil {
+	// Stop the node.
+	if err := hn.Stop(); err != nil {
 		return fmt.Errorf("restart node got error: %w", err)
 	}
 
@@ -184,11 +139,45 @@ func (nm *nodeManager) restartNodeNoUnlock(ctxt context.Context,
 		}
 	}
 
-	if node.Cfg.HasSeed {
-		return node.StartWithSeed(ctxt)
+	// Start the node without unlocking the wallet.
+	if hn.Cfg.SkipUnlock {
+		return hn.StartWithNoAuth(ctxt)
 	}
 
-	return node.Start(ctxt)
+	return hn.Start(ctxt)
+}
+
+// unlockNode unlocks the node's wallet if the password is configured.
+// Additionally, each time the node is unlocked, the caller can pass a set of
+// SCBs to pass in via the Unlock method allowing them to restore channels
+// during restart.
+func (nm *nodeManager) unlockNode(hn *node.HarnessNode,
+	chanBackups ...*lnrpc.ChanBackupSnapshot) error {
+
+	// If the node doesn't have a password set, then we can exit here as we
+	// don't need to unlock it.
+	if len(hn.Cfg.Password) == 0 {
+		return nil
+	}
+
+	// Otherwise, we'll unlock the wallet, then complete the final steps
+	// for the node initialization process.
+	unlockReq := &lnrpc.UnlockWalletRequest{
+		WalletPassword: hn.Cfg.Password,
+	}
+	if len(chanBackups) != 0 {
+		unlockReq.ChannelBackups = chanBackups[0]
+		unlockReq.RecoveryWindow = 100
+	}
+
+	err := wait.NoError(func() error {
+		return hn.Unlock(unlockReq)
+	}, DefaultTimeout)
+	if err != nil {
+		return fmt.Errorf("%s: failed to unlock: %w", hn.Name(), err)
+	}
+
+	return nil
 }
 
 // initWalletAndNode will unlock the node's wallet and finish setting up the

lntemp/node/config.go

@@ -63,8 +63,8 @@ type BaseNodeConfig struct {
 	ReadMacPath    string
 	InvoiceMacPath string
 
-	HasSeed  bool
-	Password []byte
+	SkipUnlock bool
+	Password   []byte
 
 	P2PPort     int
 	RPCPort     int
@@ -190,7 +190,7 @@ func (cfg *BaseNodeConfig) GenArgs() []string {
 	}
 	args = append(args, nodeArgs...)
 
-	if !cfg.HasSeed {
+	if cfg.Password == nil {
 		args = append(args, "--noseedbackup")
 	}
 

lntemp/node/harness_node.go

@@ -387,12 +387,12 @@ func (hn *HarnessNode) StartLndCmd(ctxb context.Context) error {
 	return nil
 }
 
-// StartWithSeed will start the lnd process, creates the grpc connection
+// StartWithNoAuth will start the lnd process, creates the grpc connection
 // without macaroon auth, and waits until the server is reported as waiting to
 // start.
 //
 // NOTE: caller needs to take extra step to create and unlock the wallet.
-func (hn *HarnessNode) StartWithSeed(ctxt context.Context) error {
+func (hn *HarnessNode) StartWithNoAuth(ctxt context.Context) error {
 	// Start lnd process and prepare logs.
 	if err := hn.StartLndCmd(ctxt); err != nil {
 		return fmt.Errorf("start lnd error: %w", err)

lntest/itest/lnd_channel_backup_test.go

@@ -349,8 +349,9 @@ func testChannelBackupRestoreBasic(ht *lntemp.HarnessTest) {
 						"", revocationWindow, nil,
 						copyPorts(oldNode),
 					)
-
-					st.RestartNode(newNode, backupSnapshot)
+					st.RestartNodeWithChanBackups(
+						newNode, backupSnapshot,
+					)
 
 					return newNode
 				}