Skip to content

Commit 1fc8c3d

Browse files
committed
No longer ever reuse keypool indexes
This fixes an issue where you could reserve a keypool entry, then top up the keypool, writing out a new key at the given index, then return they key from the pool. This isnt likely to cause issues, but given there is no reason to ever re-use keypool indexes (they're 64 bits...), best to avoid it alltogether.
1 parent 0b01935 commit 1fc8c3d

File tree

2 files changed

+9
-10
lines changed

2 files changed

+9
-10
lines changed

src/wallet/wallet.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3211,21 +3211,17 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
32113211
internal = true;
32123212
}
32133213

3214-
if (!setInternalKeyPool.empty()) {
3215-
nEnd = *(setInternalKeyPool.rbegin()) + 1;
3216-
}
3217-
if (!setExternalKeyPool.empty()) {
3218-
nEnd = std::max(nEnd, *(setExternalKeyPool.rbegin()) + 1);
3219-
}
3214+
assert(m_max_keypool_index < std::numeric_limits<int64_t>::max()); // How in the hell did you use so many keys?
3215+
int64_t index = ++m_max_keypool_index;
32203216

3221-
if (!walletdb.WritePool(nEnd, CKeyPool(GenerateNewKey(walletdb, internal), internal))) {
3217+
if (!walletdb.WritePool(index, CKeyPool(GenerateNewKey(walletdb, internal), internal))) {
32223218
throw std::runtime_error(std::string(__func__) + ": writing generated key failed");
32233219
}
32243220

32253221
if (internal) {
3226-
setInternalKeyPool.insert(nEnd);
3222+
setInternalKeyPool.insert(index);
32273223
} else {
3228-
setExternalKeyPool.insert(nEnd);
3224+
setExternalKeyPool.insert(index);
32293225
}
32303226
}
32313227
if (missingInternal + missingExternal > 0) {

src/wallet/wallet.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
701701

702702
std::set<int64_t> setInternalKeyPool;
703703
std::set<int64_t> setExternalKeyPool;
704+
int64_t m_max_keypool_index;
704705

705706
int64_t nTimeFirstKey;
706707

@@ -743,13 +744,14 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
743744
}
744745
}
745746

746-
void LoadKeyPool(int nIndex, const CKeyPool &keypool)
747+
void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool)
747748
{
748749
if (keypool.fInternal) {
749750
setInternalKeyPool.insert(nIndex);
750751
} else {
751752
setExternalKeyPool.insert(nIndex);
752753
}
754+
m_max_keypool_index = std::max(m_max_keypool_index, nIndex);
753755

754756
// If no metadata exists yet, create a default with the pool key's
755757
// creation time. Note that this may be overwritten by actually
@@ -795,6 +797,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
795797
nAccountingEntryNumber = 0;
796798
nNextResend = 0;
797799
nLastResend = 0;
800+
m_max_keypool_index = 0;
798801
nTimeFirstKey = 0;
799802
fBroadcastTransactions = false;
800803
nRelockTime = 0;

0 commit comments

Comments
 (0)