From 172213be5b174243dc501c1103ad5fe2fee67a16 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 18 Jun 2019 15:19:13 -0400 Subject: Add GetNewDestination to CWallet to fetch new destinations Instead of having the same multiple lines of code everywhere that new destinations are fetched, introduce GetNewDestination as a member function of CWallet which does the key fetching, label setting, script generation, and destination generation. --- src/interfaces/wallet.cpp | 6 ++++-- src/interfaces/wallet.h | 4 ++-- src/qt/addresstablemodel.cpp | 16 +++++++--------- src/qt/paymentserver.cpp | 8 +++----- src/test/util.cpp | 11 +++-------- src/wallet/rpcwallet.cpp | 16 ++++------------ src/wallet/test/wallet_tests.cpp | 5 +++-- src/wallet/wallet.cpp | 21 +++++++++++++++++++++ src/wallet/wallet.h | 6 +++++- 9 files changed, 52 insertions(+), 41 deletions(-) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 240670cbe7..93374ea825 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -140,9 +140,11 @@ public: void abortRescan() override { m_wallet->AbortRescan(); } bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); } std::string getWalletName() override { return m_wallet->GetName(); } - bool getKeyFromPool(bool internal, CPubKey& pub_key) override + bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) override { - return m_wallet->GetKeyFromPool(pub_key, internal); + LOCK(m_wallet->cs_wallet); + std::string error; + return m_wallet->GetNewDestination(type, label, dest, error); } bool getPubKey(const CKeyID& address, CPubKey& pub_key) override { return m_wallet->GetPubKey(address, pub_key); } bool getPrivKey(const CKeyID& address, CKey& key) override { return m_wallet->GetKey(address, key); } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 7096f54047..25815d8532 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -76,8 +76,8 @@ public: //! Get wallet name. virtual std::string getWalletName() = 0; - // Get key from pool. - virtual bool getKeyFromPool(bool internal, CPubKey& pub_key) = 0; + // Get a new address. + virtual bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) = 0; //! Get public key. virtual bool getPubKey(const CKeyID& address, CPubKey& pub_key) = 0; diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index fa6c9c9f7a..29423db3d0 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -358,12 +358,15 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con return QString(); } } + + // Add entry + walletModel->wallet().setAddressBook(DecodeDestination(strAddress), strLabel, "send"); } else if(type == Receive) { // Generate a new address to associate with given label - CPubKey newKey; - if(!walletModel->wallet().getKeyFromPool(false /* internal */, newKey)) + CTxDestination dest; + if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest)) { WalletModel::UnlockContext ctx(walletModel->requestUnlock()); if(!ctx.isValid()) @@ -372,23 +375,18 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con editStatus = WALLET_UNLOCK_FAILURE; return QString(); } - if(!walletModel->wallet().getKeyFromPool(false /* internal */, newKey)) + if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest)) { editStatus = KEY_GENERATION_FAILURE; return QString(); } } - walletModel->wallet().learnRelatedScripts(newKey, address_type); - strAddress = EncodeDestination(GetDestinationForKey(newKey, address_type)); + strAddress = EncodeDestination(dest); } else { return QString(); } - - // Add entry - walletModel->wallet().setAddressBook(DecodeDestination(strAddress), strLabel, - (type == Send ? "send" : "receive")); return QString::fromStdString(strAddress); } diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 43dccec4ea..345db7acde 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -666,16 +666,14 @@ void PaymentServer::fetchPaymentACK(WalletModel* walletModel, const SendCoinsRec payment.add_transactions(transaction.data(), transaction.size()); // Create a new refund address, or re-use: - CPubKey newKey; - if (walletModel->wallet().getKeyFromPool(false /* internal */, newKey)) { + CTxDestination dest; + const OutputType change_type = walletModel->wallet().getDefaultChangeType() != OutputType::CHANGE_AUTO ? walletModel->wallet().getDefaultChangeType() : walletModel->wallet().getDefaultAddressType(); + if (walletModel->wallet().getNewDestination(change_type, "", dest)) { // BIP70 requests encode the scriptPubKey directly, so we are not restricted to address // types supported by the receiver. As a result, we choose the address format we also // use for change. Despite an actual payment and not change, this is a close match: // it's the output type we use subject to privacy issues, but not restricted by what // other software supports. - const OutputType change_type = walletModel->wallet().getDefaultChangeType() != OutputType::CHANGE_AUTO ? walletModel->wallet().getDefaultChangeType() : walletModel->wallet().getDefaultAddressType(); - walletModel->wallet().learnRelatedScripts(newKey, change_type); - CTxDestination dest = GetDestinationForKey(newKey, change_type); std::string label = tr("Refund from %1").arg(recipient.authenticatedMerchant).toStdString(); walletModel->wallet().setAddressBook(dest, label, "refund"); diff --git a/src/test/util.cpp b/src/test/util.cpp index bc09d00b7a..44afba1a02 100644 --- a/src/test/util.cpp +++ b/src/test/util.cpp @@ -25,14 +25,9 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq std::string getnewaddress(CWallet& w) { constexpr auto output_type = OutputType::BECH32; - - CPubKey new_key; - if (!w.GetKeyFromPool(new_key)) assert(false); - - w.LearnRelatedScripts(new_key, output_type); - const auto dest = GetDestinationForKey(new_key, output_type); - - w.SetAddressBook(dest, /* label */ "", "receive"); + CTxDestination dest; + std::string error; + if (!w.GetNewDestination(output_type, "", dest, error)) assert(false); return EncodeDestination(dest); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f9baabeda4..1801493f54 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -185,19 +185,11 @@ static UniValue getnewaddress(const JSONRPCRequest& request) } } - if (!pwallet->IsLocked()) { - pwallet->TopUpKeyPool(); - } - - // Generate a new key that is added to wallet - CPubKey newKey; - if (!pwallet->GetKeyFromPool(newKey)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first"); + CTxDestination dest; + std::string error; + if (!pwallet->GetNewDestination(output_type, label, dest, error)) { + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error); } - pwallet->LearnRelatedScripts(newKey, output_type); - CTxDestination dest = GetDestinationForKey(newKey, output_type); - - pwallet->SetAddressBook(dest, label, "receive"); return EncodeDestination(dest); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 922bb0fe65..afbd8a5fba 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -466,8 +466,9 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) wallet->SetMinVersion(FEATURE_LATEST); wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); BOOST_CHECK(!wallet->TopUpKeyPool(1000)); - CPubKey pubkey; - BOOST_CHECK(!wallet->GetKeyFromPool(pubkey, false)); + CTxDestination dest; + std::string error; + BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "", dest, error)); } // Explicit calculation which is used to test the wallet constant diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bde52d7791..7a7ff2b6cb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3513,6 +3513,27 @@ bool CWallet::GetKeyFromPool(CPubKey& result, bool internal) return true; } +bool CWallet::GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, std::string& error) +{ + LOCK(cs_wallet); + error.clear(); + if (!IsLocked()) { + TopUpKeyPool(); + } + + // Generate a new key that is added to wallet + CPubKey new_key; + if (!GetKeyFromPool(new_key)) { + error = "Error: Keypool ran out, please call keypoolrefill first"; + return false; + } + LearnRelatedScripts(new_key, type); + dest = GetDestinationForKey(new_key, type); + + SetAddressBook(dest, label, "receive"); + return true; +} + static int64_t GetOldestKeyTimeInPool(const std::set& setKeyPool, WalletBatch& batch) { if (setKeyPool.empty()) { return GetTime(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 87aff09039..09c2ce3796 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -815,6 +815,9 @@ private: */ uint256 m_last_block_processed GUARDED_BY(cs_wallet); + //! Fetches a key from the keypool + bool GetKeyFromPool(CPubKey &key, bool internal = false); + public: /* * Main wallet lock. @@ -1110,7 +1113,6 @@ public: bool ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal); void KeepKey(int64_t nIndex); void ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey); - bool GetKeyFromPool(CPubKey &key, bool internal = false); int64_t GetOldestKeyPoolTime(); /** * Marks all keys in the keypool up to and including reserve_key as used. @@ -1123,6 +1125,8 @@ public: std::set GetLabelAddresses(const std::string& label) const; + bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, std::string& error); + isminetype IsMine(const CTxIn& txin) const; /** * Returns amount of debit if the input matches the -- cgit v1.2.3