Repository navigation
feat(wallet): wallet-owned rescans for DashPay receiving chains and asset-lock funding discovery #8068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
feat(wallet): wallet-owned rescans for DashPay receiving chains and asset-lock funding discovery #8068
Changes from all commits
834eb0a
0a35172
7faade6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -188,49 +188,109 @@ class WalletImpl : public Wallet | |||||||||||||||||||
| { | ||||||||||||||||||||
| return m_wallet->ChangeWalletPassphrase(old_wallet_passphrase, new_wallet_passphrase); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| //! Where a rescan from the birth of the wallet's first key starts. | ||||||||||||||||||||
| int rescanStartHeight() | ||||||||||||||||||||
| { | ||||||||||||||||||||
| int rescan_height{0}; | ||||||||||||||||||||
| std::optional<int64_t> time_first_key; | ||||||||||||||||||||
| for (auto spk_man : m_wallet->GetAllScriptPubKeyMans()) { | ||||||||||||||||||||
| int64_t time = spk_man->GetTimeFirstKey(); | ||||||||||||||||||||
| if (!time_first_key || time < *time_first_key) time_first_key = time; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if (time_first_key) { | ||||||||||||||||||||
| m_wallet->chain().findFirstBlockWithTimeAndHeight(*time_first_key - TIMESTAMP_WINDOW, rescan_height, | ||||||||||||||||||||
| FoundBlock().height(rescan_height)); | ||||||||||||||||||||
| //! The block a rescan of the blocks a key born at `time` may be paid in | ||||||||||||||||||||
| //! starts from, and its height, as importdescriptors does; nullopt when | ||||||||||||||||||||
| //! no block is that recent. | ||||||||||||||||||||
| std::optional<std::pair<uint256, int>> rescanStart(int64_t time) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| uint256 hash; | ||||||||||||||||||||
| int height{0}; | ||||||||||||||||||||
| if (!m_wallet->chain().findFirstBlockWithTimeAndHeight(time - TIMESTAMP_WINDOW, /*min_height=*/0, | ||||||||||||||||||||
| FoundBlock().hash(hash).height(height))) { | ||||||||||||||||||||
| return std::nullopt; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return rescan_height; | ||||||||||||||||||||
| return std::make_pair(hash, height); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| wallet::RescanStatus startRescan(bool from_genesis) override | ||||||||||||||||||||
| //! Scan from start and, once complete, settle the owed rescans up to | ||||||||||||||||||||
| //! owed_sequence that it covers from covered_from. | ||||||||||||||||||||
| wallet::RescanStatus rescanFrom(const WalletRescanReserver& reserver, const std::pair<uint256, int>& start, | ||||||||||||||||||||
| uint64_t owed_sequence, int64_t covered_from) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| const int rescan_height{from_genesis ? 0 : rescanStartHeight()}; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| WalletRescanReserver reserver(*m_wallet); | ||||||||||||||||||||
| if (!reserver.reserve()) { | ||||||||||||||||||||
| return wallet::RescanStatus::BUSY; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| switch (m_wallet->ScanForWalletTransactions(m_wallet->chain().getBlockHash(rescan_height), rescan_height, /*max_height=*/std::nullopt, | ||||||||||||||||||||
| reserver, /*fUpdate=*/true, /*save_progress=*/false).status) { | ||||||||||||||||||||
| switch (m_wallet->ScanForWalletTransactions(start.first, start.second, /*max_height=*/std::nullopt, reserver, | ||||||||||||||||||||
| /*fUpdate=*/true, /*save_progress=*/false).status) { | ||||||||||||||||||||
| case CWallet::ScanResult::FAILURE: | ||||||||||||||||||||
| return wallet::RescanStatus::FAILURE; | ||||||||||||||||||||
| case CWallet::ScanResult::SUCCESS: | ||||||||||||||||||||
| WITH_LOCK(m_wallet->cs_wallet, m_wallet->SettleOwedRescans(owed_sequence, covered_from)); | ||||||||||||||||||||
| return wallet::RescanStatus::SUCCESS; | ||||||||||||||||||||
|
Comment on lines
213
to
215
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🔴 Blocking: Make transaction persistence retry-safe before settling owed rescans A successful retry can reach this settlement with the recovered payment still only in memory. AddToWallet() inserts a newly discovered payment into mapWallet, wtxOrdered and setWalletUTXO before WriteTx() (wallet.cpp:1120–1132,1193–1196). If that write fails, AddToWalletIfInvolvingMe() throws without rolling those insertions back. After the caller catches the error and storage recovers, rescanning the same confirmed, unspent payment sets neither fInsertedNew nor fUpdated: its confirmation state is unchanged, and AddWalletUTXOs(..., false) finds the output already present. WriteTx() is therefore skipped, the retry returns SUCCESS, and settlement deletes the durable recovery obligation. Flush()/Close() do not rewrite mapWallet. On reload, a wallet whose saved locator already passed the historical payment loses it with nothing owed; ensuring the same complete receiving chain at the same birth is also a no-op. Although the transaction insertion behavior predates this PR, the new durable-recovery contract depends on it. Make failed transaction writes retry-safe in the transaction-storage owner, by rolling back the unpublished state or retaining and reliably retrying failed writes. Add a regression that fails the payment's TX-record write, retries a covering scan in the same session, and reloads to verify that settlement cannot discard recovery before the payment is persisted. source:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, this blocks the PR. The fix goes into D-04's settlement rather than into The bug. Your trace holds.
Why not fix it in
The fix (D-04, 0a35172). D-04 erases an owed-rescan record only while no write to the wallet's database has failed since the wallet loaded.
Tests (
Each reversion was run against the final test:
Runs, on Linux (gcc 15, depends,
🤖 Posted autonomously by Claude on behalf of pasta.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved (re-reviewed at |
||||||||||||||||||||
| case CWallet::ScanResult::USER_ABORT: | ||||||||||||||||||||
| return wallet::RescanStatus::USER_ABORT; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| Assume(false); // unreachable | ||||||||||||||||||||
| return wallet::RescanStatus::FAILURE; // fallback for release builds | ||||||||||||||||||||
| } | ||||||||||||||||||||
| wallet::RescanStatus startRescan(bool from_genesis) override | ||||||||||||||||||||
| { | ||||||||||||||||||||
| WalletRescanReserver reserver(*m_wallet); | ||||||||||||||||||||
| if (!reserver.reserve()) { | ||||||||||||||||||||
| return wallet::RescanStatus::BUSY; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| uint64_t owed; | ||||||||||||||||||||
| std::optional<int64_t> time_first_key; | ||||||||||||||||||||
| { | ||||||||||||||||||||
| LOCK(m_wallet->cs_wallet); | ||||||||||||||||||||
| owed = m_wallet->GetOwedRescanSequence(); | ||||||||||||||||||||
| if (!from_genesis) { | ||||||||||||||||||||
| for (auto spk_man : m_wallet->GetAllScriptPubKeyMans()) { | ||||||||||||||||||||
| int64_t time = spk_man->GetTimeFirstKey(); | ||||||||||||||||||||
| if (!time_first_key || time < *time_first_key) time_first_key = time; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| std::optional<std::pair<uint256, int>> start; | ||||||||||||||||||||
| if (time_first_key) start = rescanStart(*time_first_key); | ||||||||||||||||||||
| if (!start) start.emplace(m_wallet->chain().getBlockHash(0), 0); | ||||||||||||||||||||
| return rescanFrom(reserver, *start, owed, time_first_key.value_or(0)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| wallet::RescanStatus rescanFromTime(int64_t from_time) override | ||||||||||||||||||||
| { | ||||||||||||||||||||
| from_time = std::max<int64_t>(from_time, 0); | ||||||||||||||||||||
| WalletRescanReserver reserver(*m_wallet); | ||||||||||||||||||||
| if (!reserver.reserve()) { | ||||||||||||||||||||
| return wallet::RescanStatus::BUSY; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| // Taken before looking for blocks: a chain imported later may be paid | ||||||||||||||||||||
| // in one that arrives meanwhile. | ||||||||||||||||||||
| const uint64_t owed{WITH_LOCK(m_wallet->cs_wallet, return m_wallet->GetOwedRescanSequence())}; | ||||||||||||||||||||
| const auto start{rescanStart(from_time)}; | ||||||||||||||||||||
| if (!start) { | ||||||||||||||||||||
| LOCK(m_wallet->cs_wallet); | ||||||||||||||||||||
| m_wallet->chain().requestMempoolTransactions(*m_wallet); | ||||||||||||||||||||
| m_wallet->SettleOwedRescans(owed, from_time); | ||||||||||||||||||||
| return wallet::RescanStatus::SUCCESS; | ||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+256
to
+261
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🔴 Blocking: Replay existing mempool payments before settling an empty-range rescan When the active chain's maximum timestamp is older than from_time minus TIMESTAMP_WINDOW, this branch clears the obligation without replaying the mempool. A payment already present before the receiving chain was imported was ignored by its original notification because the wallet did not yet own its script. Unlike the normal ScanForWalletTransactions path, which calls requestMempoolTransactions when max_height is unset, this path leaves that payment absent while reporting successful recovery. Repeating the same import creates no new obligation. Replay the mempool before settlement, and extend the empty-block-range test with a payment present before import.
Suggested change
source:
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved (re-reviewed at |
||||||||||||||||||||
| return rescanFrom(reserver, *start, owed, from_time); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| void abortRescan() override { m_wallet->AbortRescan(); } | ||||||||||||||||||||
| std::optional<int> rescanPrunedFrom() override | ||||||||||||||||||||
| std::optional<int> rescanUnavailableFrom(int64_t from_time) override | ||||||||||||||||||||
| { | ||||||||||||||||||||
| const auto start{rescanStart(std::max<int64_t>(from_time, 0))}; | ||||||||||||||||||||
| if (!start) return std::nullopt; | ||||||||||||||||||||
| // The rescan reads up to the chain's tip, which the wallet may not have reached. | ||||||||||||||||||||
| const CBlockLocator locator{m_wallet->chain().getTipLocator()}; | ||||||||||||||||||||
| int tip_height{0}; | ||||||||||||||||||||
| if (locator.IsNull() || !m_wallet->chain().findBlock(locator.vHave.front(), FoundBlock().height(tip_height))) { | ||||||||||||||||||||
| return std::nullopt; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| const uint256& tip{locator.vHave.front()}; | ||||||||||||||||||||
| if (m_wallet->chain().hasBlocks(tip, start->second)) return std::nullopt; | ||||||||||||||||||||
| // Every block from start up to the first one without data has it. | ||||||||||||||||||||
| int first_missing{start->second}; | ||||||||||||||||||||
| int last{tip_height}; | ||||||||||||||||||||
| while (first_missing < last) { | ||||||||||||||||||||
| const int mid{first_missing + (last - first_missing) / 2}; | ||||||||||||||||||||
| if (m_wallet->chain().hasBlocks(tip, start->second, mid)) { | ||||||||||||||||||||
| first_missing = mid + 1; | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| last = mid; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return first_missing; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| std::optional<int64_t> owedRescanFrom() override | ||||||||||||||||||||
| { | ||||||||||||||||||||
| if (!m_wallet->chain().havePruned()) return std::nullopt; | ||||||||||||||||||||
| LOCK(m_wallet->cs_wallet); | ||||||||||||||||||||
| const int rescan_height{rescanStartHeight()}; | ||||||||||||||||||||
| if (m_wallet->chain().hasBlocks(m_wallet->GetLastBlockHash(), rescan_height)) return std::nullopt; | ||||||||||||||||||||
| return rescan_height; | ||||||||||||||||||||
| return m_wallet->GetOwedRescanFrom(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| void autoLockMasternodeCollaterals() override { m_wallet->AutoLockMasternodeCollaterals(); } | ||||||||||||||||||||
| bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); } | ||||||||||||||||||||
|
|
@@ -281,11 +341,21 @@ class WalletImpl : public Wallet | |||||||||||||||||||
| { | ||||||||||||||||||||
| return m_wallet->PlatformAccountReference(key, compact_xpub); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| wallet::PlatformKeyResult<wallet::FriendshipReceivingChain> getFriendshipReceivingXpub( | ||||||||||||||||||||
| const wallet::FriendshipKeychainRequest& request) override | ||||||||||||||||||||
| { | ||||||||||||||||||||
| return m_wallet->GetFriendshipReceivingXpub(request); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| wallet::PlatformKeyResult<wallet::FriendshipXpub> ensureFriendshipReceivingKeychain( | ||||||||||||||||||||
| const wallet::FriendshipKeychainRequest& request) override | ||||||||||||||||||||
| { | ||||||||||||||||||||
| return m_wallet->EnsureFriendshipReceivingKeychain(request); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| wallet::PlatformKeyResult<std::vector<wallet::AssetLockFunding>> listAssetLockFundings( | ||||||||||||||||||||
| const std::vector<wallet::AssetLockFundingKey>& keys) override | ||||||||||||||||||||
| { | ||||||||||||||||||||
| return m_wallet->ListAssetLockFundings(keys); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| bool isSpendable(const CScript& script) override | ||||||||||||||||||||
| { | ||||||||||||||||||||
| LOCK(m_wallet->cs_wallet); | ||||||||||||||||||||
|
|
||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.