Skip to content

Commit

Permalink
Merge bitcoin#11638: [tests] Dead mininode code
Browse files Browse the repository at this point in the history 
fb00c45 [tests] Explicitly disallow support for p2p versions below 60001 (John Newbery)
3858aab [tests] Remove support for p2p alert messages (John Newbery)
c0b1274 [tests] Remove support for bre-BIP31 ping messages (John Newbery)
2904e30 [tests] Remove dead code from mininode.py (John Newbery)

Pull request description:

  This is the first part of bitcoin#11518. It removes a ~150 lines of unused code from the mininode module:

  - remove unused `deliver_sleep_time` and `EarlyDisconnectError` code
  - remove support for pre-BIP31 ping messages
  - remove support for alert message
  - explicitly don't support p2p versions lower than 60001

  Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)

Tree-SHA512: 508e612ceb0b094250d18e75522d51e6b14cd069443050ba4af34d6f890c58721cb5653e8bc000b60635b9474d035b0dcd9c509c0dcdb3a7501df17b787f83b0

readd is None check

Signed-off-by: Pasta <pasta@dashboost.org>
  • Loading branch information
@gades
MarcoFalke authored and gades committed Jun 30, 2021
1 parent a2d10de commit c89c8f2
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 99 deletions.
1 change: 0 additions & 1 deletion test/functional/p2p-leaktests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ def on_verack(self, conn, message): self.bad_message(message)
def on_reject(self, conn, message): self.bad_message(message)
def on_inv(self, conn, message): self.bad_message(message)
def on_addr(self, conn, message): self.bad_message(message)
def on_alert(self, conn, message): self.bad_message(message)
def on_getdata(self, conn, message): self.bad_message(message)
def on_getblocks(self, conn, message): self.bad_message(message)
def on_tx(self, conn, message): self.bad_message(message)
Expand Down
133 changes: 35 additions & 98 deletions test/functional/test_framework/mininode.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@

import cosanta_hash

BIP0031_VERSION = 60000
MIN_VERSION_SUPPORTED = 60001
MY_VERSION = 70214 # MIN_PEER_PROTO_VERSION
MY_SUBVERSION = b"/python-mininode-tester:0.0.3/"
MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37)
Expand Down Expand Up @@ -1142,22 +1142,6 @@ def __repr__(self):
return "msg_getaddr()"


class msg_ping_prebip31():
command = b"ping"

def __init__(self):
pass

def deserialize(self, f):
pass

def serialize(self):
return b""

def __repr__(self):
return "msg_ping() (pre-bip31)"


class msg_ping():
command = b"ping"

Expand Down Expand Up @@ -1495,9 +1479,7 @@ class NodeConnCB():
"""Callback and helper functions for P2P connection to a bitcoind node.
Individual testcases should subclass this and override the on_* methods
if they want to alter message handling behaviour.
"""

if they want to alter message handling behaviour."""
def __init__(self):
# Track whether we have a P2P connection open to the node
self.connected = False
Expand All @@ -1511,25 +1493,13 @@ def __init__(self):
# A count of the number of ping messages we've sent to the node
self.ping_counter = 1

# deliver_sleep_time is helpful for debugging race conditions in p2p
# tests; it causes message delivery to sleep for the specified time
# before acquiring the global lock and delivering the next message.
self.deliver_sleep_time = None

# Message receiving methods

def deliver(self, conn, message):
"""Receive message and dispatch message to appropriate callback.
We keep a count of how many of each message type has been received
and the most recent message of each type.
Optionally waits for deliver_sleep_time before dispatching message.
"""

deliver_sleep = self.get_deliver_sleep_time()
if deliver_sleep is not None:
time.sleep(deliver_sleep)
and the most recent message of each type."""
with mininode_lock:
try:
command = message.command.decode('ascii')
Expand All @@ -1541,10 +1511,6 @@ def deliver(self, conn, message):
sys.exc_info()[0]))
raise

def get_deliver_sleep_time(self):
with mininode_lock:
return self.deliver_sleep_time

# Callback methods. Can be overridden by subclasses in individual test
# cases to provide custom message handling behaviour.

Expand Down Expand Up @@ -1582,8 +1548,7 @@ def on_inv(self, conn, message):
conn.send_message(want)

def on_ping(self, conn, message):
if conn.ver_send > BIP0031_VERSION:
conn.send_message(msg_pong(message.nonce))
conn.send_message(msg_pong(message.nonce))

def on_mnlistdiff(self, conn, message): pass
def on_clsig(self, conn, message): pass
Expand All @@ -1593,11 +1558,8 @@ def on_verack(self, conn, message):
self.verack_received = True

def on_version(self, conn, message):
if message.nVersion >= 209:
conn.send_message(msg_verack())
conn.ver_send = min(MY_VERSION, message.nVersion)
if message.nVersion < 209:
conn.ver_recv = conn.ver_send
assert message.nVersion >= MIN_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_VERSION_SUPPORTED)
conn.send_message(msg_verack())
conn.nServices = message.nServices

