Commit graph

5 commits

Author SHA1 Message Date
Gregory Sanders
c4b0c08f7c Update tx-size-small comment with relevant CVE disclosure 2019-09-18 16:21:44 -04:00
Wladimir J. van der Laan
d7d7d31506
Merge #15141: Rewrite DoS interface between validation and net_processing
0ff1c2a838 Separate reason for premature spends (coinbase/locktime) (Suhas Daftuar)
54470e767b Assert validation reasons are contextually correct (Suhas Daftuar)
2120c31521 [refactor] Update some comments in validation.cpp as we arent doing DoS there (Matt Corallo)
12dbdd7a41 [refactor] Drop unused state.DoS(), state.GetDoS(), state.CorruptionPossible() (Matt Corallo)
aa502b88d1 scripted-diff: Remove DoS calls to CValidationState (Matt Corallo)
7721ad64f4 [refactor] Prep for scripted-diff by removing some \ns which annoy sed. (Matt Corallo)
5e78c5734b Allow use of state.Invalid() for all reasons (Matt Corallo)
6b34bc6b6f Fix handling of invalid headers (Suhas Daftuar)
ef54b486d5 [refactor] Use Reasons directly instead of DoS codes (Matt Corallo)
9ab2a0412e CorruptionPossible -> BLOCK_MUTATED (Matt Corallo)
6e55b292b0 CorruptionPossible -> TX_WITNESS_MUTATED (Matt Corallo)
7df16e70e6 LookupBlockIndex -> CACHED_INVALID (Matt Corallo)
c8b0d22698 [refactor] Drop redundant nDoS, corruptionPossible, SetCorruptionPossible (Matt Corallo)
34477ccd39 [refactor] Add useful-for-dos "reason" field to CValidationState (Matt Corallo)
6a7f8777a0 Ban all peers for all block script failures (Suhas Daftuar)
7b999103e2 Clean up banning levels (Matt Corallo)
b8b4c80146 [refactor] drop IsInvalid(nDoSOut) (Matt Corallo)
8818729013 [refactor] Refactor misbehavior ban decisions to MaybePunishNode() (Matt Corallo)
00e11e61c0 [refactor] rename stateDummy -> orphan_state (Matt Corallo)
f34fa719cf Drop obsolete sigops comment (Matt Corallo)

Pull request description:

  This is a rebase of #11639 with some fixes for the last few comments which were not yet addressed.

  The original PR text, with some strikethroughs of text that is no longer correct:

  > This cleans up an old main-carryover - it made sense that main could decide what DoS scores to assign things because the DoS scores were handled in a different part of main, but now validation is telling net_processing what DoS scores to assign to different things, which is utter nonsense. Instead, we replace CValidationState's nDoS and CorruptionPossible with a general ValidationInvalidReason, which net_processing can handle as it sees fit. I keep the behavior changes here to a minimum, but in the future we can utilize these changes for other smarter behavior, such as disconnecting/preferring to rotate outbound peers based on them providing things which are invalid due to SOFT_FORK because we shouldn't ban for such cases.
  >
  > This is somewhat complementary with, though obviously conflicts heavily with #11523, which added enums in place of DoS scores, as well as a few other cleanups (which are still relevant).
  >
  > Compared with previous bans, the following changes are made:
  >
  > Txn with empty vin/vout or null prevouts move from 10 DoS
  > points to 100.
  > Loose transactions with a dependency loop now result in a ban
  > instead of 10 DoS points.
  > ~~BIP68-violation no longer results in a ban as it is SOFT_FORK.~~
  > ~~Non-SegWit SigOp violation no longer results in a ban as it
  > considers P2SH sigops and is thus SOFT_FORK.~~
  > ~~Any script violation in a block no longer results in a ban as
  > it may be the result of a SOFT_FORK. This should likely be
  > fixed in the future by differentiating between them.~~
  > Proof of work failure moves from 50 DoS points to a ban.
  > Blocks with timestamps under MTP now result in a ban, blocks
  > too far in the future continue to not result in a ban.
  > Inclusion of non-final transactions in a block now results in a
  > ban instead of 10 DoS points.

  Note: The change to ban all peers for consensus violations is actually NOT the change I'd like to make -- I'd prefer to only ban outbound peers in those situations.  The current behavior is a bit of a mess, however, and so in the interests of advancing this PR I tried to keep the changes to a minimum.  I plan to revisit the behavior in a followup PR.

  EDIT: One reviewer suggested I add some additional context for this PR:

  > The goal of this work was to make net_processing aware of the actual reasons for validation failures, rather than just deal with opaque numbers instructing it to do something.
  >
  > In the future, I'd like to make it so that we use more context to decide how to punish a peer. One example is to differentiate inbound and outbound peer misbehaviors. Another potential example is if we'd treat RECENT_CONSENSUS_CHANGE failures differently (ie after the next consensus change is implemented), and perhaps again we'd want to treat some peers differently than others.

ACKs for commit 0ff1c2:
  jnewbery:
    utACK 0ff1c2a838
  ryanofsky:
    utACK 0ff1c2a838. Only change is dropping the first commit (f3883a321bf4ab289edcd9754b12cae3a648b175), and dropping the temporary `assert(level == GetDoS())` that was in 35ee77f2832eaffce30042e00785c310c5540cdc (now c8b0d22698)

Tree-SHA512: e915a411100876398af5463d0a885920e44d473467bb6af991ef2e8f2681db6c1209bb60f848bd154be72d460f039b5653df20a6840352c5f7ea5486d9f777a3
2019-05-04 11:58:57 +02:00
Matt Corallo
7b999103e2 Clean up banning levels
Compared with previous bans, the following changes are made:
 * Txn with empty vin/vout or null prevouts move from 10 DoS
   points to 100.
 * Loose transactions with a dependency loop now result in a ban
   instead of 10 DoS points.
 * Many pre-segwit soft-fork errors now result in a ban.
   Note: Transactions that violate soft-fork script flags since P2SH do not generally
   result in a ban. Also, banning behavior for invalid blocks is dependent on
   whether the node is validating with multiple script check threads, due to a long-
   standing bug. That inconsistency is still present after this commit.
 * Proof of work failure moves from 50 DoS points to a ban.
 * Blocks with timestamps under MTP now result in a ban, blocks
   too far in the future continue to *not* result in a ban.
 * Inclusion of non-final transactions in a block now results in a
   ban instead of 10 DoS points.

Co-authored-by: Anthony Towns <aj@erisian.com.au>
2019-05-02 11:00:19 -04:00
Gregory Sanders
25b0786581 Fix missing input template by making minimal tx 2019-04-25 21:01:09 -04:00
James O'Beirne
59e387705c test: add invalid tx templates for use in functional tests
Add templates for easily constructing different kinds of invalid
transactions and use them in feature_block and p2p_invalid_tx.
2018-11-27 17:53:53 -05:00