From 3aca99ab807bb79317f1fca7fd527c788b3e0a54 Mon Sep 17 00:00:00 2001 From: Javed Khan Date: Mon, 26 Jan 2015 17:38:14 +0530 Subject: [PATCH] Mark used addresses to prevent reuse --- chainntfns.go | 3 ++ waddrmgr/address.go | 22 ++++++++++- waddrmgr/db.go | 81 ++++++++++++++++++++++++++++++++++++++-- waddrmgr/manager.go | 25 ++++++++++--- waddrmgr/manager_test.go | 21 +++++++++++ wallet.go | 38 +++++++++---------- 6 files changed, 160 insertions(+), 30 deletions(-) diff --git a/chainntfns.go b/chainntfns.go index db91956608..1f0e7f45f6 100644 --- a/chainntfns.go +++ b/chainntfns.go @@ -170,6 +170,9 @@ func (w *Wallet) addRedeemingTx(tx *btcutil.Tx, block *txstore.Block) error { if _, err := txr.AddDebits(); err != nil { return err } + if err := w.markAddrsUsed(txr); err != nil { + return err + } bs, err := w.chainSvr.BlockStamp() if err == nil { diff --git a/waddrmgr/address.go b/waddrmgr/address.go index 0727e68433..e7fbfb2357 100644 --- a/waddrmgr/address.go +++ b/waddrmgr/address.go @@ -76,6 +76,9 @@ type ManagedAddress interface { // Compressed returns true if the backing address is compressed. Compressed() bool + + // Used returns true if the backing address has been used in a transaction. + Used() bool } // ManagedPubKeyAddress extends ManagedAddress and additionally provides the @@ -119,6 +122,7 @@ type managedAddress struct { imported bool internal bool compressed bool + used bool pubKey *btcec.PublicKey privKeyEncrypted []byte privKeyCT []byte // non-nil if unlocked @@ -209,6 +213,13 @@ func (a *managedAddress) Compressed() bool { return a.compressed } +// Used returns true if the address has been used in a transaction. +// +// This is part of the ManagedAddress interface implementation. +func (a *managedAddress) Used() bool { + return a.used +} + // PubKey returns the public key associated with the address. // // This is part of the ManagedPubKeyAddress interface implementation. @@ -379,6 +390,7 @@ type scriptAddress struct { scriptEncrypted []byte scriptCT []byte scriptMutex sync.Mutex + used bool } // Enforce scriptAddress satisfies the ManagedScriptAddress interface. @@ -466,6 +478,13 @@ func (a *scriptAddress) Compressed() bool { return false } +// Used returns true if the address has been used in a transaction. +// +// This is part of the ManagedAddress interface implementation. +func (a *scriptAddress) Used() bool { + return a.used +} + // Script returns the script associated with the address. // // This implements the ScriptAddress interface. @@ -490,7 +509,7 @@ func (a *scriptAddress) Script() ([]byte, error) { } // newScriptAddress initializes and returns a new pay-to-script-hash address. -func newScriptAddress(m *Manager, account uint32, scriptHash, scriptEncrypted []byte) (*scriptAddress, error) { +func newScriptAddress(m *Manager, account uint32, scriptHash, scriptEncrypted []byte, used bool) (*scriptAddress, error) { address, err := btcutil.NewAddressScriptHashFromHash(scriptHash, m.chainParams) if err != nil { @@ -502,5 +521,6 @@ func newScriptAddress(m *Manager, account uint32, scriptHash, scriptEncrypted [] account: account, address: address, scriptEncrypted: scriptEncrypted, + used: used, }, nil } diff --git a/waddrmgr/db.go b/waddrmgr/db.go index 185fb7800c..bd61125172 100644 --- a/waddrmgr/db.go +++ b/waddrmgr/db.go @@ -29,7 +29,7 @@ import ( const ( // LatestMgrVersion is the most recent manager version. - LatestMgrVersion = 1 + LatestMgrVersion = 2 ) // maybeConvertDbError converts the passed error to a ManagerError with an @@ -101,6 +101,7 @@ type dbAddressRow struct { account uint32 addTime uint64 syncStatus syncStatus + used bool rawData []byte // Varies based on address type field. } @@ -156,6 +157,9 @@ var ( // Account related key names (account bucket). acctNumAcctsName = []byte("numaccts") + + // Used addresses (used bucket) + usedAddrBucketName = []byte("usedaddrs") ) // fetchMasterKeyParams loads the master key parameters needed to derive them @@ -554,8 +558,7 @@ func deserializeAddressRow(addressID, serializedAddress []byte) (*dbAddressRow, // serializeAddressRow returns the serialization of the passed address row. func serializeAddressRow(row *dbAddressRow) []byte { // The serialized address format is: - // - // + // // // 1 byte addrType + 4 bytes account + 8 bytes addTime + 1 byte // syncStatus + 4 bytes raw data length + raw data @@ -730,6 +733,8 @@ func fetchAddress(tx walletdb.Tx, addressID []byte) (interface{}, error) { if err != nil { return nil, err } + used := fetchAddrUsed(tx, addressID) + row.used = used switch row.addrType { case adtChain: @@ -744,6 +749,35 @@ func fetchAddress(tx walletdb.Tx, addressID []byte) (interface{}, error) { return nil, managerError(ErrDatabase, str, nil) } +// markAddressUsed flags the provided address id as used in the database. +func markAddressUsed(tx walletdb.Tx, addressID []byte) error { + bucket := tx.RootBucket().Bucket(usedAddrBucketName) + + addrHash := fastsha256.Sum256(addressID) + val := bucket.Get(addrHash[:]) + if val != nil { + return nil + } + err := bucket.Put(addrHash[:], []byte{0}) + if err != nil { + str := fmt.Sprintf("failed to mark address used %x", addressID) + return managerError(ErrDatabase, str, err) + } + return nil +} + +// fetchAddrUsed returns true if the provided address id was flagged as used. +func fetchAddrUsed(tx walletdb.Tx, addressID []byte) bool { + bucket := tx.RootBucket().Bucket(usedAddrBucketName) + + addrHash := fastsha256.Sum256(addressID) + val := bucket.Get(addrHash[:]) + if val != nil { + return true + } + return false +} + // putAddress stores the provided address information to the database. This // is used a common base for storing the various address types. func putAddress(tx walletdb.Tx, addressID []byte, row *dbAddressRow) error { @@ -1228,6 +1262,12 @@ func upgradeManager(namespace walletdb.Namespace) error { return managerError(ErrDatabase, str, err) } + _, err = rootBucket.CreateBucketIfNotExists(usedAddrBucketName) + if err != nil { + str := "failed to create used addresses bucket" + return managerError(ErrDatabase, str, err) + } + // Save the most recent database version if it isn't already // there, otherwise keep track of it for potential upgrades. verBytes := mainBucket.Get(mgrVersionName) @@ -1268,8 +1308,41 @@ func upgradeManager(namespace walletdb.Namespace) error { // Upgrade the manager as needed. if version < LatestMgrVersion { - // No upgrades yet. + // Upgrade addresses used flag + upgradeVersion1to2(namespace) } return nil } + +// upgradeVersion1to2 upgrades the database from version 1 to version 2 +// 'usedAddrBucketName' a bucket for storing addrs flagged as marked is initialized +// and it will be updated on the next rescan +func upgradeVersion1to2(namespace walletdb.Namespace) error { + err := namespace.Update(func(tx walletdb.Tx) error { + rootBucket := tx.RootBucket() + mainBucket := tx.RootBucket().Bucket(mainBucketName) + + _, err := rootBucket.CreateBucketIfNotExists(usedAddrBucketName) + if err != nil { + str := "failed to create used addresses bucket" + return managerError(ErrDatabase, str, err) + } + + var version uint32 + var buf [4]byte + version = LatestMgrVersion + binary.LittleEndian.PutUint32(buf[:], version) + err = mainBucket.Put(mgrVersionName, buf[:]) + if err != nil { + str := "failed to store latest database version" + return managerError(ErrDatabase, str, err) + } + return nil + }) + if err != nil { + str := "failed to upgrade version 1 to version 2" + return managerError(ErrDatabase, str, err) + } + return nil +} diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index eda637f808..27d07c9470 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -348,7 +348,7 @@ func (m *Manager) Close() error { // The passed derivedKey is zeroed after the new address is created. // // This function MUST be called with the manager lock held for writes. -func (m *Manager) keyToManaged(derivedKey *hdkeychain.ExtendedKey, account, branch, index uint32) (ManagedAddress, error) { +func (m *Manager) keyToManaged(derivedKey *hdkeychain.ExtendedKey, account, branch, index uint32, used bool) (ManagedAddress, error) { // Create a new managed address based on the public or private key // depending on whether the passed key is private. Also, zero the // key after creating the managed address from it. @@ -371,6 +371,7 @@ func (m *Manager) keyToManaged(derivedKey *hdkeychain.ExtendedKey, account, bran if branch == internalBranch { ma.internal = true } + ma.used = used return ma, nil } @@ -485,7 +486,7 @@ func (m *Manager) loadAccountInfo(account uint32) (*accountInfo, error) { if err != nil { return nil, err } - lastExtAddr, err := m.keyToManaged(lastExtKey, account, branch, index) + lastExtAddr, err := m.keyToManaged(lastExtKey, account, branch, index, false) if err != nil { return nil, err } @@ -500,7 +501,7 @@ func (m *Manager) loadAccountInfo(account uint32) (*accountInfo, error) { if err != nil { return nil, err } - lastIntAddr, err := m.keyToManaged(lastIntKey, account, branch, index) + lastIntAddr, err := m.keyToManaged(lastIntKey, account, branch, index, false) if err != nil { return nil, err } @@ -536,7 +537,7 @@ func (m *Manager) chainAddressRowToManaged(row *dbChainAddressRow) (ManagedAddre return nil, err } - return m.keyToManaged(addressKey, row.account, row.branch, row.index) + return m.keyToManaged(addressKey, row.account, row.branch, row.index, row.used) } // importedAddressRowToManaged returns a new managed address based on imported @@ -563,6 +564,7 @@ func (m *Manager) importedAddressRowToManaged(row *dbImportedAddressRow) (Manage } ma.privKeyEncrypted = row.encryptedPrivKey ma.imported = true + ma.used = row.used return ma, nil } @@ -577,7 +579,7 @@ func (m *Manager) scriptAddressRowToManaged(row *dbScriptAddressRow) (ManagedAdd return nil, managerError(ErrCrypto, str, err) } - return newScriptAddress(m, row.account, scriptHash, row.encryptedScript) + return newScriptAddress(m, row.account, scriptHash, row.encryptedScript, row.used) } // rowInterfaceToManaged returns a new managed address based on the given @@ -1125,7 +1127,7 @@ func (m *Manager) ImportScript(script []byte, bs *BlockStamp) (ManagedScriptAddr // since it will be cleared on lock and the script the caller passed // should not be cleared out from under the caller. scriptAddr, err := newScriptAddress(m, ImportedAddrAccount, scriptHash, - encryptedScript) + encryptedScript, false) if err != nil { return nil, err } @@ -1289,6 +1291,17 @@ func (m *Manager) Unlock(passphrase []byte) error { return nil } +// MarkUsed updates the used flag for the provided address id. +func (m *Manager) MarkUsed(addressID []byte) error { + err := m.namespace.Update(func(tx walletdb.Tx) error { + return markAddressUsed(tx, addressID) + }) + if err != nil { + return maybeConvertDbError(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/waddrmgr/manager_test.go b/waddrmgr/manager_test.go index e954384581..7f7bb675bc 100644 --- a/waddrmgr/manager_test.go +++ b/waddrmgr/manager_test.go @@ -67,6 +67,7 @@ type expectedAddr struct { addressHash []byte internal bool compressed bool + used bool imported bool pubKey []byte privKey []byte @@ -272,6 +273,12 @@ func testAddress(tc *testContext, prefix string, gotAddr waddrmgr.ManagedAddress return false } + if gotAddr.Used() != wantAddr.used { + tc.t.Errorf("%s Imported: unexpected used flag - got %v, "+ + "want %v", prefix, gotAddr.Used(), wantAddr.used) + return false + } + switch addr := gotAddr.(type) { case waddrmgr.ManagedPubKeyAddress: if !testManagedPubKeyAddress(tc, prefix, addr, wantAddr) { @@ -299,6 +306,7 @@ func testExternalAddresses(tc *testContext) bool { addressHash: hexToBytes("2b49ecd0cf72006173e6e95acf416b6735b5f889"), internal: false, compressed: true, + used: false, imported: false, pubKey: hexToBytes("02d8f88468c5a2e8e1815faf555f59cbd1979e3dbdf823f80c271b6fb70d2d519b"), privKey: hexToBytes("c27d6581b92785834b381fa697c4b0ffc4574b495743722e0acb7601b1b68b99"), @@ -309,6 +317,7 @@ func testExternalAddresses(tc *testContext) bool { addressHash: hexToBytes("e6c59a1542138d1bf08f45cd18899557cf56b356"), internal: false, compressed: true, + used: false, imported: false, pubKey: hexToBytes("02b9c175b908624f8a8eaac227d0e8c77c0eec327b8c512ad1b8b7a4b5b676971f"), privKey: hexToBytes("18f3b191019e83878a81557abebb2afda199e31d22e150d8bf4df4561671be6c"), @@ -319,6 +328,7 @@ func testExternalAddresses(tc *testContext) bool { addressHash: hexToBytes("0561e9373986965b647a57a09718e9c050215cfe"), internal: false, compressed: true, + used: false, imported: false, pubKey: hexToBytes("0329faddf1254d490d6add49e2b08cf52b561038c72baec0edb3cfacff71ff1021"), privKey: hexToBytes("ccb8f6305b73136b363644b647f6efc0fd27b6b7d9c11c7e560662ed38db7b34"), @@ -329,6 +339,7 @@ func testExternalAddresses(tc *testContext) bool { addressHash: hexToBytes("1af950be02584ca230b7078cec0cfd38dd71b468"), internal: false, compressed: true, + used: false, imported: false, pubKey: hexToBytes("03d738324e2f0ce42e46975d7f8c7117c1670e3d7912b0291aea452add99674774"), privKey: hexToBytes("d6bc8ff768814fede2adcdb74826bd846924341b3862e3b6e31cdc084e992940"), @@ -339,6 +350,7 @@ func testExternalAddresses(tc *testContext) bool { addressHash: hexToBytes("d578a267a7174c6ba7f76b0ab2397ce0ba0c5c3c"), internal: false, compressed: true, + used: false, imported: false, pubKey: hexToBytes("03a917acd5cd5b6f544b43f1921a35677e4d5320e5d2add2056039b4b44fdf905e"), privKey: hexToBytes("8563ade061110e03aee50695ffc5cb1c06c8310bde0a3674257c853c966968c0"), @@ -465,6 +477,7 @@ func testInternalAddresses(tc *testContext) bool { addressHash: hexToBytes("2ef94abb9ee8f785d087c3ec8d6ee467e92d0d0a"), internal: true, compressed: true, + used: false, imported: false, pubKey: hexToBytes("020a1290b997c0a234a95213962e7edcb761c7360f0230f698a1a3e71c37047bb0"), privKey: hexToBytes("fe4f855fcf059ec6ddf7b25f63b19aa49c771d1fcb9850b68ae3d65e20657a60"), @@ -475,6 +488,7 @@ func testInternalAddresses(tc *testContext) bool { addressHash: hexToBytes("d3c8ec46891f599bfeaa4c25918bfb3d46ea334c"), internal: true, compressed: true, + used: false, imported: false, pubKey: hexToBytes("03f79bbde32af42dde98195f011d95982602fcd0dab657fe4a1f49f9d5ada1e02d"), privKey: hexToBytes("bfef521317c65b018ae7e6d7ecc3aa700d5d0f7ea84d567be9270382d0b5e3e6"), @@ -485,6 +499,7 @@ func testInternalAddresses(tc *testContext) bool { addressHash: hexToBytes("1a0ad2a04fde3b2afe068057591e1871c289c4b8"), internal: true, compressed: true, + used: false, imported: false, pubKey: hexToBytes("023ded84afe4fe91b52b45c3deb26fd263f749cbc27747dc964dae9e0739cbc579"), privKey: hexToBytes("f506dffd4494c24006df7a35f3291f7ca0297a1a431557a1339bfed6f48738ca"), @@ -495,6 +510,7 @@ func testInternalAddresses(tc *testContext) bool { addressHash: hexToBytes("689b0249c628265215fd1de6142d5d5594eb8dc2"), internal: true, compressed: true, + used: false, imported: false, pubKey: hexToBytes("030f1e79f06824e10a259914ec310528bb2d5b8d6356341fe9dff55498591af6af"), privKey: hexToBytes("b3629de8ef6a275b4ffae41aa2bbbc2952eb92282ea6402435abbb010ecc1fb8"), @@ -505,6 +521,7 @@ func testInternalAddresses(tc *testContext) bool { addressHash: hexToBytes("c11dd8a3577978807a0453febedee2994a6144d4"), internal: true, compressed: true, + used: false, imported: false, pubKey: hexToBytes("0317d7182e26b6ca3e0f3db531c474b9cab7a763a75eabff2e14ac92f62a793238"), privKey: hexToBytes("ca747a7ef815ea0dbe68655272cecbfbd65f2a109019a9ed28e0d3dcaffe05c3"), @@ -734,6 +751,7 @@ func testImportPrivateKey(tc *testContext) bool { internal: false, imported: true, compressed: false, + used: false, pubKey: hexToBytes("04d0de0aaeaefad02b8bdc8a01a1b8b11c696bd3" + "d66a2c5f10780d95b7df42645cd85228a6fb29940e858e7e558" + "42ae2bd115d1ed7cc0e82d934e929c97648cb0a"), @@ -750,6 +768,7 @@ func testImportPrivateKey(tc *testContext) bool { internal: false, imported: true, compressed: true, + used: false, pubKey: hexToBytes("02d0de0aaeaefad02b8bdc8a01a1b8b11c696bd3d66a2c5f10780d95b7df42645c"), privKey: hexToBytes("0c28fca386c7a227600b2fe50b7cae11ec86d3bf1fbe471be89827e19d72aa1d"), // privKeyWIF is set to the in field during tests @@ -888,6 +907,7 @@ func testImportScript(tc *testContext) bool { internal: false, imported: true, compressed: false, + used: false, // script is set to the in field during tests. }, }, @@ -909,6 +929,7 @@ func testImportScript(tc *testContext) bool { internal: false, imported: true, compressed: false, + used: false, // script is set to the in field during tests. }, }, diff --git a/wallet.go b/wallet.go index 23c4b8e262..1c81110cda 100644 --- a/wallet.go +++ b/wallet.go @@ -257,6 +257,24 @@ func (w *Wallet) ListenUnconfirmedBalance() (<-chan btcutil.Amount, error) { return w.unconfirmedBalance, nil } +// markAddrsUsed marks the addresses credited by the given transaction +// record as used. +func (w *Wallet) markAddrsUsed(t *txstore.TxRecord) error { + for _, c := range t.Credits() { + // Errors don't matter here. If addrs is nil, the + // range below does nothing. + _, addrs, _, _ := c.Addresses(activeNet.Params) + for _, addr := range addrs { + addressID := addr.ScriptAddress() + if err := w.Manager.MarkUsed(addressID); err != nil { + return err + } + log.Infof("Marked address used %s", addr.EncodeAddress()) + } + } + return nil +} + func (w *Wallet) notifyConnectedBlock(block waddrmgr.BlockStamp) { w.notificationLock.Lock() if w.connectedBlocks != nil { @@ -701,25 +719,7 @@ func (w *Wallet) diskWriter() { // there are any transactions with outputs to this address in the blockchain or // the btcd mempool. func (w *Wallet) AddressUsed(addr waddrmgr.ManagedAddress) bool { - // This not only can be optimized by recording this data as it is - // read when opening a wallet, and keeping it up to date each time a - // new received tx arrives, but it probably should in case an address is - // used in a tx (made public) but the tx is eventually removed from the - // store (consider a chain reorg). - - for _, r := range w.TxStore.Records() { - for _, c := range r.Credits() { - // Errors don't matter here. If addrs is nil, the - // range below does nothing. - _, addrs, _, _ := c.Addresses(activeNet.Params) - for _, a := range addrs { - if addr.Address().String() == a.String() { - return true - } - } - } - } - return false + return addr.Used() } // CalculateBalance sums the amounts of all unspent transaction