Wallet Recovery #1005

cfromknecht · 2018-04-03T00:17:05Z

Adds wallet recovery to lnd! These changes enable aezeeds with funds to be restored into a fresh instance, and recover any on-chain balance. Most of the rescan logic is implemented in Roasbeef/btcwallet#22, this PR involves most of the scaffolding to recovery configuration options into the wallet itself. I've added an integration test to test the recovery procedure using btcd, and I have manually tested bitcoind and neutrino.

Fixes #900

zavan · 2018-04-03T01:17:48Z

Hey @cfromknecht, thanks for your work!

Is this PR ready for testing? I'd like to try it. I have deleted a wallet and recovered it from the seed, but, of course, it didn't recover the funds. I'm ready to pull and compile this, how can I trigger the rescanning process? Or should I delete my wallet and restore again?

(Using bitcoind, lnd currently @ master)

cfromknecht · 2018-04-03T02:35:29Z

Hey @zavan, yep this should be ready to start testing! Please note, this will only recover on-chain funds, so nothing regarding previous channels will be restored.

I recommend restoring to a fresh directory, don't want to mangle any data you have currently. When doing lncli create you will be prompted for the seed/passwords. After doing that correctly, lncli will also prompt you for a recovery window (default 250). This should only need to be bumped if you generated a bunch of unused address, as this defines the lookahead during the rescan.

Then, wait for the node to finish syncing and you should see your total balance. If the node crashes during recovery (or you kill the process), you should be able to restart the recovery by passing a non-zero --recovery_window to lncli unlock. I recommend passing in the same recovery window used previously.

Let me know how it goes :)

cfromknecht · 2018-04-03T02:58:28Z

FWIW, the recovery will probably take the longest when using the bitcoind backend. Currently, the logic fetches block in-order without prefetching. I have some prefetching code for btcwallet to speed that up. However, the complexity was upped a good bit because of it, so I'm gonna hold that off until a follow up PR.

cfromknecht · 2018-04-03T04:05:13Z

chainregistry.go

