From e6adae3db242a2146504bc72cf18ae58bb73401e Mon Sep 17 00:00:00 2001 From: David Gumberg Date: Thu, 2 Apr 2026 18:29:27 -0400 Subject: [PATCH 1/3] wallet: `NotifyCanGetAddressesChanged` when advancing `next_index` Even though `TopUp()` notifies, advancing `next_index` after can deplete available addresses, so make sure to notify any time it's changed. --- src/wallet/scriptpubkeyman.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index c3db3816eb0..91b45099c4c 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -905,6 +905,7 @@ util::Result DescriptorScriptPubKeyMan::GetNewDestination(const return util::Error{_("Error: Cannot extract destination from the generated scriptpubkey")}; // shouldn't happen } m_wallet_descriptor.next_index++; + NotifyCanGetAddressesChanged(); WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor); return dest; } From 0892f16f911d0e2ac7ebf40946b74c2cef485e2c Mon Sep 17 00:00:00 2001 From: David Gumberg Date: Thu, 2 Apr 2026 18:30:46 -0400 Subject: [PATCH 2/3] refactor: moveonly: Pair CanGetAddressesChanged notifications with desc range. --- src/wallet/scriptpubkeyman.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 91b45099c4c..ee22746786e 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -986,9 +986,9 @@ void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, // Only return when the index was the most recent if (m_wallet_descriptor.next_index - 1 == index) { m_wallet_descriptor.next_index--; + NotifyCanGetAddressesChanged(); } WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor); - NotifyCanGetAddressesChanged(); } std::map DescriptorScriptPubKeyMan::GetKeys() const @@ -1104,13 +1104,13 @@ bool DescriptorScriptPubKeyMan::TopUpWithDB(WalletBatch& batch, unsigned int siz m_max_cached_index++; } m_wallet_descriptor.range_end = new_range_end; + NotifyCanGetAddressesChanged(); batch.WriteDescriptor(GetID(), m_wallet_descriptor); // By this point, the cache size should be the size of the entire range assert(m_wallet_descriptor.range_end - 1 == m_max_cached_index); m_storage.TopUpCallback(new_spks, this); - NotifyCanGetAddressesChanged(); return true; } @@ -1132,6 +1132,7 @@ std::vector DescriptorScriptPubKeyMan::MarkUnusedAddresses(co ExtractDestination(scripts_temp[0], dest); result.push_back({dest, std::nullopt}); m_wallet_descriptor.next_index++; + NotifyCanGetAddressesChanged(); } } if (!TopUp()) { From e2ab8ae55142370f31d8606531a065e098be6c77 Mon Sep 17 00:00:00 2001 From: David Gumberg Date: Tue, 30 Jun 2026 17:41:45 -0700 Subject: [PATCH 3/3] wallet: spkm: Only notify CanGetAddressesChanged on change And refactor to encapsulate range changes to force notifications from DSPKM. --- src/wallet/export.cpp | 4 +- src/wallet/scriptpubkeyman.cpp | 82 +++++++++++++++++++++++----------- src/wallet/scriptpubkeyman.h | 3 ++ src/wallet/walletutil.h | 40 +++++++++++++++-- 4 files changed, 97 insertions(+), 32 deletions(-) diff --git a/src/wallet/export.cpp b/src/wallet/export.cpp index 1df51b6b260..5ce74283e4c 100644 --- a/src/wallet/export.cpp +++ b/src/wallet/export.cpp @@ -36,8 +36,8 @@ util::Expected, std::string> ExportDescriptors(const wallet_descriptor.creation_time, wallet.IsActiveScriptPubKeyMan(*desc_spk_man), wallet.IsInternalScriptPubKeyMan(desc_spk_man), - is_range ? std::optional(std::make_pair(wallet_descriptor.range_start, wallet_descriptor.range_end)) : std::nullopt, - wallet_descriptor.next_index + is_range ? std::optional(std::make_pair(wallet_descriptor.GetStart(), wallet_descriptor.GetEnd())) : std::nullopt, + wallet_descriptor.GetNext() ); } return wallet_descriptors; diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index ee22746786e..904593af263 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -871,6 +871,42 @@ std::unique_ptr DescriptorScriptPubKeyMan::GenerateNe return spkm; } +void DescriptorScriptPubKeyMan::IncIndex() +{ + AssertLockHeld(cs_desc_man); + + const auto old_can = CanGetAddresses(); + m_wallet_descriptor.IncNext(); + const auto new_can = CanGetAddresses(); + if (old_can != new_can) { + NotifyCanGetAddressesChanged(); + } +} + +void DescriptorScriptPubKeyMan::DecIndex() +{ + AssertLockHeld(cs_desc_man); + + const auto old_can = CanGetAddresses(); + m_wallet_descriptor.DecNext(); + const auto new_can = CanGetAddresses(); + if (old_can != new_can) { + NotifyCanGetAddressesChanged(); + } +} + +void DescriptorScriptPubKeyMan::SetRangeEnd(int32_t end) +{ + AssertLockHeld(cs_desc_man); + + const auto old_can = CanGetAddresses(); + m_wallet_descriptor.SetEnd(end); + const auto new_can = CanGetAddresses(); + if (old_can != new_can) { + NotifyCanGetAddressesChanged(); + } +} + util::Result DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type) { // Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later @@ -891,11 +927,11 @@ util::Result DescriptorScriptPubKeyMan::GetNewDestination(const // Get the scriptPubKey from the descriptor FlatSigningProvider out_keys; std::vector scripts_temp; - if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) { + if (m_wallet_descriptor.GetEnd() <= m_max_cached_index && !TopUp(1)) { // We can't generate anymore keys return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } - if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, out_keys)) { + if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.GetNext(), m_wallet_descriptor.cache, scripts_temp, out_keys)) { // We can't generate anymore keys return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } @@ -904,8 +940,7 @@ util::Result DescriptorScriptPubKeyMan::GetNewDestination(const if (!ExtractDestination(scripts_temp[0], dest)) { return util::Error{_("Error: Cannot extract destination from the generated scriptpubkey")}; // shouldn't happen } - m_wallet_descriptor.next_index++; - NotifyCanGetAddressesChanged(); + IncIndex(); WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor); return dest; } @@ -976,7 +1011,7 @@ util::Result DescriptorScriptPubKeyMan::GetReservedDestination(c { LOCK(cs_desc_man); auto op_dest = GetNewDestination(type); - index = m_wallet_descriptor.next_index - 1; + index = m_wallet_descriptor.GetNext() - 1; return op_dest; } @@ -984,9 +1019,8 @@ void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, { LOCK(cs_desc_man); // Only return when the index was the most recent - if (m_wallet_descriptor.next_index - 1 == index) { - m_wallet_descriptor.next_index--; - NotifyCanGetAddressesChanged(); + if (m_wallet_descriptor.GetNext() - 1 == index) { + DecIndex(); } WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor); } @@ -1061,13 +1095,11 @@ bool DescriptorScriptPubKeyMan::TopUpWithDB(WalletBatch& batch, unsigned int siz } // Calculate the new range_end - int32_t new_range_end = std::max(m_wallet_descriptor.next_index + (int32_t)target_size, m_wallet_descriptor.range_end); + int32_t new_range_end = std::max(m_wallet_descriptor.GetNext() + (int32_t)target_size, m_wallet_descriptor.GetEnd()); // If the descriptor is not ranged, we actually just want to fill the first cache item if (!m_wallet_descriptor.descriptor->IsRange()) { new_range_end = 1; - m_wallet_descriptor.range_end = 1; - m_wallet_descriptor.range_start = 0; } FlatSigningProvider provider; @@ -1103,12 +1135,11 @@ bool DescriptorScriptPubKeyMan::TopUpWithDB(WalletBatch& batch, unsigned int siz } m_max_cached_index++; } - m_wallet_descriptor.range_end = new_range_end; - NotifyCanGetAddressesChanged(); + SetRangeEnd(new_range_end); batch.WriteDescriptor(GetID(), m_wallet_descriptor); // By this point, the cache size should be the size of the entire range - assert(m_wallet_descriptor.range_end - 1 == m_max_cached_index); + assert(m_wallet_descriptor.GetEnd() - 1 == m_max_cached_index); m_storage.TopUpCallback(new_spks, this); return true; @@ -1120,19 +1151,18 @@ std::vector DescriptorScriptPubKeyMan::MarkUnusedAddresses(co std::vector result; if (IsMine(script)) { int32_t index = m_map_script_pub_keys[script]; - if (index >= m_wallet_descriptor.next_index) { + if (index >= m_wallet_descriptor.GetNext()) { WalletLogPrintf("%s: Detected a used keypool item at index %d, mark all keypool items up to this item as used\n", __func__, index); auto out_keys = std::make_unique(); std::vector scripts_temp; - while (index >= m_wallet_descriptor.next_index) { - if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, *out_keys)) { + while (index >= m_wallet_descriptor.GetNext()) { + if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.GetNext(), m_wallet_descriptor.cache, scripts_temp, *out_keys)) { throw std::runtime_error(std::string(__func__) + ": Unable to expand descriptor from cache"); } CTxDestination dest; ExtractDestination(scripts_temp[0], dest); result.push_back({dest, std::nullopt}); - m_wallet_descriptor.next_index++; - NotifyCanGetAddressesChanged(); + IncIndex(); } } if (!TopUp()) { @@ -1224,7 +1254,7 @@ bool DescriptorScriptPubKeyMan::CanGetAddresses(bool internal) const LOCK(cs_desc_man); return m_wallet_descriptor.descriptor->IsSingleType() && m_wallet_descriptor.descriptor->IsRange() && - (HavePrivateKeys() || m_wallet_descriptor.next_index < m_wallet_descriptor.range_end || m_wallet_descriptor.descriptor->CanSelfExpand()); + (HavePrivateKeys() || m_wallet_descriptor.GetNext() < m_wallet_descriptor.GetEnd() || m_wallet_descriptor.descriptor->CanSelfExpand()); } bool DescriptorScriptPubKeyMan::HavePrivateKeys() const @@ -1242,7 +1272,7 @@ bool DescriptorScriptPubKeyMan::HaveCryptedKeys() const unsigned int DescriptorScriptPubKeyMan::GetKeyPoolSize() const { LOCK(cs_desc_man); - return m_wallet_descriptor.range_end - m_wallet_descriptor.next_index; + return m_wallet_descriptor.GetEnd() - m_wallet_descriptor.GetNext(); } int64_t DescriptorScriptPubKeyMan::GetTimeFirstKey() const @@ -1481,7 +1511,7 @@ void DescriptorScriptPubKeyMan::Load() { LOCK(cs_desc_man); std::set new_spks; - for (int32_t i = m_wallet_descriptor.range_start; i < m_wallet_descriptor.range_end; ++i) { + for (int32_t i = m_wallet_descriptor.GetStart(); i < m_wallet_descriptor.GetEnd(); ++i) { FlatSigningProvider out_keys; std::vector scripts_temp; if (!m_wallet_descriptor.descriptor->ExpandFromCache(i, m_wallet_descriptor.cache, scripts_temp, out_keys)) { @@ -1647,12 +1677,12 @@ bool DescriptorScriptPubKeyMan::CanUpdateToWalletDescriptor(const WalletDescript return true; } - if (descriptor.range_start > m_wallet_descriptor.range_start || - descriptor.range_end < m_wallet_descriptor.range_end) { + if (descriptor.GetStart() > m_wallet_descriptor.GetStart() || + descriptor.GetEnd() < m_wallet_descriptor.GetEnd()) { // Use inclusive range for error error = strprintf("new range must include current range = [%d,%d]", - m_wallet_descriptor.range_start, - m_wallet_descriptor.range_end - 1); + m_wallet_descriptor.GetStart(), + m_wallet_descriptor.GetEnd() - 1); return false; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 6cebae052bf..809a999d26a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -340,6 +340,9 @@ protected: {} WalletDescriptor m_wallet_descriptor GUARDED_BY(cs_desc_man); + void IncIndex() EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); + void DecIndex() EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); + void SetRangeEnd(int32_t end) EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); //! Same as 'TopUp' but designed for use within a batch transaction context bool TopUpWithDB(WalletBatch& batch, unsigned int size = 0); diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h index bbf279329fb..a85a339a2da 100644 --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -62,15 +62,41 @@ fs::path GetWalletDir(); /** Descriptor with some wallet metadata */ class WalletDescriptor { +private: + int32_t range_start = 0; // First item in range; start of range, inclusive, i.e. [range_start, range_end). This never changes. + int32_t next_index = 0; // Position of the next item to generate + int32_t range_end = 0; // Item after the last; end of range, exclusive, i.e. [range_start, range_end). This will increment with each TopUp() public: std::shared_ptr descriptor; uint256 id; // Descriptor ID (calculated once at descriptor initialization/deserialization) uint64_t creation_time = 0; - int32_t range_start = 0; // First item in range; start of range, inclusive, i.e. [range_start, range_end). This never changes. - int32_t range_end = 0; // Item after the last; end of range, exclusive, i.e. [range_start, range_end). This will increment with each TopUp() - int32_t next_index = 0; // Position of the next item to generate DescriptorCache cache; + int32_t GetStart() const { return range_start; } + int32_t GetNext() const { return next_index; } + int32_t GetEnd() const { return range_end; } + + //! Increments the next_index of the descriptor. + void IncNext() + { + next_index++; + } + + //! Increments the next_index of the descriptor. + void DecNext() + { + next_index--; + } + + //! Sets the range_end of the descriptor. + void SetEnd(int32_t end) + { + if (!descriptor->IsRange()) { + CHECK_NONFATAL(end == 1); + } + range_end = end; + } + void DeserializeDescriptor(const std::string& str) { std::string error; @@ -95,7 +121,13 @@ public: } WalletDescriptor() = default; - WalletDescriptor(std::shared_ptr descriptor, uint64_t creation_time, int32_t range_start, int32_t range_end, int32_t next_index) : descriptor(descriptor), id(DescriptorID(*descriptor)), creation_time(creation_time), range_start(range_start), range_end(range_end), next_index(next_index) { } + WalletDescriptor(std::shared_ptr descriptor, uint64_t creation_time, int32_t range_start, int32_t range_end, int32_t next_index) + : range_start(descriptor->IsRange() ? range_start : 0), + next_index(next_index), + range_end(descriptor->IsRange() ? range_end : 1), + descriptor(descriptor), + id(DescriptorID(*descriptor)), + creation_time(creation_time) {} }; WalletDescriptor GenerateWalletDescriptor(const CExtPubKey& master_key, const OutputType& output_type, bool internal);