wallet: Avoid use of Chain::Lock in importmulti

ryanofsky · ryanofsky · commit bc96a9bfc61a · 2020-03-31T08:36:02.000-05:00
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change only affects behavior in the case where wallet last block processed
falls behind the chain tip, in which case it may use a more accurate rescan
time.
diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
@@ -87,13 +87,6 @@ class LockImpl : public Chain::Lock, public UniqueLock<RecursiveMutex>
         assert(block != nullptr);
         return block->GetBlockTime();
     }
-    int64_t getBlockMedianTimePast(int height) override
-    {
-        LockAssertion lock(::cs_main);
-        CBlockIndex* block = ::ChainActive()[height];
-        assert(block != nullptr);
-        return block->GetMedianTimePast();
-    }
     bool haveBlockOnDisk(int height) override
     {
         LockAssertion lock(::cs_main);
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
@@ -103,10 +103,6 @@ class Chain
         //! Get block time. Height must be valid or this function will abort.
         virtual int64_t getBlockTime(int height) = 0;
 
-        //! Get block median time past. Height must be valid or this function
-        //! will abort.
-        virtual int64_t getBlockMedianTimePast(int height) = 0;
-
         //! Check that the block is available on disk (i.e. has not been
         //! pruned), and contains transactions.
         virtual bool haveBlockOnDisk(int height) = 0;
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
@@ -1379,20 +1379,13 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
         EnsureWalletIsUnlocked(pwallet);
 
         // Verify all timestamps are present before importing any keys.
-        const Optional<int> tip_height = locked_chain->getHeight();
-        now = tip_height ? locked_chain->getBlockMedianTimePast(*tip_height) : 0;
+        CHECK_NONFATAL(pwallet->chain().findBlock(pwallet->GetLastBlockHash(), FoundBlock().time(nLowestTimestamp).mtpTime(now)));
         for (const UniValue& data : requests.getValues()) {
             GetImportTimestamp(data, now);
         }
 
         const int64_t minimumTimestamp = 1;
 
-        if (fRescan && tip_height) {
-            nLowestTimestamp = locked_chain->getBlockTime(*tip_height);
-        } else {
-            fRescan = false;
-        }
-
         for (const UniValue& data : requests.getValues()) {
             const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp);
             const UniValue result = ProcessImport(pwallet, data, timestamp);
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
@@ -152,6 +152,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
     {
         std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
         wallet->SetupLegacyScriptPubKeyMan();
+        WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()));
         AddWallet(wallet);
         UniValue keys;
         keys.setArray();

Original file line number	Diff line number	Diff line change
`@@ -87,13 +87,6 @@ class LockImpl : public Chain::Lock, public UniqueLock<RecursiveMutex>`
`87`	`87`	`assert(block != nullptr);`
`88`	`88`	`return block->GetBlockTime();`
`89`	`89`	`}`
`90`		`- int64_t getBlockMedianTimePast(int height) override`
`91`		`- {`
`92`		`- LockAssertion lock(::cs_main);`
`93`		`- CBlockIndex* block = ::ChainActive()[height];`
`94`		`- assert(block != nullptr);`
`95`		`- return block->GetMedianTimePast();`
`96`		`- }`
`97`	`90`	`bool haveBlockOnDisk(int height) override`
`98`	`91`	`{`
`99`	`92`	`LockAssertion lock(::cs_main);`
Original file line number	Diff line number	Diff line change
`@@ -152,6 +152,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)`
`152`	`152`	`{`
`153`	`153`	`std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());`
`154`	`154`	`wallet->SetupLegacyScriptPubKeyMan();`
	`155`	`+ WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()));`
`155`	`156`	`AddWallet(wallet);`
`156`	`157`	`UniValue keys;`
`157`	`158`	`keys.setArray();`