Skip to content
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

Batch write imported stuff in importmulti #15741

Merged
merged 6 commits into from May 29, 2019

Conversation

Projects
None yet
@achow101
Copy link
Member

commented Apr 3, 2019

Instead of writing each item to the wallet database individually, do them in batches so that the import runs faster.

This was tested by importing a ranged descriptor for 10,000 keys.

Current master

$ time src/bitcoin-cli -regtest -rpcwallet=importbig importmulti '[{"desc": "sh(wpkh([73111820/44h/1h/0h]tpubDDoT2SgEjaU5rerQpfcRDWPAcwyZ5g7xxHgVAfPwidgPDKVjm89d6jJ8AQotp35Np3m6VaysfUY1C2g68wFqUmraGbzhSsMF9YBuTGxpBaW/1/*))#3w7php47", "range": [0, 10000], "timestamp": "now", "internal": true, "keypool": false, "watchonly": true}]'
...

real	7m45.29s

This PR:

$ time src/bitcoin-cli -regtest -rpcwallet=importbig4 importmulti '[{"desc": "pkh([73111820/44h/1h/0h]tpubDDoT2SgEjaU5rerQpfcRDWPAcwyZ5g7xxHgVAfPwidgPDKVjm89d6jJ8AQotp35Np3m6VaysfUY1C2g68wFqUmraGbzhSsMF9YBuTGxpBaW/1/*)#v65yjgmc", "range": [0, 10000], "timestamp": "now", "internal": true, "keypool": false, "watchonly": true}]'
...

real	3.93s

Fixes #15739

@gwillen

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

utACK although it would be good to factor out the counting into a WalletWriteCache or something, that flushes itself at intervals so you don't have to do it manually.

@@ -1282,33 +1287,46 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
}
const CPubKey& pubkey = entry->second;
CPubKey temp;
if (!pwallet->GetPubKey(id, temp) && !pwallet->AddWatchOnly(GetScriptForRawPubKey(pubkey), timestamp)) {
if (!pwallet->GetPubKey(id, temp) && !pwallet->AddWatchOnlyWithDB(*batch, GetScriptForRawPubKey(pubkey), timestamp)) {

This comment has been minimized.

Copy link
@gwillen

gwillen Apr 3, 2019

Contributor

There is a weird indentation glitch here, although it looks like it was preexisting.

}
CTxDestination dest;
ExtractDestination(script, dest);
if (!internal && IsValidDestination(dest)) {
pwallet->SetAddressBook(dest, label, "receive");
}
}
batch.reset();

This comment has been minimized.

Copy link
@gwillen

gwillen Apr 3, 2019

Contributor

Is this necessary? It's about to go out of scope, right?

This comment has been minimized.

Copy link
@achow101

achow101 Apr 4, 2019

Author Member

Probably not. I did that just as a belt-and-suspenders thing.

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15876 ([rpc] signer bump fee by Sjors)
  • #15761 (Replace -upgradewallet startup option with upgradewallet RPC by achow101)
  • #15424 ([wallet] wallet-tool: command to remove key metadata by Sjors)
  • #15414 ([wallet] allow adding pubkeys from imported private keys to keypool by Sjors)
  • #14912 ([WIP] External signer support (e.g. hardware wallet) by Sjors)
  • #13756 (wallet: "avoid_reuse" wallet flag for improved privacy by kallewoof)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@Empact

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Empact@dd5b0aa CWallet::AddKeyOrigin(const CPubKey& pubkey, const KeyOriginInfo& info) and CWallet::WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, const bool overwrite) can be removed.

Empact@c5d9eec How about moving this functionality into CWallet, so that we don't operate externally on CWallet::database any more than we already do. As the GetDBHandle comments say:

bitcoin/src/wallet/wallet.h

Lines 750 to 753 in ba54342

/** Get database handle used by this wallet. Ideally this function would
* not be necessary.
*/
WalletDatabase& GetDBHandle()

