diff --git a/src/account.cpp b/src/account.cpp index 0e3babc230..0ff665691e 100644 --- a/src/account.cpp +++ b/src/account.cpp @@ -425,6 +425,18 @@ CAccountHD::CAccountHD(CExtPubKey accountKey_, boost::uuids::uuid seedID, Accoun m_readOnly = true; } +bool CAccountHD::GetKeyIDWithHighestIndex(CKeyID& HDKeyID, int nChain) const +{ + if (nChain == KEYCHAIN_EXTERNAL) + { + return externalKeyStore.GetKeyIDWithHighestIndex(HDKeyID); + } + else + { + return internalKeyStore.GetKeyIDWithHighestIndex(HDKeyID); + } +} + bool CAccountHD::GetKey(CExtKey& childKey, int nChain) const { assert(!m_readOnly); @@ -1073,13 +1085,25 @@ uint64_t CAccount::getEarliestPossibleCreationTime() return earliestPossibleCreationTime; } -//fixme: (FUT) (ACCOUNTS) (CLEANUP) - relook at how the wallet reports keypool size in an accounts context. unsigned int CAccount::GetKeyPoolSize() { AssertLockHeld(cs_keypool); // setKeyPool return setKeyPoolExternal.size(); } +unsigned int CAccount::GetKeyPoolSize(int nChain) +{ + AssertLockHeld(cs_keypool); // setKeyPool + if (nChain == KEYCHAIN_EXTERNAL) + { + return setKeyPoolExternal.size(); + } + else + { + return setKeyPoolInternal.size(); + } +} + std::string CAccount::getLabel() const { return accountLabel; diff --git a/src/account.h b/src/account.h index e534d38f38..8df32784e7 100644 --- a/src/account.h +++ b/src/account.h @@ -360,7 +360,8 @@ class CAccount : public CCryptoKeyStore virtual bool AddKeyPubKey(int64_t HDKeyIndex, const CPubKey &pubkey, int keyChain); void AddChild(CAccount* childAccount); - unsigned int GetKeyPoolSize(); + unsigned int GetKeyPoolSize(int nchain); + [[deprecated]] unsigned int GetKeyPoolSize(); std::string getLabel() const; void setLabel(const std::string& label, CWalletDB* Db); @@ -435,6 +436,7 @@ class CAccountHD: public CAccount //For serialization only. CAccountHD(){}; + virtual bool GetKeyIDWithHighestIndex(CKeyID& HDKeyID, int nChain) const; virtual bool GetKey(CExtKey& childKey, int nChain) const; virtual bool GetKey(const CKeyID& keyID, CKey& key) const override; virtual bool GetKey(const CKeyID &address, std::vector& encryptedKeyOut) const override; diff --git a/src/coins.cpp b/src/coins.cpp index c2d192409d..1e1e5f9c42 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -125,7 +125,19 @@ void CCoinsViewCache::validateInsert(const COutPoint &outpoint, uint64_t block, else { // no entry in cacheCoinRefs, so it should be absent in cacheCoins also - assert(!outpoint.isHash || cacheCoins.find(outpoint) == cacheCoins.end()); + if (outpoint.isHash) + { + if (cacheCoins.find(outpoint) != cacheCoins.end()) + { + CCoinsMap::iterator it = cacheCoins.find(outpoint); + std::string warning = strprintf("Warning: outpoint mismatch.\nPlease notify the developers with this information to assist them.\n\n" + "ophash:[%s]\nblock:[%d] txidx:[%d] outidx:[%d] flags:[%d] spent:[%s] lookupishash: [%s]", + outpoint.getTransactionHash().ToString(), + block, txIndex, voutIndex, it->second.flags, (it->second.coin.IsSpent()?"yes":"no"), (outpoint.isHash?"yes":"no")); + uiInterface.NotifyUIAlertChanged(warning); + } + } + // The non-hash output definitely shouldn't be in the cacheCoins ever assert(outpoint.isHash || cacheCoins.find(COutPoint(block, txIndex, voutIndex)) == cacheCoins.end()); } } diff --git a/src/keystore.h b/src/keystore.h index 8edd249a71..1cdc15bcec 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -139,6 +139,24 @@ class CBasicKeyStore : public CKeyStore } return false; } + bool GetKeyIDWithHighestIndex(CKeyID& HDKeyID) const + { + int64_t highestIndex=0; + { + LOCK(cs_KeyStore); + if (mapHDKeys.empty()) + return false; + for (const auto& [keyID, keyIndex] : mapHDKeys) + { + if (keyIndex >= highestIndex) + { + highestIndex = keyIndex; + HDKeyID = keyID; + } + } + } + return true; + } virtual bool AddCScript(const CScript& redeemScript); virtual bool HaveCScript(const CScriptID &hash) const; virtual bool GetCScript(const CScriptID &hash, CScript& redeemScriptOut) const; diff --git a/src/unity/unity_impl.cpp b/src/unity/unity_impl.cpp index 1d78ceec3d..afd96211e2 100644 --- a/src/unity/unity_impl.cpp +++ b/src/unity/unity_impl.cpp @@ -205,18 +205,23 @@ TransactionRecord calculateTransactionRecordForWalletTransaction(const CWalletTx //fixme: (UNITY) - rather calculate this once and pass it in instead of for every call.. std::vector accountsToTry; - for ( const auto& accountPair : pactiveWallet->mapAccounts ) + accountsToTry.push_back(pactiveWallet->activeAccount); + for (const auto& [accountUUID, account] : pactiveWallet->mapAccounts) { - if(accountPair.second->getParentUUID() == pactiveWallet->activeAccount->getUUID()) + (unused) accountUUID; + if (account->getParentUUID() == pactiveWallet->activeAccount->getUUID()) { - accountsToTry.push_back(accountPair.second); + accountsToTry.push_back(account); } - accountsToTry.push_back(pactiveWallet->activeAccount); } - - int64_t subtracted = wtx.GetDebit(ISMINE_SPENDABLE, pactiveWallet->activeAccount, true); - int64_t added = wtx.GetCredit(ISMINE_SPENDABLE, pactiveWallet->activeAccount, true); + int64_t subtracted; + int64_t added; + for (const auto& account : accountsToTry) + { + subtracted += wtx.GetDebit(ISMINE_SPENDABLE, account, true); + added += wtx.GetCredit(ISMINE_SPENDABLE, account, true); + } CAmount fee = 0; // if any funds were subtracted the transaction was sent by us diff --git a/src/wallet/Gulden/guldenwallet.cpp b/src/wallet/Gulden/guldenwallet.cpp index 917424ad51..54ad46ce8d 100644 --- a/src/wallet/Gulden/guldenwallet.cpp +++ b/src/wallet/Gulden/guldenwallet.cpp @@ -357,6 +357,23 @@ void CGuldenWallet::MarkKeyUsed(CKeyID keyID, uint64_t usageTime) { keyUsedSet.insert(keyID); + if (forAccount->IsMiningAccount()) + { + CKeyID highestKeyID; + //fixme: (HIGH) (KEYPOOL) All HD accounts should use a similar (not identical) method to this + //To obtain a proper key gap instead of what we have now + //Probably we should be storing the 'highest used key' somewhere in this function + //And then topupkeypool should utilise it for allocation (instead of the allocation trick we use here) + if (dynamic_cast(forAccount)->GetKeyIDWithHighestIndex(highestKeyID, KEYCHAIN_EXTERNAL) && (highestKeyID == keyID)) + { + // Assign 1 extra key, because mining accounts never discard keys the keypool size always grows when new keys are allocated + // Ideally most mining accounts will only have 1 key, but due to a previous bug some have more + LOCK(forAccount->cs_keypool); + uint64_t topupSize = forAccount->GetKeyPoolSize(KEYCHAIN_EXTERNAL)+1; + static_cast(this)->TopUpKeyPool(topupSize, 10, forAccount, topupSize); + } + } + if (forAccount->m_State != AccountState::Normal && forAccount->m_State != AccountState::ShadowChild) { forAccount->m_State = AccountState::Normal; diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index f4fbaa6746..7f761c6848 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -319,6 +319,15 @@ bool CCryptoKeyStore::GetKey(const CKeyID &address, std::vector& return false; } +bool CCryptoKeyStore::GetKeyIDWithHighestIndex(CKeyID &address) const +{ + // For HD we don't encrypt anything here - as the public key we need access to anyway, and the index is not special info - we derive the private key when we need it. + { + LOCK(cs_KeyStore); + return CBasicKeyStore::GetKeyIDWithHighestIndex(address); + } +} + bool CCryptoKeyStore::GetKey(const CKeyID &address, CKey& keyOut) const { { diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index 12ac434b53..d764950cab 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -187,6 +187,7 @@ class CCryptoKeyStore : public CBasicKeyStore } virtual bool GetKey(const CKeyID &address, std::vector& encryptedKeyOut) const; virtual bool GetKey(const CKeyID &address, CKey& keyOut) const; + virtual bool GetKeyIDWithHighestIndex(CKeyID& address) const; virtual bool GetKey(const CKeyID &address, int64_t& HDKeyIndex) const; virtual bool GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const; virtual void GetKeys(std::set &setAddress) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6329a1a580..1ea7b3a1c8 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -826,7 +826,7 @@ class CWallet : public CGuldenWallet /* size_t KeypoolCountExternalKeys(); */ - int TopUpKeyPool(unsigned int nTargetKeypoolSize = 0, unsigned int nMaxNewAllocations = 0, CAccount* forAccount = nullptr); + int TopUpKeyPool(unsigned int nTargetKeypoolSize = 0, unsigned int nMaxNewAllocations = 0, CAccount* forAccount = nullptr, unsigned int nMinimalKeypoolOverride = 1); void ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, CAccount* forAccount, int64_t keyChain); void KeepKey(int64_t nIndex); void ReturnKey(int64_t nIndex, CAccount* forAccount, int64_t keyChain); diff --git a/src/wallet/wallet_keypool.cpp b/src/wallet/wallet_keypool.cpp index b365e38a1d..6fe7cb959a 100644 --- a/src/wallet/wallet_keypool.cpp +++ b/src/wallet/wallet_keypool.cpp @@ -59,7 +59,7 @@ bool CWallet::NewKeyPool() } //fixme: (FUT) (ACCOUNTS) GULDEN Note for HD this should actually care more about maintaining a gap above the last used address than it should about the size of the pool. -int CWallet::TopUpKeyPool(unsigned int nTargetKeypoolSize, unsigned int nMaxNewAllocations, CAccount* topupForAccount) +int CWallet::TopUpKeyPool(unsigned int nTargetKeypoolSize, unsigned int nMaxNewAllocations, CAccount* topupForAccount, unsigned int nMinimalKeypoolOverride) { // Return -1 if we fail to allocate any -and- one of the accounts is not HD -and- it is locked. uint32_t nNew = 0; @@ -103,7 +103,7 @@ int CWallet::TopUpKeyPool(unsigned int nTargetKeypoolSize, unsigned int nMaxNewA uint32_t nFinalAccountTargetSize = nAccountTargetSize; if (loopForAccount->IsMinimalKeyPool()) { - nFinalAccountTargetSize = 1; + nFinalAccountTargetSize = nMinimalKeypoolOverride; } for (auto& keyChain : { KEYCHAIN_EXTERNAL, KEYCHAIN_CHANGE }) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index ce56e465d2..4e5e649b07 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -284,8 +284,7 @@ bool CWalletDB::ErasePool(CWallet* pwallet, const CKeyID& id, bool forceErase) { if ((iter.second->setKeyPoolExternal.find(keyIndex) != iter.second->setKeyPoolExternal.end()) || (iter.second->setKeyPoolInternal.find(keyIndex) != iter.second->setKeyPoolInternal.end())) { - allowErase = false; - break; + return true; } } } @@ -295,11 +294,8 @@ bool CWalletDB::ErasePool(CWallet* pwallet, const CKeyID& id, bool forceErase) //Remove from internal keypool, key has been used so shouldn't circulate anymore - address will now reside only in address book. for (auto iter : pwallet->mapAccounts) { - if (forceErase || !iter.second->IsFixedKeyPool()) - { - iter.second->setKeyPoolExternal.erase(keyIndex); - iter.second->setKeyPoolInternal.erase(keyIndex); - } + iter.second->setKeyPoolExternal.erase(keyIndex); + iter.second->setKeyPoolInternal.erase(keyIndex); } //Remove from disk