# Connection helper methods
Expand Down Expand Up @@ -1669,9 +1631,10 @@ def sync_with_ping(self, timeout=60):
wait_until(test_function, timeout=timeout, lock=mininode_lock)
self.ping_counter += 1

# The actual NodeConn class
# This class provides an interface for a p2p connection to a specified node
class NodeConn(asyncore.dispatcher):
"""The actual NodeConn class
This class provides an interface for a p2p connection to a specified node."""
messagemap = {
b"version": msg_version,
b"verack": msg_verack,
Expand Down Expand Up @@ -1799,43 +1762,30 @@ def got_data(self):
return
if self.recvbuf[:4] != self.MAGIC_BYTES[self.network]:
raise ValueError("got garbage %s" % repr(self.recvbuf))
if self.ver_recv < 209:
if len(self.recvbuf) < 4 + 12 + 4:
return
command = self.recvbuf[4:4+12].split(b"\x00", 1)[0]
msglen = struct.unpack("<i", self.recvbuf[4+12:4+12+4])[0]
checksum = None
if len(self.recvbuf) < 4 + 12 + 4 + msglen:
return
msg = self.recvbuf[4+12+4:4+12+4+msglen]
self.recvbuf = self.recvbuf[4+12+4+msglen:]
else:
if len(self.recvbuf) < 4 + 12 + 4 + 4:
return
command = self.recvbuf[4:4+12].split(b"\x00", 1)[0]
msglen = struct.unpack("<i", self.recvbuf[4+12:4+12+4])[0]
checksum = self.recvbuf[4+12+4:4+12+4+4]
if len(self.recvbuf) < 4 + 12 + 4 + 4 + msglen:
return
msg = self.recvbuf[4+12+4+4:4+12+4+4+msglen]
th = sha256(msg)
h = sha256(th)
if checksum != h[:4]:
raise ValueError("got bad checksum " + repr(self.recvbuf))
self.recvbuf = self.recvbuf[4+12+4+4+msglen:]
if command in self.messagemap:
if self.messagemap[command] is None:
# Command is known but we don't want/need to handle it
continue
f = BytesIO(msg)
t = self.messagemap[command]()
t.deserialize(f)
self.got_message(t)
else:
logger.warning("Received unknown command from %s:%d: '%s' %s" % (self.dstaddr, self.dstport, str(command), repr(msg)))
raise ValueError("Unknown command: '%s'" % (command))
if len(self.recvbuf) < 4 + 12 + 4 + 4:
return
command = self.recvbuf[4:4+12].split(b"\x00", 1)[0]
msglen = struct.unpack("<i", self.recvbuf[4+12:4+12+4])[0]
checksum = self.recvbuf[4+12+4:4+12+4+4]
if len(self.recvbuf) < 4 + 12 + 4 + 4 + msglen:
return
msg = self.recvbuf[4+12+4+4:4+12+4+4+msglen]
th = sha256(msg)
h = sha256(th)
if checksum != h[:4]:
raise ValueError("got bad checksum " + repr(self.recvbuf))
self.recvbuf = self.recvbuf[4+12+4+4+msglen:]
if command not in self.messagemap:
raise ValueError("Received unknown command from %s:%d: '%s' %s" % (self.dstaddr, self.dstport, command, repr(msg)))
if self.messagemap[command] is None:
# Command is known but we don't want/need to handle it
continue
f = BytesIO(msg)
t = self.messagemap[command]()
t.deserialize(f)
self.got_message(t)
except Exception as e:
logger.exception('got_data:', repr(e))
logger.exception('Error reading message:', repr(e))
raise

def send_message(self, message, pushbuf=False):
Expand All @@ -1848,10 +1798,9 @@ def send_message(self, message, pushbuf=False):
tmsg += command
tmsg += b"\x00" * (12 - len(command))
tmsg += struct.pack("<I", len(data))
if self.ver_send >= 209:
th = sha256(data)
h = sha256(th)
tmsg += h[:4]
th = sha256(data)
h = sha256(th)
tmsg += h[:4]
tmsg += data
with mininode_lock:
if (len(self.sendbuf) == 0 and not pushbuf):
Expand All @@ -1865,9 +1814,6 @@ def send_message(self, message, pushbuf=False):
self.last_sent = time.time()

def got_message(self, message):
if message.command == b"version":
if message.nVersion <= BIP0031_VERSION:
self.messagemap[b'ping'] = msg_ping_prebip31
if self.last_sent + 30 * 60 < time.time():
self.send_message(self.messagemap[b'ping']())
self._log_message("receive", message)
Expand Down Expand Up @@ -1924,15 +1870,6 @@ def network_thread_join(timeout=10):
thread.join(timeout)
assert not thread.is_alive()

# An exception we can raise if we detect a potential disconnect
# (p2p or rpc) before the test is complete
class EarlyDisconnectError(Exception):
def __init__(self, value):
self.value = value

def __str__(self):
return repr(self.value)

class P2PDataStore(NodeConnCB):
"""A P2P data store class.
Expand Down

0 comments on commit c89c8f2

Please sign in to comment.