Page MenuHomePhabricator

Improve ScanForWalletTransactions return value
ClosedPublic

Authored by deadalnix on Jan 12 2018, 23:56.

Details

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

Diff Detail

Repository
rABC Bitcoin ABC
Branch
scanwalletreturn
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1591
Build 1591: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Jan 14 2018, 02:27
This revision was automatically updated to reflect the committed changes.