lnd+walletunlocker: move history drop to unlocker

Apparently dropping the wallet transaction history only fully takes
effect after opening the wallet from scratch again. To do this cleanly,
we need to do it in the unlocker instead of lnd.

lnd.go

@@ -1146,6 +1146,7 @@ func waitForWalletPassword(cfg *Config, restEndpoints []net.Addr,
 	pwService := walletunlocker.New(
 		chainConfig.ChainDir, cfg.ActiveNetParams.Params,
 		!cfg.SyncFreelist, macaroonFiles, cfg.DB.Bolt.DBTimeout,
+		cfg.ResetWalletTransactions,
 	)
 
 	// Set up a new PasswordService, which will listen for passwords
@@ -1313,21 +1314,10 @@ func waitForWalletPassword(cfg *Config, restEndpoints []net.Addr,
 		// remind the user to turn off the setting again after
 		// successful completion.
 		if cfg.ResetWalletTransactions {
-			ltndLog.Warnf("Dropping all transaction history from " +
+			ltndLog.Warnf("Dropped all transaction history from " +
 				"on-chain wallet. Remember to disable " +
 				"reset-wallet-transactions flag for next " +
 				"start of lnd")
-
-			err := wallet.DropTransactionHistory(
-				unlockMsg.Wallet.Database(), true,
-			)
-			if err != nil {
-				if err := unlockMsg.UnloadWallet(); err != nil {
-					ltndLog.Errorf("Could not unload "+
-						"wallet: %v", err)
-				}
-				return nil, shutdown, err
-			}
 		}
 
 		return &WalletUnlockParams{

walletunlocker/service.go

@@ -133,11 +133,16 @@ type UnlockerService struct {
 	// dbTimeout specifies the timeout value to use when opening the wallet
 	// database.
 	dbTimeout time.Duration
+
+	// resetWalletTransactions indicates that the wallet state should be
+	// reset on unlock to force a full chain rescan.
+	resetWalletTransactions bool
 }
 
 // New creates and returns a new UnlockerService.
 func New(chainDir string, params *chaincfg.Params, noFreelistSync bool,
-	macaroonFiles []string, dbTimeout time.Duration) *UnlockerService {
+	macaroonFiles []string, dbTimeout time.Duration,
+	resetWalletTransactions bool) *UnlockerService {
 
 	return &UnlockerService{
 		InitMsgs:   make(chan *WalletInitMsg, 1),
@@ -151,6 +156,7 @@ func New(chainDir string, params *chaincfg.Params, noFreelistSync bool,
 		macaroonFiles:           macaroonFiles,
 		dbTimeout:               dbTimeout,
 		noFreelistSync:          noFreelistSync,
+		resetWalletTransactions: resetWalletTransactions,
 	}
 }
 
@@ -400,6 +406,37 @@ func (u *UnlockerService) UnlockWallet(ctx context.Context,
 		return nil, err
 	}
 
+	// The user requested to drop their whole wallet transaction state to
+	// force a full chain rescan for wallet addresses. Dropping the state
+	// only properly takes effect after opening the wallet. That's why we
+	// start, drop, stop and start again.
+	if u.resetWalletTransactions {
+		dropErr := wallet.DropTransactionHistory(
+			unlockedWallet.Database(), true,
+		)
+
+		// Even if dropping the history fails, we'll want to unload the
+		// wallet. If unloading fails, that error is probably more
+		// important to be returned to the user anyway.
+		if err := loader.UnloadWallet(); err != nil {
+			return nil, fmt.Errorf("could not unload "+
+				"wallet (tx history drop err: %v): %v", dropErr,
+				err)
+		}
+
+		// If dropping failed but unloading didn't, we'll still abort
+		// and inform the user.
+		if dropErr != nil {
+			return nil, dropErr
+		}
+
+		// All looks good, let's now open the wallet again.
+		unlockedWallet, err = loader.OpenExistingWallet(password, false)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	// We successfully opened the wallet and pass the instance back to
 	// avoid it needing to be unlocked again.
 	walletUnlockMsg := &WalletUnlockMsg{

walletunlocker/service_test.go

@@ -148,6 +148,7 @@ func TestGenSeed(t *testing.T) {
 
 	service := walletunlocker.New(
 		testDir, testNetParams, true, nil, kvdb.DefaultDBTimeout,
+		false,
 	)
 
 	// Now that the service has been created, we'll ask it to generate a
@@ -185,6 +186,7 @@ func TestGenSeedGenerateEntropy(t *testing.T) {
 	}()
 	service := walletunlocker.New(
 		testDir, testNetParams, true, nil, kvdb.DefaultDBTimeout,
+		false,
 	)
 
 	// Now that the service has been created, we'll ask it to generate a
@@ -221,6 +223,7 @@ func TestGenSeedInvalidEntropy(t *testing.T) {
 	}()
 	service := walletunlocker.New(
 		testDir, testNetParams, true, nil, kvdb.DefaultDBTimeout,
+		false,
 	)
 
 	// Now that the service has been created, we'll ask it to generate a
@@ -254,6 +257,7 @@ func TestInitWallet(t *testing.T) {
 	// Create new UnlockerService.
 	service := walletunlocker.New(
 		testDir, testNetParams, true, nil, kvdb.DefaultDBTimeout,
+		false,
 	)
 
 	// Once we have the unlocker service created, we'll now instantiate a
@@ -342,6 +346,7 @@ func TestCreateWalletInvalidEntropy(t *testing.T) {
 	// Create new UnlockerService.
 	service := walletunlocker.New(
 		testDir, testNetParams, true, nil, kvdb.DefaultDBTimeout,
+		false,
 	)
 
 	// We'll attempt to init the wallet with an invalid cipher seed and
@@ -370,9 +375,11 @@ func TestUnlockWallet(t *testing.T) {
 		_ = os.RemoveAll(testDir)
 	}()
 
-	// Create new UnlockerService.
+	// Create new UnlockerService that'll also drop the wallet's history on
+	// unlock.
 	service := walletunlocker.New(
 		testDir, testNetParams, true, nil, kvdb.DefaultDBTimeout,
+		true,
 	)
 
 	ctx := context.Background()
@@ -464,6 +471,7 @@ func TestChangeWalletPasswordNewRootkey(t *testing.T) {
 	// Create a new UnlockerService with our temp files.
 	service := walletunlocker.New(
 		testDir, testNetParams, true, tempFiles, kvdb.DefaultDBTimeout,
+		false,
 	)
 
 	ctx := context.Background()
@@ -572,9 +580,11 @@ func TestChangeWalletPasswordStateless(t *testing.T) {
 	nonExistingFile := path.Join(testDir, "does-not-exist")
 
 	// Create a new UnlockerService with our temp files.
-	service := walletunlocker.New(testDir, testNetParams, true, []string{
+	service := walletunlocker.New(
+		testDir, testNetParams, true, []string{
 			tempMacFile, nonExistingFile,
-	}, kvdb.DefaultDBTimeout)
+		}, kvdb.DefaultDBTimeout, false,
+	)
 
 	// Create a wallet we can try to unlock. We use the default password
 	// so we can check that the unlocker service defaults to this when