HomePhabricator

make per-tx sigops limit contextual

Description

make per-tx sigops limit contextual

Summary:
The per-tx sigops check in CheckTransactionCommon (used by CheckCoinbase
and CheckRegularTransaction) happens without any context. To deactivate
sigops, just as with D5061, this needs to be moved into a contextual zone
that is aware of block index.

This is a fairly delicate change, so please review it carefully (and I
will be taking another very careful look at this myself before landing):
Although for most transactions this check is in fact optional, since
they will be rechecked in ConnectBlock using the more strict P2SH
counting, it cannot be removed naively since it is the *only* place
where the coinbase sigops limit is enforced! I've added a test for
that in D5073, and put a second check in ConnectBlock just for
good measure.

Note that like D5061 this is a behaviour change for old blocks,
since it now only counts the checkdatasig sigops for transactions
in blocks after that upgrade.

Also, as for mempool behaviour, previously we would ban for txns
with >20000 sigops but only reject >4000 sigops as nonstandard.
Now, we just reject both without banning. I think that's fine.

Depends on D5061 D5062 D5072 D5073.

Test Plan: ninja check-extended

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Maniphest Tasks: T704

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

Details

Provenance
Mark Lundeberg <markblundeberg@users.noreply.github.com>Authored on Jan 27 2020, 05:35
markblundebergPushed on Jan 28 2020, 12:16
Reviewer
Restricted Project
Differential Revision
D5076: make per-tx sigops limit contextual
Parents
rSTAGING445da7dc7b93: rework AcceptToMemoryPoolWorker sigops counting
Branches
Unknown
Tags
Unknown
Tasks
T704: sigChecks implementation