Practically speaking, this also explicitly limits the scope of the batch operations on database to within the lifecycle of the CWallet instance.

Empact@b363aa7 Could also apply the batch treatment to SetAddressBook

@@ -1222,6 +1224,7 @@ class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notific

/** Add a KeyOriginInfo to the wallet */
bool AddKeyOrigin(const CPubKey& pubkey, const KeyOriginInfo& info);
bool AddKeyOriginWithDB(WalletBatch& batch, const CPubKey& pubkey, const KeyOriginInfo& info);

This comment has been minimized.

Copy link
@Empact

Empact Apr 4, 2019

Member

nit: May be a bad example to follow, but AddKeypoolPubkeyWithDB has batch as the last argument, which makes as much sense to me.

This comment has been minimized.

Copy link
@achow101

achow101 Apr 4, 2019

Author Member

I was following the AddKeyPubKeyWithDB way of argument ordering.

@Empact

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Concept ACK

@promag
Copy link
Member

left a comment

Concept ACK, nice improvement.

throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
}
pwallet->UpdateTimeFirstKey(timestamp);
if (++cnt % 1000 == 0) {

This comment has been minimized.

Copy link
@promag

promag Apr 4, 2019

Member

What is the problem of one batch only?

This comment has been minimized.

Copy link
@achow101

achow101 Apr 4, 2019

Author Member

The batch can get very large in memory if lots of keys are involved, so we limit the size to avoid blowing up memory during imports.

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

See also:

  • Use a single wallet batch for UpgradeKeyMetadata #15433

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from 508c486 to 7455c9d Apr 4, 2019

@achow101

This comment has been minimized.

Copy link
Member Author

commented Apr 4, 2019

I've pulled in @Empact's changes (Empact/bitcoin@dd5b0aa was squashed into 59ec412, cherry-picked the other two commits).

@gwillen

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

Especially if there are now multiple places we're manually batching writes like this, and more where we're considering it: Let me suggest again and more strongly that the batch object should be handling autoflushing after every X calls, instead of open-coding it in the caller like this. If it's appropriate for every caller, it could go into WalletBatch, else into a new class that's an extremely thin wrapper around it. I bet the total number of lines added in this PR would actually go down.

@Sjors

This comment has been minimized.

Copy link
Member

commented Apr 5, 2019

Concept ACK. I tried the first two commits (so without address book change) with #14145 and importing 2x 1000 keys seems about 6 times faster.

@Empact

This comment has been minimized.

Copy link
Member

commented Apr 5, 2019

@gwillen Agree it should be done but would be appropriate as a follow-up

@practicalswift

This comment has been minimized.

Copy link
Member

commented Apr 5, 2019

Looks like AddKeypoolPubkey is no longer used and can be removed?

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from 7455c9d to 27fdf12 Apr 5, 2019

@achow101

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

I've implemented @gwillen's suggestion. WalletBatch will flush to disk automatically after every 1000 writes. This appears to be faster too as it does not close and reopen the database on every flush. With this change, I've also updated UpgradeKeyMetadata.

Looks like AddKeypoolPubkey is no longer used and can be removed?

Done

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from 27fdf12 to 72ec512 Apr 5, 2019

}
}
}
batch.reset(); //write before setting the flag

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Apr 5, 2019

Member

in commit ebb0ea8:

Why is this no longer needed?

This comment has been minimized.

Copy link
@achow101

achow101 Apr 5, 2019

Author Member

When batch goes out of scope, it will write. This was just a belt and suspenders.

This comment has been minimized.

Copy link
@gwillen

gwillen Apr 5, 2019

Contributor

The comment suggests that in this case -- unlike the batch write in importmulti -- there's a specific reason to force it to write before the end of the scope.

This comment has been minimized.

Copy link
@achow101

achow101 Apr 5, 2019

Author Member

reverted

@achow101

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

