diff options
author | Wladimir J. van der Laan <laanwj@protonmail.com> | 2019-10-09 11:26:44 +0200 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@protonmail.com> | 2019-10-09 11:51:58 +0200 |
commit | c08bf2b574636023cd9d68cb43b3dada5b0cc737 (patch) | |
tree | f262d6ebc83588e35ef82c5cefccca7590257853 /test/functional | |
parent | 1d9ac7fded554b1baf828d10f9ae7062fbf9c9eb (diff) | |
parent | fa25f43ac5692082dba3f90456c501eb08f1b75c (diff) |
Merge #15437: p2p: Remove BIP61 reject messages
fa25f43ac5692082dba3f90456c501eb08f1b75c p2p: Remove BIP61 reject messages (MarcoFalke)
Pull request description:
Reject messages (BIP 61) appear in the following settings:
* Parsing of reject messages (in case `-debug=net` is set, off by default). This has only been used for a single `LogPrint` call for several releases now. Such logging is completely meaningless to us and should thus be removed.
* The sending of reject messages (in case `-enablebip61` is set, off by default). This can be used to debug a node that is under our control. Instead of hacking this debugging into the p2p protocol, it could be more easily achieved by parsing the debug log. (Use `-printtoconsole` to have it as stream, or read from the `debug.log` file like our python function `assert_debug_log` in the test framework does)
Having to maintain all of this logic and code to accommodate debugging, which can be achieved by other means a lot easier, is a burden. It makes review on net processing changes a lot harder, since the reject message logic has to be carried around without introducing any errors or DOS vectors.
ACKs for top commit:
jnewbery:
utACK fa25f43ac5692082dba3f90456c501eb08f1b75c
laanwj:
I'm still not 100% convinced that I like getting rid of BIP61 conceptually, but apparently everyone wants it, code review ACK fa25f43ac5692082dba3f90456c501eb08f1b75c.
ryanofsky:
Code review ACK fa25f43ac5692082dba3f90456c501eb08f1b75c
Tree-SHA512: daf55254202925e56be3d6cfb3c1c804e7a82cecb1dd1e5bd7b472bae989fd68ac4f21ec53fc46751353056fd645f7f877bebcb0b40920257991423a3d99e0be
Diffstat (limited to 'test/functional')
-rwxr-xr-x | test/functional/feature_cltv.py | 3 | ||||
-rwxr-xr-x | test/functional/feature_dersig.py | 14 | ||||
-rwxr-xr-x | test/functional/test_framework/messages.py | 32 | ||||
-rwxr-xr-x | test/functional/test_framework/mininode.py | 2 |
4 files changed, 2 insertions, 49 deletions
diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index e00219ca4a..13f1c9216a 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -22,9 +22,6 @@ from io import BytesIO CLTV_HEIGHT = 1351 -# Reject codes that we might receive in this test -REJECT_INVALID = 16 -REJECT_NONSTANDARD = 64 def cltv_invalidate(tx): '''Modify the signature in vin 0 of the tx to fail CLTV diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index 1bd9586364..92f6eea5b2 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -9,19 +9,15 @@ Test that the DERSIG soft-fork activates at (regtest) height 1251. from test_framework.blocktools import create_coinbase, create_block, create_transaction from test_framework.messages import msg_block -from test_framework.mininode import mininode_lock, P2PInterface +from test_framework.mininode import P2PInterface from test_framework.script import CScript from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - wait_until, ) DERSIG_HEIGHT = 1251 -# Reject codes that we might receive in this test -REJECT_INVALID = 16 -REJECT_NONSTANDARD = 64 # A canonical signature consists of: # <30> <total len> <02> <len R> <R> <02> <len S> <S> <hashtype> @@ -44,7 +40,7 @@ def unDERify(tx): class BIP66Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [['-whitelist=127.0.0.1', '-par=1', '-enablebip61']] # Use only one script thread to get the exact reject reason for testing + self.extra_args = [['-whitelist=127.0.0.1', '-par=1']] # Use only one script thread to get the exact log msg for testing self.setup_clean_chain = True self.rpc_timeout = 120 @@ -129,12 +125,6 @@ class BIP66Test(BitcoinTestFramework): assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip) self.nodes[0].p2p.sync_with_ping() - wait_until(lambda: "reject" in self.nodes[0].p2p.last_message.keys(), lock=mininode_lock) - with mininode_lock: - assert self.nodes[0].p2p.last_message["reject"].code in [REJECT_INVALID, REJECT_NONSTANDARD] - assert_equal(self.nodes[0].p2p.last_message["reject"].data, block.sha256) - assert b'Non-canonical DER signature' in self.nodes[0].p2p.last_message["reject"].reason - self.log.info("Test that a version 3 block with a DERSIG-compliant transaction is accepted") block.vtx[1] = create_transaction(self.nodes[0], self.coinbase_txids[1], self.nodeaddress, amount=1.0) block.hashMerkleRoot = block.calc_merkle_root() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 3cb5f56bee..25520a2151 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1317,38 +1317,6 @@ class msg_headers: return "msg_headers(headers=%s)" % repr(self.headers) -class msg_reject: - __slots__ = ("code", "data", "message", "reason") - command = b"reject" - REJECT_MALFORMED = 1 - - def __init__(self): - self.message = b"" - self.code = 0 - self.reason = b"" - self.data = 0 - - def deserialize(self, f): - self.message = deser_string(f) - self.code = struct.unpack("<B", f.read(1))[0] - self.reason = deser_string(f) - if (self.code != self.REJECT_MALFORMED and - (self.message == b"block" or self.message == b"tx")): - self.data = deser_uint256(f) - - def serialize(self): - r = ser_string(self.message) - r += struct.pack("<B", self.code) - r += ser_string(self.reason) - if (self.code != self.REJECT_MALFORMED and - (self.message == b"block" or self.message == b"tx")): - r += ser_uint256(self.data) - return r - - def __repr__(self): - return "msg_reject: %s %d %s [%064x]" \ - % (self.message, self.code, self.reason, self.data) - class msg_feefilter: __slots__ = ("feerate",) diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index 166438cf70..f95c158a68 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -41,7 +41,6 @@ from test_framework.messages import ( msg_notfound, msg_ping, msg_pong, - msg_reject, msg_sendcmpct, msg_sendheaders, msg_tx, @@ -74,7 +73,6 @@ MESSAGEMAP = { b"notfound": msg_notfound, b"ping": msg_ping, b"pong": msg_pong, - b"reject": msg_reject, b"sendcmpct": msg_sendcmpct, b"sendheaders": msg_sendheaders, b"tx": msg_tx, |