Merge #16021: p2p: Avoid logging transaction decode errors to stderr

fa2b52af32f6a4b9c22c270f36e92960c29ef364 Disallow extended encoding for non-witness transactions (take 3) (MarcoFalke)

Pull request description:

  (previous title "p2p: Disallow extended encoding for non-witness transactions (take 3)")

  Remote peers can send us illegally encoded txs and thus have us write to stderr. Fix that by not writing to stderr.

  This is a follow up to the previous (incomplete) attempts at this:

  *  Disallow extended encoding for non-witness transactions #14039
  *  Add test for superfluous witness record in deserialization #15893

ACKs for commit fa2b52:
  laanwj:
    utACK fa2b52af32f6a4b9c22c270f36e92960c29ef364
  ryanofsky:
    utACK fa2b52af32f6a4b9c22c270f36e92960c29ef364. Would change title to something like "Avoid logging transaction decode errors to stderr" instead of "Disallow extended encoding for non-witness transactions." The current title is confusing because this PR isn't really allowing or disallowing anything, just logging the condition differently. "Disallow" also seems to contradict the "Allow exceptions from..." comments in the actual code.

Tree-SHA512: c66990e69b432d00dc1c5510bf976a1188664d0890a32d1e5c6459094e7e27da82a5d227627afcbc203676f5540eec74b7d9b1d71d2c62d3b2069e1781824b4d

1 files changed, 23 insertions, 3 deletions

diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py
index 000c30646a..df0f9fa365 100755
--- a/test/functional/p2p_segwit.py
+++ b/test/functional/p2p_segwit.py
@@ -2038,9 +2038,9 @@ class SegWitTest(BitcoinTestFramework):
         # TODO: test p2sh sigop counting
 
     def test_superfluous_witness(self):
-        # Serialization of tx that puts witness flag to 1 always
+        # Serialization of tx that puts witness flag to 3 always
         def serialize_with_bogus_witness(tx):
-            flags = 1
+            flags = 3
             r = b""
             r += struct.pack("<i", tx.nVersion)
             if flags:
@@ -2059,9 +2059,29 @@ class SegWitTest(BitcoinTestFramework):
             r += struct.pack("<I", tx.nLockTime)
             return r
 
-        raw = self.nodes[0].createrawtransaction([{"txid":"00"*32, "vout":0}], {self.nodes[0].getnewaddress():1})
+        class msg_bogus_tx(msg_tx):
+            def serialize(self):
+                return serialize_with_bogus_witness(self.tx)
+
+        self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(address_type='bech32'), 5)
+        self.nodes[0].generate(1)
+        unspent = next(u for u in self.nodes[0].listunspent() if u['spendable'] and u['address'].startswith('bcrt'))
+
+        raw = self.nodes[0].createrawtransaction([{"txid": unspent['txid'], "vout": unspent['vout']}], {self.nodes[0].getnewaddress(): 1})
         tx = FromHex(CTransaction(), raw)
         assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].decoderawtransaction, serialize_with_bogus_witness(tx).hex())
+        with self.nodes[0].assert_debug_log(['Superfluous witness record']):
+            self.nodes[0].p2p.send_message(msg_bogus_tx(tx))
+            self.nodes[0].p2p.sync_with_ping()
+        raw = self.nodes[0].signrawtransactionwithwallet(raw)
+        assert raw['complete']
+        raw = raw['hex']
+        tx = FromHex(CTransaction(), raw)
+        assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].decoderawtransaction, serialize_with_bogus_witness(tx).hex())
+        with self.nodes[0].assert_debug_log(['Unknown transaction optional data']):
+            self.nodes[0].p2p.send_message(msg_bogus_tx(tx))
+            self.nodes[0].p2p.sync_with_ping()
+
 
 if __name__ == '__main__':
     SegWitTest().main()