Merge #15141: Rewrite DoS interface between validation and net_processing

0ff1c2a838da9e8dc7f77609adc89124bbea3e2b Separate reason for premature spends (coinbase/locktime) (Suhas Daftuar)
54470e767bab37f9b7089782b1be73d5883bb244 Assert validation reasons are contextually correct (Suhas Daftuar)
2120c31521aa51aa1984ee33250b8320506d3a0f [refactor] Update some comments in validation.cpp as we arent doing DoS there (Matt Corallo)
12dbdd7a41bac73e51ed8f7b290b7671196bf9ea [refactor] Drop unused state.DoS(), state.GetDoS(), state.CorruptionPossible() (Matt Corallo)
aa502b88d10c2c3ac56d9163555849b96dc4df1e scripted-diff: Remove DoS calls to CValidationState (Matt Corallo)
7721ad64f40a0c67edefaaf7353264d78df8803e [refactor] Prep for scripted-diff by removing some \ns which annoy sed. (Matt Corallo)
5e78c5734bb0c9aae7b0a7019a745b2d7059b3d9 Allow use of state.Invalid() for all reasons (Matt Corallo)
6b34bc6b6f54f85537494cbea3846d5d195a06d9 Fix handling of invalid headers (Suhas Daftuar)
ef54b486d5333dfc85c56e6b933c81735196a25d [refactor] Use Reasons directly instead of DoS codes (Matt Corallo)
9ab2a0412e96e87956fe61257387683635213035 CorruptionPossible -> BLOCK_MUTATED (Matt Corallo)
6e55b292b0ea944897b6dc2f766446fd209af484 CorruptionPossible -> TX_WITNESS_MUTATED (Matt Corallo)
7df16e70e67c753c871797ce947ea09d7cb0e519 LookupBlockIndex -> CACHED_INVALID (Matt Corallo)
c8b0d22698385f91215ce8145631e3d5826dc977 [refactor] Drop redundant nDoS, corruptionPossible, SetCorruptionPossible (Matt Corallo)
34477ccd39a8d4bfa8ad612f22d5a46291922185 [refactor] Add useful-for-dos "reason" field to CValidationState (Matt Corallo)
6a7f8777a0b193fae4f976196f3464ffac01bf1b Ban all peers for all block script failures (Suhas Daftuar)
7b999103e21509e1c2dec10f68e48744ffe90f55 Clean up banning levels (Matt Corallo)
b8b4c80146780f9011abbd1be72343cc965c07b9 [refactor] drop IsInvalid(nDoSOut) (Matt Corallo)
8818729013e17c650a25f030b2b80e0997389155 [refactor] Refactor misbehavior ban decisions to MaybePunishNode() (Matt Corallo)
00e11e61c0211a62788611cd6a6714a393fdc26c [refactor] rename stateDummy -> orphan_state (Matt Corallo)
f34fa719cf33a51d11f1d2219cbe73ccff6fd697 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 0ff1c2a838da9e8dc7f77609adc89124bbea3e2b
  ryanofsky:
    utACK 0ff1c2a838da9e8dc7f77609adc89124bbea3e2b. Only change is dropping the first commit (f3883a321bf4ab289edcd9754b12cae3a648b175), and dropping the temporary `assert(level == GetDoS())` that was in 35ee77f2832eaffce30042e00785c310c5540cdc (now c8b0d22698385f91215ce8145631e3d5826dc977)

Tree-SHA512: e915a411100876398af5463d0a885920e44d473467bb6af991ef2e8f2681db6c1209bb60f848bd154be72d460f039b5653df20a6840352c5f7ea5486d9f777a3

2 files changed, 9 insertions, 9 deletions

diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py
index d262dae5aa..454eb583f7 100644
--- a/test/functional/data/invalid_txs.py
+++ b/test/functional/data/invalid_txs.py
@@ -58,7 +58,7 @@ class BadTxTemplate:
 
 class OutputMissing(BadTxTemplate):
     reject_reason = "bad-txns-vout-empty"
-    expect_disconnect = False
+    expect_disconnect = True
 
     def get_tx(self):
         tx = CTransaction()
