rpc: Early call once CWallet::MarkDirty in import calls #14303

pull promag wants to merge 2 commits into bitcoin:master from promag:2018-09-wallet-mark-dirty changing 3 files +16 −20

promag commented at 11:22 PM on September 23, 2018: member

This gives a small, but not relevant, performance improvement when calling importmulti RPC since MarkDirty is called only once.

Note that ImportScript is not atomic: https://github.com/bitcoin/bitcoin/blob/920c090f63f4990bf0f3b3d1a6d3d8a8bcd14ba0/src/wallet/rpcdump.cpp#L227-L242 ie something can fail after a successful AddWatchOnly, which could lead to invalid cached balances. That's why the MarkDirty call is done as early as possible.
promag cross-referenced this on Sep 23, 2018 from issue Import pubkeys when importing p2sh with importmulti by achow101
fanquake added the label Wallet on Sep 23, 2018
fanquake added the label RPC/REST/ZMQ on Sep 23, 2018
DrahtBot commented at 12:08 AM on September 24, 2018: contributor
Reviewers, this pull request conflicts with the following ones:
- #14565 (Overhaul importmulti logic by sipa)
- #14454 (Add SegWit support to importmulti by MeshCollider)
- #13787 (Test for Windows encoding issue by ken2812221)
- #9381 (Remove CWalletTx merging logic from AddToWallet by ryanofsky)
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.
promag force-pushed on Sep 24, 2018
DrahtBot cross-referenced this on Sep 24, 2018 from issue Remove CWalletTx merging logic from AddToWallet by ryanofsky
DrahtBot cross-referenced this on Sep 24, 2018 from issue utils: Add fstream wrapper to allow to pass unicode filename on Windows by ken2812221
DrahtBot cross-referenced this on Sep 24, 2018 from issue Test for Windows encoding issue by ken2812221
DrahtBot commented at 5:17 AM on September 28, 2018: contributor



Coverage Change (pull 14303) Reference (master)

Lines +0.0339 % 87.0361 %

Functions +0.1390 % 84.1130 %

Branches +0.0039 % 51.5451 %
meshcollider commented at 8:52 AM on October 8, 2018: contributor

Concept ACK
achow101 commented at 1:10 AM on October 9, 2018: member

utACK 0a1bbf739890408c44dafd96f6643d86d54c2537
DrahtBot cross-referenced this on Oct 10, 2018 from issue Add SegWit support to importmulti by meshcollider
meshcollider commented at 10:51 PM on October 18, 2018: contributor

utACK https://github.com/bitcoin/bitcoin/pull/14303/commits/0a1bbf739890408c44dafd96f6643d86d54c2537 Needs rebase
meshcollider added the label Refactoring on Oct 18, 2018
DrahtBot added the label Needs rebase on Oct 20, 2018
rpc: Early call once CWallet::MarkDirty in import calls 93196eeacc
wallet: Refactor CWallet::MarkDirty to EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) 852f295ea6
promag force-pushed on Oct 20, 2018
promag commented at 10:05 AM on October 20, 2018: member

Rebased.
DrahtBot removed the label Needs rebase on Oct 20, 2018
DrahtBot cross-referenced this on Oct 24, 2018 from issue rpc: Require solvability in importmulti if importing more than the scriptPubKey by achow101
DrahtBot cross-referenced this on Oct 25, 2018 from issue Overhaul importmulti logic by sipa
DrahtBot added the label Needs rebase on Oct 31, 2018
DrahtBot commented at 5:02 PM on October 31, 2018: contributor

Needs rebase
promag closed this on Oct 31, 2018
laanwj removed the label Needs rebase on Oct 24, 2019
bitcoin locked this on Dec 16, 2021

Coverage	Change (pull 14303)	Reference (master)
Lines	+0.0339 %	87.0361 %
Functions	+0.1390 %	84.1130 %
Branches	+0.0039 %	51.5451 %

Contributors

Labels

Refactoring Wallet RPC/REST/ZMQ

Linked (view graph)

#14019 Import pubkeys when importing p2sh with importmulti