Skip to content

Commit

Permalink
p2p/discovery: Workaround a parity bug that causes pongs with wrong t…
Browse files Browse the repository at this point in the history
…oken

Parity 1.10 (and earlier) nodes send the wrong token on pong messages
(openethereum/parity-ethereum#8038). This change keeps
a mapping of wrong tokens to the ones we expect, and uses that whenever
we get a pong with one of those wrong tokens.
  • Loading branch information
gsalgado committed Jul 13, 2018
1 parent 3b7cc5b commit d7e1cf2
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 18 deletions.
8 changes: 3 additions & 5 deletions p2p/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,9 @@
)
ROPSTEN_BOOTNODES = (
'enode://30b7ab30a01c124a6cceca36863ece12c4f5fa68e3ba9b0b51407ccc002eeed3b3102d20a88f1c1d3c3154e2449317b8ef95090e77b312d5cc39354f86d5d606@52.176.7.10:30303', # noqa: E501
# FIXME: Re-enable those bootnodes once we've figured out why we can't "bond" with them:
# https://github.com/ethereum/py-evm/issues/438
# 'enode://865a63255b3bb68023b6bffd5095118fcc13e79dcf014fe4e47e065c350c7cc72af2e53eff895f11ba1bbb6a2b33271c1116ee870f266618eadfc2e78aa7349c@52.176.100.77:30303', # noqa: E501
# 'enode://6332792c4a00e3e4ee0926ed89e0d27ef985424d97b6a45bf0f23e51f0dcb5e66b875777506458aea7af6f9e4ffb69f43f3778ee73c81ed9d34c51c4b16b0b0f@52.232.243.152:30303', # noqa: E501
# 'enode://94c15d1b9e2fe7ce56e458b9a3b672ef11894ddedd0c6f247e0f1d3487f52b66208fb4aeb8179fce6e3a749ea93ed147c37976d67af557508d199d9594c35f09@192.81.208.223:30303', # noqa: E501
'enode://865a63255b3bb68023b6bffd5095118fcc13e79dcf014fe4e47e065c350c7cc72af2e53eff895f11ba1bbb6a2b33271c1116ee870f266618eadfc2e78aa7349c@52.176.100.77:30303', # noqa: E501
'enode://6332792c4a00e3e4ee0926ed89e0d27ef985424d97b6a45bf0f23e51f0dcb5e66b875777506458aea7af6f9e4ffb69f43f3778ee73c81ed9d34c51c4b16b0b0f@52.232.243.152:30303', # noqa: E501
'enode://94c15d1b9e2fe7ce56e458b9a3b672ef11894ddedd0c6f247e0f1d3487f52b66208fb4aeb8179fce6e3a749ea93ed147c37976d67af557508d199d9594c35f09@192.81.208.223:30303', # noqa: E501
)

# Maximum peers number, we'll try to keep open connections up to this number of peers
Expand Down
37 changes: 25 additions & 12 deletions p2p/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def connection_made(self, transport: asyncio.BaseTransport) -> None:
self.transport = cast(asyncio.DatagramTransport, transport)

async def bootstrap(self) -> None:
self.logger.debug("boostrapping with %s", self.bootstrap_nodes)
self.logger.info("boostrapping with %s", self.bootstrap_nodes)

This comment has been minimized.

Copy link
@veox

veox Sep 28, 2018

Contributor

Typo remains: boostrapping. 😁

try:
await self.kademlia.bootstrap(self.bootstrap_nodes, self.cancel_token)
except OperationCancelled as e:
Expand Down Expand Up @@ -221,6 +221,11 @@ def send_ping(self, node: kademlia.Node) -> bytes:
# Return the msg hash, which is used as a token to identify pongs.
token = message[:MAC_SIZE]
self.logger.debug('>>> ping %s (token == %s)', node, encode_hex(token))
# XXX: This hack is needed because there are lots of parity 1.10 nodes out there that send
# the wrong token on pong msgs (https://github.com/paritytech/parity/issues/8038). We
# should get rid of this once there are no longer too many parity 1.10 nodes out there.
parity_token = keccak(message[HEAD_SIZE + 1:])
self.kademlia.parity_pong_tokens[parity_token] = token
return token

