Page MenuHomePhabricator

sigChecks implementation
Open, Needs TriagePublic

Description

Pull request on bitcoincashorg repo: https://github.com/bitcoincashorg/bitcoincash.org/pull/443

Implementation plan below is not up to date, but roughly accurate.

Rough spec of desired behaviour: https://gist.github.com/markblundeberg/becf85ac6ba48bebfeb7871ce2de27a7

Implementation plan (not including tests):

  • Update EvalScript to do the desired counting (easy).
  • One new flag (permanent, standard-only) to control the per-input-limiter rule.
  • One new flag (temporary, consensus) to control sigChecks count being reported out of interpreter.cpp and into validation.cpp
    • This makes it so that pre-activation transactions are guaranteed to count as sigChecks=0. Preactivation blocks which will thus all have sigChecks=0 by definition, and the the consensus limit on sigChecks can be in place for all time.
    • Upon activation when the mempool is reprocessed anyway (and scripts are evaluated with new flags), the script cache will be filled with new values that have correct SigChecks count.
    • Some days after activation has passed, we can remove this temporary flag and start counting sigchecks on all blocks, current and historical. A retconned historical limit can be applied to the pre-activation blocks. This lets us remove the sigops limit on historical blocks, safe in the knowledge that a new node coming online cannot be tricked into consuming ridiculous low-difficult blocks that consume endless CPU cycles.
  • Upgrade scriptcache to function as map cache instead of set cache. The same cuckoocache structure is good and can be repurposed to act as a map.
    • Most of desired functionality can be gotten by adding a new method get() to cuckoocache (similar to contains() but returns the value) and then making an Element type which contains key and value fields (with operator== only looking at key), and a hasher that only looks at the key.
    • Question: should insert() be modified so that if it finds an extant entry, it overwrites said entry (key is same but the new value may differ)? Can this be done in some elegant way so it does not slow down set-type operations at all? Not really relevant to scriptcache but a map ought to update on insert.
  • Update CheckInputs to dispatch SigChecks information correctly (return the value from cache, or evaluate now if uncached and mempool, or schedule CScriptCheck if connectblock).
    • Note that during ConnectBlock, the call to CheckInputs will only read+erase from the cache, never insert (insertion into cuckoocache is not thread safe so this would conflict with parallel validation!). Thus during ConnectBlock, CheckInputs will:
      • Return immediately with the cached total SigChecks value for that transaction, if it gets a cache hit.
      • Otherwise, schedule a parallel CScriptCheck that is has access to update a SigChecks counter. Each input;s validation will separately modify the SigChecks counter.
      • We can make the SigChecks counter start at the maximum value and decrement atomically after each input is validated. In this way, the parallel checkers can observe when the SigChecks limit has been exceeded and bail early (bailing early is essential to prevent the invalid-block DoS attack here!). Thankfully, such bailing based on "global" information does not lead to corrupting of any cache, since the system does not assume that a failing CScriptCheck means the tx is bad (rather it only means the block is bad).
    • During mempool admittance, CheckInputs will either:
      • Return immediately with the cached total SigChecks value for that transaction, if it gets a cache hit.
      • Otherwise, construct CScriptChecks and evaluate the immediately, and (if valid) return the total SigChecks, after inserting the total into the script cache.
    • Note, we will probably only allocate 2 bytes to the SigChecks cached value and leave 30 bytes for the key. It should be impossible to exceed this due to standardness limits - the most possible SigChecks in a standard tx will be 0 before activation, and ~3000 after activation. But if the unimaginable happens and a tx somehow reaches CheckInputs with more than 65535 sigchecks, we can simply not insert that entry into the cache.
  • Update the wallet bare multisig signer so it always uses Schnorr, and/or backport Wuille's PR (https://github.com/bitcoin/bitcoin/pull/13002) which drops the automatic acquisition of bare multisig outputs into wallet.
    • Why: Currently a bare multisig 1-of-20 can be sent into your wallet, and will be automatically picked up if all the pubkeys belong to you. Although this is nonstandard to create, it is standard to spend from. But the spending will currently use ECDSA, which means it will produce extremely dense SigChecks. Such spending will become nonstandard once the new per-input sigchecks rule comes into play.

Revisions and Commits

rABC Bitcoin ABC
Needs Revision
Changes Planned
Changes Planned
Abandoned
Abandoned
rSTAGING Bitcoin ABC staging
D5297
D5179
D5029
D5224
D5220
D5160
D5127
D5153
D5128
D5126
D4834
D5076
D5061
D5060
D5062
D5018
D4940
D4920
D4617
D4918
D4835
D4678
D4677

Event Timeline