-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Allow for aborting rescans in the GUI #11200
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
Conversation
1bb1af0
to
e6e8b0c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK modulo Travis issues
src/qt/bitcoingui.cpp
Outdated
progressDialog->setAutoClose(false); | ||
progressDialog->setValue(0); | ||
|
||
if (cancel) { | ||
progressDialog->setCancelButtonText("Cancel"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this "Cancel" need a translation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it does. Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be possible to abort rescans caused by the RPC calls?
I'm not very fond of callbacks. Have you thought in alternatives?
src/qt/bitcoingui.cpp
Outdated
else if (progressDialog) { | ||
if (progressDialog->wasCanceled()) { | ||
cancel(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} else {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/qt/bitcoingui.cpp
Outdated
@@ -1119,16 +1120,21 @@ void BitcoinGUI::detectShutdown() | |||
} | |||
} | |||
|
|||
void BitcoinGUI::showProgress(const QString &title, int nProgress) | |||
void BitcoinGUI::showProgress(const QString &title, int nProgress, const std::function<void(void)> &cancel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&
are next to the type. Same for remaining changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
I don't know what is causing the travis failures.
Yes. There is an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK e6e8b0c259594b09e405fc9123d1f8473e42b0e4 with RPC bugfixes
src/qt/bitcoingui.cpp
Outdated
@@ -487,7 +487,7 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel) | |||
connect(_clientModel, SIGNAL(message(QString,QString,unsigned int)), this, SLOT(message(QString,QString,unsigned int))); | |||
|
|||
// Show progress dialog | |||
connect(_clientModel, SIGNAL(showProgress(QString,int)), this, SLOT(showProgress(QString,int))); | |||
connect(_clientModel, SIGNAL(showProgress(QString,int, std::function<void(void)>)), this, SLOT(showProgress(QString,int, std::function<void(void)>))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "Add cancel function reference to ShowProgress"
A typedef or using declaration for std::function<void(void)>
might make the changes throughout this commit be more readable, and also make this code less fragile because SIGNAL and SLOT macros rely on string comparison.
src/txdb.cpp
Outdated
@@ -386,7 +386,7 @@ bool CCoinsViewDB::Upgrade() { | |||
if (count++ % 256 == 0) { | |||
uint32_t high = 0x100 * *key.second.begin() + *(key.second.begin() + 1); | |||
int percentageDone = (int)(high * 100.0 / 65536.0 + 0.5); | |||
uiInterface.ShowProgress(_("Upgrading UTXO database") + "\n"+ _("(press q to shutdown and continue later)") + "\n", percentageDone); | |||
uiInterface.ShowProgress(_("Upgrading UTXO database") + "\n"+ _("(press q to shutdown and continue later)") + "\n", percentageDone, std::function<void(void)>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "Add cancel function reference to ShowProgress"
No need to write std::function<void(void)>()
here and 12 other places. You can just pass {} or nullptr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't realize that that would work. Done.
src/qt/bitcoingui.cpp
Outdated
progressDialog->setValue(nProgress); | ||
else if (progressDialog) { | ||
if (progressDialog->wasCanceled()) { | ||
cancel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "Add cancel button to rescan progress dialog"
Maybe assert(cancel)
src/wallet/rpcdump.cpp
Outdated
@@ -535,7 +556,14 @@ UniValue importwallet(const JSONRPCRequest& request) | |||
file.close(); | |||
pwallet->ShowProgress("", 100, std::function<void(void)>()); // hide progress dialog in GUI | |||
pwallet->UpdateTimeFirstKey(nTimeBegin); | |||
pwallet->RescanFromTime(nTimeBegin, false /* update */); | |||
int64_t scanned_time = pwallet->RescanFromTime(nTimeBegin, false /* update */); | |||
if (scanned_time > TIMESTAMP_MIN) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "Give an error when rescan is aborted by the user"
s/TIMESTAMP_MIN/nTimeBegin/ here to fix test failures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
b42c0a9
to
c8720f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK c8720f0a320eff0cc83a459dc87f28087d518825. Only changes were implementing various suggestions.
I'm not very fond of callbacks. Have you thought in alternatives?
Reason callback seems to be used is to avoid the need for qt code to call CWallet's AbortRescan method or set the fAbortRescan flag directly. The alternatives would be for Qt to call a different function or set a different flag, but don't seem like they would be much better or worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK
src/wallet/wallet.cpp
Outdated
@@ -1568,13 +1568,13 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool f | |||
fAbortRescan = false; | |||
fScanningWallet = true; | |||
|
|||
ShowProgress(_("Rescanning..."), 0); // show rescan progress in GUI as dialog or on splashscreen, if -rescan on startup | |||
ShowProgress(_("Rescanning..."), 0, boost::bind(&CWallet::AbortRescan, this)); // show rescan progress in GUI as dialog or on splashscreen, if -rescan on startup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: use std::bind
(and below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/ui_interface.h
Outdated
@@ -95,7 +95,7 @@ class CClientUIInterface | |||
boost::signals2::signal<void (CWallet* wallet)> LoadWallet; | |||
|
|||
/** Show progress e.g. for verifychain */ | |||
boost::signals2::signal<void (const std::string &title, int nProgress)> ShowProgress; | |||
boost::signals2::signal<void (const std::string &title, int nProgress, const std::function<void(void)>& cancel)> ShowProgress; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use nullptr
as default parameter for cancel
to avoid adding nullptr
to all non-callback ShowProgress
calls?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't set a default parameter in the signal, so I'm not sure it is possible to do that. Or I'm just doing something wrong.
c8720f0
to
3498226
Compare
Tested a bit and there is a concurrency issue (maybe not related to this code but since you finally can test multiple rescans during the same runtime this PR may reveal the issue): Sometimes the progress window does not show up (happened to me only if I canceled a rescan before) and the GUI is fully in locked state (due to the rescan). |
Needs rebase |
aca0587
to
4657e40
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 4657e40f1516d506095f969dc726da6f97d57a99. Only change since last review is rebase and some whitespace diffs.
src/qt/bitcoingui.cpp
Outdated
@@ -487,7 +487,7 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel) | |||
connect(_clientModel, SIGNAL(message(QString,QString,unsigned int)), this, SLOT(message(QString,QString,unsigned int))); | |||
|
|||
// Show progress dialog | |||
connect(_clientModel, SIGNAL(showProgress(QString,int)), this, SLOT(showProgress(QString,int))); | |||
connect(_clientModel, SIGNAL(showProgress(QString,int, bool, std::function<void(void)>)), this, SLOT(showProgress(QString,int, std::function<void(void)>))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could just write void()
instead of void(void)
everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/qt/splashscreen.cpp
Outdated
@@ -181,7 +181,7 @@ static void ShowProgress(SplashScreen *splash, const std::string &title, int nPr | |||
#ifdef ENABLE_WALLET | |||
void SplashScreen::ConnectWallet(CWallet* wallet) | |||
{ | |||
wallet->ShowProgress.connect(boost::bind(ShowProgress, this, _1, _2, false)); | |||
wallet->ShowProgress.connect(boost::bind(ShowProgress, this, _1, _2, false, _4)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You added both resume_possible
and cancel
arguments to the ShowProgress signal in wallet.h so this should be _3, _4
, instead of false, _4
.
(Another alternative if you want to ensure wallet resume_possible value is always false, is to not add resume_possible in wallet.h, and then change this to false, _3
. But adding the argument and then silently ignoring like this does now isn't good.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually changing this seems to be causing a segfault, so I will leave it as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following diff appears to be working for me. Could you help me find the steps to reproduce the segfault? Otherwise, I think this is a sensible suggestion.
--- a/src/qt/splashscreen.cpp
+++ b/src/qt/splashscreen.cpp
@@ -181,7 +181,7 @@ static void ShowProgress(SplashScreen *splash, const std::string &title, int nPr
#ifdef ENABLE_WALLET
void SplashScreen::ConnectWallet(CWallet* wallet)
{
- wallet->ShowProgress.connect(boost::bind(ShowProgress, this, _1, _2, false, _4));
+ wallet->ShowProgress.connect(boost::bind(ShowProgress, this, _1, _2, _3, _4));
connectedWallets.push_back(wallet);
}
#endif
@@ -203,7 +203,7 @@ void SplashScreen::unsubscribeFromCoreSignals()
uiInterface.ShowProgress.disconnect(boost::bind(ShowProgress, this, _1, _2, _3, _4));
#ifdef ENABLE_WALLET
for (CWallet* const & pwallet : connectedWallets) {
- pwallet->ShowProgress.disconnect(boost::bind(ShowProgress, this, _1, _2, false, _4));
+ pwallet->ShowProgress.disconnect(boost::bind(ShowProgress, this, _1, _2, _3, _4));
}
#endif
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that diff works for me. I don't remember what I did that caused a segfault. Added this change.
utACK 4657e40. Have you thought adding tests for the new RPC errors? (might be impossible?) |
Reminder: this PR currently causes a deadlock (at least on OSX): |
@jonasschnelli, I don't see how this could be causing any deadlock that wasn't occurring before. The cancel callback isn't acquiring any locks, just updating an atomic<bool>. Are you sure there is a problem in the current version of this PR? |
@ryanofsky I think it is more that a deadlock currently exists in the rescanning code which this PR makes more obvious and easier to hit. |
350d235
to
70550fd
Compare
I know that's the claim, but I don't understand what the deadlock is. What does the rescan code have to do with transactiontablemodel and why would shorter aborted scans cause more UI problems than longer full scans? |
79ff4a5
to
449dd52
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 449dd52b417d2a49b4de4440b391f6ebccb093b4. Same as before, just gets rid of spurious whitespace changes and void(void)
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments, will test later.
@@ -172,7 +172,13 @@ UniValue importprivkey(const JSONRPCRequest& request) | |||
} | |||
} | |||
if (fRescan) { | |||
pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); | |||
int64_t scanned_time = pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is repeated 4 times, maybe it's worth moving it to a function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "Give an error when rescan is aborted by the user"
This code is repeated 4 times, maybe it's worth moving it to a function?
Could be nice, though to be honest a lot of other code is duplicated between the import* RPCs too. So effort might be better spent deduping larger chunks or deprecating older RPCs in favor of importmulti.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that should be part of a separate refactor for deduplicating all of the import* RPCs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
src/interfaces/wallet.h
Outdated
@@ -247,6 +247,9 @@ class Wallet | |||
//! Register handler for watchonly changed messages. | |||
using WatchOnlyChangedFn = std::function<void(bool have_watch_only)>; | |||
virtual std::unique_ptr<Handler> handleWatchOnlyChanged(WatchOnlyChangedFn fn) = 0; | |||
|
|||
//! Abort a rescan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would suggest renaming this method abortRescan
and moving it above backupWallet
(both here and in wallet.cpp
). These interfaces are organized with regular methods on top and callback methods below. Also roughly with whole-wallet operations above operations on individual addresses and transactions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (moved and added period).
src/qt/walletmodel.cpp
Outdated
|
||
void WalletModel::AbortRescan() const | ||
{ | ||
wallet().AbortRescan(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, m_wallet->AbortRescan()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "Add cancel button to rescan progress dialog"
Would suggest dropping this method and just calling model.wallet().abortRescan() directly. I don't think there's a benefit to wrapping individual Wallet
calls inside WalletModel
methods unless they're doing extra work like synchronization or caching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed and called directly from walletview
src/qt/walletmodel.h
Outdated
@@ -204,6 +204,8 @@ class WalletModel : public QObject | |||
QString getWalletName() const; | |||
|
|||
bool isMultiwallet(); | |||
|
|||
void AbortRescan() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, void abortRescan() const
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
I agree with @ryanofsky here. I'll try to push #11826 forward. Not that this shouldn't go forward. |
Could finally test this. Tested ACK bc2003cdf7102e03710dbc19e9c397b92e124fb4. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK bc2003cdf7102e03710dbc19e9c397b92e124fb4. This PR has changed quite a lot since it was opened but is very simple at this point and provides useful functionality.
src/interfaces/wallet.h
Outdated
@@ -247,6 +247,9 @@ class Wallet | |||
//! Register handler for watchonly changed messages. | |||
using WatchOnlyChangedFn = std::function<void(bool have_watch_only)>; | |||
virtual std::unique_ptr<Handler> handleWatchOnlyChanged(WatchOnlyChangedFn fn) = 0; | |||
|
|||
//! Abort a rescan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would suggest renaming this method abortRescan
and moving it above backupWallet
(both here and in wallet.cpp
). These interfaces are organized with regular methods on top and callback methods below. Also roughly with whole-wallet operations above operations on individual addresses and transactions.
src/qt/walletmodel.cpp
Outdated
|
||
void WalletModel::AbortRescan() const | ||
{ | ||
wallet().AbortRescan(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "Add cancel button to rescan progress dialog"
Would suggest dropping this method and just calling model.wallet().abortRescan() directly. I don't think there's a benefit to wrapping individual Wallet
calls inside WalletModel
methods unless they're doing extra work like synchronization or caching.
@@ -172,7 +172,13 @@ UniValue importprivkey(const JSONRPCRequest& request) | |||
} | |||
} | |||
if (fRescan) { | |||
pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); | |||
int64_t scanned_time = pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "Give an error when rescan is aborted by the user"
This code is repeated 4 times, maybe it's worth moving it to a function?
Could be nice, though to be honest a lot of other code is duplicated between the import* RPCs too. So effort might be better spent deduping larger chunks or deprecating older RPCs in favor of importmulti.
071710e
to
66ee39f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 66ee39f227b28a32c6c1cb6cd68c0eee0cc8637f
src/wallet/rpcdump.cpp
Outdated
@@ -534,9 +534,9 @@ UniValue importwallet(const JSONRPCRequest& request) | |||
int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg()); | |||
file.seekg(0, file.beg); | |||
|
|||
pwallet->ShowProgress(_("Importing..."), 0); // show progress dialog in GUI | |||
uiInterface.ShowProgress(_("Importing..."), 0, false); // show progress dialog in GUI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "Add cancel button to rescan progress dialog"
These changes from wallet to global ShowProgress
calls seem unrelated. Are they intentional? It might make sense to change these if removing the wallet ShowProgress
signal, but since that is still used in CWallet::ScanForWalletTransactions
, this change just makes these calls inconsistent.
If this change is necessary it would be good to have a comment saying why CWallet::ShowProgress is not used here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because CWallet::ShowProgress
has a cancel button now which is directly tied to AbortRescan
(because it isn't used for anything except showing rescan progress), I needed a different progress bar which does not have a cancel button. This change makes that ShowProgress
use BitcoinGUI::ShowProgress
(which is tied toe uiInterface
)which does not have a cancel button (and is also not used anywhere else AFAICT). This progress bar indicates the import progress, not rescan progress which is why a different ShowProgress
is needed
I added a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK deaffd812e3cdaeb7ec978c2d86f4bf688add96f, thanks for the comment!
Adds a cancel button to the rescan progress dialog. When it is clicked, AbortRescan is called to abort a rescan
deaffd8
to
ae1d2b0
Compare
Squashed the squashme. Is this ready for merging? |
utACK ae1d2b0. Going to test this later... |
ae1d2b0 Give an error when rescan is aborted by the user (Andrew Chow) 69b01e6 Add cancel button to rescan progress dialog (Andrew Chow) Pull request description: A cancel button is added to the `showProgress` dialog that is used only for rescans. When clicked, `AbortRescan` is called directly to cancel the rescan. Rescans triggered from the debug console will now be cancelable by clicking the cancel button. Rescans triggered by a command (e.g. `importmulti`) will now give an error indicating that the rescan was aborted by the user (either by the `abortrescan` command or by clicking cancel). Tree-SHA512: 4bb14998766de686e2318fbc9805758eccf5dbe628a7257d072c9ae2fb4f61303a0876f49988d6e5eddb261969b8a307c81c0c2df0a42ae909a43d738af3dc1b
Summary: ae1d2b0 Give an error when rescan is aborted by the user (Andrew Chow) 69b01e6 Add cancel button to rescan progress dialog (Andrew Chow) Pull request description: A cancel button is added to the `showProgress` dialog that is used only for rescans. When clicked, `AbortRescan` is called directly to cancel the rescan. Rescans triggered from the debug console will now be cancelable by clicking the cancel button. Rescans triggered by a command (e.g. `importmulti`) will now give an error indicating that the rescan was aborted by the user (either by the `abortrescan` command or by clicking cancel). Tree-SHA512: 4bb14998766de686e2318fbc9805758eccf5dbe628a7257d072c9ae2fb4f61303a0876f49988d6e5eddb261969b8a307c81c0c2df0a42ae909a43d738af3dc1b Backport of Core PR11200 bitcoin/bitcoin#11200 Test Plan: make check ./bitcoin-qt -> help -> debug -> console -> rescanblockchain -> select cancel in the new window. Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D3891
…scanFromTime 7f81250 Mention that other RPC calls report keys as "imported" while txns are still missing (Jonas Schnelli) ccd8ef6 Reduce cs_main lock in ReadBlockFromDisk, only read GetBlockPos under the lock (Jonas Schnelli) bc356b4 Make sure WalletRescanReserver has successfully reserved the rescan (Jonas Schnelli) dbf8556 Add RAII wallet rescan reserver (Jonas Schnelli) 8d0b610 Avoid pemanent cs_main/cs_wallet lock during wallet rescans (Jonas Schnelli) Pull request description: Right now, we are holding `cs_main`/`cs_wallet` during the whole rescan process (which can take a couple of hours). This was probably only done because of laziness and it is an important show-stopper for bitcoin#11200 (GUI rescan abort). Tree-SHA512: 0fc3f82d0ee9b2f013e6bacba8d59f7334306660cd676cd64c47bb305c4cb7c7a36219d6a6f76023b74e5fe87f3ab9fc7fd2439e939f71aef653fddb0a1e23b1 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/wallet/rpcdump.cpp # src/wallet/test/wallet_tests.cpp # src/wallet/wallet.h
…escanFromTime (#3411) * Merge bitcoin#11281: Avoid permanent cs_main/cs_wallet lock during RescanFromTime 7f81250 Mention that other RPC calls report keys as "imported" while txns are still missing (Jonas Schnelli) ccd8ef6 Reduce cs_main lock in ReadBlockFromDisk, only read GetBlockPos under the lock (Jonas Schnelli) bc356b4 Make sure WalletRescanReserver has successfully reserved the rescan (Jonas Schnelli) dbf8556 Add RAII wallet rescan reserver (Jonas Schnelli) 8d0b610 Avoid pemanent cs_main/cs_wallet lock during wallet rescans (Jonas Schnelli) Pull request description: Right now, we are holding `cs_main`/`cs_wallet` during the whole rescan process (which can take a couple of hours). This was probably only done because of laziness and it is an important show-stopper for bitcoin#11200 (GUI rescan abort). Tree-SHA512: 0fc3f82d0ee9b2f013e6bacba8d59f7334306660cd676cd64c47bb305c4cb7c7a36219d6a6f76023b74e5fe87f3ab9fc7fd2439e939f71aef653fddb0a1e23b1 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/wallet/rpcdump.cpp # src/wallet/test/wallet_tests.cpp # src/wallet/wallet.h * remove "LearnAllRelatedScripts" Signed-off-by: Pasta <pasta@dashboost.org> * fix importelectrumwallet Signed-off-by: Pasta <pasta@dashboost.org> Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
ae1d2b0 Give an error when rescan is aborted by the user (Andrew Chow) 69b01e6 Add cancel button to rescan progress dialog (Andrew Chow) Pull request description: A cancel button is added to the `showProgress` dialog that is used only for rescans. When clicked, `AbortRescan` is called directly to cancel the rescan. Rescans triggered from the debug console will now be cancelable by clicking the cancel button. Rescans triggered by a command (e.g. `importmulti`) will now give an error indicating that the rescan was aborted by the user (either by the `abortrescan` command or by clicking cancel). Tree-SHA512: 4bb14998766de686e2318fbc9805758eccf5dbe628a7257d072c9ae2fb4f61303a0876f49988d6e5eddb261969b8a307c81c0c2df0a42ae909a43d738af3dc1b
ae1d2b0 Give an error when rescan is aborted by the user (Andrew Chow) 69b01e6 Add cancel button to rescan progress dialog (Andrew Chow) Pull request description: A cancel button is added to the `showProgress` dialog that is used only for rescans. When clicked, `AbortRescan` is called directly to cancel the rescan. Rescans triggered from the debug console will now be cancelable by clicking the cancel button. Rescans triggered by a command (e.g. `importmulti`) will now give an error indicating that the rescan was aborted by the user (either by the `abortrescan` command or by clicking cancel). Tree-SHA512: 4bb14998766de686e2318fbc9805758eccf5dbe628a7257d072c9ae2fb4f61303a0876f49988d6e5eddb261969b8a307c81c0c2df0a42ae909a43d738af3dc1b
ae1d2b0 Give an error when rescan is aborted by the user (Andrew Chow) 69b01e6 Add cancel button to rescan progress dialog (Andrew Chow) Pull request description: A cancel button is added to the `showProgress` dialog that is used only for rescans. When clicked, `AbortRescan` is called directly to cancel the rescan. Rescans triggered from the debug console will now be cancelable by clicking the cancel button. Rescans triggered by a command (e.g. `importmulti`) will now give an error indicating that the rescan was aborted by the user (either by the `abortrescan` command or by clicking cancel). Tree-SHA512: 4bb14998766de686e2318fbc9805758eccf5dbe628a7257d072c9ae2fb4f61303a0876f49988d6e5eddb261969b8a307c81c0c2df0a42ae909a43d738af3dc1b
…escanFromTime (dashpay#3411) * Merge bitcoin#11281: Avoid permanent cs_main/cs_wallet lock during RescanFromTime 7f81250 Mention that other RPC calls report keys as "imported" while txns are still missing (Jonas Schnelli) ccd8ef6 Reduce cs_main lock in ReadBlockFromDisk, only read GetBlockPos under the lock (Jonas Schnelli) bc356b4 Make sure WalletRescanReserver has successfully reserved the rescan (Jonas Schnelli) dbf8556 Add RAII wallet rescan reserver (Jonas Schnelli) 8d0b610 Avoid pemanent cs_main/cs_wallet lock during wallet rescans (Jonas Schnelli) Pull request description: Right now, we are holding `cs_main`/`cs_wallet` during the whole rescan process (which can take a couple of hours). This was probably only done because of laziness and it is an important show-stopper for bitcoin#11200 (GUI rescan abort). Tree-SHA512: 0fc3f82d0ee9b2f013e6bacba8d59f7334306660cd676cd64c47bb305c4cb7c7a36219d6a6f76023b74e5fe87f3ab9fc7fd2439e939f71aef653fddb0a1e23b1 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/wallet/rpcdump.cpp # src/wallet/test/wallet_tests.cpp # src/wallet/wallet.h * remove "LearnAllRelatedScripts" Signed-off-by: Pasta <pasta@dashboost.org> * fix importelectrumwallet Signed-off-by: Pasta <pasta@dashboost.org> Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
A cancel button is added to the
showProgress
dialog that is used only for rescans. When clicked,AbortRescan
is called directly to cancel the rescan.Rescans triggered from the debug console will now be cancelable by clicking the cancel button.
Rescans triggered by a command (e.g.
importmulti
) will now give an error indicating that the rescan was aborted by the user (either by theabortrescan
command or by clicking cancel).