wallet: Remove `CWallet::GetKeyFromPool`

Legacy transparent addresses for external use are now obtained directly
via `GenerateNewKey(true)`.

qa/rpc-tests/keypool.py

@@ -40,13 +40,13 @@ class KeyPoolTest(BitcoinTestFramework):
         bitcoind_processes[0].wait()
         # Restart node 0
         nodes[0] = start_node(0, self.options.tmpdir)
-        # Keep creating keys
-        addr = nodes[0].getnewaddress()
+        # We can't create any external addresses, which don't use the keypool.
+        # We should get an error that we need to unlock the wallet.
         try:
             addr = nodes[0].getnewaddress()
-            raise AssertionError('Keypool should be exhausted after one address')
+            raise AssertionError('Wallet should be locked.')
         except JSONRPCException as e:
-            assert(e.error['code']==-12)
+            assert_equal(e.error['code'], -13)
 
         # put three new keys in the keypool
         nodes[0].walletpassphrase('test', 12000)

qa/rpc-tests/walletbackup.py

@@ -96,10 +96,10 @@ class WalletBackupTest(BitcoinTestFramework):
         self.sync_all()
 
     # As above, this mirrors the original bash test.
-    def start_three(self):
-        self.nodes[0] = start_node(0, self.options.tmpdir)
-        self.nodes[1] = start_node(1, self.options.tmpdir)
-        self.nodes[2] = start_node(2, self.options.tmpdir)
+    def start_three(self, extra_args=None):
+        self.nodes[0] = start_node(0, self.options.tmpdir, extra_args)
+        self.nodes[1] = start_node(1, self.options.tmpdir, extra_args)
+        self.nodes[2] = start_node(2, self.options.tmpdir, extra_args)
         connect_nodes(self.nodes[0], 3)
         connect_nodes(self.nodes[1], 3)
         connect_nodes(self.nodes[2], 3)
@@ -191,6 +191,30 @@ class WalletBackupTest(BitcoinTestFramework):
         self.start_three()
         sync_blocks(self.nodes)
 
+        # We made extra transactions that involved addresses generated after the
+        # backups were taken, and external addresses do not use the keypool, so
+        # the balances shouldn't line up.
+        balance0backup = self.nodes[0].getbalance()
+        balance1backup = self.nodes[1].getbalance()
+        balance2backup = self.nodes[2].getbalance()
+        assert(balance0backup != balance0)
+        assert(balance1backup != balance1)
+        assert(balance2backup != balance2)
+
+        # However, because addresses are derived deterministically, we can
+        # recover the balances by generating the extra addresses and then
+        # rescanning.
+        for i in range(5):
+            self.nodes[0].getnewaddress()
+            self.nodes[1].getnewaddress()
+            self.nodes[2].getnewaddress()
+
+        logging.info("Re-starting nodes with -rescan")
+        self.stop_three()
+        self.start_three(['-rescan'])
+        sync_blocks(self.nodes)
+
+        # TODO: Alter keypool to not use external addresses, so this passes.
         assert_equal(self.nodes[0].getbalance(), balance0)
         assert_equal(self.nodes[1].getbalance(), balance1)
         assert_equal(self.nodes[2].getbalance(), balance2)
@@ -215,9 +239,9 @@ class WalletBackupTest(BitcoinTestFramework):
 
         sync_blocks(self.nodes)
 
-        assert_equal(self.nodes[0].getbalance(), balance0)
-        assert_equal(self.nodes[1].getbalance(), balance1)
-        assert_equal(self.nodes[2].getbalance(), balance2)
+        assert_equal(self.nodes[0].getbalance(), balance0backup)
+        assert_equal(self.nodes[1].getbalance(), balance1backup)
+        assert_equal(self.nodes[2].getbalance(), balance2backup)
 
 
 if __name__ == '__main__':

src/wallet/rpcwallet.cpp

@@ -182,14 +182,11 @@ UniValue getnewaddress(const UniValue& params, bool fHelp)
     const CChainParams& chainparams = Params();
     EnsureWalletIsBackedUp(chainparams);
 
-    if (!pwalletMain->IsLocked())
-        pwalletMain->TopUpKeyPool();
+    EnsureWalletIsUnlocked();
 
     // Generate a new key that is added to wallet
-    std::optional<CPubKey> newKey = pwalletMain->GetKeyFromPool();
-    if (!newKey.has_value())
-        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
-    CKeyID keyID = newKey.value().GetID();
+    CPubKey newKey = pwalletMain->GenerateNewKey(true);
+    CKeyID keyID = newKey.GetID();
 
     std::string dummy_account;
     pwalletMain->SetAddressBook(keyID, dummy_account, "receive");

src/wallet/wallet.cpp

@@ -5130,23 +5130,6 @@ void CWallet::ReturnKey(int64_t nIndex)
     LogPrintf("keypool return %d\n", nIndex);
 }
 
-std::optional<CPubKey> CWallet::GetKeyFromPool()
-{
-    int64_t nIndex = 0;
-    CKeyPool keypool;
-    {
-        LOCK(cs_wallet);
-        ReserveKeyFromKeyPool(nIndex, keypool);
-        if (nIndex == -1)
-        {
-            if (IsLocked()) return std::nullopt;
-            return GenerateNewKey(true);
-        }
-        KeepKey(nIndex);
-        return keypool.vchPubKey;
-    }
-}
-
 int64_t CWallet::GetOldestKeyPoolTime()
 {
     int64_t nIndex = 0;
@@ -5715,9 +5698,9 @@ bool CWallet::InitLoadWallet(const CChainParams& params, bool clearWitnessCaches
     if (fFirstRun)
     {
         // Create new keyUser and set as default key
-        std::optional<CPubKey> newDefaultKey = walletInstance->GetKeyFromPool();
-        if (newDefaultKey.has_value()) {
-            walletInstance->SetDefaultKey(newDefaultKey.value());
+        if (!walletInstance->IsCrypted()) {
+            CPubKey newDefaultKey = walletInstance->GenerateNewKey(true);
+            walletInstance->SetDefaultKey(newDefaultKey);
             if (!walletInstance->SetAddressBook(walletInstance->vchDefaultKey.GetID(), "", "receive"))
                 return UIError(_("Cannot write default address") += "\n");
         }

src/wallet/wallet.h

@@ -1536,7 +1536,6 @@ public:
     void ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool);
     void KeepKey(int64_t nIndex);
     void ReturnKey(int64_t nIndex);
-    std::optional<CPubKey> GetKeyFromPool();
     int64_t GetOldestKeyPoolTime();
     void GetAllReserveKeys(std::set<CKeyID>& setAddress) const;