Page MenuHomePhabricator

rearrange ATMP in preparation for SigChecks accounting in mempool
ClosedPublic

Authored by markblundeberg on Feb 2 2020, 14:40.

Details

Summary

Once we start using sigchecks in place of sigops, anything relying
on that number can only be run *after* CheckInputs. Notably, the
virtualsize computation and the generation of the CTxMemPoolEntry
(and things using the CTxMemPoolEntry).

This perhaps creates some concerns of DoS vector, however it can
be ameliorated by a future change that runs the standard script
checks with a dummy signature verifier (no signature verification)
and only the consensus script checks need signature verification.
The standard sigcheck count would be put into mempool, however if
the sigchecks counts disagree the transaction should be dropped.

Test Plan

ninja check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
atmp_rearrange
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9227
Build 16399: Default Diff Build & Tests
Build 16398: arc lint + arc unit

Event Timeline

If this rearrangement is not desired, an alternative is to do the following:

  1. pass a flag to CTxMemPoolEntry that disables use of sigops count when calculating virtual size (so that virtual size computations would not be used for sigchecks), and,
  2. allow CTxMemPoolEntry to have its sigops count updated after creation. This would be done just before the insertion into mempool with addUnchecked ... obviously such an update shouldn't occur once it's in mempool.
This revision is now accepted and ready to land.Feb 2 2020, 22:18

It's all good. With the change we did around NULLFAIL and multisig, we should move things toward verifying signature lazily anyways.