From 51b362d7c515211bcd47348de847cf6ed075abe2 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 24 Jan 2020 15:18:09 -0800 Subject: [PATCH 01/25] waddrmgr+wallet: only watch addresses within default key scopes It was discovered that the wallet can scan the chain for unnecessary additional addresses that are derived by higher-level applications using custom key scopes. This isn't much of an issue for full nodes, but it can cause light clients to scan more than what's required, triggering more false positive matches which lead to block retrieval. Now, we'll only scan the chain for addresses that exist within the default key scopes, as those are the only ones the wallet should be concerned about. --- waddrmgr/manager.go | 23 +++++++++++++++++++++++ wallet/wallet.go | 20 ++++++++++++-------- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index 85d034d96c..0540a4bbb7 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -748,6 +748,29 @@ func (m *Manager) ForEachAccountAddress(ns walletdb.ReadBucket, account uint32, return nil } +// ForEachDefaultScopeActiveAddress calls the given function with each active +// address stored in the manager within the default scopes, breaking early on +// error. +func (m *Manager) ForEachDefaultScopeActiveAddress(ns walletdb.ReadBucket, + fn func(addr btcutil.Address) error) error { + + m.mtx.RLock() + defer m.mtx.RUnlock() + + for _, keyScope := range DefaultKeyScopes { + scopedMgr, ok := m.scopedManagers[keyScope] + if !ok { + return fmt.Errorf("manager for default key scope with "+ + "purpose %v not found", keyScope.Purpose) + } + if err := scopedMgr.ForEachActiveAddress(ns, fn); err != nil { + return err + } + } + + return nil +} + // ChainParams returns the chain parameters for this address manager. func (m *Manager) ChainParams() *chaincfg.Params { // NOTE: No need for mutex here since the net field does not change diff --git a/wallet/wallet.go b/wallet/wallet.go index 49e8e20078..2309b6962a 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -303,18 +303,22 @@ func (w *Wallet) SetChainSynced(synced bool) { w.chainClientSyncMtx.Unlock() } -// activeData returns the currently-active receiving addresses and all unspent -// outputs. This is primarely intended to provide the parameters for a -// rescan request. -func (w *Wallet) activeData(dbtx walletdb.ReadTx) ([]btcutil.Address, []wtxmgr.Credit, error) { +// activeData returns the currently-active receiving addresses that exist within +// the wallet's default key scopes and all unspent outputs. This is primarily +// intended to provide the parameters for a rescan request. +func (w *Wallet) activeData(dbtx walletdb.ReadTx) ([]btcutil.Address, + []wtxmgr.Credit, error) { + addrmgrNs := dbtx.ReadBucket(waddrmgrNamespaceKey) txmgrNs := dbtx.ReadBucket(wtxmgrNamespaceKey) var addrs []btcutil.Address - err := w.Manager.ForEachActiveAddress(addrmgrNs, func(addr btcutil.Address) error { - addrs = append(addrs, addr) - return nil - }) + err := w.Manager.ForEachDefaultScopeActiveAddress( + addrmgrNs, func(addr btcutil.Address) error { + addrs = append(addrs, addr) + return nil + }, + ) if err != nil { return nil, nil, err } From f2ed9c1c77a29c9e56e0f6a6e3c6fc5883e15bec Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Tue, 18 Feb 2020 17:08:12 +0100 Subject: [PATCH 02/25] walletdb: generic param passing for walletdb test This commit removes bbolt specific parameters from the interface test to enable testing of other DB drivers trough the same entry point. --- walletdb/bdb/interface_test.go | 2 +- walletdb/walletdbtest/interface.go | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/walletdb/bdb/interface_test.go b/walletdb/bdb/interface_test.go index ec10fec6f3..1dcf605e01 100644 --- a/walletdb/bdb/interface_test.go +++ b/walletdb/bdb/interface_test.go @@ -23,5 +23,5 @@ import ( func TestInterface(t *testing.T) { dbPath := "interfacetest.db" defer os.RemoveAll(dbPath) - walletdbtest.TestInterface(t, dbType, dbPath) + walletdbtest.TestInterface(t, dbType, dbPath, true) } diff --git a/walletdb/walletdbtest/interface.go b/walletdb/walletdbtest/interface.go index fe338a7669..037de1ce24 100644 --- a/walletdb/walletdbtest/interface.go +++ b/walletdb/walletdbtest/interface.go @@ -7,7 +7,6 @@ package walletdbtest import ( "bytes" "fmt" - "os" "reflect" "sync" @@ -793,13 +792,12 @@ func testBatchInterface(tc *testContext) bool { } // TestInterface performs all interfaces tests for this database driver. -func TestInterface(t Tester, dbType, dbPath string) { - db, err := walletdb.Create(dbType, dbPath, true) +func TestInterface(t Tester, dbType string, args ...interface{}) { + db, err := walletdb.Create(dbType, args...) if err != nil { t.Errorf("Failed to create test database (%s) %v", dbType, err) return } - defer os.Remove(dbPath) defer db.Close() // Run all of the interface tests against the database. From fc64a1c70473500ca3adc740a79c5cdb5c96907a Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 13 Mar 2020 10:59:51 +0100 Subject: [PATCH 03/25] wallet: wait for chain sync on Neutrino recovery Normally the wallet doesn't wait for the chain backend to be synced on regtest/simnet because there we cannot be certain if we are at the chain tip, as there are no other nodes to compare to. For Neutrino, this is a bit different because we rely on the cfheader server to tell us what it thinks the chain tip is. For a wallet recovery on Neutrino we therefore need to make sure we are at least synced up to what the server thinks is the tip. --- wallet/wallet.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/wallet/wallet.go b/wallet/wallet.go index 2309b6962a..a3e092cb3d 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -336,10 +336,21 @@ func (w *Wallet) syncWithChain(birthdayStamp *waddrmgr.BlockStamp) error { return err } + // Neutrino relies on the information given to it by the cfheader server + // so it knows exactly whether it's synced up to the server's state or + // not, even on dev chains. To recover a Neutrino wallet, we need to + // make sure it's synced before we start scanning for addresses, + // otherwise we might miss some if we only scan up to its current sync + // point. + neutrinoRecovery := chainClient.BackEnd() == "neutrino" && + w.recoveryWindow > 0 + // We'll wait until the backend is synced to ensure we get the latest // MaxReorgDepth blocks to store. We don't do this for development - // environments as we can't guarantee a lively chain. - if !w.isDevEnv() { + // environments as we can't guarantee a lively chain, except for + // Neutrino, where the cfheader server tells us what it believes the + // chain tip is. + if !w.isDevEnv() || neutrinoRecovery { log.Debug("Waiting for chain backend to sync to tip") if err := w.waitUntilBackendSynced(chainClient); err != nil { return err From 43b5326afa6a39e94a6e8ae615df106919491b63 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 25 Mar 2020 15:28:30 -0700 Subject: [PATCH 04/25] walletdb: update to bolt v1.3.4 --- walletdb/bdb/db.go | 2 +- walletdb/go.mod | 5 +---- walletdb/go.sum | 12 ++++-------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/walletdb/bdb/db.go b/walletdb/bdb/db.go index cf3fc2e2d4..2408fad3f8 100644 --- a/walletdb/bdb/db.go +++ b/walletdb/bdb/db.go @@ -9,7 +9,7 @@ import ( "os" "github.com/btcsuite/btcwallet/walletdb" - "github.com/coreos/bbolt" + "go.etcd.io/bbolt" ) // convertErr converts some bolt errors to the equivalent walletdb error. diff --git a/walletdb/go.mod b/walletdb/go.mod index 59f6db3d65..a93460855a 100644 --- a/walletdb/go.mod +++ b/walletdb/go.mod @@ -4,9 +4,6 @@ go 1.12 require ( github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f - github.com/coreos/bbolt v1.3.3 github.com/davecgh/go-spew v1.1.1 - go.etcd.io/bbolt v1.3.3 // indirect - golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e - golang.org/x/sys v0.0.0-20190904154756-749cb33beabd // indirect + go.etcd.io/bbolt v1.3.4 ) diff --git a/walletdb/go.sum b/walletdb/go.sum index ca356ac361..baadeb0e77 100644 --- a/walletdb/go.sum +++ b/walletdb/go.sum @@ -1,12 +1,8 @@ github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo= github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA= -github.com/coreos/bbolt v1.3.3 h1:n6AiVyVRKQFNb6mJlwESEvvLoDyiTzXX7ORAUlkeBdY= -github.com/coreos/bbolt v1.3.3/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -go.etcd.io/bbolt v1.3.3 h1:MUGmc65QhB3pIlaQ5bB4LwqSj6GIonVJXpZiaKNyaKk= -go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= -golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY= -golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20190904154756-749cb33beabd h1:DBH9mDw0zluJT/R+nGuV3jWFWLFaHyYZWD4tOT+cjn0= -golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +go.etcd.io/bbolt v1.3.4 h1:hi1bXHMVrlQh6WwxAy+qZCV/SYIlqo+Ushwdpa4tAKg= +go.etcd.io/bbolt v1.3.4/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ= +golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5 h1:LfCXLvNmTYH9kEmVgqbnsWfruoXZIrh4YBgqVHtDvw0= +golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= From ba45925bcb33c5dc95ab7d9f4039567562145bcb Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 25 Mar 2020 18:53:31 -0700 Subject: [PATCH 05/25] build: update to walletdb v1.3.0 --- go.mod | 2 +- go.sum | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index b117a0c6fb..f0be0fd4df 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d github.com/btcsuite/btcwallet/wallet/txauthor v1.0.0 github.com/btcsuite/btcwallet/wallet/txrules v1.0.0 - github.com/btcsuite/btcwallet/walletdb v1.2.0 + github.com/btcsuite/btcwallet/walletdb v1.3.0 github.com/btcsuite/btcwallet/wtxmgr v1.0.0 github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792 github.com/davecgh/go-spew v1.1.1 diff --git a/go.sum b/go.sum index 3f267ec86b..82215cec86 100644 --- a/go.sum +++ b/go.sum @@ -21,8 +21,6 @@ github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792 h1:R8vQdOQdZ9Y3 github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792/go.mod h1:ghJtEyQwv5/p4Mg4C0fgbePVuGr935/5ddU9Z3TmDRY= github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46fmI40EZs= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= -github.com/coreos/bbolt v1.3.3 h1:n6AiVyVRKQFNb6mJlwESEvvLoDyiTzXX7ORAUlkeBdY= -github.com/coreos/bbolt v1.3.3/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495 h1:6IyqGr3fnd0tM3YxipK27TUskaOVUjU2nG45yzwcQKY= github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -69,8 +67,8 @@ github.com/onsi/gomega v1.4.1 h1:PZSj/UFNaVp3KxrzHOcS7oyuWA7LoOY/77yCTEFu21U= github.com/onsi/gomega v1.4.1/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.4.3 h1:RE1xgDvH7imwFD45h+u2SgIfERHlS2yNG4DObb5BSKU= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= -go.etcd.io/bbolt v1.3.3 h1:MUGmc65QhB3pIlaQ5bB4LwqSj6GIonVJXpZiaKNyaKk= -go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= +go.etcd.io/bbolt v1.3.4 h1:hi1bXHMVrlQh6WwxAy+qZCV/SYIlqo+Ushwdpa4tAKg= +go.etcd.io/bbolt v1.3.4/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ= golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44 h1:9lP3x0pW80sDI6t1UMSLA4to18W7R7imwAI/sWS9S8Q= golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67 h1:ng3VDlRp5/DHpSWl02R4rM9I+8M2rhmsuLwAMmkLQWE= @@ -89,12 +87,12 @@ golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAG golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY= -golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190904154756-749cb33beabd h1:DBH9mDw0zluJT/R+nGuV3jWFWLFaHyYZWD4tOT+cjn0= golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5 h1:LfCXLvNmTYH9kEmVgqbnsWfruoXZIrh4YBgqVHtDvw0= +golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2 h1:z99zHgr7hKfrUcX/KsoJk5FJfjTceCKIp96+biqP4To= From 60fce250f41d5102063a31a497562b2631ee8469 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 30 Mar 2020 15:30:53 -0700 Subject: [PATCH 06/25] wallet: derive change addresses from the provided key scope Previously, the wallet would determine the key scope to use for change addresses by locating the one compatible with P2WPKH addresses, but this wasn't always safe like in the case when multiple key scopes that supported these addresses existed within the address manager, leading the change address to be created outside of the intended key scope. --- wallet/createtx.go | 16 +++++++++++----- wallet/wallet.go | 11 +++-------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/wallet/createtx.go b/wallet/createtx.go index 9fa0973e82..6bc5aaed65 100644 --- a/wallet/createtx.go +++ b/wallet/createtx.go @@ -135,15 +135,21 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, account uint32, inputSource := makeInputSource(eligible) changeSource := func() ([]byte, error) { - // Derive the change output script. As a hack to allow - // spending from the imported account, change addresses are - // created from account 0. + // Derive the change output script. We'll use the default key + // scope responsible for P2WPKH addresses to do so. As a hack to + // allow spending from the imported account, change addresses + // are created from account 0. var changeAddr btcutil.Address var err error + changeKeyScope := waddrmgr.KeyScopeBIP0084 if account == waddrmgr.ImportedAddrAccount { - changeAddr, err = w.newChangeAddress(addrmgrNs, 0) + changeAddr, err = w.newChangeAddress( + addrmgrNs, 0, changeKeyScope, + ) } else { - changeAddr, err = w.newChangeAddress(addrmgrNs, account) + changeAddr, err = w.newChangeAddress( + addrmgrNs, account, changeKeyScope, + ) } if err != nil { return nil, err diff --git a/wallet/wallet.go b/wallet/wallet.go index a3e092cb3d..b760618ffe 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -2946,7 +2946,7 @@ func (w *Wallet) NewChangeAddress(account uint32, err = walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { addrmgrNs := tx.ReadWriteBucket(waddrmgrNamespaceKey) var err error - addr, err = w.newChangeAddress(addrmgrNs, account) + addr, err = w.newChangeAddress(addrmgrNs, account, scope) return err }) if err != nil { @@ -2968,14 +2968,9 @@ func (w *Wallet) NewChangeAddress(account uint32, // method in order to detect when an on-chain transaction pays to the address // being created. func (w *Wallet) newChangeAddress(addrmgrNs walletdb.ReadWriteBucket, - account uint32) (btcutil.Address, error) { + account uint32, scope waddrmgr.KeyScope) (btcutil.Address, error) { - // As we're making a change address, we'll fetch the type of manager - // that is able to make p2wkh output as they're the most efficient. - scopes := w.Manager.ScopesForExternalAddrType( - waddrmgr.WitnessPubKey, - ) - manager, err := w.Manager.FetchScopedKeyManager(scopes[0]) + manager, err := w.Manager.FetchScopedKeyManager(scope) if err != nil { return nil, err } From 43e19da86842b2d997f02f228a9bc02381968443 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 30 Mar 2020 15:31:44 -0700 Subject: [PATCH 07/25] Revert "waddrmgr+wallet: only watch addresses within default key scopes" The commit being reverted resulted in the discovery of a bug in which change addresses could at times be created outside of the default key scopes, causing us to not properly determine their spends. --- waddrmgr/manager.go | 23 ----------------------- wallet/wallet.go | 20 ++++++++------------ 2 files changed, 8 insertions(+), 35 deletions(-) diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index f2895ca3de..b94c6ee9f3 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -756,29 +756,6 @@ func (m *Manager) ForEachAccountAddress(ns walletdb.ReadBucket, account uint32, return nil } -// ForEachDefaultScopeActiveAddress calls the given function with each active -// address stored in the manager within the default scopes, breaking early on -// error. -func (m *Manager) ForEachDefaultScopeActiveAddress(ns walletdb.ReadBucket, - fn func(addr btcutil.Address) error) error { - - m.mtx.RLock() - defer m.mtx.RUnlock() - - for _, keyScope := range DefaultKeyScopes { - scopedMgr, ok := m.scopedManagers[keyScope] - if !ok { - return fmt.Errorf("manager for default key scope with "+ - "purpose %v not found", keyScope.Purpose) - } - if err := scopedMgr.ForEachActiveAddress(ns, fn); err != nil { - return err - } - } - - return nil -} - // ChainParams returns the chain parameters for this address manager. func (m *Manager) ChainParams() *chaincfg.Params { // NOTE: No need for mutex here since the net field does not change diff --git a/wallet/wallet.go b/wallet/wallet.go index b760618ffe..cdb1862377 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -303,22 +303,18 @@ func (w *Wallet) SetChainSynced(synced bool) { w.chainClientSyncMtx.Unlock() } -// activeData returns the currently-active receiving addresses that exist within -// the wallet's default key scopes and all unspent outputs. This is primarily -// intended to provide the parameters for a rescan request. -func (w *Wallet) activeData(dbtx walletdb.ReadTx) ([]btcutil.Address, - []wtxmgr.Credit, error) { - +// activeData returns the currently-active receiving addresses and all unspent +// outputs. This is primarely intended to provide the parameters for a +// rescan request. +func (w *Wallet) activeData(dbtx walletdb.ReadTx) ([]btcutil.Address, []wtxmgr.Credit, error) { addrmgrNs := dbtx.ReadBucket(waddrmgrNamespaceKey) txmgrNs := dbtx.ReadBucket(wtxmgrNamespaceKey) var addrs []btcutil.Address - err := w.Manager.ForEachDefaultScopeActiveAddress( - addrmgrNs, func(addr btcutil.Address) error { - addrs = append(addrs, addr) - return nil - }, - ) + err := w.Manager.ForEachActiveAddress(addrmgrNs, func(addr btcutil.Address) error { + addrs = append(addrs, addr) + return nil + }) if err != nil { return nil, nil, err } From 12850499231eaf6c38c165ef2e3365d0a8a2a157 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 30 Mar 2020 15:32:34 -0700 Subject: [PATCH 08/25] wallet: include addresses from relevant key scopes in rescan Due to a no longer existing bug within the wallet, it was possible for change addresses to be created outside of their intended key scope (the default), so wallets affected by this now need to ensure they scan the chain for all addresses within the default key scopes (as expected), and all _internal_ addresses (branch used for change addresses) within any other registered key scopes to reflect their proper balance. --- waddrmgr/manager.go | 37 +++++++++++++++++++++++++++++++++++++ waddrmgr/scoped_manager.go | 27 +++++++++++++++++++++++++++ wallet/wallet.go | 10 ++++++---- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index b94c6ee9f3..8dc279a7b2 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -738,6 +738,43 @@ func (m *Manager) ForEachActiveAddress(ns walletdb.ReadBucket, fn func(addr btcu return nil } +// ForEachRelevantActiveAddress invokes the given closure on each active +// address relevant to the wallet. Ideally, only addresses within the default +// key scopes would be relevant, but due to a bug (now fixed) in which change +// addresses could be created outside of the default key scopes, we now need to +// check for those as well. +func (m *Manager) ForEachRelevantActiveAddress(ns walletdb.ReadBucket, + fn func(addr btcutil.Address) error) error { + + m.mtx.RLock() + defer m.mtx.RUnlock() + + for _, scopedMgr := range m.scopedManagers { + // If the manager is for a default key scope, we'll return all + // addresses, otherwise we'll only return internal addresses, as + // that's the branch used for change addresses. + isDefaultKeyScope := false + for _, defaultKeyScope := range DefaultKeyScopes { + if scopedMgr.Scope() == defaultKeyScope { + isDefaultKeyScope = true + break + } + } + + var err error + if isDefaultKeyScope { + err = scopedMgr.ForEachActiveAddress(ns, fn) + } else { + err = scopedMgr.ForEachInternalActiveAddress(ns, fn) + } + if err != nil { + return err + } + } + + return nil +} + // ForEachAccountAddress calls the given function with each address of // the given account stored in the manager, breaking early on error. func (m *Manager) ForEachAccountAddress(ns walletdb.ReadBucket, account uint32, diff --git a/waddrmgr/scoped_manager.go b/waddrmgr/scoped_manager.go index b9acda3d62..16bd83a2af 100644 --- a/waddrmgr/scoped_manager.go +++ b/waddrmgr/scoped_manager.go @@ -1760,3 +1760,30 @@ func (s *ScopedKeyManager) ForEachActiveAddress(ns walletdb.ReadBucket, return nil } + +// ForEachInternalActiveAddress invokes the given closure on each _internal_ +// active address belonging to the scoped key manager, breaking early on error. +func (s *ScopedKeyManager) ForEachInternalActiveAddress(ns walletdb.ReadBucket, + fn func(addr btcutil.Address) error) error { + + s.mtx.Lock() + defer s.mtx.Unlock() + + addrFn := func(rowInterface interface{}) error { + managedAddr, err := s.rowInterfaceToManaged(ns, rowInterface) + if err != nil { + return err + } + // Skip any non-internal branch addresses. + if !managedAddr.Internal() { + return nil + } + return fn(managedAddr.Address()) + } + + if err := forEachActiveAddress(ns, &s.scope, addrFn); err != nil { + return maybeConvertDbError(err) + } + + return nil +} diff --git a/wallet/wallet.go b/wallet/wallet.go index cdb1862377..3a8167bf26 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -311,10 +311,12 @@ func (w *Wallet) activeData(dbtx walletdb.ReadTx) ([]btcutil.Address, []wtxmgr.C txmgrNs := dbtx.ReadBucket(wtxmgrNamespaceKey) var addrs []btcutil.Address - err := w.Manager.ForEachActiveAddress(addrmgrNs, func(addr btcutil.Address) error { - addrs = append(addrs, addr) - return nil - }) + err := w.Manager.ForEachRelevantActiveAddress( + addrmgrNs, func(addr btcutil.Address) error { + addrs = append(addrs, addr) + return nil + }, + ) if err != nil { return nil, nil, err } From 31c027e19f1dc3c996e50d8b3d688f4a4a215278 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 30 Mar 2020 15:33:23 -0700 Subject: [PATCH 09/25] wallet: perform recovery on all registered key scopes In similar fashion to the previous commit, due to a no longer existing bug within the wallet, it was possible for change addresses to be created outside of their intended key scope (the default), so wallets affected by this now need to ensure upon recovery that they scan the chain for _all_ existing key scopes, rather than just the default ones, to reflect their proper balance. Through manual testing, it was shown that the impact of recovering the additional key scopes is negligible in most cases for both full nodes and light clients. --- wallet/wallet.go | 62 +++++++++++------------------------------------- 1 file changed, 14 insertions(+), 48 deletions(-) diff --git a/wallet/wallet.go b/wallet/wallet.go index 3a8167bf26..aaa9ff6638 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -637,13 +637,15 @@ func (w *Wallet) recovery(chainClient chain.Interface, ) // In the event that this recovery is being resumed, we will need to - // repopulate all found addresses from the database. For basic recovery, - // we will only do so for the default scopes. - scopedMgrs, err := w.defaultScopeManagers() - if err != nil { - return err + // repopulate all found addresses from the database. Ideally, for basic + // recovery, we would only do so for the default scopes, but due to a + // bug in which the wallet would create change addresses outside of the + // default scopes, it's necessary to attempt all registered key scopes. + scopedMgrs := make(map[waddrmgr.KeyScope]*waddrmgr.ScopedKeyManager) + for _, scopedMgr := range w.Manager.ActiveScopedKeyManagers() { + scopedMgrs[scopedMgr.Scope()] = scopedMgr } - err = walletdb.View(w.db, func(tx walletdb.ReadTx) error { + err := walletdb.View(w.db, func(tx walletdb.ReadTx) error { txMgrNS := tx.ReadBucket(wtxmgrNamespaceKey) credits, err := w.TxStore.UnspentOutputs(txMgrNS) if err != nil { @@ -712,9 +714,9 @@ func (w *Wallet) recovery(chainClient chain.Interface, return err } } - return w.recoverDefaultScopes( + return w.recoverScopedAddresses( chainClient, tx, ns, recoveryBatch, - recoveryMgr.State(), + recoveryMgr.State(), scopedMgrs, ) }) if err != nil { @@ -737,48 +739,10 @@ func (w *Wallet) recovery(chainClient chain.Interface, return nil } -// defaultScopeManagers fetches the ScopedKeyManagers from the wallet using the -// default set of key scopes. -func (w *Wallet) defaultScopeManagers() ( - map[waddrmgr.KeyScope]*waddrmgr.ScopedKeyManager, error) { - - scopedMgrs := make(map[waddrmgr.KeyScope]*waddrmgr.ScopedKeyManager) - for _, scope := range waddrmgr.DefaultKeyScopes { - scopedMgr, err := w.Manager.FetchScopedKeyManager(scope) - if err != nil { - return nil, err - } - - scopedMgrs[scope] = scopedMgr - } - - return scopedMgrs, nil -} - -// recoverDefaultScopes attempts to recover any addresses belonging to any -// active scoped key managers known to the wallet. Recovery of each scope's -// default account will be done iteratively against the same batch of blocks. -// TODO(conner): parallelize/pipeline/cache intermediate network requests -func (w *Wallet) recoverDefaultScopes( - chainClient chain.Interface, - tx walletdb.ReadWriteTx, - ns walletdb.ReadWriteBucket, - batch []wtxmgr.BlockMeta, - recoveryState *RecoveryState) error { - - scopedMgrs, err := w.defaultScopeManagers() - if err != nil { - return err - } - - return w.recoverScopedAddresses( - chainClient, tx, ns, batch, recoveryState, scopedMgrs, - ) -} - -// recoverAccountAddresses scans a range of blocks in attempts to recover any +// recoverScopedAddresses scans a range of blocks in attempts to recover any // previously used addresses for a particular account derivation path. At a high // level, the algorithm works as follows: +// // 1) Ensure internal and external branch horizons are fully expanded. // 2) Filter the entire range of blocks, stopping if a non-zero number of // address are contained in a particular block. @@ -786,6 +750,8 @@ func (w *Wallet) recoverDefaultScopes( // 4) Record any outpoints found in the block that should be watched for spends // 5) Trim the range of blocks up to and including the one reporting the addrs. // 6) Repeat from (1) if there are still more blocks in the range. +// +// TODO(conner): parallelize/pipeline/cache intermediate network requests func (w *Wallet) recoverScopedAddresses( chainClient chain.Interface, tx walletdb.ReadWriteTx, From 90a0744ae4fafdcfebf1535c39263c57942cc86c Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 24 Mar 2020 16:23:41 -0700 Subject: [PATCH 10/25] chain: batch filter fetches for neutrino chain client This greatly speeds up wallet recovery with a neutrino backend, as we'll avoid unnecessary round trips for each block filter fetch. --- chain/neutrino.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/chain/neutrino.go b/chain/neutrino.go index 71ef8009ff..28987f7a32 100644 --- a/chain/neutrino.go +++ b/chain/neutrino.go @@ -199,6 +199,11 @@ func (s *NeutrinoClient) FilterBlocks( // the filter returns a positive match, the full block is then requested // and scanned for addresses using the block filterer. for i, blk := range req.Blocks { + // TODO(wilmer): Investigate why polling it still necessary + // here. While testing, I ran into a few instances where the + // filter was not retrieved, leading to a panic. This should not + // happen in most cases thanks to the query logic revamp within + // Neutrino, but it seems there's still an uncovered edge case. filter, err := s.pollCFilter(&blk.Hash) if err != nil { return nil, err @@ -312,7 +317,9 @@ func (s *NeutrinoClient) pollCFilter(hash *chainhash.Hash) (*gcs.Filter, error) time.Sleep(100 * time.Millisecond) } - filter, err = s.CS.GetCFilter(*hash, wire.GCSFilterRegular) + filter, err = s.CS.GetCFilter( + *hash, wire.GCSFilterRegular, neutrino.OptimisticBatch(), + ) if err != nil { count++ continue From e51b4821ce72300828fa12b96ee3ba41d76df103 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 3 Apr 2020 14:13:53 -0700 Subject: [PATCH 11/25] walletdb: use temp dir for db filepaths in tests --- walletdb/bdb/driver_test.go | 22 ++++++++++++++++++---- walletdb/bdb/interface_test.go | 11 ++++++++++- walletdb/db_test.go | 12 ++++++++++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/walletdb/bdb/driver_test.go b/walletdb/bdb/driver_test.go index 8fdcab17c8..aa4d87210f 100644 --- a/walletdb/bdb/driver_test.go +++ b/walletdb/bdb/driver_test.go @@ -6,7 +6,9 @@ package bdb_test import ( "fmt" + "io/ioutil" "os" + "path/filepath" "reflect" "testing" @@ -71,13 +73,19 @@ func TestCreateOpenFail(t *testing.T) { // Ensure operations against a closed database return the expected // error. - dbPath := "createfail.db" + tempDir, err := ioutil.TempDir("", "createfail") + if err != nil { + t.Errorf("unable to create temp dir: %v", err) + return + } + defer os.Remove(tempDir) + + dbPath := filepath.Join(tempDir, "db") db, err := walletdb.Create(dbType, dbPath, true) if err != nil { t.Errorf("Create: unexpected error: %v", err) return } - defer os.Remove(dbPath) db.Close() wantErr = walletdb.ErrDbNotOpen @@ -92,13 +100,19 @@ func TestCreateOpenFail(t *testing.T) { // reopening the database. func TestPersistence(t *testing.T) { // Create a new database to run tests against. - dbPath := "persistencetest.db" + tempDir, err := ioutil.TempDir("", "persistencetest") + if err != nil { + t.Errorf("unable to create temp dir: %v", err) + return + } + defer os.Remove(tempDir) + + dbPath := filepath.Join(tempDir, "db") db, err := walletdb.Create(dbType, dbPath, true) if err != nil { t.Errorf("Failed to create test database (%s) %v", dbType, err) return } - defer os.Remove(dbPath) defer db.Close() // Create a namespace and put some values into it so they can be tested diff --git a/walletdb/bdb/interface_test.go b/walletdb/bdb/interface_test.go index 1dcf605e01..3755b1d690 100644 --- a/walletdb/bdb/interface_test.go +++ b/walletdb/bdb/interface_test.go @@ -13,7 +13,9 @@ package bdb_test import ( + "io/ioutil" "os" + "path/filepath" "testing" "github.com/btcsuite/btcwallet/walletdb/walletdbtest" @@ -21,7 +23,14 @@ import ( // TestInterface performs all interfaces tests for this database driver. func TestInterface(t *testing.T) { - dbPath := "interfacetest.db" + tempDir, err := ioutil.TempDir("", "interfacetest") + if err != nil { + t.Errorf("unable to create temp dir: %v", err) + return + } + defer os.Remove(tempDir) + + dbPath := filepath.Join(tempDir, "db") defer os.RemoveAll(dbPath) walletdbtest.TestInterface(t, dbType, dbPath, true) } diff --git a/walletdb/db_test.go b/walletdb/db_test.go index 48b60c3111..30c4341bc8 100644 --- a/walletdb/db_test.go +++ b/walletdb/db_test.go @@ -6,7 +6,9 @@ package walletdb_test import ( "fmt" + "io/ioutil" "os" + "path/filepath" "testing" "github.com/btcsuite/btcwallet/walletdb" @@ -54,14 +56,20 @@ func TestAddDuplicateDriver(t *testing.T) { "got %v, want %v", err, walletdb.ErrDbTypeRegistered) } - dbPath := "dupdrivertest.db" + tempDir, err := ioutil.TempDir("", "dupdrivertest") + if err != nil { + t.Errorf("unable to create temp dir: %v", err) + return + } + defer os.Remove(tempDir) + + dbPath := filepath.Join(tempDir, "db") db, err := walletdb.Create(dbType, dbPath, true) if err != nil { t.Errorf("failed to create database: %v", err) return } db.Close() - _ = os.Remove(dbPath) } From 57be08fbbf0202b0a27af1c2f6f851fa2c5f9c85 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 3 Apr 2020 14:18:22 -0700 Subject: [PATCH 12/25] Revert "walletdb: update to bolt v1.3.4" The v1.3.4 version of the bolt module contained a bug fix to allow the use of go1.14, but it uncovered a GC issue. --- walletdb/bdb/db.go | 2 +- walletdb/go.mod | 5 ++++- walletdb/go.sum | 12 ++++++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/walletdb/bdb/db.go b/walletdb/bdb/db.go index 2408fad3f8..cf3fc2e2d4 100644 --- a/walletdb/bdb/db.go +++ b/walletdb/bdb/db.go @@ -9,7 +9,7 @@ import ( "os" "github.com/btcsuite/btcwallet/walletdb" - "go.etcd.io/bbolt" + "github.com/coreos/bbolt" ) // convertErr converts some bolt errors to the equivalent walletdb error. diff --git a/walletdb/go.mod b/walletdb/go.mod index a93460855a..59f6db3d65 100644 --- a/walletdb/go.mod +++ b/walletdb/go.mod @@ -4,6 +4,9 @@ go 1.12 require ( github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f + github.com/coreos/bbolt v1.3.3 github.com/davecgh/go-spew v1.1.1 - go.etcd.io/bbolt v1.3.4 + go.etcd.io/bbolt v1.3.3 // indirect + golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e + golang.org/x/sys v0.0.0-20190904154756-749cb33beabd // indirect ) diff --git a/walletdb/go.sum b/walletdb/go.sum index baadeb0e77..ca356ac361 100644 --- a/walletdb/go.sum +++ b/walletdb/go.sum @@ -1,8 +1,12 @@ github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo= github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA= +github.com/coreos/bbolt v1.3.3 h1:n6AiVyVRKQFNb6mJlwESEvvLoDyiTzXX7ORAUlkeBdY= +github.com/coreos/bbolt v1.3.3/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -go.etcd.io/bbolt v1.3.4 h1:hi1bXHMVrlQh6WwxAy+qZCV/SYIlqo+Ushwdpa4tAKg= -go.etcd.io/bbolt v1.3.4/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ= -golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5 h1:LfCXLvNmTYH9kEmVgqbnsWfruoXZIrh4YBgqVHtDvw0= -golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +go.etcd.io/bbolt v1.3.3 h1:MUGmc65QhB3pIlaQ5bB4LwqSj6GIonVJXpZiaKNyaKk= +go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= +golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY= +golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190904154756-749cb33beabd h1:DBH9mDw0zluJT/R+nGuV3jWFWLFaHyYZWD4tOT+cjn0= +golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= From d2ce512b4bcbfbde6034218dab21714095f8626e Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 3 Apr 2020 15:08:07 -0700 Subject: [PATCH 13/25] build: pin to walletdb v1.3.1 --- go.mod | 2 +- go.sum | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index f0be0fd4df..a20589cb62 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d github.com/btcsuite/btcwallet/wallet/txauthor v1.0.0 github.com/btcsuite/btcwallet/wallet/txrules v1.0.0 - github.com/btcsuite/btcwallet/walletdb v1.3.0 + github.com/btcsuite/btcwallet/walletdb v1.3.1 github.com/btcsuite/btcwallet/wtxmgr v1.0.0 github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792 github.com/davecgh/go-spew v1.1.1 diff --git a/go.sum b/go.sum index 82215cec86..3f267ec86b 100644 --- a/go.sum +++ b/go.sum @@ -21,6 +21,8 @@ github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792 h1:R8vQdOQdZ9Y3 github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792/go.mod h1:ghJtEyQwv5/p4Mg4C0fgbePVuGr935/5ddU9Z3TmDRY= github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46fmI40EZs= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= +github.com/coreos/bbolt v1.3.3 h1:n6AiVyVRKQFNb6mJlwESEvvLoDyiTzXX7ORAUlkeBdY= +github.com/coreos/bbolt v1.3.3/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495 h1:6IyqGr3fnd0tM3YxipK27TUskaOVUjU2nG45yzwcQKY= github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -67,8 +69,8 @@ github.com/onsi/gomega v1.4.1 h1:PZSj/UFNaVp3KxrzHOcS7oyuWA7LoOY/77yCTEFu21U= github.com/onsi/gomega v1.4.1/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.4.3 h1:RE1xgDvH7imwFD45h+u2SgIfERHlS2yNG4DObb5BSKU= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= -go.etcd.io/bbolt v1.3.4 h1:hi1bXHMVrlQh6WwxAy+qZCV/SYIlqo+Ushwdpa4tAKg= -go.etcd.io/bbolt v1.3.4/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ= +go.etcd.io/bbolt v1.3.3 h1:MUGmc65QhB3pIlaQ5bB4LwqSj6GIonVJXpZiaKNyaKk= +go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44 h1:9lP3x0pW80sDI6t1UMSLA4to18W7R7imwAI/sWS9S8Q= golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67 h1:ng3VDlRp5/DHpSWl02R4rM9I+8M2rhmsuLwAMmkLQWE= @@ -87,12 +89,12 @@ golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAG golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY= +golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190904154756-749cb33beabd h1:DBH9mDw0zluJT/R+nGuV3jWFWLFaHyYZWD4tOT+cjn0= golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5 h1:LfCXLvNmTYH9kEmVgqbnsWfruoXZIrh4YBgqVHtDvw0= -golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2 h1:z99zHgr7hKfrUcX/KsoJk5FJfjTceCKIp96+biqP4To= From 4c5bc1b15d3c0dc0d070ca019f4653ff95657452 Mon Sep 17 00:00:00 2001 From: Dev Random Date: Fri, 24 Apr 2020 17:44:21 -0700 Subject: [PATCH 14/25] waddrmgr: create watch-only address managers and accounts 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. Co-authored-by: Ken Sedgwick --- waddrmgr/README.md | 3 +- waddrmgr/db.go | 4 + waddrmgr/manager.go | 335 ++++++++++++++++------------- waddrmgr/manager_test.go | 414 +++++++++++++++++++++++++++++------- waddrmgr/scoped_manager.go | 85 +++++++- wallet/loader.go | 36 +++- wallet/wallet.go | 57 +++-- wallet/watchingonly_test.go | 34 +++ 8 files changed, 722 insertions(+), 246 deletions(-) create mode 100644 wallet/watchingonly_test.go diff --git a/waddrmgr/README.md b/waddrmgr/README.md index 6499c6d33a..ae39edc3e5 100644 --- a/waddrmgr/README.md +++ b/waddrmgr/README.md @@ -32,8 +32,9 @@ report. Package waddrmgr is licensed under the liberal ISC license. - Import WIF keys - Import pay-to-script-hash scripts for things such as multi-signature transactions - - Ability to export a watching-only version which does not contain any private + - Ability to start in watching-only mode which does not contain any private key material + - Ability to convert to watching-only mode - Programmatically detectable errors, including encapsulation of errors from packages it relies on - Address synchronization capabilities diff --git a/waddrmgr/db.go b/waddrmgr/db.go index 97ecc5ac93..19167ea2b4 100644 --- a/waddrmgr/db.go +++ b/waddrmgr/db.go @@ -850,6 +850,7 @@ func forEachAccount(ns walletdb.ReadBucket, scope *KeyScope, } // fetchLastAccount retrieves the last account from the database. +// If no accounts, returns twos-complement representation of -1, so that the next account is zero func fetchLastAccount(ns walletdb.ReadBucket, scope *KeyScope) (uint32, error) { scopedBucket, err := fetchReadScopeBucket(ns, scope) if err != nil { @@ -859,6 +860,9 @@ func fetchLastAccount(ns walletdb.ReadBucket, scope *KeyScope) (uint32, error) { metaBucket := scopedBucket.NestedReadBucket(metaBucketName) val := metaBucket.Get(lastAccountName) + if val == nil { + return (1 << 32) - 1, nil + } if len(val) != 4 { str := fmt.Sprintf("malformed metadata '%s' stored in database", lastAccountName) diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index 8dc279a7b2..b0f3bee2c3 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -438,53 +438,61 @@ func (m *Manager) Close() { // // TODO(roasbeef): addrtype of raw key means it'll look in scripts to possibly // mark as gucci? -func (m *Manager) NewScopedKeyManager(ns walletdb.ReadWriteBucket, scope KeyScope, - addrSchema ScopeAddrSchema) (*ScopedKeyManager, error) { +func (m *Manager) NewScopedKeyManager(ns walletdb.ReadWriteBucket, + scope KeyScope, addrSchema ScopeAddrSchema) (*ScopedKeyManager, error) { m.mtx.Lock() defer m.mtx.Unlock() - // If the manager is locked, then we can't create a new scoped manager. - if m.locked { - return nil, managerError(ErrLocked, errLocked, nil) - } + var rootPriv *hdkeychain.ExtendedKey + if !m.watchingOnly { + // If the manager is locked, then we can't create a new scoped + // manager. + if m.locked { + return nil, managerError(ErrLocked, errLocked, nil) + } - // Now that we know the manager is unlocked, we'll need to fetch the - // root master HD private key. This is required as we'll be attempting - // the following derivation: m/purpose'/cointype' - // - // Note that the path to the coin type is requires hardened derivation, - // therefore this can only be done if the wallet's root key hasn't been - // neutered. - masterRootPrivEnc, _, err := fetchMasterHDKeys(ns) - if err != nil { - return nil, err - } + // Now that we know the manager is unlocked, we'll need to + // fetch the root master HD private key. This is required as + // we'll be attempting the following derivation: + // m/purpose'/cointype' + // + // Note that the path to the coin type is requires hardened + // derivation, therefore this can only be done if the wallet's + // root key hasn't been neutered. + masterRootPrivEnc, _, err := fetchMasterHDKeys(ns) + if err != nil { + return nil, err + } - // If the master root private key isn't found within the database, but - // we need to bail here as we can't create the cointype key without the - // master root private key. - if masterRootPrivEnc == nil { - return nil, managerError(ErrWatchingOnly, "", nil) - } + // If the master root private key isn't found within the + // database, but we need to bail here as we can't create the + // cointype key without the master root private key. + if masterRootPrivEnc == nil { + return nil, managerError(ErrWatchingOnly, "", nil) + } - // Before we can derive any new scoped managers using this key, we'll - // need to fully decrypt it. - serializedMasterRootPriv, err := m.cryptoKeyPriv.Decrypt(masterRootPrivEnc) - if err != nil { - str := fmt.Sprintf("failed to decrypt master root serialized private key") - return nil, managerError(ErrLocked, str, err) - } + // Before we can derive any new scoped managers using this + // key, we'll need to fully decrypt it. + serializedMasterRootPriv, err := + m.cryptoKeyPriv.Decrypt(masterRootPrivEnc) + if err != nil { + str := fmt.Sprintf("failed to decrypt master root " + + "serialized private key") + return nil, managerError(ErrLocked, str, err) + } - // Now that we know the root priv is within the database, we'll decode - // it into a usable object. - rootPriv, err := hdkeychain.NewKeyFromString( - string(serializedMasterRootPriv), - ) - zero.Bytes(serializedMasterRootPriv) - if err != nil { - str := fmt.Sprintf("failed to create master extended private key") - return nil, managerError(ErrKeyChain, str, err) + // Now that we know the root priv is within the database, + // we'll decode it into a usable object. + rootPriv, err = hdkeychain.NewKeyFromString( + string(serializedMasterRootPriv), + ) + zero.Bytes(serializedMasterRootPriv) + if err != nil { + str := fmt.Sprintf("failed to create master extended " + + "private key") + return nil, managerError(ErrKeyChain, str, err) + } } // Now that we have the root private key, we'll fetch the scope bucket @@ -506,19 +514,21 @@ func (m *Manager) NewScopedKeyManager(ns walletdb.ReadWriteBucket, scope KeyScop } scopeKey := scopeToBytes(&scope) schemaBytes := scopeSchemaToBytes(&addrSchema) - err = scopeSchemas.Put(scopeKey[:], schemaBytes) + err := scopeSchemas.Put(scopeKey[:], schemaBytes) if err != nil { return nil, err } - // With the database state created, we'll now derive the cointype key - // using the master HD private key, then encrypt it along with the - // first account using our crypto keys. - err = createManagerKeyScope( - ns, scope, rootPriv, m.cryptoKeyPub, m.cryptoKeyPriv, - ) - if err != nil { - return nil, err + if !m.watchingOnly { + // With the database state created, we'll now derive the + // cointype key using the master HD private key, then encrypt + // it along with the first account using our crypto keys. + err = createManagerKeyScope( + ns, scope, rootPriv, m.cryptoKeyPub, m.cryptoKeyPriv, + ) + if err != nil { + return nil, err + } } // Finally, we'll register this new scoped manager with the root @@ -1298,7 +1308,7 @@ func newManager(chainParams *chaincfg.Params, masterKeyPub *snacl.SecretKey, masterKeyPriv *snacl.SecretKey, cryptoKeyPub EncryptorDecryptor, cryptoKeyPrivEncrypted, cryptoKeyScriptEncrypted []byte, syncInfo *syncState, birthday time.Time, privPassphraseSalt [saltSize]byte, - scopedManagers map[KeyScope]*ScopedKeyManager) *Manager { + scopedManagers map[KeyScope]*ScopedKeyManager, watchingOnly bool) *Manager { m := &Manager{ chainParams: chainParams, @@ -1316,6 +1326,7 @@ func newManager(chainParams *chaincfg.Params, masterKeyPub *snacl.SecretKey, scopedManagers: scopedManagers, externalAddrSchemas: make(map[AddressType][]KeyScope), internalAddrSchemas: make(map[AddressType][]KeyScope), + watchingOnly: watchingOnly, } for _, sMgr := range m.scopedManagers { @@ -1540,9 +1551,8 @@ func loadManager(ns walletdb.ReadBucket, pubPassphrase []byte, mgr := newManager( chainParams, &masterKeyPub, &masterKeyPriv, cryptoKeyPub, cryptoKeyPrivEnc, cryptoKeyScriptEnc, syncInfo, - birthday, privPassphraseSalt, scopedManagers, + birthday, privPassphraseSalt, scopedManagers, watchingOnly, ) - mgr.watchingOnly = watchingOnly for _, scopedManager := range scopedManagers { scopedManager.rootManager = mgr @@ -1676,27 +1686,40 @@ func createManagerKeyScope(ns walletdb.ReadWriteBucket, ) } -// Create creates a new address manager in the given namespace. The seed must -// conform to the standards described in hdkeychain.NewMaster and will be used -// to create the master root node from which all hierarchical deterministic -// addresses are derived. This allows all chained addresses in the address -// manager to be recovered by using the same seed. +// Create creates a new address manager in the given namespace. // -// All private and public keys and information are protected by secret keys -// derived from the provided private and public passphrases. The public -// passphrase is required on subsequent opens of the address manager, and the -// private passphrase is required to unlock the address manager in order to -// gain access to any private keys and information. +// The seed must conform to the standards described in +// hdkeychain.NewMaster and will be used to create the master root +// node from which all hierarchical deterministic addresses are +// derived. This allows all chained addresses in the address manager +// to be recovered by using the same seed. // -// If a config structure is passed to the function, that configuration will -// override the defaults. +// If the provided seed value is nil the address manager will be +// created in watchingOnly mode in which case no default accounts or +// scoped managers are created - it is up to the caller to create a +// new one with NewAccountWatchingOnly and NewScopedKeyManager. +// +// All private and public keys and information are protected by secret +// keys derived from the provided private and public passphrases. The +// public passphrase is required on subsequent opens of the address +// manager, and the private passphrase is required to unlock the +// address manager in order to gain access to any private keys and +// information. // -// A ManagerError with an error code of ErrAlreadyExists will be returned the -// address manager already exists in the specified namespace. -func Create(ns walletdb.ReadWriteBucket, seed, pubPassphrase, privPassphrase []byte, +// If a config structure is passed to the function, that configuration +// will override the defaults. +// +// A ManagerError with an error code of ErrAlreadyExists will be +// returned the address manager already exists in the specified +// namespace. +func Create(ns walletdb.ReadWriteBucket, + seed, pubPassphrase, privPassphrase []byte, chainParams *chaincfg.Params, config *ScryptOptions, birthday time.Time) error { + // If the seed argument is nil we create in watchingOnly mode. + isWatchingOnly := seed == nil + // Return an error if the manager has already been created in // the given database namespace. exists := managerExists(ns) @@ -1705,13 +1728,17 @@ func Create(ns walletdb.ReadWriteBucket, seed, pubPassphrase, privPassphrase []b } // Ensure the private passphrase is not empty. - if len(privPassphrase) == 0 { + if !isWatchingOnly && len(privPassphrase) == 0 { str := "private passphrase may not be empty" return managerError(ErrEmptyPassphrase, str, nil) } // Perform the initial bucket creation and database namespace setup. - if err := createManagerNS(ns, ScopeAddrMap); err != nil { + defaultScopes := map[KeyScope]ScopeAddrSchema{} + if !isWatchingOnly { + defaultScopes = ScopeAddrMap + } + if err := createManagerNS(ns, defaultScopes); err != nil { return maybeConvertDbError(err) } @@ -1726,22 +1753,6 @@ func Create(ns walletdb.ReadWriteBucket, seed, pubPassphrase, privPassphrase []b str := "failed to master public key" return managerError(ErrCrypto, str, err) } - masterKeyPriv, err := newSecretKey(&privPassphrase, config) - if err != nil { - str := "failed to master private key" - return managerError(ErrCrypto, str, err) - } - defer masterKeyPriv.Zero() - - // Generate the private passphrase salt. This is used when hashing - // passwords to detect whether an unlock can be avoided when the manager - // is already unlocked. - var privPassphraseSalt [saltSize]byte - _, err = rand.Read(privPassphraseSalt[:]) - if err != nil { - str := "failed to read random source for passphrase salt" - return managerError(ErrCrypto, str, err) - } // Generate new crypto public, private, and script keys. These keys are // used to protect the actual public and private data such as addresses, @@ -1751,18 +1762,6 @@ func Create(ns walletdb.ReadWriteBucket, seed, pubPassphrase, privPassphrase []b str := "failed to generate crypto public key" return managerError(ErrCrypto, str, err) } - cryptoKeyPriv, err := newCryptoKey() - if err != nil { - str := "failed to generate crypto private key" - return managerError(ErrCrypto, str, err) - } - defer cryptoKeyPriv.Zero() - cryptoKeyScript, err := newCryptoKey() - if err != nil { - str := "failed to generate crypto script key" - return managerError(ErrCrypto, str, err) - } - defer cryptoKeyScript.Zero() // Encrypt the crypto keys with the associated master keys. cryptoKeyPubEnc, err := masterKeyPub.Encrypt(cryptoKeyPub.Bytes()) @@ -1770,16 +1769,6 @@ func Create(ns walletdb.ReadWriteBucket, seed, pubPassphrase, privPassphrase []b str := "failed to encrypt crypto public key" return managerError(ErrCrypto, str, err) } - cryptoKeyPrivEnc, err := masterKeyPriv.Encrypt(cryptoKeyPriv.Bytes()) - if err != nil { - str := "failed to encrypt crypto private key" - return managerError(ErrCrypto, str, err) - } - cryptoKeyScriptEnc, err := masterKeyPriv.Encrypt(cryptoKeyScript.Bytes()) - if err != nil { - str := "failed to encrypt crypto script key" - return managerError(ErrCrypto, str, err) - } // Use the genesis block for the passed chain as the created at block // for the default. @@ -1788,52 +1777,108 @@ func Create(ns walletdb.ReadWriteBucket, seed, pubPassphrase, privPassphrase []b // Create the initial sync state. syncInfo := newSyncState(createdAt, createdAt) - // Save the master key params to the database. pubParams := masterKeyPub.Marshal() - privParams := masterKeyPriv.Marshal() - err = putMasterKeyParams(ns, pubParams, privParams) - if err != nil { - return maybeConvertDbError(err) - } - // Generate the BIP0044 HD key structure to ensure the provided seed - // can generate the required structure with no issues. + var privParams []byte = nil + var masterKeyPriv *snacl.SecretKey + var cryptoKeyPrivEnc []byte = nil + var cryptoKeyScriptEnc []byte = nil + if !isWatchingOnly { + masterKeyPriv, err = newSecretKey(&privPassphrase, config) + if err != nil { + str := "failed to master private key" + return managerError(ErrCrypto, str, err) + } + defer masterKeyPriv.Zero() - // Derive the master extended key from the seed. - rootKey, err := hdkeychain.NewMaster(seed, chainParams) - if err != nil { - str := "failed to derive master extended key" - return managerError(ErrKeyChain, str, err) - } - rootPubKey, err := rootKey.Neuter() - if err != nil { - str := "failed to neuter master extended key" - return managerError(ErrKeyChain, str, err) - } + // Generate the private passphrase salt. This is used when + // hashing passwords to detect whether an unlock can be + // avoided when the manager is already unlocked. + var privPassphraseSalt [saltSize]byte + _, err = rand.Read(privPassphraseSalt[:]) + if err != nil { + str := "failed to read random source for passphrase salt" + return managerError(ErrCrypto, str, err) + } - // Next, for each registers default manager scope, we'll create the - // hardened cointype key for it, as well as the first default account. - for _, defaultScope := range DefaultKeyScopes { - err := createManagerKeyScope( - ns, defaultScope, rootKey, cryptoKeyPub, cryptoKeyPriv, - ) + cryptoKeyPriv, err := newCryptoKey() + if err != nil { + str := "failed to generate crypto private key" + return managerError(ErrCrypto, str, err) + } + defer cryptoKeyPriv.Zero() + cryptoKeyScript, err := newCryptoKey() + if err != nil { + str := "failed to generate crypto script key" + return managerError(ErrCrypto, str, err) + } + defer cryptoKeyScript.Zero() + + cryptoKeyPrivEnc, err = + masterKeyPriv.Encrypt(cryptoKeyPriv.Bytes()) + if err != nil { + str := "failed to encrypt crypto private key" + return managerError(ErrCrypto, str, err) + } + cryptoKeyScriptEnc, err = + masterKeyPriv.Encrypt(cryptoKeyScript.Bytes()) + if err != nil { + str := "failed to encrypt crypto script key" + return managerError(ErrCrypto, str, err) + } + + // Generate the BIP0044 HD key structure to ensure the + // provided seed can generate the required structure with no + // issues. + + // Derive the master extended key from the seed. + rootKey, err := hdkeychain.NewMaster(seed, chainParams) + if err != nil { + str := "failed to derive master extended key" + return managerError(ErrKeyChain, str, err) + } + rootPubKey, err := rootKey.Neuter() + if err != nil { + str := "failed to neuter master extended key" + return managerError(ErrKeyChain, str, err) + } + + // Next, for each registers default manager scope, we'll + // create the hardened cointype key for it, as well as the + // first default account. + for _, defaultScope := range DefaultKeyScopes { + err := createManagerKeyScope( + ns, defaultScope, rootKey, cryptoKeyPub, cryptoKeyPriv, + ) + if err != nil { + return maybeConvertDbError(err) + } + } + + // Before we proceed, we'll also store the root master private + // key within the database in an encrypted format. This is + // required as in the future, we may need to create additional + // scoped key managers. + masterHDPrivKeyEnc, err := + cryptoKeyPriv.Encrypt([]byte(rootKey.String())) + if err != nil { + return maybeConvertDbError(err) + } + masterHDPubKeyEnc, err := + cryptoKeyPub.Encrypt([]byte(rootPubKey.String())) + if err != nil { + return maybeConvertDbError(err) + } + err = putMasterHDKeys(ns, masterHDPrivKeyEnc, masterHDPubKeyEnc) if err != nil { return maybeConvertDbError(err) } - } - // Before we proceed, we'll also store the root master private key - // within the database in an encrypted format. This is required as in - // the future, we may need to create additional scoped key managers. - masterHDPrivKeyEnc, err := cryptoKeyPriv.Encrypt([]byte(rootKey.String())) - if err != nil { - return maybeConvertDbError(err) - } - masterHDPubKeyEnc, err := cryptoKeyPub.Encrypt([]byte(rootPubKey.String())) - if err != nil { - return maybeConvertDbError(err) + privParams = masterKeyPriv.Marshal() } - err = putMasterHDKeys(ns, masterHDPrivKeyEnc, masterHDPubKeyEnc) + + // Save the master key params to the database. + err = putMasterKeyParams(ns, pubParams, privParams) if err != nil { return maybeConvertDbError(err) } @@ -1845,9 +1890,9 @@ func Create(ns walletdb.ReadWriteBucket, seed, pubPassphrase, privPassphrase []b return maybeConvertDbError(err) } - // Save the fact this is not a watching-only address manager to the + // Save the watching-only mode of the address manager to the // database. - err = putWatchingOnly(ns, false) + err = putWatchingOnly(ns, isWatchingOnly) if err != nil { return maybeConvertDbError(err) } diff --git a/waddrmgr/manager_test.go b/waddrmgr/manager_test.go index cb80a7d846..0db0a88cb7 100644 --- a/waddrmgr/manager_test.go +++ b/waddrmgr/manager_test.go @@ -17,6 +17,7 @@ import ( "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcutil" + "github.com/btcsuite/btcutil/hdkeychain" "github.com/btcsuite/btcwallet/snacl" "github.com/btcsuite/btcwallet/walletdb" "github.com/davecgh/go-spew/spew" @@ -71,6 +72,7 @@ func failingSecretKeyGen(passphrase *[]byte, // spent. type testContext struct { t *testing.T + caseName string db walletdb.DB rootManager *Manager manager *ScopedKeyManager @@ -112,7 +114,7 @@ func testNamePrefix(tc *testContext) string { prefix = "Create " } - return prefix + fmt.Sprintf("account #%d", tc.account) + return fmt.Sprintf("(%s) %s account #%d", tc.caseName, prefix, tc.account) } // testManagedPubKeyAddress ensures the data returned by all exported functions @@ -1049,7 +1051,7 @@ func testImportScript(tc *testContext) bool { } // testMarkUsed ensures used addresses are flagged as such. -func testMarkUsed(tc *testContext) bool { +func testMarkUsed(tc *testContext, doScript bool) bool { tests := []struct { name string typ addrType @@ -1067,9 +1069,12 @@ func testMarkUsed(tc *testContext) bool { }, } - prefix := "MarkUsed" + prefix := fmt.Sprintf("(%s) MarkUsed", tc.caseName) chainParams := tc.manager.ChainParams() for i, test := range tests { + if !doScript && test.typ == addrScriptHash { + continue + } addrHash := test.in var addr btcutil.Address @@ -1116,7 +1121,7 @@ func testMarkUsed(tc *testContext) bool { return nil }) if err != nil { - tc.t.Errorf("Unexpected error %v", err) + tc.t.Errorf("(%s) Unexpected error %v", tc.caseName, err) } } @@ -1126,10 +1131,12 @@ func testMarkUsed(tc *testContext) bool { // testChangePassphrase ensures changes both the public and private passphrases // works as intended. func testChangePassphrase(tc *testContext) bool { + pfx := fmt.Sprintf("(%s) ", tc.caseName) + // Force an error when changing the passphrase due to failure to // generate a new secret key by replacing the generation function one // that intentionally errors. - testName := "ChangePassphrase (public) with invalid new secret key" + testName := pfx + "ChangePassphrase (public) with invalid new secret key" oldKeyGen := SetSecretKeyGen(failingSecretKeyGen) err := walletdb.Update(tc.db, func(tx walletdb.ReadWriteTx) error { @@ -1143,7 +1150,7 @@ func testChangePassphrase(tc *testContext) bool { } // Attempt to change public passphrase with invalid old passphrase. - testName = "ChangePassphrase (public) with invalid old passphrase" + testName = pfx + "ChangePassphrase (public) with invalid old passphrase" SetSecretKeyGen(oldKeyGen) err = walletdb.Update(tc.db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) @@ -1156,7 +1163,7 @@ func testChangePassphrase(tc *testContext) bool { } // Change the public passphrase. - testName = "ChangePassphrase (public)" + testName = pfx + "ChangePassphrase (public)" err = walletdb.Update(tc.db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) return tc.rootManager.ChangePassphrase( @@ -1192,7 +1199,7 @@ func testChangePassphrase(tc *testContext) bool { // Attempt to change private passphrase with invalid old passphrase. // The error should be ErrWrongPassphrase or ErrWatchingOnly depending // on the type of the address manager. - testName = "ChangePassphrase (private) with invalid old passphrase" + testName = pfx + "ChangePassphrase (private) with invalid old passphrase" err = walletdb.Update(tc.db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) return tc.rootManager.ChangePassphrase( @@ -1216,7 +1223,7 @@ func testChangePassphrase(tc *testContext) bool { } // Change the private passphrase. - testName = "ChangePassphrase (private)" + testName = pfx + "ChangePassphrase (private)" err = walletdb.Update(tc.db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) return tc.rootManager.ChangePassphrase( @@ -1379,6 +1386,18 @@ func testLookupAccount(tc *testContext) bool { defaultAccountName: DefaultAccountNum, ImportedAddrAccountName: ImportedAddrAccount, } + + var expectedLastAccount uint32 = 1 + if !tc.create { + // Existing wallet manager will have 3 accounts + expectedLastAccount = 2 + } + + return testLookupExpectedAccount(tc, expectedAccounts, expectedLastAccount) +} + +func testLookupExpectedAccount(tc *testContext, expectedAccounts map[string]uint32, + expectedLastAccount uint32) bool { for acctName, expectedAccount := range expectedAccounts { var account uint32 err := walletdb.View(tc.db, func(tx walletdb.ReadTx) error { @@ -1418,19 +1437,12 @@ func testLookupAccount(tc *testContext) bool { lastAccount, err = tc.manager.LastAccount(ns) return err }) - var expectedLastAccount uint32 - expectedLastAccount = 1 - if !tc.create { - // Existing wallet manager will have 3 accounts - expectedLastAccount = 2 - } if lastAccount != expectedLastAccount { tc.t.Errorf("LookupAccount "+ "account mismatch -- got %d, "+ "want %d", lastAccount, expectedLastAccount) return false } - // Test account lookup for default account adddress var expectedAccount uint32 for i, addr := range expectedAddrs { @@ -1608,31 +1620,51 @@ func testForEachAccountAddress(tc *testContext) bool { // testManagerAPI tests the functions provided by the Manager API as well as // the ManagedAddress, ManagedPubKeyAddress, and ManagedScriptAddress // interfaces. -func testManagerAPI(tc *testContext) { - testLocking(tc) - testExternalAddresses(tc) - testInternalAddresses(tc) - testImportPrivateKey(tc) - testImportScript(tc) - testMarkUsed(tc) - testChangePassphrase(tc) - - // Reset default account - tc.account = 0 - testNewAccount(tc) - testLookupAccount(tc) - testForEachAccount(tc) - testForEachAccountAddress(tc) - - // Rename account 1 "acct-create" - tc.account = 1 - testRenameAccount(tc) +func testManagerAPI(tc *testContext, caseCreatedWatchingOnly bool) { + if !caseCreatedWatchingOnly { + // Test API for normal create (w/ seed) case. + testLocking(tc) + testExternalAddresses(tc) + testInternalAddresses(tc) + testImportPrivateKey(tc) + testImportScript(tc) + testMarkUsed(tc, true) + testChangePassphrase(tc) + + // Reset default account + tc.account = 0 + testNewAccount(tc) + testLookupAccount(tc) + testForEachAccount(tc) + testForEachAccountAddress(tc) + + // Rename account 1 "acct-create" + tc.account = 1 + testRenameAccount(tc) + } else { + // Test API for created watch-only case. + testExternalAddresses(tc) + testInternalAddresses(tc) + testMarkUsed(tc, false) + testChangePassphrase(tc) + + testNewAccount(tc) + expectedAccounts := map[string]uint32{ + defaultAccountName: DefaultAccountNum, + } + testLookupExpectedAccount(tc, expectedAccounts, 0) + //testForEachAccount(tc) + testForEachAccountAddress(tc) + } } -// testWatchingOnly tests various facets of a watching-only address +// testConvertWatchingOnly tests various facets of a watching-only address // manager such as running the full set of API tests against a newly converted // copy as well as when it is opened from an existing namespace. -func testWatchingOnly(tc *testContext) bool { +func testConvertWatchingOnly(tc *testContext) bool { + // These tests check the case where the manager was not initially + // created watch-only, but converted to watch only ... + // Make a copy of the current database so the copy can be converted to // watching only. woMgrName := "mgrtestwo.bin" @@ -1689,13 +1721,14 @@ func testWatchingOnly(tc *testContext) bool { } testManagerAPI(&testContext{ t: tc.t, + caseName: tc.caseName, db: db, rootManager: mgr, manager: scopedMgr, account: 0, create: false, watchingOnly: true, - }) + }, false) mgr.Close() // Open the watching-only manager and run all the tests again. @@ -1719,13 +1752,14 @@ func testWatchingOnly(tc *testContext) bool { testManagerAPI(&testContext{ t: tc.t, + caseName: tc.caseName, db: db, rootManager: mgr, manager: scopedMgr, account: 0, create: false, watchingOnly: true, - }) + }, false) return true } @@ -1739,7 +1773,8 @@ func testSync(tc *testContext) bool { return tc.rootManager.SetSyncedTo(ns, nil) }) if err != nil { - tc.t.Errorf("SetSyncedTo unexpected err on nil: %v", err) + tc.t.Errorf("(%s) SetSyncedTo unexpected err on nil: %v", + tc.caseName, err) return false } blockStamp := BlockStamp{ @@ -1748,8 +1783,8 @@ func testSync(tc *testContext) bool { } gotBlockStamp := tc.rootManager.SyncedTo() if gotBlockStamp != blockStamp { - tc.t.Errorf("SyncedTo unexpected block stamp on nil -- "+ - "got %v, want %v", gotBlockStamp, blockStamp) + tc.t.Errorf("(%s) SyncedTo unexpected block stamp on nil -- "+ + "got %v, want %v", tc.caseName, gotBlockStamp, blockStamp) return false } @@ -1789,39 +1824,79 @@ func testSync(tc *testContext) bool { func TestManager(t *testing.T) { t.Parallel() + tests := []struct { + name string + createdWatchingOnly bool + seed []byte + privPassphrase []byte + }{ + { + name: "created with seed", + createdWatchingOnly: false, + seed: seed, + privPassphrase: privPassphrase, + }, + { + name: "created watch-only", + createdWatchingOnly: true, + seed: nil, + privPassphrase: nil, + }, + } + + for _, test := range tests { + // Need to wrap in a call so the defers work correctly. + testManagerCase(t, test.name, test.createdWatchingOnly, + test.seed, test.privPassphrase) + } +} + +func testManagerCase(t *testing.T, caseName string, + caseCreatedWatchingOnly bool, caseSeed, casePrivPassphrase []byte) { + teardown, db := emptyDB(t) defer teardown() - // Open manager that does not exist to ensure the expected error is - // returned. - err := walletdb.View(db, func(tx walletdb.ReadTx) error { - ns := tx.ReadBucket(waddrmgrNamespaceKey) - _, err := Open(ns, pubPassphrase, &chaincfg.MainNetParams) - return err - }) - if !checkManagerError(t, "Open non-existant", err, ErrNoExist) { - return + if !caseCreatedWatchingOnly { + // Open manager that does not exist to ensure the expected error is + // returned. + err := walletdb.View(db, func(tx walletdb.ReadTx) error { + ns := tx.ReadBucket(waddrmgrNamespaceKey) + _, err := Open(ns, pubPassphrase, &chaincfg.MainNetParams) + return err + }) + if !checkManagerError(t, "Open non-existent", err, ErrNoExist) { + return + } } // Create a new manager. var mgr *Manager - err = walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { ns, err := tx.CreateTopLevelBucket(waddrmgrNamespaceKey) if err != nil { return err } err = Create( - ns, seed, pubPassphrase, privPassphrase, + ns, caseSeed, pubPassphrase, casePrivPassphrase, &chaincfg.MainNetParams, fastScrypt, time.Time{}, ) if err != nil { return err } mgr, err = Open(ns, pubPassphrase, &chaincfg.MainNetParams) + if err != nil { + return err + } + + if caseCreatedWatchingOnly { + _, err = mgr.NewScopedKeyManager( + ns, KeyScopeBIP0044, ScopeAddrMap[KeyScopeBIP0044]) + } return err }) if err != nil { - t.Errorf("Create/Open: unexpected error: %v", err) + t.Errorf("(%s) Create/Open: unexpected error: %v", caseName, err) return } @@ -1833,30 +1908,58 @@ func TestManager(t *testing.T) { err = walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) return Create( - ns, seed, pubPassphrase, privPassphrase, + ns, caseSeed, pubPassphrase, casePrivPassphrase, &chaincfg.MainNetParams, fastScrypt, time.Time{}, ) }) - if !checkManagerError(t, "Create existing", err, ErrAlreadyExists) { + if !checkManagerError(t, fmt.Sprintf("(%s) Create existing", caseName), + err, ErrAlreadyExists) { mgr.Close() return } - // Run all of the manager API tests in create mode and close the - // manager after they've completed scopedMgr, err := mgr.FetchScopedKeyManager(KeyScopeBIP0044) if err != nil { - t.Fatalf("unable to fetch default scope: %v", err) + t.Fatalf("(%s) unable to fetch default scope: %v", caseName, err) + } + + if caseCreatedWatchingOnly { + accountKey := deriveTestAccountKey(t) + if accountKey == nil { + return + } + + acctKeyPub, err := accountKey.Neuter() + if err != nil { + t.Errorf("(%s) Neuter: unexpected error: %v", caseName, err) + return + } + + // Create the default account + err = walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) + _, err = scopedMgr.NewAccountWatchingOnly( + ns, defaultAccountName, acctKeyPub) + return err + }) + if err != nil { + t.Errorf("NewAccountWatchingOnly: unexpected error: %v", err) + return + } } + + // Run all of the manager API tests in create mode and close the + // manager after they've completed testManagerAPI(&testContext{ t: t, + caseName: caseName, db: db, manager: scopedMgr, rootManager: mgr, account: 0, create: true, - watchingOnly: false, - }) + watchingOnly: caseCreatedWatchingOnly, + }, caseCreatedWatchingOnly) mgr.Close() // Open the manager and run all the tests again in open mode which @@ -1868,42 +1971,66 @@ func TestManager(t *testing.T) { return err }) if err != nil { - t.Errorf("Open: unexpected error: %v", err) + t.Errorf("(%s) Open: unexpected error: %v", caseName, err) return } defer mgr.Close() scopedMgr, err = mgr.FetchScopedKeyManager(KeyScopeBIP0044) if err != nil { - t.Fatalf("unable to fetch default scope: %v", err) + t.Fatalf("(%s) unable to fetch default scope: %v", caseName, err) } tc := &testContext{ t: t, + caseName: caseName, db: db, manager: scopedMgr, rootManager: mgr, account: 0, create: false, - watchingOnly: false, + watchingOnly: caseCreatedWatchingOnly, } - testManagerAPI(tc) + testManagerAPI(tc, caseCreatedWatchingOnly) - // Now that the address manager has been tested in both the newly - // created and opened modes, test a watching-only version. - testWatchingOnly(tc) + if !caseCreatedWatchingOnly { + // Now that the address manager has been tested in both the newly + // created and opened modes, test a watching-only version. + testConvertWatchingOnly(tc) + } // Ensure that the manager sync state functionality works as expected. testSync(tc) - // Unlock the manager so it can be closed with it unlocked to ensure - // it works without issue. - err = walletdb.View(db, func(tx walletdb.ReadTx) error { - ns := tx.ReadBucket(waddrmgrNamespaceKey) - return mgr.Unlock(ns, privPassphrase) - }) + if !caseCreatedWatchingOnly { + // Unlock the manager so it can be closed with it unlocked to ensure + // it works without issue. + err = walletdb.View(db, func(tx walletdb.ReadTx) error { + ns := tx.ReadBucket(waddrmgrNamespaceKey) + return mgr.Unlock(ns, casePrivPassphrase) + }) + if err != nil { + t.Errorf("Unlock: unexpected error: %v", err) + } + } +} + +func deriveTestAccountKey(t *testing.T) *hdkeychain.ExtendedKey { + masterKey, err := hdkeychain.NewMaster(seed, &chaincfg.MainNetParams) if err != nil { - t.Errorf("Unlock: unexpected error: %v", err) + t.Errorf("NewMaster: unexpected error: %v", err) + return nil } + scopeKey, err := deriveCoinTypeKey(masterKey, KeyScopeBIP0044) + if err != nil { + t.Errorf("derive: unexpected error: %v", err) + return nil + } + accountKey, err := deriveAccountKey(scopeKey, 0) + if err != nil { + t.Errorf("derive: unexpected error: %v", err) + return nil + } + return accountKey } // TestManagerIncorrectVersion ensures that that the manager cannot be accessed @@ -2454,10 +2581,145 @@ func TestNewRawAccount(t *testing.T) { t.Fatalf("unable to create new account: %v", err) } + testNewRawAccount(t, mgr, db, accountNum, scopedMgr) +} + +// TestNewRawAccountWatchingOnly tests that callers are able to +// properly create, and use watching-only raw accounts created with +// only an account number, and not a string which is eventually mapped +// to an account number. +func TestNewRawAccountWatchingOnly(t *testing.T) { + t.Parallel() + + teardown, db := emptyDB(t) + defer teardown() + + // We'll start the test by creating a new root manager that will be + // used for the duration of the test. + var mgr *Manager + err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + ns, err := tx.CreateTopLevelBucket(waddrmgrNamespaceKey) + if err != nil { + return err + } + err = Create( + ns, nil, pubPassphrase, nil, + &chaincfg.MainNetParams, fastScrypt, time.Time{}, + ) + if err != nil { + return err + } + mgr, err = Open(ns, pubPassphrase, &chaincfg.MainNetParams) + if err != nil { + return err + } + + _, err = mgr.NewScopedKeyManager( + ns, KeyScopeBIP0044, ScopeAddrMap[KeyScopeBIP0044]) + return err + }) + if err != nil { + t.Fatalf("create/open: unexpected error: %v", err) + } + defer mgr.Close() + + // Now that we have the manager created, we'll fetch one of the default + // scopes for usage within this test. + scopedMgr, err := mgr.FetchScopedKeyManager(KeyScopeBIP0044) + if err != nil { + t.Fatalf("unable to fetch scope %v: %v", KeyScopeBIP0044, err) + } + + accountKey := deriveTestAccountKey(t) + if accountKey == nil { + return + } + + // With the scoped manager retrieved, we'll attempt to create a new raw + // account by number. + const accountNum = 1000 + err = walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) + return scopedMgr.NewRawAccountWatchingOnly(ns, accountNum, accountKey) + }) + if err != nil { + t.Fatalf("unable to create new account: %v", err) + } + + testNewRawAccount(t, mgr, db, accountNum, scopedMgr) +} + +// TestNewRawAccountHybrid is similar to TestNewRawAccountWatchingOnly +// 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) { + t.Parallel() + + teardown, db := emptyDB(t) + defer teardown() + + // We'll start the test by creating a new root manager that will be + // used for the duration of the test. + var mgr *Manager + err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + ns, err := tx.CreateTopLevelBucket(waddrmgrNamespaceKey) + if err != nil { + return err + } + err = Create( + ns, seed, pubPassphrase, privPassphrase, + &chaincfg.MainNetParams, fastScrypt, time.Time{}, + ) + if err != nil { + return err + } + mgr, err = Open(ns, pubPassphrase, &chaincfg.MainNetParams) + return err + }) + if err != nil { + t.Fatalf("create/open: unexpected error: %v", err) + } + defer mgr.Close() + + // Now that we have the manager created, we'll fetch one of the default + // scopes for usage within this test. + scopedMgr, err := mgr.FetchScopedKeyManager(KeyScopeBIP0044) + if err != nil { + t.Fatalf("unable to fetch scope %v: %v", KeyScopeBIP0044, err) + } + + accountKey := deriveTestAccountKey(t) + if accountKey == nil { + return + } + + acctKeyPub, err := accountKey.Neuter() + if err != nil { + t.Errorf("Neuter: unexpected error: %v", err) + return + } + + // With the scoped manager retrieved, we'll attempt to create a new raw + // account by number. + const accountNum = 1000 + err = walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) + return scopedMgr.NewRawAccountWatchingOnly(ns, accountNum, acctKeyPub) + }) + if err != nil { + t.Fatalf("unable to create new account: %v", err) + } + + testNewRawAccount(t, mgr, db, accountNum, scopedMgr) +} + +func testNewRawAccount(t *testing.T, mgr *Manager, db walletdb.DB, + accountNum uint32, scopedMgr *ScopedKeyManager) { // With the account created, we should be able to derive new addresses // from the account. var accountAddrNext ManagedAddress - err = walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) addrs, err := scopedMgr.NextExternalAddresses( diff --git a/waddrmgr/scoped_manager.go b/waddrmgr/scoped_manager.go index 16bd83a2af..44f9b959a7 100644 --- a/waddrmgr/scoped_manager.go +++ b/waddrmgr/scoped_manager.go @@ -1187,7 +1187,7 @@ func (s *ScopedKeyManager) LastInternalAddress(ns walletdb.ReadBucket, } // NewRawAccount creates a new account for the scoped manager. This method -// differs from the NewAccount method in that this method takes the acount +// differs from the NewAccount method in that this method takes the account // number *directly*, rather than taking a string name for the account, then // mapping that to the next highest account number. func (s *ScopedKeyManager) NewRawAccount(ns walletdb.ReadWriteBucket, number uint32) error { @@ -1209,6 +1209,24 @@ func (s *ScopedKeyManager) NewRawAccount(ns walletdb.ReadWriteBucket, number uin return s.newAccount(ns, number, name) } +// NewRawAccountWatchingOnly creates a new watching only account for +// the scoped manager. This method differs from the +// NewAccountWatchingOnly method in that this method takes the account +// number *directly*, rather than taking a string name for the +// account, then mapping that to the next highest account number. +func (s *ScopedKeyManager) NewRawAccountWatchingOnly( + ns walletdb.ReadWriteBucket, number uint32, + pubKey *hdkeychain.ExtendedKey) error { + s.mtx.Lock() + defer s.mtx.Unlock() + + // As this is an ad hoc account that may not follow our normal linear + // derivation, we'll create a new name for this account based off of + // the account number. + name := fmt.Sprintf("act:%v", number) + return s.newAccountWatchingOnly(ns, number, name, pubKey) +} + // NewAccount creates and returns a new account stored in the manager based on // the given account name. If an account with the same name already exists, // ErrDuplicateAccount will be returned. Since creating a new account requires @@ -1326,6 +1344,70 @@ func (s *ScopedKeyManager) newAccount(ns walletdb.ReadWriteBucket, return putLastAccount(ns, &s.scope, account) } +// NewAccountWatchingOnly is similar to NewAccount, but for watch-only wallets. +func (s *ScopedKeyManager) NewAccountWatchingOnly(ns walletdb.ReadWriteBucket, name string, + pubKey *hdkeychain.ExtendedKey) (uint32, error) { + s.mtx.Lock() + defer s.mtx.Unlock() + + // Fetch latest account, and create a new account in the same + // transaction Fetch the latest account number to generate the next + // account number + account, err := fetchLastAccount(ns, &s.scope) + if err != nil { + return 0, err + } + account++ + + // With the name validated, we'll create a new account for the new + // contiguous account. + if err := s.newAccountWatchingOnly(ns, account, name, pubKey); err != nil { + return 0, err + } + + return account, nil +} + +// newAccountWatchingOnly is similar to newAccount, but for watching-only wallets. +// +// NOTE: This function MUST be called with the manager lock held for writes. +func (s *ScopedKeyManager) newAccountWatchingOnly(ns walletdb.ReadWriteBucket, account uint32, name string, + pubKey *hdkeychain.ExtendedKey) error { + + // Validate the account name. + if err := ValidateAccountName(name); err != nil { + return err + } + + // Check that account with the same name does not exist + _, err := s.lookupAccount(ns, name) + if err == nil { + str := fmt.Sprintf("account with the same name already exists") + return managerError(ErrDuplicateAccount, str, err) + } + + // Encrypt the default account keys with the associated crypto keys. + acctPubEnc, err := s.rootManager.cryptoKeyPub.Encrypt( + []byte(pubKey.String()), + ) + if err != nil { + str := "failed to encrypt public key for account" + return managerError(ErrCrypto, str, err) + } + + // We have the encrypted account extended keys, so save them to the + // database + err = putAccountInfo( + ns, &s.scope, account, acctPubEnc, nil, 0, 0, name, + ) + if err != nil { + return err + } + + // Save last account metadata + return putLastAccount(ns, &s.scope, account) +} + // RenameAccount renames an account stored in the manager based on the given // account number with the given name. If an account with the same name // already exists, ErrDuplicateAccount will be returned. @@ -1700,6 +1782,7 @@ func (s *ScopedKeyManager) ForEachAccount(ns walletdb.ReadBucket, } // LastAccount returns the last account stored in the manager. +// If no accounts, returns twos-complement representation of -1 func (s *ScopedKeyManager) LastAccount(ns walletdb.ReadBucket) (uint32, error) { return fetchLastAccount(ns, &s.scope) } diff --git a/wallet/loader.go b/wallet/loader.go index 5ae807e3fc..17f761f922 100644 --- a/wallet/loader.go +++ b/wallet/loader.go @@ -100,6 +100,25 @@ func (l *Loader) RunAfterLoad(fn func(*Wallet)) { func (l *Loader) CreateNewWallet(pubPassphrase, privPassphrase, seed []byte, bday time.Time) (*Wallet, error) { + return l.createNewWallet( + pubPassphrase, privPassphrase, seed, bday, false, + ) +} + +// CreateNewWatchingOnlyWallet creates a new wallet using the provided +// public passphrase. No seed or private passphrase may be provided +// since the wallet is watching-only. +func (l *Loader) CreateNewWatchingOnlyWallet(pubPassphrase []byte, + bday time.Time) (*Wallet, error) { + + return l.createNewWallet( + pubPassphrase, nil, nil, bday, true, + ) +} + +func (l *Loader) createNewWallet(pubPassphrase, privPassphrase, + seed []byte, bday time.Time, isWatchingOnly bool) (*Wallet, error) { + defer l.mu.Unlock() l.mu.Lock() @@ -127,11 +146,18 @@ func (l *Loader) CreateNewWallet(pubPassphrase, privPassphrase, seed []byte, } // Initialize the newly created database for the wallet before opening. - err = Create( - db, pubPassphrase, privPassphrase, seed, l.chainParams, bday, - ) - if err != nil { - return nil, err + if isWatchingOnly { + err = CreateWatchingOnly(db, pubPassphrase, l.chainParams, bday) + if err != nil { + return nil, err + } + } else { + err = Create( + db, pubPassphrase, privPassphrase, seed, l.chainParams, bday, + ) + if err != nil { + return nil, err + } } // Open the newly-created wallet. diff --git a/wallet/wallet.go b/wallet/wallet.go index aaa9ff6638..476eea8850 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -3579,23 +3579,45 @@ func (w *Wallet) Database() walletdb.DB { // Create creates an new wallet, writing it to an empty database. If the passed // seed is non-nil, it is used. Otherwise, a secure random seed of the // recommended length is generated. -func Create(db walletdb.DB, pubPass, privPass, seed []byte, params *chaincfg.Params, - birthday time.Time) error { - - // If a seed was provided, ensure that it is of valid length. Otherwise, - // we generate a random seed for the wallet with the recommended seed - // length. - if seed == nil { - hdSeed, err := hdkeychain.GenerateSeed( - hdkeychain.RecommendedSeedLen) - if err != nil { - return err +func Create(db walletdb.DB, pubPass, privPass, seed []byte, + params *chaincfg.Params, birthday time.Time) error { + + return create( + db, pubPass, privPass, seed, params, birthday, false, + ) +} + +// CreateWatchingOnly creates an new watch-only wallet, writing it to +// an empty database. No seed can be provided as this wallet will be +// watching only. Likewise no private passphrase may be provided +// either. +func CreateWatchingOnly(db walletdb.DB, pubPass []byte, + params *chaincfg.Params, birthday time.Time) error { + + return create( + db, pubPass, nil, nil, params, birthday, true, + ) +} + +func create(db walletdb.DB, pubPass, privPass, seed []byte, + params *chaincfg.Params, birthday time.Time, isWatchingOnly bool) error { + + if !isWatchingOnly { + // If a seed was provided, ensure that it is of valid length. Otherwise, + // we generate a random seed for the wallet with the recommended seed + // length. + if seed == nil { + hdSeed, err := hdkeychain.GenerateSeed( + hdkeychain.RecommendedSeedLen) + if err != nil { + return err + } + seed = hdSeed + } + if len(seed) < hdkeychain.MinSeedBytes || + len(seed) > hdkeychain.MaxSeedBytes { + return hdkeychain.ErrInvalidSeedLen } - seed = hdSeed - } - if len(seed) < hdkeychain.MinSeedBytes || - len(seed) > hdkeychain.MaxSeedBytes { - return hdkeychain.ErrInvalidSeedLen } return walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { @@ -3609,8 +3631,7 @@ func Create(db walletdb.DB, pubPass, privPass, seed []byte, params *chaincfg.Par } err = waddrmgr.Create( - addrmgrNs, seed, pubPass, privPass, params, nil, - birthday, + addrmgrNs, seed, pubPass, privPass, params, nil, birthday, ) if err != nil { return err diff --git a/wallet/watchingonly_test.go b/wallet/watchingonly_test.go new file mode 100644 index 0000000000..f7846b1cc2 --- /dev/null +++ b/wallet/watchingonly_test.go @@ -0,0 +1,34 @@ +// Copyright (c) 2018 The btcsuite developers +// Use of this source code is governed by an ISC +// license that can be found in the LICENSE file. + +package wallet + +import ( + "io/ioutil" + "os" + "testing" + "time" + + "github.com/btcsuite/btcd/chaincfg" + _ "github.com/btcsuite/btcwallet/walletdb/bdb" +) + +// TestCreateWatchingOnly checks that we can construct a watching-only +// wallet. +func TestCreateWatchingOnly(t *testing.T) { + // Set up a wallet. + dir, err := ioutil.TempDir("", "watchingonly_test") + if err != nil { + t.Fatalf("Failed to create db dir: %v", err) + } + defer os.RemoveAll(dir) + + pubPass := []byte("hello") + + loader := NewLoader(&chaincfg.TestNet3Params, dir, true, 250) + _, err = loader.CreateNewWatchingOnlyWallet(pubPass, time.Now()) + if err != nil { + t.Fatalf("unable to create wallet: %v", err) + } +} From 50869085ebb41b843dc014a17b7c9480c1de9aff Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 30 Apr 2020 09:08:00 +0200 Subject: [PATCH 15/25] wtxmgr: add put and fetch functions for optional transaction label Add and test functions which can be used to write optional transaction labels to disk in their own bucket. These labels are keyed by txid and write the labels to disk using-length value encoding scheme. Although the length field is not required at present, it is added to allow future extensibility without a migration. This approach is chosen over adding this information to txRecords, Because a migration would be required to add a field after the variable Length serialized tx. The put label function will overwrite existing labels if called more than once for the same txid. User side validation of whether we want to override this label should be performed by calling code. Labels must be > 0 characters and <= 500 characters (an arbitrarily chosen limit). --- wtxmgr/db.go | 1 + wtxmgr/tx.go | 91 ++++++++++++++++++++++++++++++++++++++++++++++ wtxmgr/tx_test.go | 92 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+) diff --git a/wtxmgr/db.go b/wtxmgr/db.go index 28839a6425..0a90f44747 100644 --- a/wtxmgr/db.go +++ b/wtxmgr/db.go @@ -62,6 +62,7 @@ var _ [32]byte = chainhash.Hash{} var ( bucketBlocks = []byte("b") bucketTxRecords = []byte("t") + bucketTxLabels = []byte("l") bucketCredits = []byte("c") bucketUnspent = []byte("u") bucketDebits = []byte("d") diff --git a/wtxmgr/tx.go b/wtxmgr/tx.go index fbf04274a1..512fe1a343 100644 --- a/wtxmgr/tx.go +++ b/wtxmgr/tx.go @@ -7,6 +7,8 @@ package wtxmgr import ( "bytes" + "encoding/binary" + "errors" "fmt" "time" @@ -18,6 +20,28 @@ import ( "github.com/btcsuite/btcwallet/walletdb" ) +// TxLabelLimit is the length limit we impose on transaction labels. +const TxLabelLimit = 500 + +var ( + // ErrEmptyLabel is returned when an attempt to write a label that is + // empty is made. + ErrEmptyLabel = errors.New("empty transaction label not allowed") + + // ErrLabelTooLong is returned when an attempt to write a label that is + // to long is made. + ErrLabelTooLong = errors.New("transaction label exceeds limit") + + // ErrNoLabelBucket is returned when the bucket holding optional + // transaction labels is not found. This occurs when no transactions + // have been labelled yet. + ErrNoLabelBucket = errors.New("labels bucket does not exist") + + // ErrTxLabelNotFound is returned when no label is found for a + // transaction hash. + ErrTxLabelNotFound = errors.New("label for transaction not found") +) + // Block contains the minimum amount of data to uniquely identify any block on // either the best or side chain. type Block struct { @@ -935,3 +959,70 @@ func (s *Store) Balance(ns walletdb.ReadBucket, minConf int32, syncHeight int32) return bal, nil } + +// PutTxLabel validates transaction labels and writes them to disk if they +// are non-zero and within the label length limit. The entry is keyed by the +// transaction hash: +// [0:32] Transaction hash (32 bytes) +// +// The label itself is written to disk in length value format: +// [0:2] Label length +// [2: +len] Label +func (s *Store) PutTxLabel(ns walletdb.ReadWriteBucket, txid chainhash.Hash, + label string) error { + + if len(label) == 0 { + return ErrEmptyLabel + } + + if len(label) > TxLabelLimit { + return ErrLabelTooLong + } + + labelBucket, err := ns.CreateBucketIfNotExists(bucketTxLabels) + if err != nil { + return err + } + + // We expect the label length to be limited on creation, so we can + // store the label's length as a uint16. + labelLen := uint16(len(label)) + + var buf bytes.Buffer + + var b [2]byte + binary.BigEndian.PutUint16(b[:], labelLen) + if _, err := buf.Write(b[:]); err != nil { + return err + } + + if _, err := buf.WriteString(label); err != nil { + return err + } + + return labelBucket.Put(txid[:], buf.Bytes()) +} + +// FetchTxLabel reads a transaction label from the tx labels bucket. If a label +// with 0 length was written, we return an error, since this is unexpected. +func FetchTxLabel(ns walletdb.ReadBucket, txid chainhash.Hash) (string, error) { + labelBucket := ns.NestedReadBucket(bucketTxLabels) + if labelBucket == nil { + return "", ErrNoLabelBucket + } + + v := labelBucket.Get(txid[:]) + if v == nil { + return "", ErrTxLabelNotFound + } + + // If the label is empty, return an error. + length := binary.BigEndian.Uint16(v[0:2]) + if length == 0 { + return "", ErrEmptyLabel + } + + // Read the remainder of the bytes into a label string. + label := string(v[2:]) + return label, nil +} diff --git a/wtxmgr/tx_test.go b/wtxmgr/tx_test.go index 84a60233d1..e2f2c2a34c 100644 --- a/wtxmgr/tx_test.go +++ b/wtxmgr/tx_test.go @@ -2263,3 +2263,95 @@ func TestInsertMempoolTxAndConfirm(t *testing.T) { } }) } + +// TestTxLabel tests reading and writing of transaction labels. +func TestTxLabel(t *testing.T) { + t.Parallel() + + store, db, teardown, err := testStore() + if err != nil { + t.Fatal(err) + } + defer teardown() + + // txid is the transaction hash we will use to write and get labels for. + txid := TstRecvTx.Hash() + + // txidNotFound is distinct from txid, and will not have a label written + // to disk. + txidNotFound := TstSpendingTx.Hash() + + // getBucket gets the top level bucket, and fails the test if it is + // not found. + getBucket := func(tx walletdb.ReadWriteTx) walletdb.ReadWriteBucket { + testBucket := tx.ReadWriteBucket(namespaceKey) + if testBucket == nil { + t.Fatalf("could not get bucket: %v", err) + } + + return testBucket + } + + // tryPutLabel attempts to write a label to disk. + tryPutLabel := func(label string) error { + return walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + // Try to write the label to disk. + return store.PutTxLabel(getBucket(tx), *txid, label) + }) + } + + // tryReadLabel attempts to retrieve a label for a given txid. + tryReadLabel := func(labelTx chainhash.Hash) (string, error) { + var label string + + err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + var err error + label, err = FetchTxLabel(getBucket(tx), labelTx) + return err + }) + + return label, err + } + + // First, try to lookup a label when the labels bucket does not exist + // yet. + _, err = tryReadLabel(*txid) + if err != ErrNoLabelBucket { + t.Fatalf("expected: %v, got: %v", ErrNoLabelBucket, err) + } + + // Now try to write an empty label, which should fail. + err = tryPutLabel("") + if err != ErrEmptyLabel { + t.Fatalf("expected: %v, got: %v", ErrEmptyLabel, err) + } + + // Create a label which exceeds the length limit. + longLabel := make([]byte, TxLabelLimit+1) + err = tryPutLabel(string(longLabel)) + if err != ErrLabelTooLong { + t.Fatalf("expected: %v, got: %v", ErrLabelTooLong, err) + } + + // Write an acceptable length label to disk, this should succeed. + testLabel := "test label" + err = tryPutLabel(testLabel) + if err != nil { + t.Fatalf("expected: no error, got: %v", err) + } + + diskLabel, err := tryReadLabel(*txid) + if err != nil { + t.Fatalf("expected: no error, got: %v", err) + } + if diskLabel != testLabel { + t.Fatalf("expected: %v, got: %v", testLabel, diskLabel) + } + + // Finally, try to read a label for a transaction which does not have + // one. + _, err = tryReadLabel(*txidNotFound) + if err != ErrTxLabelNotFound { + t.Fatalf("expected: %v, got: %v", ErrTxLabelNotFound, err) + } +} From d2f9185f6a180272c664e94907aa0435ca93bcca Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 30 Apr 2020 09:08:11 +0200 Subject: [PATCH 16/25] wallet: add label to PublishTransaction All label parameter to PublishTransaction. Pass in an empty string in rpc call as a placeholder for follow up PR which will add a label parameter to the PublishTransaction request. --- rpc/rpcserver/server.go | 2 +- wallet/wallet.go | 24 +++++++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/rpc/rpcserver/server.go b/rpc/rpcserver/server.go index a2250a6a21..fa3bf58023 100644 --- a/rpc/rpcserver/server.go +++ b/rpc/rpcserver/server.go @@ -519,7 +519,7 @@ func (s *walletServer) PublishTransaction(ctx context.Context, req *pb.PublishTr "Bytes do not represent a valid raw transaction: %v", err) } - err = s.wallet.PublishTransaction(&msgTx) + err = s.wallet.PublishTransaction(&msgTx, "") if err != nil { return nil, translateError(err) } diff --git a/wallet/wallet.go b/wallet/wallet.go index 476eea8850..167d4021f4 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -3118,7 +3118,7 @@ func (w *Wallet) SendOutputs(outputs []*wire.TxOut, account uint32, return nil, err } - txHash, err := w.reliablyPublishTransaction(createdTx.Tx) + txHash, err := w.reliablyPublishTransaction(createdTx.Tx, "") if err != nil { return nil, err } @@ -3311,8 +3311,8 @@ func (e *ErrReplacement) Unwrap() error { // // This function is unstable and will be removed once syncing code is moved out // of the wallet. -func (w *Wallet) PublishTransaction(tx *wire.MsgTx) error { - _, err := w.reliablyPublishTransaction(tx) +func (w *Wallet) PublishTransaction(tx *wire.MsgTx, label string) error { + _, err := w.reliablyPublishTransaction(tx, label) return err } @@ -3321,7 +3321,9 @@ func (w *Wallet) PublishTransaction(tx *wire.MsgTx) error { // relevant database state, and finally possible removing the transaction from // the database (along with cleaning up all inputs used, and outputs created) if // the transaction is rejected by the backend. -func (w *Wallet) reliablyPublishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) { +func (w *Wallet) reliablyPublishTransaction(tx *wire.MsgTx, + label string) (*chainhash.Hash, error) { + chainClient, err := w.requireChainClient() if err != nil { return nil, err @@ -3336,7 +3338,19 @@ func (w *Wallet) reliablyPublishTransaction(tx *wire.MsgTx) (*chainhash.Hash, er return nil, err } err = walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error { - return w.addRelevantTx(dbTx, txRec, nil) + if err := w.addRelevantTx(dbTx, txRec, nil); err != nil { + return err + } + + // If the tx label is empty, we can return early. + if len(label) == 0 { + return nil + } + + // If there is a label we should write, get the namespace key + // and record it in the tx store. + txmgrNs := dbTx.ReadWriteBucket(wtxmgrNamespaceKey) + return w.TxStore.PutTxLabel(txmgrNs, tx.TxHash(), label) }) if err != nil { return nil, err From 9e2f2ce157cbf53e2f491c48860564020e8a048a Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 30 Apr 2020 09:08:11 +0200 Subject: [PATCH 17/25] wallet: add label parameter to SendOutputs --- rpc/legacyrpc/methods.go | 2 +- wallet/wallet.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rpc/legacyrpc/methods.go b/rpc/legacyrpc/methods.go index be714a142a..7fe9addec2 100644 --- a/rpc/legacyrpc/methods.go +++ b/rpc/legacyrpc/methods.go @@ -1378,7 +1378,7 @@ func sendPairs(w *wallet.Wallet, amounts map[string]btcutil.Amount, if err != nil { return "", err } - tx, err := w.SendOutputs(outputs, account, minconf, feeSatPerKb) + tx, err := w.SendOutputs(outputs, account, minconf, feeSatPerKb, "") if err != nil { if err == txrules.ErrAmountNegative { return "", ErrNeedPositiveAmount diff --git a/wallet/wallet.go b/wallet/wallet.go index 167d4021f4..e38e6c0fbc 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -3094,7 +3094,7 @@ func (w *Wallet) TotalReceivedForAddr(addr btcutil.Address, minConf int32) (btcu // SendOutputs creates and sends payment transactions. It returns the // transaction upon success. func (w *Wallet) SendOutputs(outputs []*wire.TxOut, account uint32, - minconf int32, satPerKb btcutil.Amount) (*wire.MsgTx, error) { + minconf int32, satPerKb btcutil.Amount, label string) (*wire.MsgTx, error) { // Ensure the outputs to be created adhere to the network's consensus // rules. @@ -3118,7 +3118,7 @@ func (w *Wallet) SendOutputs(outputs []*wire.TxOut, account uint32, return nil, err } - txHash, err := w.reliablyPublishTransaction(createdTx.Tx, "") + txHash, err := w.reliablyPublishTransaction(createdTx.Tx, label) if err != nil { return nil, err } From 9aed49070deccb57400dac5473fa5c744edc7907 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 30 Apr 2020 09:08:11 +0200 Subject: [PATCH 18/25] wtxmgr: update rangeBlockTransactions to use minedTxDetails Code is duplicated across these two functions, so update rangeBlockTransactions to use minedTxDetails. When iterating through a block, we do have the block metadata already (which is looked up by minedTxDetails). This change can be further optimized to split minedTxDetails into minedTxDetails and minedTxDetailsWithoutBlock to reduce this redundancy. --- wtxmgr/query.go | 45 +++------------------------------------------ 1 file changed, 3 insertions(+), 42 deletions(-) diff --git a/wtxmgr/query.go b/wtxmgr/query.go index 6e7f40ffe2..6235da28c3 100644 --- a/wtxmgr/query.go +++ b/wtxmgr/query.go @@ -296,52 +296,13 @@ func (s *Store) rangeBlockTransactions(ns walletdb.ReadBucket, begin, end int32, "block %v", txHash, block.Height) return false, storeError(ErrData, str, nil) } - detail := TxDetails{ - Block: BlockMeta{ - Block: block.Block, - Time: block.Time, - }, - } - err := readRawTxRecord(&txHash, v, &detail.TxRecord) + + detail, err := s.minedTxDetails(ns, &txHash, k, v) if err != nil { return false, err } - credIter := makeReadCreditIterator(ns, k) - for credIter.next() { - if int(credIter.elem.Index) >= len(detail.MsgTx.TxOut) { - str := "saved credit index exceeds number of outputs" - return false, storeError(ErrData, str, nil) - } - - // The credit iterator does not record whether - // this credit was spent by an unmined - // transaction, so check that here. - if !credIter.elem.Spent { - k := canonicalOutPoint(&txHash, credIter.elem.Index) - spent := existsRawUnminedInput(ns, k) != nil - credIter.elem.Spent = spent - } - detail.Credits = append(detail.Credits, credIter.elem) - } - if credIter.err != nil { - return false, credIter.err - } - - debIter := makeReadDebitIterator(ns, k) - for debIter.next() { - if int(debIter.elem.Index) >= len(detail.MsgTx.TxIn) { - str := "saved debit index exceeds number of inputs" - return false, storeError(ErrData, str, nil) - } - - detail.Debits = append(detail.Debits, debIter.elem) - } - if debIter.err != nil { - return false, debIter.err - } - - details = append(details, detail) + details = append(details, *detail) } // Every block record must have at least one transaction, so it From f852d2f99152e72171a27b789101928561585699 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 30 Apr 2020 09:08:11 +0200 Subject: [PATCH 19/25] wtxmgr: add transaction label to TxDetails --- wtxmgr/query.go | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/wtxmgr/query.go b/wtxmgr/query.go index 6235da28c3..2ba8b3b621 100644 --- a/wtxmgr/query.go +++ b/wtxmgr/query.go @@ -39,6 +39,7 @@ type TxDetails struct { Block BlockMeta Credits []CreditRecord Debits []DebitRecord + Label string } // minedTxDetails fetches the TxDetails for the mined transaction with hash @@ -90,7 +91,17 @@ func (s *Store) minedTxDetails(ns walletdb.ReadBucket, txHash *chainhash.Hash, r details.Debits = append(details.Debits, debIter.elem) } - return &details, debIter.err + if debIter.err != nil { + return nil, debIter.err + } + + // Finally, we add the transaction label to details. + details.Label, err = s.TxLabel(ns, *txHash) + if err != nil { + return nil, err + } + + return &details, nil } // unminedTxDetails fetches the TxDetails for the unmined transaction with the @@ -158,9 +169,40 @@ func (s *Store) unminedTxDetails(ns walletdb.ReadBucket, txHash *chainhash.Hash, }) } + // Finally, we add the transaction label to details. + details.Label, err = s.TxLabel(ns, *txHash) + if err != nil { + return nil, err + } + return &details, nil } +// TxLabel looks up a transaction label for the txHash provided. If the store +// has no labels in it, or the specific txHash does not have a label, an empty +// string and no error are returned. +func (s *Store) TxLabel(ns walletdb.ReadBucket, txHash chainhash.Hash) (string, + error) { + + label, err := FetchTxLabel(ns, txHash) + switch err { + // If there are no saved labels yet (the bucket has not been created) or + // there is not a label for this particular tx, we ignore the error. + case ErrNoLabelBucket: + fallthrough + case ErrTxLabelNotFound: + return "", nil + + // If we found the label, we return it. + case nil: + return label, nil + } + + // Otherwise, another error occurred while looking uo the label, so we + // return it. + return "", err +} + // TxDetails looks up all recorded details regarding a transaction with some // hash. In case of a hash collision, the most recent transaction with a // matching hash is returned. From 3809a6d553168a867044d668d71cea1403d9073c Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 30 Apr 2020 09:08:11 +0200 Subject: [PATCH 20/25] wallet: add transaction label to transaction summary --- wallet/notifications.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/wallet/notifications.go b/wallet/notifications.go index f76e6d9c78..d815576ab8 100644 --- a/wallet/notifications.go +++ b/wallet/notifications.go @@ -148,6 +148,7 @@ func makeTxSummary(dbtx walletdb.ReadTx, w *Wallet, details *wtxmgr.TxDetails) T MyOutputs: outputs, Fee: fee, Timestamp: details.Received.Unix(), + Label: details.Label, } } @@ -365,6 +366,7 @@ type TransactionSummary struct { MyOutputs []TransactionSummaryOutput Fee btcutil.Amount Timestamp int64 + Label string } // TransactionSummaryInput describes a transaction input that is relevant to the From 3465d2ecc628dbbe2761913647ab98ddf3c41031 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 30 Apr 2020 09:08:12 +0200 Subject: [PATCH 21/25] wallet/test: extract testWallet generation into separate function --- wallet/createtx_test.go | 31 ++------------------------- wallet/example_test.go | 47 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 29 deletions(-) create mode 100644 wallet/example_test.go diff --git a/wallet/createtx_test.go b/wallet/createtx_test.go index 9747630003..04a52c103d 100644 --- a/wallet/createtx_test.go +++ b/wallet/createtx_test.go @@ -6,16 +6,12 @@ package wallet import ( "bytes" - "io/ioutil" - "os" "testing" "time" - "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" - "github.com/btcsuite/btcutil/hdkeychain" "github.com/btcsuite/btcwallet/waddrmgr" "github.com/btcsuite/btcwallet/walletdb" _ "github.com/btcsuite/btcwallet/walletdb/bdb" @@ -26,31 +22,8 @@ import ( // request a dry run of the txToOutputs call. It also makes sure a subsequent // non-dry run call produces a similar transaction to the dry-run. func TestTxToOutputsDryRun(t *testing.T) { - // Set up a wallet. - dir, err := ioutil.TempDir("", "createtx_test") - if err != nil { - t.Fatalf("Failed to create db dir: %v", err) - } - defer os.RemoveAll(dir) - - seed, err := hdkeychain.GenerateSeed(hdkeychain.MinSeedBytes) - if err != nil { - t.Fatalf("unable to create seed: %v", err) - } - - pubPass := []byte("hello") - privPass := []byte("world") - - loader := NewLoader(&chaincfg.TestNet3Params, dir, true, 250) - w, err := loader.CreateNewWallet(pubPass, privPass, seed, time.Now()) - if err != nil { - t.Fatalf("unable to create wallet: %v", err) - } - chainClient := &mockChainClient{} - w.chainClient = chainClient - if err := w.Unlock(privPass, time.After(10*time.Minute)); err != nil { - t.Fatalf("unable to unlock wallet: %v", err) - } + w, cleanup := testWallet(t) + defer cleanup() // Create an address we can use to send some coins to. addr, err := w.CurrentAddress(0, waddrmgr.KeyScopeBIP0044) diff --git a/wallet/example_test.go b/wallet/example_test.go new file mode 100644 index 0000000000..c6d1b82921 --- /dev/null +++ b/wallet/example_test.go @@ -0,0 +1,47 @@ +package wallet + +import ( + "io/ioutil" + "os" + "testing" + "time" + + "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcutil/hdkeychain" +) + +// testWallet creates a test wallet and unlocks it. +func testWallet(t *testing.T) (*Wallet, func()) { + // Set up a wallet. + dir, err := ioutil.TempDir("", "test_wallet") + if err != nil { + t.Fatalf("Failed to create db dir: %v", err) + } + + cleanup := func() { + if err := os.RemoveAll(dir); err != nil { + t.Fatalf("could not cleanup test: %v", err) + } + } + + seed, err := hdkeychain.GenerateSeed(hdkeychain.MinSeedBytes) + if err != nil { + t.Fatalf("unable to create seed: %v", err) + } + + pubPass := []byte("hello") + privPass := []byte("world") + + loader := NewLoader(&chaincfg.TestNet3Params, dir, true, 250) + w, err := loader.CreateNewWallet(pubPass, privPass, seed, time.Now()) + if err != nil { + t.Fatalf("unable to create wallet: %v", err) + } + chainClient := &mockChainClient{} + w.chainClient = chainClient + if err := w.Unlock(privPass, time.After(10*time.Minute)); err != nil { + t.Fatalf("unable to unlock wallet: %v", err) + } + + return w, cleanup +} From 212575f7d1467d37ac778fb8f7af0a6a17754f46 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 30 Apr 2020 09:08:12 +0200 Subject: [PATCH 22/25] wallet: add label transaction function Add function which allows you to retrospectively label a transaction, optionally overwriting the existing label. --- wallet/wallet.go | 62 ++++++++++++++++++++++ wallet/wallet_test.go | 119 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+) diff --git a/wallet/wallet.go b/wallet/wallet.go index e38e6c0fbc..cf2990fcd3 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -65,6 +65,16 @@ var ( // down. ErrWalletShuttingDown = errors.New("wallet shutting down") + // ErrUnknownTransaction is returned when an attempt is made to label + // a transaction that is not known to the wallet. + ErrUnknownTransaction = errors.New("cannot label transaction not " + + "known to wallet") + + // ErrTxLabelExists is returned when a transaction already has a label + // and an attempt has been made to label it without setting overwrite + // to true. + ErrTxLabelExists = errors.New("transaction already labelled") + // Namespace bucket keys. waddrmgrNamespaceKey = []byte("waddrmgr") wtxmgrNamespaceKey = []byte("wtxmgr") @@ -1591,6 +1601,58 @@ func (w *Wallet) PubKeyForAddress(a btcutil.Address) (*btcec.PublicKey, error) { return pubKey, err } +// LabelTransaction adds a label to the transaction with the hash provided. The +// call will fail if the label is too long, or if the transaction already has +// a label and the overwrite boolean is not set. +func (w *Wallet) LabelTransaction(hash chainhash.Hash, label string, + overwrite bool) error { + + // Check that the transaction is known to the wallet, and fail if it is + // unknown. If the transaction is known, check whether it already has + // a label. + err := walletdb.View(w.db, func(tx walletdb.ReadTx) error { + txmgrNs := tx.ReadBucket(wtxmgrNamespaceKey) + + dbTx, err := w.TxStore.TxDetails(txmgrNs, &hash) + if err != nil { + return err + } + + // If the transaction looked up is nil, it was not found. We + // do not allow labelling of unknown transactions so we fail. + if dbTx == nil { + return ErrUnknownTransaction + } + + _, err = wtxmgr.FetchTxLabel(txmgrNs, hash) + return err + }) + + switch err { + // If no labels have been written yet, we can silence the error. + // Likewise if there is no label, we do not need to do any overwrite + // checks. + case wtxmgr.ErrNoLabelBucket: + case wtxmgr.ErrTxLabelNotFound: + + // If we successfully looked up a label, fail if the overwrite param + // is not set. + case nil: + if !overwrite { + return ErrTxLabelExists + } + + // In another unrelated error occurred, return it. + default: + return err + } + + return walletdb.Update(w.db, func(tx walletdb.ReadWriteTx) error { + txmgrNs := tx.ReadWriteBucket(wtxmgrNamespaceKey) + return w.TxStore.PutTxLabel(txmgrNs, hash, label) + }) +} + // PrivKeyForAddress looks up the associated private key for a P2PKH or P2PK // address. func (w *Wallet) PrivKeyForAddress(a btcutil.Address) (*btcec.PrivateKey, error) { diff --git a/wallet/wallet_test.go b/wallet/wallet_test.go index f70f43ca3d..5be8d74f29 100644 --- a/wallet/wallet_test.go +++ b/wallet/wallet_test.go @@ -1,8 +1,20 @@ package wallet import ( + "encoding/hex" "testing" "time" + + "github.com/btcsuite/btcwallet/walletdb" + "github.com/btcsuite/btcwallet/wtxmgr" + + "github.com/btcsuite/btcutil" +) + +var ( + TstSerializedTx, _ = hex.DecodeString("010000000114d9ff358894c486b4ae11c2a8cf7851b1df64c53d2e511278eff17c22fb7373000000008c493046022100995447baec31ee9f6d4ec0e05cb2a44f6b817a99d5f6de167d1c75354a946410022100c9ffc23b64d770b0e01e7ff4d25fbc2f1ca8091053078a247905c39fce3760b601410458b8e267add3c1e374cf40f1de02b59213a82e1d84c2b94096e22e2f09387009c96debe1d0bcb2356ffdcf65d2a83d4b34e72c62eccd8490dbf2110167783b2bffffffff0280969800000000001976a914479ed307831d0ac19ebc5f63de7d5f1a430ddb9d88ac38bfaa00000000001976a914dadf9e3484f28b385ddeaa6c575c0c0d18e9788a88ac00000000") + TstTx, _ = btcutil.NewTxFromBytes(TstSerializedTx) + TstTxHash = TstTx.Hash() ) // TestLocateBirthdayBlock ensures we can properly map a block in the chain to a @@ -83,3 +95,110 @@ func TestLocateBirthdayBlock(t *testing.T) { } } } + +// TestLabelTransaction tests labelling of transactions with invalid labels, +// and failure to label a transaction when it already has a label. +func TestLabelTransaction(t *testing.T) { + tests := []struct { + name string + + // Whether the transaction should be known to the wallet. + txKnown bool + + // Whether the test should write an existing label to disk. + existingLabel bool + + // The overwrite parameter to call label transaction with. + overwrite bool + + // The error we expect to be returned. + expectedErr error + }{ + { + name: "existing label, not overwrite", + txKnown: true, + existingLabel: true, + overwrite: false, + expectedErr: ErrTxLabelExists, + }, + { + name: "existing label, overwritten", + txKnown: true, + existingLabel: true, + overwrite: true, + expectedErr: nil, + }, + { + name: "no prexisting label, ok", + txKnown: true, + existingLabel: false, + overwrite: false, + expectedErr: nil, + }, + { + name: "transaction unknown", + txKnown: false, + existingLabel: false, + overwrite: false, + expectedErr: ErrUnknownTransaction, + }, + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + w, cleanup := testWallet(t) + defer cleanup() + + // If the transaction should be known to the store, we + // write txdetail to disk. + if test.txKnown { + rec, err := wtxmgr.NewTxRecord( + TstSerializedTx, time.Now(), + ) + if err != nil { + t.Fatal(err) + } + + err = walletdb.Update(w.db, + func(tx walletdb.ReadWriteTx) error { + + ns := tx.ReadWriteBucket( + wtxmgrNamespaceKey, + ) + + return w.TxStore.InsertTx( + ns, rec, nil, + ) + }) + if err != nil { + t.Fatalf("could not insert tx: %v", err) + } + } + + // If we want to setup an existing label for the purpose + // of the test, write one to disk. + if test.existingLabel { + err := w.LabelTransaction( + *TstTxHash, "existing label", false, + ) + if err != nil { + t.Fatalf("could not write label: %v", + err) + } + } + + newLabel := "new label" + err := w.LabelTransaction( + *TstTxHash, newLabel, test.overwrite, + ) + if err != test.expectedErr { + t.Fatalf("expected: %v, got: %v", + test.expectedErr, err) + } + }) + } +} From ce888ed941af9f614d506da3c72d1456f2dd4268 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 13 May 2020 08:54:23 -0700 Subject: [PATCH 23/25] wallet: use GetBlockHeaderVerbose to retrieve block heights in GetTransactions There's no need to retrieve the full block as we're only interesting in retrieve its corresponding height, which can be done with GetBlockHeaderVerbose. --- wallet/wallet.go | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/wallet/wallet.go b/wallet/wallet.go index cf2990fcd3..ad16ab942f 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -20,7 +20,6 @@ import ( "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" - "github.com/btcsuite/btcd/rpcclient" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" @@ -2202,7 +2201,6 @@ func (w *Wallet) GetTransactions(startBlock, endBlock *BlockIdentifier, cancel < // TODO: Fetching block heights by their hashes is inherently racy // because not all block headers are saved but when they are for SPV the // db can be queried directly without this. - var startResp, endResp rpcclient.FutureGetBlockVerboseResult if startBlock != nil { if startBlock.hash == nil { start = startBlock.height @@ -2212,7 +2210,13 @@ func (w *Wallet) GetTransactions(startBlock, endBlock *BlockIdentifier, cancel < } switch client := chainClient.(type) { case *chain.RPCClient: - startResp = client.GetBlockVerboseTxAsync(startBlock.hash) + startHeader, err := client.GetBlockHeaderVerbose( + startBlock.hash, + ) + if err != nil { + return nil, err + } + start = startHeader.Height case *chain.BitcoindClient: var err error start, err = client.GetBlockHeight(startBlock.hash) @@ -2237,7 +2241,19 @@ func (w *Wallet) GetTransactions(startBlock, endBlock *BlockIdentifier, cancel < } switch client := chainClient.(type) { case *chain.RPCClient: - endResp = client.GetBlockVerboseTxAsync(endBlock.hash) + endHeader, err := client.GetBlockHeaderVerbose( + endBlock.hash, + ) + if err != nil { + return nil, err + } + end = endHeader.Height + case *chain.BitcoindClient: + var err error + start, err = client.GetBlockHeight(endBlock.hash) + if err != nil { + return nil, err + } case *chain.NeutrinoClient: var err error end, err = client.GetBlockHeight(endBlock.hash) @@ -2247,20 +2263,6 @@ func (w *Wallet) GetTransactions(startBlock, endBlock *BlockIdentifier, cancel < } } } - if startResp != nil { - resp, err := startResp.Receive() - if err != nil { - return nil, err - } - start = int32(resp.Height) - } - if endResp != nil { - resp, err := endResp.Receive() - if err != nil { - return nil, err - } - end = int32(resp.Height) - } var res GetTransactionsResult err := walletdb.View(w.db, func(dbtx walletdb.ReadTx) error { From 17c23a92669959771fe93095536d000a74f501d0 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 13 May 2020 08:57:18 -0700 Subject: [PATCH 24/25] build: update to latest btcd version --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index a20589cb62..f55c89f1f0 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,7 @@ module github.com/btcsuite/btcwallet require ( - github.com/btcsuite/btcd v0.20.1-beta + github.com/btcsuite/btcd v0.20.1-beta.0.20200513120220-b470eee47728 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d github.com/btcsuite/btcwallet/wallet/txauthor v1.0.0 diff --git a/go.sum b/go.sum index 3f267ec86b..7c1715ae01 100644 --- a/go.sum +++ b/go.sum @@ -5,6 +5,8 @@ github.com/btcsuite/btcd v0.0.0-20190824003749-130ea5bddde3 h1:A/EVblehb75cUgXA5 github.com/btcsuite/btcd v0.0.0-20190824003749-130ea5bddde3/go.mod h1:3J08xEfcugPacsc34/LKRU2yO7YmuT8yt28J8k2+rrI= github.com/btcsuite/btcd v0.20.1-beta h1:Ik4hyJqN8Jfyv3S4AGBOmyouMsYE3EdYODkMbQjwPGw= github.com/btcsuite/btcd v0.20.1-beta/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13Px/pDuV7OomQ= +github.com/btcsuite/btcd v0.20.1-beta.0.20200513120220-b470eee47728 h1:kF1MN22IdIZ1I7VMgS5WuihFuYaWOoP69Btm4bLUBxY= +github.com/btcsuite/btcd v0.20.1-beta.0.20200513120220-b470eee47728/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13Px/pDuV7OomQ= github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo= github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA= github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d h1:yJzD/yFppdVCf6ApMkVy8cUxV0XrxdP9rVf6D87/Mng= From ae285bb9f3872013794d2a6e4142069c0266020f Mon Sep 17 00:00:00 2001 From: losh11 Date: Wed, 17 Jun 2020 23:36:26 +0100 Subject: [PATCH 25/25] update deps --- go.mod | 14 +++----------- go.sum | 6 ++++++ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index 60ef19fb35..2234a6e2a7 100644 --- a/go.mod +++ b/go.mod @@ -9,25 +9,17 @@ require ( github.com/jrick/logrotate v1.0.0 github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf github.com/lightningnetwork/lnd/queue v1.0.4 // indirect - github.com/ltcsuite/ltcd v0.20.1-beta + github.com/ltcsuite/ltcd v0.20.1-beta.0.20200617222819-81094527da29 github.com/ltcsuite/ltclog v0.0.0-20160817181405-73889fb79bd6 - github.com/ltcsuite/ltcutil v1.0.2 + github.com/ltcsuite/ltcutil v1.0.2-beta github.com/ltcsuite/ltcwallet/wallet/txauthor v1.0.0 github.com/ltcsuite/ltcwallet/wallet/txrules v1.0.0 github.com/ltcsuite/ltcwallet/walletdb v1.3.1 - github.com/ltcsuite/ltcwallet/wtxmgr v1.0.0 + github.com/ltcsuite/ltcwallet/wtxmgr v1.1.0 github.com/ltcsuite/neutrino v0.11.0 golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 google.golang.org/grpc v1.18.0 ) -replace github.com/ltcsuite/ltcd => ../ltcd - -replace github.com/ltcsuite/ltcutil => ../ltcutil - -replace github.com/ltcsuite/ltcwallet/walletdb => ./walletdb - -replace github.com/ltcsuite/ltcwallet/wtxmgr => ./wtxmgr - go 1.13 diff --git a/go.sum b/go.sum index dbf02f9f00..c6b4eab812 100644 --- a/go.sum +++ b/go.sum @@ -77,7 +77,13 @@ github.com/ltcsuite/lnd/queue v1.0.3/go.mod h1:L0MMGRrsJFPHhTInek8YgW2v7NyB6pXrA github.com/ltcsuite/lnd/ticker v1.0.1/go.mod h1:WZKpekfDVAVv7Gsrr0GAWC/U1XURfGesFg9sQYJbeL4= github.com/ltcsuite/ltcd v0.20.1-beta h1:ka9ZwUG7oUPppl+7ptuh5VDxGD7TWEJXu/IOOOz1yfY= github.com/ltcsuite/ltcd v0.20.1-beta/go.mod h1:ZFQaYdYULIuTQiWqs7AUiHD2XhDFeeHW1IH+UYMdABU= +github.com/ltcsuite/ltcd v0.20.1-beta.0.20200617222819-81094527da29 h1:k7X+QeOhE8u/DttC5eUAqaYhhEEd74DrZYxrJEb4D1k= +github.com/ltcsuite/ltcd v0.20.1-beta.0.20200617222819-81094527da29/go.mod h1:k4GH6YHNIenJnZY4zAETuiyiYdWKd00kIc6M2lXt/4g= github.com/ltcsuite/ltclog v0.0.0-20160817181405-73889fb79bd6/go.mod h1:0vobaAoyUrwaVFd11+PtAx4afDrzzyFopOaky5ytLBw= +github.com/ltcsuite/ltcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:8Vg/LTOO0KYa/vlHWJ6XZAevPQThGH5sufO0Hrou/lA= +github.com/ltcsuite/ltcutil v0.0.0-20191227053721-6bec450ea6ad/go.mod h1:8Vg/LTOO0KYa/vlHWJ6XZAevPQThGH5sufO0Hrou/lA= +github.com/ltcsuite/ltcutil v1.0.2-beta h1:IS86frABIvbpw9ilpQ/zt8t30pFog6DD4tBcgbjdj8g= +github.com/ltcsuite/ltcutil v1.0.2-beta/go.mod h1:G1JGpaqtMm0mPtheTryXnDd9a4KAFuGevdQirlJO1Nw= github.com/ltcsuite/ltcwallet v0.0.0-20190105125346-3fa612e326e5/go.mod h1:lLxtFelndCZougRANrysOgq+5aBCgBraH4/ovLb1fo0= github.com/ltcsuite/ltcwallet v0.11.1-beta/go.mod h1:NDOrqKG1Wo5f9WbKzEW0oQFRcFrA6znu+zFYgmFvyJE= github.com/ltcsuite/ltcwallet/wallet/txauthor v1.0.0 h1:V5iIFayENLKYw63swa3HsHEHvW8oKHzrVaqqhn48Xgc=