The removeprunedfunds() RPC was returning misleading or incorrect error
codes (for example RPC_INTERNAL_ERROR when the transaction was
not found in the wallet). This commit fixes those error codes:
- RPC_INTERNAL_ERROR should not be returned for application-level
errors, only for genuine internal errors such as corrupted data.
This error code has been replaced with RPC_WALLET_ERROR.
This commit also updates the test cases to explicitly test the error code.
Previously if an existing watch only key was reimported with a new timestamp,
the new timestamp would not be saved in the key metadata, and would not be used
to update the wallet nTimeFirstKey value (which could cause rescanning to start
at the wrong point and miss transactions).
Issue was pointed out by Jonas Schnelli <dev@jonasschnelli.ch> in
https://github.com/bitcoin/bitcoin/pull/9108#issuecomment-279715550
Remove "nLowestTimestamp <= chainActive.Tip()->GetBlockTimeMax()" check from
importmulti, which is always true because nLowestTimestamp is set to the
minimum of the most recent block time and all the imported key timestamps,
which is necessarily lower than the maximum block time.
When importing a watch-only address over importmulti with a specific timestamp,
the wallet's nTimeFirstKey is currently set to 1. After this change, the
provided timestamp will be used and stored as metadata associated with
watch-only key. This can improve wallet performance because it can avoid the
need to scan the entire blockchain for watch only addresses when timestamps are
provided.
Also adds timestamp to validateaddress return value (needed for tests).
Fixes#9034.
Additionally, accept a "now" timestamp, to allow avoiding rescans for keys
which are known never to have been used.
Note that the behavior when "now" is specified is slightly different than the
previous behavior when no timestamp was specified at all. Previously, when no
timestamp was specified, it would avoid rescanning during the importmulti call,
but set the key's nCreateTime value to 1, which would not prevent future block
reads in later ScanForWalletTransactions calls. With this change, passing a
"now" timestamp will set the key's nCreateTime to the current block time
instead of 1.
Fixes#9491
In spite of the name FindLatestBefore used std::lower_bound to try
to find the earliest block with a nTime greater or equal to the
the requested value. But lower_bound uses bisection and requires
the input to be ordered with respect to the comparison operation.
Block times are not well ordered.
I don't know what lower_bound is permitted to do when the data
is not sufficiently ordered, but it's probably not good.
(I could construct an implementation which would infinite loop...)
To resolve the issue this commit introduces a maximum-so-far to the
block indexes and searches that.
For clarity the function is renamed to reflect what it actually does.
An issue that remains is that there is no grace period in importmulti:
If a address is created at time T and a send is immediately broadcast
and included by a miner with a slow clock there may not yet have been
any block with at least time T.
The normal rescan has a grace period of 7200 seconds, but importmulti
does not.
The meaning is clear from the context, and we're inconsistent here.
Also save typing when using named arguments.
- `bitcoinaddress` -> `address`
- `bitcoinprivkey` -> `privkey`
- `bitcoinpubkey` -> `pubkey`
Putting the build date in the executable is a practice that has no place
in these days, now that deterministic building is increasingly common.
Continues #7732 which did this for the GUI.
Before this, if someone imported a scriptPubKey directly (in hex form) using
importaddress, outputs sending to it would be treated as change, as the
corresponding CTxDestination was not added to the address book.
Fix this by trying to detect scriptPubKeys that are in fact convertible to a
CTxDestination and add them anyway. Add a warning to the RPC help to warn
against importing raw non-standard scripts.
Complete rescan is incompatible with pruning, but rescan is optional on
our wallet key import RPCs. Import on use is very useful in some common
situations in conjunction with pruning, e.g. merchant payment tracking.
This reenables importprivkey/importaddress/importpubkey when rescan
is not used.
In the future we should consider changing the rescan argument to allow depth
or date to allow limited rescanning when compatible with the retained
block depth.
Karl-Johan Alm