HomePhabricator

Improve ScanForWalletTransactions return value

Description

Improve ScanForWalletTransactions return value

Summary:
Change ScanForWalletTransactions return value so it is possible to distinguish
scans that skip reading every block (due to the nTimeFirstKey optimization)
from scans that fail while reading the chainActive.Tip() block. Return value is
now non-null in the non-failing case.

This change doesn't affect any user-visible behavior, it is only an internal
API improvement. The only code currently using the ScanForWalletTransactions
return value is in importmulti, and importmulti always calls
ScanForWalletTransactions with a pindex pointing to the first block in
chainActive whose block time is >= (nLowestTimestamp - 7200), while
ScanForWalletTransactions would only return null without reading blocks when
pindex and every block after it had a block time < (nTimeFirstKey - 7200).
These conditions could never happen at the same time because nTimeFirstKey <=
nLowestTimestamp.

I'm planning to make a more substantial API improvement in the future (making
ScanForWalletTransactions private and exposing a higher level rescan method to
RPC code), but Matt Corallo <git@bluematt.me> pointed out this odd behavior
introduced by e2e2f4c "Return errors from importmulti if complete rescans are
not successful" yesterday, so I'm following up now to get rid of badness
introduced by that merge.

This is a backport of core's PR9827

Test Plan:

make check

Reviewers: schancel, #bitcoin_abc

Reviewed By: schancel, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D942

Details

Provenance
Russell Yanofsky <russ@yanofsky.org>Authored on Feb 22 2017, 18:07
deadalnixCommitted on Jan 14 2018, 02:30
deadalnixPushed on Jan 14 2018, 14:18
Reviewer
Restricted Project
Differential Revision
D942: Improve ScanForWalletTransactions return value
Parents
rSTAGING259fd7ccf822: Define 7200 second timestamp window constant
Branches
Unknown
Tags
Unknown