HomePhabricator

ConnectBlock: fix slow usage of AddCoins

Description

ConnectBlock: fix slow usage of AddCoins

Summary:
In D2076 the check=true parameter was passed, which doesn't just
silently hide overwrites but also causes an extra HaveCoin lookup
for each coin. This is a whole database lookup regardless of the
caches, since it's always going to be a miss in normal operation.
We're better off just catching the exception than doing the silent
overwrite thing.

As a quick benchmark, I ran reindex-chainstate on testnet without/with
this change, and the runtime changed from 29 minutes to 20 minutes.
And that's even with the coins db on an SSD and probably fully in
the OS cache.

I found the comment a bit confusing:

  • CheckBlock does not check for duplicate transactions except the special case of the merkle tree vuln.
  • BIP30 is nice but it's not being enforced on recent blocks (again, because HaveCoin misses are slow), only BIP34 is enforced. -- see https://github.com/bitcoin/bitcoin/pull/6931

Test Plan:
ninja check-extended

(since CTOR is always enabled on regtest, and also regtest always
enforces BIP30, it is unfortunately not possible to write a functional
test for this.)

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

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

Details

Provenance
Mark Lundeberg <markblundeberg@users.noreply.github.com>Authored on Jan 28 2020, 12:58
FabienPushed on Feb 3 2020, 10:27
Reviewer
Restricted Project
Differential Revision
D5081: ConnectBlock: fix slow usage of AddCoins
Parents
rSTAGINGe31a329a4dd5: nit: functional test chmod +x
Branches
Unknown
Tags
Unknown
References
tag: phabricator/base/15955, tag: phabricator/base/15954

Event Timeline

Mark Lundeberg <markblundeberg@users.noreply.github.com> committed rSTAGING1e1a278bdee6: ConnectBlock: fix slow usage of AddCoins (authored by Mark Lundeberg <markblundeberg@users.noreply.github.com>).Feb 3 2020, 08:27