I'm unsure why tests are currently failing.

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from 72ec512 to f2b9b32 Apr 5, 2019

@@ -157,6 +157,9 @@ class WalletBatch
return false;
}
m_database.IncrementUpdateCounter();
if (m_database.nUpdateCounter % 1000 == 0) {

This comment has been minimized.

Copy link
@promag

promag Apr 5, 2019

Member

I get that this makes the code simple, but IMO this defeats the batch purpose. Like, it shouldn't be something to worry about here.

This comment has been minimized.

Copy link
@gwillen

gwillen Apr 5, 2019

Contributor

I don't understand what you mean by 'defeats the batch purpose'. Are there cases where the caller is depending on the batch not being flushed?

I think it is far better engineering practice to deal with periodic flushing once, inside the batch-management code, than to deal with it independently at each callsite. If there's some situation where periodic flushing is undesirable, I think it would be better to offer both versions.

This comment has been minimized.

Copy link
@promag

promag Apr 8, 2019

Member

@gwillen I mean you should be able to create a batch with 1001 changes no? I was suggesting something on top of WalletBatch to do that.

This comment has been minimized.

Copy link
@gwillen

gwillen Apr 8, 2019

Contributor

Well, you can add as many changes as you want, it just flushes after every 1000. This should be completely transparent to the user. Unless there is some reason that flushing can hurt, like the caller is using the batch as though it were an atomic transaction?

This comment has been minimized.

Copy link
@achow101

achow101 Apr 8, 2019

Author Member

Unless there is some reason that flushing can hurt, like the caller is using the batch as though it were an atomic transaction?

There is only one place where an atomic transaction is used, and if an atomic transaction is active, then Flush() will do nothing.

This comment has been minimized.

Copy link
@promag

promag Apr 14, 2019

Member

Well I may be wrong but I see the batch as an atomic transaction. Making 1500 updates and then aborting the batch should not commit 1000 updates IMO.

This comment has been minimized.

Copy link
@achow101

achow101 Apr 14, 2019

Author Member

Well I may be wrong but I see the batch as an atomic transaction. Making 1500 updates and then aborting the batch should not commit 1000 updates IMO.

Except that's not how the batch works normally. Batches are not atomic unless you explicitly make them so by starting a transaction with the underlying BerkeleyBatch. In that case, then the flushing won't do anything. In the normal case, each update in a batch itself is atomic and should be expected to be written to the wallet file. There is no aborting. As soon as a batch goes out of scope, every single update is committed to the file. If you make 1500 updates, the first 1000 will be written after you do the 1000th write operation, and then the last 500 will be written when the batch object is deleted.

This comment has been minimized.

Copy link
@Sjors

Sjors May 16, 2019

Member

So is the description of WalletBatch out of date? It says This represents a single transaction at the database. It will be committed when the object goes out of scope.

This comment has been minimized.

Copy link
@achow101

achow101 May 18, 2019

Author Member

I believe that is out of date

This comment has been minimized.

Copy link
@achow101

achow101 May 18, 2019

Author Member

I've updated the comment.

@@ -614,7 +614,9 @@ void BerkeleyBatch::Flush()
if (fReadOnly)
nMinutes = 1;

env->dbenv->txn_checkpoint(nMinutes ? gArgs.GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);
if (env) {

This comment has been minimized.

Copy link
@promag

promag Apr 5, 2019

Member

Why this change?

This comment has been minimized.

Copy link
@achow101

achow101 Apr 5, 2019

Author Member

So that tests pass. When there's a dummy database, env will be nulltpr and this causes a segfault in tests which use a dummy database.

This comment has been minimized.

Copy link
@promag

promag Apr 8, 2019

Member

Can't we make env is defined instead? Otherwise a comment would make sense as this doesn't really happen.

This comment has been minimized.

Copy link
@achow101

achow101 Apr 8, 2019

Author Member

Can't we make env is defined instead?

No. env is nullptr for dummy databases which are used in tests. A check like this is used in many other places in this file in the form of BerkeleyDatabase::IsDummy().

Otherwise a comment would make sense as this doesn't really happen.

Done.

This comment has been minimized.

Copy link
@Empact

Empact Apr 10, 2019

Member

nit: could deal with it earlier (i.e. early return), and use IsDummy to make it more explicit.

This comment has been minimized.

Copy link
@achow101

achow101 Apr 14, 2019

Author Member

IsDummy is done by BerkeleyDatabase, not BerkeleyBatch so is not available to do here (although the check is completely identical).

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from f2b9b32 to 2811f37 Apr 5, 2019

@jnewbery jnewbery added this to Blockers in High-priority for review Apr 8, 2019

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from 2811f37 to ffafc25 Apr 8, 2019

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from ffafc25 to a14a5ba Apr 9, 2019

@DrahtBot DrahtBot removed the Needs rebase label Apr 9, 2019

@promag

This comment has been minimized.

Copy link
Member

commented May 18, 2019

@achow101 looks like CWallet::UnsetWalletFlag should reuse the batch instance:
Screenshot 2019-05-18 at 10 45 19
Screenshot 2019-05-18 at 11 07 28
Please test/banchmark with 76c3906 (https://github.com/promag/bitcoin/tree/pr15741), at least here it gives a significant improvement.

@jonasschnelli

This comment has been minimized.

Copy link
Member

commented May 18, 2019

Concept ACK

@jb55

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

@promag's patch brings my import speeds down from 8 minutes to about 10 seconds for 10000 keys

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from a9ae323 to f8b0c08 May 18, 2019

@achow101

This comment has been minimized.

Copy link
Member Author

commented May 18, 2019

I've pulled in @promag's patch and updated it to match the convention we've been using for passing in a WalletBatch.

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from f8b0c08 to c6e4421 May 18, 2019

@promag

This comment has been minimized.

Copy link
Member

commented May 18, 2019

@achow101 please update times in OP.

achow101 added some commits Apr 3, 2019

Add AddWatchOnlyWithDB, AddKeyOriginWithDB, AddCScriptWithDB functions
AddWatchOnlyWithDB, AddKeyOriginWithDB, and AddCScriptWithDB add their
respective data to the wallet using the provided WalletBatch instead
of creating a new WalletBatch object every time. This allows for batching
writes to the database.
Have WalletBatch automatically flush every 1000 updates
Since it now automatically flushes, we don't need to have
UpgradeKeyMetadata count and flush separately
Batch writes for importmulti
When writing all of the imported data to the wallet, use a common
WalletBatch object so that batch writes are done and the writes
finish more quickly.

AddKeypoolPubkey is no longer needed so it is also removed

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from c6e4421 to 87f7bef May 18, 2019

@achow101

This comment has been minimized.

Copy link
Member Author

commented May 18, 2019

I noticed that importing scripts would also be slow so I've added AddCScriptWIthDB in order to apply the same batching performance boost to such imports.

Also updated OP.

@jb55

This comment has been minimized.

Copy link
Contributor

commented May 19, 2019

tACK 87f7bef sooo fast. tested a rescan with my all my trezor keys from HWI and it worked great. thanks @achow101, hw wallet + core is actually usable now (at least for watching :)!

@promag

This comment has been minimized.

Copy link
Member

commented May 19, 2019

Restarted travis, it was failing with 😕

47/121 - rpc_psbt.py failed, Duration: 5 s
stdout:
2019-05-18T17:46:43.670000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20190518_173555/rpc_psbt_71
2019-05-18T17:46:47.538000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 194, in main
    self.run_test()
  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/rpc_psbt.py", line 156, in run_test
    assert_raises_rpc_error(-22, "", self.nodes[0].converttopsbt, signedtx['hex'])
  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/util.py", line 105, in assert_raises_rpc_error
    assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
AssertionError: No exception raised
@promag

This comment has been minimized.

Copy link
Member

commented May 19, 2019

utACK 87f7bef. nit, ampersand should be before space.

@@ -1204,6 +1204,7 @@ class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notific

/** Unsets a single wallet flag */
void UnsetWalletFlag(uint64_t flag);
void UnsetWalletFlagWithDB(WalletBatch& batch, uint64_t flag);

This comment has been minimized.

Copy link
@Empact

Empact May 27, 2019

Member

nit: could be private

This comment has been minimized.

Copy link
@achow101

achow101 May 28, 2019

Author Member

Done

@@ -872,6 +883,7 @@ class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notific
//! Adds an encrypted key to the store, without saving it to disk (used by LoadWallet)
bool LoadCryptedKey(const CPubKey &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret);
bool AddCScript(const CScript& redeemScript) override;
bool AddCScriptWithDB(WalletBatch& batch, const CScript& script);

This comment has been minimized.

Copy link
@Empact

Empact May 27, 2019

Member

nit: could be private

This comment has been minimized.

Copy link
@achow101

achow101 May 28, 2019

Author Member

Done

@Empact

This comment has been minimized.

Copy link
Member

commented May 27, 2019

utACK 87f7bef

Empact and others added some commits Apr 4, 2019

Move some of ProcessImport into CWallet::Import*
This maintains encapsulation of CWallet::database in the face of
batching, e.g. allows making the `WithDB` methods private.

@achow101 achow101 force-pushed the achow101:batch-write-importmulti branch from 87f7bef to 0db94e5 May 28, 2019

@jb55

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

utACK 0db94e5

@ariard

This comment has been minimized.

Copy link
Contributor

commented May 29, 2019

Tested ACK 0db94e5

Drop import time for 10000 keys ranged descriptor from 495.57s to 3.626s with 2.8 GHz Intel Core i7.

@Empact

This comment has been minimized.

Copy link
Member

commented May 29, 2019

re-utACK 0db94e5 only change is re the privacy of UnsetWalletFlagWithDB and AddCScriptWithDB.

@meshcollider meshcollider merged commit 0db94e5 into bitcoin:master May 29, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

meshcollider added a commit that referenced this pull request May 29, 2019

Merge #15741: Batch write imported stuff in importmulti
0db94e5 wallet: Pass WalletBatch to CWallet::UnsetWalletFlag (João Barbosa)
6cb888b Apply the batch treatment to CWallet::SetAddressBook via ImportScriptPubKeys (Ben Woosley)
6154a09 Move some of ProcessImport into CWallet::Import* (Ben Woosley)
ccb26cf Batch writes for importmulti (Andrew Chow)
d6576e3 Have WalletBatch automatically flush every 1000 updates (Andrew Chow)
366fe0b Add AddWatchOnlyWithDB, AddKeyOriginWithDB, AddCScriptWithDB functions (Andrew Chow)

Pull request description:

  Instead of writing each item to the wallet database individually, do them in batches so that the import runs faster.

  This was tested by importing a ranged descriptor for 10,000 keys.

  Current master

  ```
  $ time src/bitcoin-cli -regtest -rpcwallet=importbig importmulti '[{"desc": "sh(wpkh([73111820/44h/1h/0h]tpubDDoT2SgEjaU5rerQpfcRDWPAcwyZ5g7xxHgVAfPwidgPDKVjm89d6jJ8AQotp35Np3m6VaysfUY1C2g68wFqUmraGbzhSsMF9YBuTGxpBaW/1/*))#3w7php47", "range": [0, 10000], "timestamp": "now", "internal": true, "keypool": false, "watchonly": true}]'
  ...

  real	7m45.29s
  ```

  This PR:

  ```
  $ time src/bitcoin-cli -regtest -rpcwallet=importbig4 importmulti '[{"desc": "pkh([73111820/44h/1h/0h]tpubDDoT2SgEjaU5rerQpfcRDWPAcwyZ5g7xxHgVAfPwidgPDKVjm89d6jJ8AQotp35Np3m6VaysfUY1C2g68wFqUmraGbzhSsMF9YBuTGxpBaW/1/*)#v65yjgmc", "range": [0, 10000], "timestamp": "now", "internal": true, "keypool": false, "watchonly": true}]'
  ...

  real	3.93s
  ```

  Fixes #15739

ACKs for commit 0db94e:
  jb55:
    utACK 0db94e5
  ariard:
    Tested ACK 0db94e5
  Empact:
    re-utACK 0db94e5 only change is re the privacy of `UnsetWalletFlagWithDB` and `AddCScriptWithDB`.

Tree-SHA512: 3481308a64c99b6129f7bd328113dc291fe58743464628931feaebdef0e6ec770ddd5c19e4f9fbc1249a200acb04aaf62a8d914d53b0a29ac1e557576659c0cc

@meshcollider meshcollider removed this from Blockers in High-priority for review May 29, 2019

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 30, 2019

Merge bitcoin#15741: Batch write imported stuff in importmulti
0db94e5 wallet: Pass WalletBatch to CWallet::UnsetWalletFlag (João Barbosa)
6cb888b Apply the batch treatment to CWallet::SetAddressBook via ImportScriptPubKeys (Ben Woosley)
6154a09 Move some of ProcessImport into CWallet::Import* (Ben Woosley)
ccb26cf Batch writes for importmulti (Andrew Chow)
d6576e3 Have WalletBatch automatically flush every 1000 updates (Andrew Chow)
366fe0b Add AddWatchOnlyWithDB, AddKeyOriginWithDB, AddCScriptWithDB functions (Andrew Chow)

Pull request description:

  Instead of writing each item to the wallet database individually, do them in batches so that the import runs faster.

  This was tested by importing a ranged descriptor for 10,000 keys.

  Current master

  ```
  $ time src/bitcoin-cli -regtest -rpcwallet=importbig importmulti '[{"desc": "sh(wpkh([73111820/44h/1h/0h]tpubDDoT2SgEjaU5rerQpfcRDWPAcwyZ5g7xxHgVAfPwidgPDKVjm89d6jJ8AQotp35Np3m6VaysfUY1C2g68wFqUmraGbzhSsMF9YBuTGxpBaW/1/*))#3w7php47", "range": [0, 10000], "timestamp": "now", "internal": true, "keypool": false, "watchonly": true}]'
  ...

  real	7m45.29s
  ```

  This PR:

  ```
  $ time src/bitcoin-cli -regtest -rpcwallet=importbig4 importmulti '[{"desc": "pkh([73111820/44h/1h/0h]tpubDDoT2SgEjaU5rerQpfcRDWPAcwyZ5g7xxHgVAfPwidgPDKVjm89d6jJ8AQotp35Np3m6VaysfUY1C2g68wFqUmraGbzhSsMF9YBuTGxpBaW/1/*)#v65yjgmc", "range": [0, 10000], "timestamp": "now", "internal": true, "keypool": false, "watchonly": true}]'
  ...

  real	3.93s
  ```

  Fixes bitcoin#15739

ACKs for commit 0db94e:
  jb55:
    utACK 0db94e5
  ariard:
    Tested ACK 0db94e5
  Empact:
    re-utACK bitcoin@0db94e5 only change is re the privacy of `UnsetWalletFlagWithDB` and `AddCScriptWithDB`.

Tree-SHA512: 3481308a64c99b6129f7bd328113dc291fe58743464628931feaebdef0e6ec770ddd5c19e4f9fbc1249a200acb04aaf62a8d914d53b0a29ac1e557576659c0cc

@LitecoinZ LitecoinZ referenced this pull request May 31, 2019

Open

Backports from upstream #1

44 of 244 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.