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