def send_find_node(self, node: kademlia.Node, target_node_id: int) -> None:
Expand Down Expand Up @@ -431,28 +436,36 @@ def _unpack(message: bytes) -> Tuple[datatypes.PublicKey, int, List[Any], bytes]


def _test() -> None:
import argparse
import signal
from p2p import constants
from p2p import ecies

logging.basicConfig(level=logging.INFO, format='%(levelname)s: %(message)s')

loop = asyncio.get_event_loop()
loop.set_debug(True)

listen_host = '0.0.0.0'
# Listen on a port other than 30303 in case we want to test against a local geth instance
parser = argparse.ArgumentParser()
parser.add_argument('-bootnode', type=str, help="The enode to use as bootnode")
parser.add_argument('-debug', action="store_true")
args = parser.parse_args()

log_level = logging.INFO
if args.debug:
log_level = logging.DEBUG
logging.basicConfig(level=log_level, format='%(asctime)s %(levelname)s: %(message)s')

listen_host = '127.0.0.1'
# Listen on a port other than 30303 so that we can test against a local geth instance
# running on that port.
listen_port = 30303
listen_port = 30304
privkey = ecies.generate_privkey()
addr = kademlia.Address(listen_host, listen_port, listen_port)
bootstrap_nodes = tuple(
kademlia.Node.from_uri(enode) for enode in constants.ROPSTEN_BOOTNODES
)
if args.bootnode:
bootstrap_nodes = tuple([kademlia.Node.from_uri(args.bootnode)])
else:
bootstrap_nodes = tuple(
kademlia.Node.from_uri(enode) for enode in constants.ROPSTEN_BOOTNODES)
discovery = DiscoveryProtocol(privkey, addr, bootstrap_nodes)
# local_bootnodes = [
# kademlia.Node.from_uri('enode://0x3a514176466fa815ed481ffad09110a2d344f6c9b78c1d14afc351c3a51be33d8072e77939dc03ba44790779b7a1025baf3003f6732430e20cd9b76d953391b3@127.0.0.1:30303')] # noqa: E501
# discovery = DiscoveryProtocol(privkey, addr, local_bootnodes)
loop.run_until_complete(
loop.create_datagram_endpoint(lambda: discovery, local_addr=('0.0.0.0', listen_port)))

Expand Down
18 changes: 17 additions & 1 deletion p2p/kademlia.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
Any,
Callable,
cast,
Dict,
Hashable,
Iterable,
Iterator,
Expand All @@ -24,6 +25,8 @@
)
from urllib import parse as urlparse

import cytoolz

from eth_utils import (
big_endian_to_int,
decode_hex,
Expand Down Expand Up @@ -396,6 +399,7 @@ def __init__(self, node: Node, wire: 'DiscoveryProtocol') -> None:
self.pong_callbacks = CallbackManager()
self.ping_callbacks = CallbackManager()
self.neighbours_callbacks = CallbackManager()
self.parity_pong_tokens: Dict[bytes, bytes] = {}

def recv_neighbours(self, remote: Node, neighbours: List[Node]) -> None:
"""Process a neighbours response.
Expand All @@ -421,13 +425,25 @@ def recv_pong(self, remote: Node, token: bytes) -> None:
left to the callback from pong_callbacks, which is added (and removed after it's done
or timed out) in wait_pong().
"""
# XXX: This hack is needed because there are lots of parity 1.10 nodes out there that send
# the wrong token on pong msgs (https://github.com/paritytech/parity/issues/8038). We
# should get rid of this once there are no longer too many parity 1.10 nodes out there.
if token in self.parity_pong_tokens:
# This is a pong from a buggy parity node, so need to lookup the actual token we're
# expecting.
token = self.parity_pong_tokens.pop(token)
else:
# This is a pong from a non-buggy node, so just cleanup self.parity_pong_tokens.
self.parity_pong_tokens = cytoolz.valfilter(
lambda val: val != token, self.parity_pong_tokens)

self.logger.debug('<<< pong from %s (token == %s)', remote, encode_hex(token))
pingid = self._mkpingid(token, remote)

try:
callback = self.pong_callbacks.get_callback(pingid)
except KeyError:
self.logger.debug('unexpected pong from %s (token == %s)', remote, encode_hex(token))
self.logger.info('unexpected pong from %s (token == %s)', remote, encode_hex(token))
else:
callback()

Expand Down

0 comments on commit d7e1cf2

Please sign in to comment.