Optional update rescan option in importmulti RPC by pedrobranco · Pull Request #11484 · bitcoin/bitcoin

pedrobranco · 2017-10-11T10:33:50Z

This PR adds a new option rescanUpdate in importmulti RPC method to optionally do not rescan transactions that already exist in the wallet.

This is very important for large wallets when importing a single address with a old timestamp, which in this moment triggers a rescan to the whole wallet, replaying several already known wallet events (via notify-wallet).

promag

Maybe option should be just update? Needs a test thought.

promag · 2017-10-11T11:04:08Z

src/wallet/rpcdump.cpp

Remove : <false>, these <false> and <true> will be removed IMO.

See:

bitcoin/src/wallet/rpcwallet.cpp

Line 3060 in 364da2c

" \"replaceable\" (boolean, optional, default true) Whether the new transaction should still be\n"

promag · 2017-10-11T11:08:38Z

src/wallet/rpcdump.cpp

Remove comment.

pedrobranco · 2017-10-18T14:08:30Z

Maybe option should be just update? Needs a test thought.

Just update isn't too generic? Does not refer to a rescan property.

promag

Missing test test.

Regarding the option name, even rescanUpdate doesn't really explain the underlying consequence. Maybe onlyNotifyNew=false or notifyExisting=true.

promag · 2017-10-18T14:10:39Z

src/wallet/rpcdump.cpp

Nit, care to remove : <false> here too?

pedrobranco · 2017-10-18T14:55:53Z

Regarding the option name, even rescanUpdate doesn't really explain the underlying consequence. Maybe onlyNotifyNew=false or notifyExisting=true.

Maybe notifyKnownTransactions=true or notifyKnown=true?

Missing test test.

Yes, I will add it soon.

pedrobranco · 2018-03-12T11:25:04Z

After checking the Wallet::RescanFromTime() method, the argument which triggers the notifiers is called fUpdate, so I'm going by your approach of calling it update.

The -help of the command importmulti should be the one that should explain the underlying consequence of using update.

DrahtBot · 2018-10-20T11:16:00Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#14942 (wallet: Make scan / abort status private to CWallet by Empact)
#14918 (RPCHelpMan: Check default values are given at compile-time by MarcoFalke)

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.

maflcko · 2018-12-05T18:36:03Z

src/wallet/rpcdump.cpp

New code should be snake_case, without the prefix

Should we also rename the other variables on this method?

Just new code.

promag · 2019-01-26T16:14:56Z

This definitely needs a test.

maflcko · 2019-02-12T23:57:12Z

src/wallet/rpcdump.cpp

                    {"options", RPCArg::Type::OBJ, /* opt */ true, /* default_val */ "null", "",
                        {
                            {"rescan", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "true", "Stating if should rescan the blockchain after all imports"},
+                            {"update", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "true", "Stating if rescan should notify existent transactions"},


Suggested change

{"update", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "true", "Stating if rescan should notify existent transactions"},

{"update", RPCArg::Type::BOOL, /* default */ "true", "Stating if rescan should notify existent transactions"},

fanquake added the RPC/REST/ZMQ label Oct 11, 2017

promag reviewed Oct 11, 2017

View reviewed changes

pedrobranco force-pushed the enhancement/optional-update-rescan-on-importmulti branch from f8cb517 to b20645a Compare October 11, 2017 11:28

promag reviewed Oct 18, 2017

View reviewed changes

pedrobranco force-pushed the enhancement/optional-update-rescan-on-importmulti branch from b20645a to dc31515 Compare March 12, 2018 14:29

DrahtBot mentioned this pull request Jul 13, 2018

[moveonly] Extract RescanWallet to handle a simple rescan #13658

Merged

DrahtBot closed this Jul 21, 2018

DrahtBot reopened this Jul 21, 2018

This was referenced Oct 20, 2018

Rpc help helper class #14502

Closed

docs: Consistent type names in RPC help descriptions #14459

Closed

DrahtBot added the Needs rebase label Dec 5, 2018

maflcko reviewed Dec 5, 2018

View reviewed changes

pedrobranco force-pushed the enhancement/optional-update-rescan-on-importmulti branch from dc31515 to 0482356 Compare January 26, 2019 15:05

Optional update rescan on importmulti

2fd985d

pedrobranco force-pushed the enhancement/optional-update-rescan-on-importmulti branch from 0482356 to 2fd985d Compare January 26, 2019 15:06

DrahtBot removed the Needs rebase label Jan 26, 2019

maflcko added the Needs rebase label Feb 12, 2019

maflcko reviewed Feb 12, 2019

View reviewed changes

bitcoin deleted a comment from DrahtBot Feb 12, 2019

pedrobranco closed this May 19, 2019

promag mentioned this pull request May 19, 2019

rpc: Control if rescan updates/notifies existing transactions on importmulti #16050

Closed

laanwj removed the Needs rebase label Oct 24, 2019

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

	{"update", RPCArg::Type::BOOL, /* opt / true, / default_val */ "true", "Stating if rescan should notify existent transactions"},
	{"update", RPCArg::Type::BOOL, /* default */ "true", "Stating if rescan should notify existent transactions"},

Conversation

pedrobranco commented Oct 11, 2017 • edited by DrahtBot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

promag Oct 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

promag Oct 11, 2017

Choose a reason for hiding this comment

Uh oh!

promag Oct 11, 2017

Choose a reason for hiding this comment

Uh oh!

pedrobranco commented Oct 18, 2017

Uh oh!

promag left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

promag Oct 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedrobranco commented Oct 18, 2017

Uh oh!

pedrobranco commented Mar 12, 2018

Uh oh!

DrahtBot commented Oct 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

maflcko Dec 5, 2018

Choose a reason for hiding this comment

Uh oh!

pedrobranco Jan 26, 2019

Choose a reason for hiding this comment

Uh oh!

promag Jan 26, 2019

Choose a reason for hiding this comment

Uh oh!

promag commented Jan 26, 2019

Uh oh!

maflcko Feb 12, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pedrobranco commented Oct 11, 2017 •

edited by DrahtBot

Loading

promag Oct 11, 2017 •

edited

Loading

promag left a comment •

edited

Loading

promag Oct 18, 2017 •

edited

Loading

DrahtBot commented Oct 20, 2018 •

edited

Loading