@@ -69,7 +69,7 @@ class OutputMissing(BadTxTemplate):
 
 class InputMissing(BadTxTemplate):
     reject_reason = "bad-txns-vin-empty"
-    expect_disconnect = False
+    expect_disconnect = True
 
     # We use a blank transaction here to make sure
     # it is interpreted as a non-witness transaction.
diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py
index 72eb4f804f..3ad83cd2b3 100755
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -281,7 +281,7 @@ class FullBlockTest(BitcoinTestFramework):
         self.log.info("Reject a block spending an immature coinbase.")
         self.move_tip(15)
         b20 = self.next_block(20, spend=out[7])
-        self.send_blocks([b20], success=False, reject_reason='bad-txns-premature-spend-of-coinbase')
+        self.send_blocks([b20], success=False, reject_reason='bad-txns-premature-spend-of-coinbase', reconnect=True)
 
         # Attempt to spend a coinbase at depth too low (on a fork this time)
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
@@ -294,7 +294,7 @@ class FullBlockTest(BitcoinTestFramework):
         self.send_blocks([b21], False)
 
         b22 = self.next_block(22, spend=out[5])
-        self.send_blocks([b22], success=False, reject_reason='bad-txns-premature-spend-of-coinbase')
+        self.send_blocks([b22], success=False, reject_reason='bad-txns-premature-spend-of-coinbase', reconnect=True)
 
         # Create a block on either side of MAX_BLOCK_BASE_SIZE and make sure its accepted/rejected
         #     genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6  (3)
@@ -616,7 +616,7 @@ class FullBlockTest(BitcoinTestFramework):
         while b47.sha256 < target:
             b47.nNonce += 1
             b47.rehash()
-        self.send_blocks([b47], False, force_send=True, reject_reason='high-hash')
+        self.send_blocks([b47], False, force_send=True, reject_reason='high-hash', reconnect=True)
 
         self.log.info("Reject a block with a timestamp >2 hours in the future")
         self.move_tip(44)
@@ -667,7 +667,7 @@ class FullBlockTest(BitcoinTestFramework):
         b54 = self.next_block(54, spend=out[15])
         b54.nTime = b35.nTime - 1
         b54.solve()
-        self.send_blocks([b54], False, force_send=True, reject_reason='time-too-old')
+        self.send_blocks([b54], False, force_send=True, reject_reason='time-too-old', reconnect=True)
 
         # valid timestamp
         self.move_tip(53)
@@ -813,7 +813,7 @@ class FullBlockTest(BitcoinTestFramework):
         assert tx.vin[0].nSequence < 0xffffffff
         tx.calc_sha256()
         b62 = self.update_block(62, [tx])
-        self.send_blocks([b62], success=False, reject_reason='bad-txns-nonfinal')
+        self.send_blocks([b62], success=False, reject_reason='bad-txns-nonfinal', reconnect=True)
 
         # Test a non-final coinbase is also rejected
         #
@@ -827,7 +827,7 @@ class FullBlockTest(BitcoinTestFramework):
         b63.vtx[0].vin[0].nSequence = 0xDEADBEEF
         b63.vtx[0].rehash()
         b63 = self.update_block(63, [])
-        self.send_blocks([b63], success=False, reject_reason='bad-txns-nonfinal')
+        self.send_blocks([b63], success=False, reject_reason='bad-txns-nonfinal', reconnect=True)
 
         #  This checks that a block with a bloated VARINT between the block_header and the array of tx such that
         #  the block is > MAX_BLOCK_BASE_SIZE with the bloated varint, but <= MAX_BLOCK_BASE_SIZE without the bloated varint,
@@ -1241,7 +1241,7 @@ class FullBlockTest(BitcoinTestFramework):
 
         self.log.info("Reject a block with an invalid block header version")
         b_v1 = self.next_block('b_v1', version=1)
-        self.send_blocks([b_v1], success=False, force_send=True, reject_reason='bad-version(0x00000001)')
+        self.send_blocks([b_v1], success=False, force_send=True, reject_reason='bad-version(0x00000001)', reconnect=True)
 
         self.move_tip(chain1_tip + 2)
         b_cb34 = self.next_block('b_cb34', version=4)