Create watch-only address managers and accounts #667

devrandom · 2019-12-14T21:38:33Z

This PR allows the creation of managers and accounts that are watch-only. The state of the database after creation would be identical to the state after calling Manager.ConvertToWatchingOnly, assuming accounts with the right xpubs were created in the former case.

Of course, creating a watch-only database is safer, since you don't have to worry about private data ending up in a swap file or the system having been compromised before creation.

devrandom · 2019-12-14T21:56:50Z

waddrmgr/manager.go

+// particular coin type and BIP0043 purpose. This is useful as it enables
+// callers to create an arbitrary BIP0043 like schema with a stand alone
+// manager.
+func (m *Manager) NewWatchingOnlyScopedKeyManager(ns walletdb.ReadWriteBucket, scope KeyScope,


This could be merged with NewScopedKeyManager, with a conditional on watchingOnly. WDYT?

SGTM, there's a ton of duplication as is.

Yes, that would definitely be preferable. You could split the content into multiple smaller methods and call them depending on the flag. This would avoid large chunks of code inside if conditions.

Let us know if looks good.

Roasbeef

After meditating on this a bit more, I realized that non of the restrictions (well most of them) that I previously thought were present actually exists. As a result, I think this PR takes a step towards putting some scaffolding in places to address both of our respective goals and more!

I've completed an initial pass through this PR, thanks again for getting this rolling!

Roasbeef · 2020-01-16T02:20:57Z

waddrmgr/manager.go

+// particular coin type and BIP0043 purpose. This is useful as it enables
+// callers to create an arbitrary BIP0043 like schema with a stand alone
+// manager.
+func (m *Manager) NewWatchingOnlyScopedKeyManager(ns walletdb.ReadWriteBucket, scope KeyScope,


SGTM, there's a ton of duplication as is.

Roasbeef · 2020-01-16T02:25:52Z

waddrmgr/db.go

@@ -859,6 +860,9 @@ func fetchLastAccount(ns walletdb.ReadBucket, scope *KeyScope) (uint32, error) {
 	metaBucket := scopedBucket.NestedReadBucket(metaBucketName)

 	val := metaBucket.Get(lastAccountName)
+	if val == nil {


Why wasn't this needed before? Or is this for the case where we create the new wallet as pure watch-only?

I think this only occurs when the wallet is created as pure watch-only.

Ah, because no default accounts are created, right.

waddrmgr/manager.go

waddrmgr/manager_test.go

waddrmgr/scoped_manager.go

Roasbeef · 2020-01-16T02:53:27Z

cc @guggero

guggero

Thank you for the PR, nice work! This does indeed look like what we need for our use case in lnd.
My comments are also mainly around code duplication and putting everything behind a flag (which will also make it easier for us to use the code in lnd).

guggero · 2020-01-16T10:17:21Z

waddrmgr/manager.go

+// particular coin type and BIP0043 purpose. This is useful as it enables
+// callers to create an arbitrary BIP0043 like schema with a stand alone
+// manager.
+func (m *Manager) NewWatchingOnlyScopedKeyManager(ns walletdb.ReadWriteBucket, scope KeyScope,


Yes, that would definitely be preferable. You could split the content into multiple smaller methods and call them depending on the flag. This would avoid large chunks of code inside if conditions.

waddrmgr/manager.go

waddrmgr/manager_test.go

guggero

This looks much better without the code duplication, thanks!

Maybe it's a bit late to suggest this, but while figuring out how exactly one would use this new functionality I couldn't help but wonder if it wouldn't be possible to create a new manager with the master xpub directly instead of later adding accounts.

So perhaps creating a new CreateWatchingOnly function and keeping the old one for compatibility but internally all routing it to a non-exported function that has all the logic.

Something like:

func CreateWatchingOnly(ns walletdb.ReadWriteBucket,
	pubKey, pubPassphrase []byte,
	chainParams *chaincfg.Params, config *ScryptOptions,
	birthday time.Time) error {

	return create(
		ns, pubKey, pubPassphrase, nil, chainParams, config,
		birthday, true,
	)
}

func Create(ns walletdb.ReadWriteBucket,
	seed, pubPassphrase, privPassphrase []byte,
	chainParams *chaincfg.Params, config *ScryptOptions,
	birthday time.Time) error {

	return create(
		ns, seed, pubPassphrase, privPassphrase, chainParams, config,
		birthday, false,
	)
}

func create(ns walletdb.ReadWriteBucket,
	seedOrPubKey, pubPassphrase, privPassphrase []byte,
	chainParams *chaincfg.Params, config *ScryptOptions,
	birthday time.Time, isWatchingOnly bool) error {

// logic for parsing the pubKey as a neutered master key and initializing the default accounts with it.
}

This could also be done in a follow-up PR if we want to push forward with this.

waddrmgr/error.go

waddrmgr/manager.go

guggero · 2020-03-05T12:23:39Z

waddrmgr/db.go

@@ -859,6 +860,9 @@ func fetchLastAccount(ns walletdb.ReadBucket, scope *KeyScope) (uint32, error) {
 	metaBucket := scopedBucket.NestedReadBucket(metaBucketName)

 	val := metaBucket.Get(lastAccountName)
+	if val == nil {


Ah, because no default accounts are created, right.

waddrmgr/scoped_manager.go

devrandom · 2020-03-05T21:39:23Z

Maybe it's a bit late to suggest this, but while figuring out how exactly one would use this new functionality I couldn't help but wonder if it wouldn't be possible to create a new manager with the master xpub directly instead of later adding accounts.

Account derivation is hardened, so it's not possible to derive account xpubs from the parent(s).

guggero · 2020-03-06T07:59:51Z

Right, I didn't remember that. Then I think the current design of the PR makes a lot of sense 👍

ksedgwic · 2020-03-23T14:09:21Z

I think everything is addressed, pls let me know if I missed something ...

guggero · 2020-03-24T12:38:44Z

Thanks, looks better now!

Just one last question: I started implementing something on top of this and noticed that currently there is no way to create a new wallet wit a nil seed, because by default a random seed is created in the wallet.Create function. Maybe I still don't understand how you intend to use this functionality. Is the idea to create a new wallet normally, create all the key scopes and accounts, then turn it into a watch-only wallet?

ksedgwic · 2020-03-24T14:12:34Z

No, the idea is definitely to create the wallet without a seed and add accounts using XPUB from the remote signer. Investigating ...

ksedgwic · 2020-03-25T20:43:04Z

Oliver, I think we missed that. We'll add mechanism to create an empty watch-only wallet to this PR straightaway. Thanks for catching!

ksedgwic · 2020-03-25T23:36:08Z

Did a force push to rebase on master, fixes coming ...

ksedgwic · 2020-03-26T19:28:29Z

Ok, I've added Loader.CreateNewWatchingOnlyWallet and Wallet.CreateWatchingOnly, thoughts?

guggero

Thanks, looks good to me!
Just two small nits, but they aren't blocking in my opinion.

wallet/wallet.go

wallet/loader.go

…tManager

…ded.

ksedgwic · 2020-04-09T15:52:04Z

Force pushed rebase onto master, about to fix nits ...

ksedgwic · 2020-04-23T19:21:55Z

ping, everything look ok with this?

guggero · 2020-04-23T19:26:22Z

cc @Roasbeef.

Roasbeef

LGTM 💎

Roasbeef · 2020-04-25T00:43:07Z

waddrmgr/manager_test.go

+// except that the manager is created normally with a seed. This test
+// shows that watch-only accounts can be added to managers with
+// non-watch-only accounts.
+func TestNewRawAccountHybrid(t *testing.T) {


Roasbeef · 2020-04-25T00:46:27Z

Y'all can follow the progress for the remaining lnd changes to support a watch-only node and a segregated signer here: lightningnetwork/lnd#3943

devrandom force-pushed the watchonly branch 2 times, most recently from 9221af9 to 82c55fc Compare December 14, 2019 21:49

devrandom commented Dec 14, 2019

View reviewed changes

devrandom force-pushed the watchonly branch from 01dd6d6 to 330896a Compare December 15, 2019 21:34

Roasbeef requested changes Jan 16, 2020

View reviewed changes

guggero reviewed Jan 16, 2020

View reviewed changes

Roasbeef mentioned this pull request Jan 21, 2020

lnwallet+lnd: support watch-only initialization for lnd nodes lightningnetwork/lnd#3943

Closed

3 tasks

ksedgwic force-pushed the watchonly branch from 330896a to 0f2148e Compare January 21, 2020 21:58

Roasbeef self-requested a review February 18, 2020 20:44

guggero reviewed Mar 5, 2020

View reviewed changes

ksedgwic force-pushed the watchonly branch from 0f2148e to 679e247 Compare March 17, 2020 22:04

ksedgwic force-pushed the watchonly branch from 679e247 to 52afd8c Compare March 25, 2020 23:33

guggero approved these changes Apr 2, 2020

View reviewed changes

wallet/wallet.go Outdated Show resolved Hide resolved

wallet/loader.go Outdated Show resolved Hide resolved

devrandom and others added 6 commits April 9, 2020 08:49

Create watch-only address managers and accounts

01f4d11

README

9ccb7c5

Addressed review issues; factored watch-only code into Create and Tes…

ec885b6

…tManager

Removed errMustBeWatchingOnly and ErrMustBeWatchingOnly because unnee…

4038979

…ded.

Fixed comments and lines too long.

0deb3c1

Added Wallet and Loader watching-only wallet creation.

b99d34d

ksedgwic force-pushed the watchonly branch from b97681b to b99d34d Compare April 9, 2020 15:51

Made "internal" routine name changes as suggested.

40da314

Roasbeef approved these changes Apr 25, 2020

View reviewed changes

Roasbeef merged commit 4c5bc1b into btcsuite:master Apr 25, 2020

wpaulino mentioned this pull request Feb 17, 2021

wallet: support derived public key import #732

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create watch-only address managers and accounts #667

Create watch-only address managers and accounts #667

devrandom commented Dec 14, 2019 •

edited

Loading

devrandom Dec 14, 2019

Roasbeef Jan 16, 2020

guggero Jan 16, 2020

ksedgwic Jan 21, 2020

devrandom Jan 21, 2020

Roasbeef left a comment

Roasbeef Jan 16, 2020

Roasbeef Jan 16, 2020

ksedgwic Jan 16, 2020

guggero Mar 5, 2020

Roasbeef commented Jan 16, 2020

guggero left a comment

guggero Jan 16, 2020

guggero left a comment •

edited

Loading

guggero Mar 5, 2020

devrandom commented Mar 5, 2020

guggero commented Mar 6, 2020

ksedgwic commented Mar 23, 2020

guggero commented Mar 24, 2020

ksedgwic commented Mar 24, 2020

ksedgwic commented Mar 25, 2020 •

edited

Loading

ksedgwic commented Mar 25, 2020

ksedgwic commented Mar 26, 2020

guggero left a comment

ksedgwic commented Apr 9, 2020

ksedgwic commented Apr 23, 2020

guggero commented Apr 23, 2020

Roasbeef left a comment

Roasbeef Apr 25, 2020

Roasbeef commented Apr 25, 2020

Create watch-only address managers and accounts #667

Create watch-only address managers and accounts #667

Conversation

devrandom commented Dec 14, 2019 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef commented Jan 16, 2020

guggero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guggero left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devrandom commented Mar 5, 2020

guggero commented Mar 6, 2020

ksedgwic commented Mar 23, 2020

guggero commented Mar 24, 2020

ksedgwic commented Mar 24, 2020

ksedgwic commented Mar 25, 2020 • edited Loading

ksedgwic commented Mar 25, 2020

ksedgwic commented Mar 26, 2020

guggero left a comment

Choose a reason for hiding this comment

ksedgwic commented Apr 9, 2020

ksedgwic commented Apr 23, 2020

guggero commented Apr 23, 2020

Roasbeef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef commented Apr 25, 2020

devrandom commented Dec 14, 2019 •

edited

Loading

guggero left a comment •

edited

Loading

ksedgwic commented Mar 25, 2020 •

edited

Loading