Interrupt rescan on shutdown request #12507

promag · 2018-02-22T01:01:33Z

Here are the steps to test the feature:

start bitcoind, generate a couple of transactions and then stop:

bitcoind -regtest -printtoconsole
bitcoin-cli -regtest generate 100

apply the following patch

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 2478d67ce..8f8cea40c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1671,6 +1671,7 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock
         }
         while (pindex && !fAbortRescan && !ShutdownRequested())
         {
+            MilliSleep(500);
             if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0) {
                 double gvp = 0;
                 {

start bitcoind with rescan flag, interrupt with CTRL+C and the output should look like:

bitcoind -regtest -printtoconsole -rescan
...
^C2018-02-22 01:00:55 AddToWallet e8bfb4501b630ad2acb91e88ab0112a779766536d2c564b04faae45ae90e18f7
2018-02-22 01:00:55 Rescan interrupted by shutdown request at block 5. Progress=1.000000
2018-02-22 01:00:55  rescan                 1774ms
2018-02-22 01:00:55 setKeyPool.size() = 1995
2018-02-22 01:00:55 mapWallet.size() = 10145
2018-02-22 01:00:55 mapAddressBook.size() = 3
2018-02-22 01:00:55 Shutdown: In progress...
2018-02-22 01:00:55 scheduler thread interrupt
2018-02-22 01:00:55 Shutdown: done

jonasschnelli · 2018-02-25T00:43:50Z

Seems not urgently important, but maybe worth to do.
My only concern is to include init.h in wallet.cpp (since the long term goal is to make the wallet more independent from the sublayers).

promag · 2018-02-25T11:25:00Z

I thought the concern was having the init depending on the wallet.

promag · 2018-02-25T11:30:14Z

Rebased after #12287.

promag · 2018-02-25T23:22:11Z

I wonder if the rescan should resume in the next run? The problem is the same if abortrescan is called - keys are stored but transactions can be missed.

promag · 2018-03-07T21:18:54Z

I guess it fixes #12596.

eklitzke

utACK 3c647844d960c80f45f0359bcb311638f4a733b2

maflcko · 2018-03-12T16:15:04Z

Am I reading this correctly, that the rescan would be aborted instead of paused (and resumed on the next run), similar to reindex? This might be confusing to users.

promag · 2018-03-12T16:23:40Z

@MarcoFalke right, but if abortrescan is called, the rescan is also not resumed. Also, not interrupting might lead the user to force kill which is not desirable.

I can implement the "resume rescan" option here, just waiting for more feedback.

sipa · 2018-03-17T00:01:05Z

@MarcoFalke In what way do you think this is not expected behavior?

maflcko · 2018-03-17T15:50:50Z

Since rescanning might take several hours, some users might prefer to shut down the node temporarily and then continue the rescan process whenever they start the node again. (Without having to manually specify rescanblockchain with a block height.)

Valid use cases are probably

(1) abort rescan, (2) shut down (3) restart and don't resume rescan
(1) shut down, (2) restart and resume rescan

maflcko · 2018-03-17T16:19:19Z

Since the rescan in the gui is specifically advertised to be non-resumable, the following code can be used to fix the gui:

--- a/src/qt/splashscreen.cpp
+++ b/src/qt/splashscreen.cpp
@@ -225,8 +225,16 @@ void SplashScreen::paintEvent(QPaintEvent *event)
     painter.drawText(r, curAlignment, curMessage);
 }
 
-void SplashScreen::closeEvent(QCloseEvent *event)
+void SplashScreen::closeEvent(QCloseEvent* event)
 {
+#ifdef ENABLE_WALLET
+    for (CWallet* wallet : connectedWallets) {
+        if (wallet->IsScanning()) {
+            // We don't resume the ongoing rescan
+            wallet->AbortRescan();
+        }
+    }
+#endif
     StartShutdown(); // allows an "emergency" shutdown during startup
     event->ignore();
 }

I am not sure how to abort an ongoing rescan in bitcoind on startup, since the rpc will be started later.

eklitzke · 2018-03-19T07:24:03Z

I think the issue about interrupted re-scans should be tracked in a separate issue elsewhere. That's something I'm interested in working on at some point, i.e. in general if you shut down you node in the middle of a rescan it should make a best-effort to restart at the same point it was interrupted at.

jonasschnelli · 2018-04-15T18:05:12Z

Needs rebase
utACK 3c647844d960c80f45f0359bcb311638f4a733b2
Agree with @eklitzke that interruptible rescans could be next and can be independent from this PR.

promag · 2018-04-27T09:02:15Z

Rebased.

c4fda76 wallet: Interrupt rescan on shutdown request (João Barbosa) Pull request description: Fixes #10987. Here are the steps to test the feature: 1. start bitcoind, generate a couple of transactions and then stop: ``` bitcoind -regtest -printtoconsole bitcoin-cli -regtest generate 100 ``` 2. apply the following patch ```diff diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2478d67ce..8f8cea40c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1671,6 +1671,7 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock } while (pindex && !fAbortRescan && !ShutdownRequested()) { + MilliSleep(500); if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0) { double gvp = 0; { ``` 3. start bitcoind with rescan flag, interrupt with CTRL+C and the output should look like: ``` bitcoind -regtest -printtoconsole -rescan ... ^C2018-02-22 01:00:55 AddToWallet e8bfb4501b630ad2acb91e88ab0112a779766536d2c564b04faae45ae90e18f7 2018-02-22 01:00:55 Rescan interrupted by shutdown request at block 5. Progress=1.000000 2018-02-22 01:00:55 rescan 1774ms 2018-02-22 01:00:55 setKeyPool.size() = 1995 2018-02-22 01:00:55 mapWallet.size() = 10145 2018-02-22 01:00:55 mapAddressBook.size() = 3 2018-02-22 01:00:55 Shutdown: In progress... 2018-02-22 01:00:55 scheduler thread interrupt 2018-02-22 01:00:55 Shutdown: done ``` Tree-SHA512: f9bebe2cdacf0359b6cbfcbc48ac2818a3ae7aa7822ff0c2c0de4ca2fff7c88493380b74a1c5ff2ce1de01fe605b0e5ef3576f124ea9cff8ef25a9e762477b92

maflcko · 2018-05-04T14:47:35Z

src/wallet/wallet.cpp

@@ -1794,6 +1795,8 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock
        }
        if (pindex && fAbortRescan) {
            LogPrintf("Rescan aborted at block %d. Progress=%f\n", pindex->nHeight, gvp);
+        } else if (pindex && ShutdownRequested()) {
+            LogPrintf("Rescan interrupted by shutdown request at block %d. Progress=%f\n", pindex->nHeight, gvp);


Doesn't interrupt mean it will continue later on? I think this should say "aborted".

Summary: c4fda76 wallet: Interrupt rescan on shutdown request (João Barbosa) Pull request description: Fixes #10987. Here are the steps to test the feature: 1. start bitcoind, generate a couple of transactions and then stop: ``` bitcoind -regtest -printtoconsole bitcoin-cli -regtest generate 100 ``` 2. apply the following patch ```diff diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2478d67ce..8f8cea40c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1671,6 +1671,7 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock } while (pindex && !fAbortRescan && !ShutdownRequested()) { + MilliSleep(500); if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0) { double gvp = 0; { ``` 3. start bitcoind with rescan flag, interrupt with CTRL+C and the output should look like: ``` bitcoind -regtest -printtoconsole -rescan ... ^C2018-02-22 01:00:55 AddToWallet e8bfb4501b630ad2acb91e88ab0112a779766536d2c564b04faae45ae90e18f7 2018-02-22 01:00:55 Rescan interrupted by shutdown request at block 5. Progress=1.000000 2018-02-22 01:00:55 rescan 1774ms 2018-02-22 01:00:55 setKeyPool.size() = 1995 2018-02-22 01:00:55 mapWallet.size() = 10145 2018-02-22 01:00:55 mapAddressBook.size() = 3 2018-02-22 01:00:55 Shutdown: In progress... 2018-02-22 01:00:55 scheduler thread interrupt 2018-02-22 01:00:55 Shutdown: done ``` Tree-SHA512: f9bebe2cdacf0359b6cbfcbc48ac2818a3ae7aa7822ff0c2c0de4ca2fff7c88493380b74a1c5ff2ce1de01fe605b0e5ef3576f124ea9cff8ef25a9e762477b92 Backport of Core PR12507 bitcoin/bitcoin#12507 Test Plan: make check ./bitcoind -printtoconsole -rescan CTRL+C Verify console output Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: deadalnix, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D3975

c4fda76 wallet: Interrupt rescan on shutdown request (João Barbosa) Pull request description: Fixes bitcoin#10987. Here are the steps to test the feature: 1. start bitcoind, generate a couple of transactions and then stop: ``` bitcoind -regtest -printtoconsole bitcoin-cli -regtest generate 100 ``` 2. apply the following patch ```diff diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2478d67ce..8f8cea40c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1671,6 +1671,7 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock } while (pindex && !fAbortRescan && !ShutdownRequested()) { + MilliSleep(500); if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0) { double gvp = 0; { ``` 3. start bitcoind with rescan flag, interrupt with CTRL+C and the output should look like: ``` bitcoind -regtest -printtoconsole -rescan ... ^C2018-02-22 01:00:55 AddToWallet e8bfb4501b630ad2acb91e88ab0112a779766536d2c564b04faae45ae90e18f7 2018-02-22 01:00:55 Rescan interrupted by shutdown request at block 5. Progress=1.000000 2018-02-22 01:00:55 rescan 1774ms 2018-02-22 01:00:55 setKeyPool.size() = 1995 2018-02-22 01:00:55 mapWallet.size() = 10145 2018-02-22 01:00:55 mapAddressBook.size() = 3 2018-02-22 01:00:55 Shutdown: In progress... 2018-02-22 01:00:55 scheduler thread interrupt 2018-02-22 01:00:55 Shutdown: done ``` Tree-SHA512: f9bebe2cdacf0359b6cbfcbc48ac2818a3ae7aa7822ff0c2c0de4ca2fff7c88493380b74a1c5ff2ce1de01fe605b0e5ef3576f124ea9cff8ef25a9e762477b92

fanquake added the Wallet label Feb 22, 2018

promag force-pushed the 2018-02-shutdown-on-rescan branch from 6d534b1 to 3c64784 Compare February 25, 2018 11:28

aikkyu35 approved these changes Feb 25, 2018

View reviewed changes

eklitzke approved these changes Mar 11, 2018

View reviewed changes

promag force-pushed the 2018-02-shutdown-on-rescan branch from 3c64784 to 79ca4b0 Compare April 27, 2018 09:01

wallet: Interrupt rescan on shutdown request

c4fda76

promag force-pushed the 2018-02-shutdown-on-rescan branch from 79ca4b0 to c4fda76 Compare May 2, 2018 10:58

laanwj merged commit c4fda76 into bitcoin:master May 3, 2018

ryanofsky mentioned this pull request May 3, 2018

Refactor: separate wallet from node #10973

Merged

Empact mentioned this pull request May 3, 2018

Fix ScanForWalletTransactions to return an enum indicating scan result: success / failure / user_abort #13076

Merged

Bushstar mentioned this pull request May 4, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#320

Merged

maflcko reviewed May 4, 2018

View reviewed changes

promag deleted the 2018-02-shutdown-on-rescan branch May 23, 2018 21:10

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Interrupt rescan on shutdown request #12507

Interrupt rescan on shutdown request #12507

Uh oh!

promag commented Feb 22, 2018

Uh oh!

jonasschnelli commented Feb 25, 2018

Uh oh!

promag commented Feb 25, 2018

Uh oh!

promag commented Feb 25, 2018

Uh oh!

promag commented Feb 25, 2018

Uh oh!

promag commented Mar 7, 2018

Uh oh!

eklitzke left a comment

Uh oh!

maflcko commented Mar 12, 2018

Uh oh!

promag commented Mar 12, 2018

Uh oh!

sipa commented Mar 17, 2018

Uh oh!

maflcko commented Mar 17, 2018

Uh oh!

maflcko commented Mar 17, 2018

Uh oh!

eklitzke commented Mar 19, 2018

Uh oh!

jonasschnelli commented Apr 15, 2018

Uh oh!

promag commented Apr 27, 2018

Uh oh!

maflcko May 4, 2018

Uh oh!

Uh oh!

Interrupt rescan on shutdown request #12507

Interrupt rescan on shutdown request #12507

Uh oh!

Conversation

promag commented Feb 22, 2018

Uh oh!

jonasschnelli commented Feb 25, 2018

Uh oh!

promag commented Feb 25, 2018

Uh oh!

promag commented Feb 25, 2018

Uh oh!

promag commented Feb 25, 2018

Uh oh!

promag commented Mar 7, 2018

Uh oh!

eklitzke left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko commented Mar 12, 2018

Uh oh!

promag commented Mar 12, 2018

Uh oh!

sipa commented Mar 17, 2018

Uh oh!

maflcko commented Mar 17, 2018

Uh oh!

maflcko commented Mar 17, 2018

Uh oh!

eklitzke commented Mar 19, 2018

Uh oh!

jonasschnelli commented Apr 15, 2018

Uh oh!

promag commented Apr 27, 2018

Uh oh!

maflcko May 4, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!