+	// beginning of the bitcoin blockchain in case this wallet has yet to be
+	// created. If the wallet has already been created, this value will be
+	// ignored.
+	if walletConfig.Birthday.IsZero() {


Would appreciate some feedback on the correct behavior here. Other alternatives seem to be using time.Now() or just not do anything as it should be ignored.

why just not keep it Zero?

cfromknecht · 2018-04-03T04:06:06Z

lnd.go

@@ -832,12 +850,21 @@ func genMacaroons(ctx context.Context, svc *macaroons.Service,
 	return nil
 }

+type WalletInitParams struct {


Add godoc comments

cfromknecht · 2018-04-03T04:06:27Z

lnd_test.go

+	AddrTypeNestedPubkeyHash  = lnrpc.NewAddressRequest_NESTED_PUBKEY_HASH
+)
+
+// testOnchainFundRecovery


fill in godoc comment

cfromknecht · 2018-04-03T04:07:41Z

lntest/harness.go

+	return node, nil
+}
+
+func (n *NetworkHarness) RegisterNode(node *HarnessNode) {


add godoc comment

halseth · 2018-04-05T12:46:31Z

lnd.go

-		privateWalletPw, publicWalletPw, err = waitForWalletPassword(
-			cfg.RPCListeners, cfg.RESTListeners, serverOpts, proxyOpts,
-			tlsConf, macaroonService,
+		walletInitParams, err := waitForWalletPassword(


what will happen if --noencryptwallet is specified? Atm I don't think we even give users a seed (hence they won't be able to recover the funds), but this is not very intuitive. We should probably let a user run with --noencryptwallet and recover from a seed, or deprecate --noencryptwallet (at least on mainnet).

yeah we don't have rescans for unencrypted wallets, because the users don't have seeds.
currently the recovery window is initialized as 0, and can only be set when --noencryptwallet is not provided, so it will just sync as normal

halseth · 2018-04-05T12:48:11Z

cmd/lncli/commands.go

+			if err != nil {
+				fmt.Println("Unable to parse recovery "+
+					"window: %v", err)
+				goto readRecoveryWindow


continue?

halseth · 2018-04-05T12:48:26Z

cmd/lncli/commands.go

+
+			if len(answer) == 0 {
+				recoveryWindow = defaultRecoveryWindow
+				break readRecoveryWindow


just break?

halseth · 2018-04-05T12:48:38Z

cmd/lncli/commands.go

+			}
+
+			recoveryWindow = int32(lookAhead)
+			break readRecoveryWindow


halseth · 2018-04-05T12:49:54Z

cmd/lncli/commands.go

+	Flags: []cli.Flag{
+		cli.IntFlag{
+			Name:  "recovery_window",
+			Usage: "address lookahead for rescan",


not very clear if this will trigger a new rescan (from genesis?) if specified

I updated the wording to say that it will resume recovery and expand on how to properly set the value.

i guess it'd be worth also noting that this won't do anything if you're already synced, and that it's meant to be used to recover/restore onchain funds?

halseth · 2018-04-05T12:53:03Z

chainregistry.go

+	// beginning of the bitcoin blockchain in case this wallet has yet to be
+	// created. If the wallet has already been created, this value will be
+	// ignored.
+	if walletConfig.Birthday.IsZero() {


why just not keep it Zero?

Roasbeef

Excited to finally get this in! Performed an initial review sweep, and didn't find anything fundemental. Will now move on to testing live with a few seeds from mainnet and testnet to ensure all funds are properly recovered.

Roasbeef · 2018-04-10T02:23:50Z

lntest/node.go

 }

 // Assert *HarnessNode implements the lnrpc.LightningClient interface.
 var _ lnrpc.LightningClient = (*HarnessNode)(nil)
+var _ lnrpc.WalletUnlockerClient = (*HarnessNode)(nil)


Roasbeef · 2018-04-10T02:49:24Z

Needs rebase in order to trigger a fresh travis run on top of the latest changes.

halseth · 2018-04-20T10:54:56Z

cmd/lncli/commands.go

+	args := ctx.Args()
+
+	// Parse the optional recovery window if it is specified. By default,
+	// the recovery window will be 0, indicating no lookahead should be


some places it says it should be non-zero, here it says it can be zero. What's correct?

Recovery window of 0 indicates no-lookahead, and no recovery will be attempted. 0 is a valid parameter, unless the user is actually trying to attempting to recover funds

halseth · 2018-04-20T10:57:50Z

lnd_test.go

+	// First, create a new node with strong passphrase and grab the mnemonic
+	// used for key derivation. This will bring up Carol with an empty
+	// wallet, and such that she is synced up.
+	password := []byte("topkek9000")


password leak

lol, didn't mean to expose you ;)

Roasbeef · 2018-04-21T02:12:24Z

After latest rebase, tested again on multiple seeds on testnet, confirmed that all funds were recovered in full! Also confirmed that if the daemon is killed mid recovery, then a start with lncli unlock --recovery_window results in a resumption of the initial rescan.

cfromknecht · 2018-04-25T02:42:59Z

PR has been rebased on master and includes latest btcwallet updates and unit tests for 1-in-1-out spends.

Roasbeef · 2018-04-26T22:14:39Z

Still has a proto conflict.

Roasbeef · 2018-04-26T22:35:11Z

With the btcwallet PR merged, this should also update the dep files during rebase as well.

cfromknecht · 2018-04-26T23:30:07Z

PR has been rebased with latest btcwallet merge! Will ping when green

Roasbeef

⚡️⚡️LGTM ⚡️⚡️

cfromknecht force-pushed the wallet-recovery branch 3 times, most recently from 8ea431b to 12d7fbe Compare April 3, 2018 00:57

zavan mentioned this pull request Apr 3, 2018

restore from seed has zero balance #900

Closed

cfromknecht force-pushed the wallet-recovery branch 3 times, most recently from 0ecf7be to d99607c Compare April 3, 2018 02:25

meshcollider added wallet The wallet (lnwallet) which LND uses crypto Related to the cryptography underlying LND recovery Related to the backup/restoration of LND data (e.g. wallet seeds) labels Apr 3, 2018

cfromknecht commented Apr 3, 2018

View reviewed changes

cfromknecht force-pushed the wallet-recovery branch from d99607c to 2e01efc Compare April 3, 2018 04:20

halseth reviewed Apr 5, 2018

View reviewed changes

cfromknecht force-pushed the wallet-recovery branch 5 times, most recently from 82428f3 to a8a077a Compare April 6, 2018 01:57

Roasbeef reviewed Apr 10, 2018

View reviewed changes

cfromknecht force-pushed the wallet-recovery branch 4 times, most recently from 51cf084 to 6a9d641 Compare April 18, 2018 04:21

halseth reviewed Apr 20, 2018

View reviewed changes

cfromknecht force-pushed the wallet-recovery branch 3 times, most recently from b06ac03 to d9a9916 Compare April 25, 2018 02:41

cfromknecht added 13 commits April 26, 2018 16:03

aezeed: expose BirthdayTime conversion from offset

c824af1

walletunlocker: accept recovery window from InitWallet

f8c0357

walletunlocker/service_test: check recovery window is threaded

3cb87f3

lnrpc: add RecoveryWindow to InitWalletRequest

0ae62c3

chainregistry: initialize wallet w/ recovery window

1972a79

keychain/interface_test: init test wallet w/o recovery window

b9df5e7

lnd: threads recovery window param from walletunlocker

603228c

lnwallet/btcwallet/btcwallet: pass recovery window to CreateNewWallet

ae60406

lnwallet/btcwallet/config: add recovery window and bday

06a53aa

lnwallet/interfaace_test: init neutrino with chain params

a2fdd85

cmd/lncli/commands: ask for recovery window after restore seed

92a2ba1

lntest/harness: add NewNodeWithSeed and RestoreNodeWithSeed

329793d

lntest/node: add ability to unlock/init HarnessNode

c977ebb

cfromknecht force-pushed the wallet-recovery branch 2 times, most recently from b8c00fa to 0c93cd4 Compare April 26, 2018 23:29

cfromknecht added 2 commits April 26, 2018 16:34

lnd_test: adds wallet recovery test

5049d39

dep: update btcwallet w/ wallet recovery

1ae3d4f

cfromknecht force-pushed the wallet-recovery branch from 0c93cd4 to 1ae3d4f Compare April 26, 2018 23:34

Roasbeef changed the title ~~[WIP] Wallet Recovery~~ Wallet Recovery Apr 27, 2018

Roasbeef approved these changes Apr 27, 2018

View reviewed changes

Roasbeef merged commit 4ab2bba into lightningnetwork:master Apr 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wallet Recovery #1005

Wallet Recovery #1005

cfromknecht commented Apr 3, 2018 •

edited

Loading

zavan commented Apr 3, 2018 •

edited

Loading

cfromknecht commented Apr 3, 2018 •

edited

Loading

cfromknecht commented Apr 3, 2018

cfromknecht Apr 3, 2018

halseth Apr 5, 2018

cfromknecht Apr 3, 2018

cfromknecht Apr 3, 2018

cfromknecht Apr 3, 2018

halseth Apr 5, 2018

cfromknecht Apr 6, 2018

halseth Apr 5, 2018

cfromknecht Apr 6, 2018

halseth Apr 5, 2018

cfromknecht Apr 6, 2018

halseth Apr 5, 2018

cfromknecht Apr 6, 2018

halseth Apr 5, 2018

cfromknecht Apr 6, 2018

halseth Apr 5, 2018

Roasbeef left a comment

Roasbeef Apr 10, 2018

Roasbeef commented Apr 10, 2018

halseth Apr 20, 2018

cfromknecht Apr 26, 2018

halseth Apr 20, 2018

Roasbeef Apr 26, 2018

cfromknecht Apr 26, 2018 •

edited

Loading

Roasbeef commented Apr 21, 2018

cfromknecht commented Apr 25, 2018

Roasbeef commented Apr 26, 2018

Roasbeef commented Apr 26, 2018

cfromknecht commented Apr 26, 2018

Roasbeef left a comment

Wallet Recovery #1005

Wallet Recovery #1005

Conversation

cfromknecht commented Apr 3, 2018 • edited Loading

zavan commented Apr 3, 2018 • edited Loading

cfromknecht commented Apr 3, 2018 • edited Loading

cfromknecht commented Apr 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef commented Apr 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfromknecht Apr 26, 2018 • edited Loading

Choose a reason for hiding this comment

Roasbeef commented Apr 21, 2018

cfromknecht commented Apr 25, 2018

Roasbeef commented Apr 26, 2018

Roasbeef commented Apr 26, 2018

cfromknecht commented Apr 26, 2018

Roasbeef left a comment

Choose a reason for hiding this comment

cfromknecht commented Apr 3, 2018 •

edited

Loading

zavan commented Apr 3, 2018 •

edited

Loading

cfromknecht commented Apr 3, 2018 •

edited

Loading

cfromknecht Apr 26, 2018 •

edited

Loading