diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramer.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramer.java index 918c55aeb9..088bbab447 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramer.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramer.java @@ -168,12 +168,17 @@ protected void decode(final ChannelHandlerContext ctx, final ByteBuf in, final L } private Peer createPeer(final PeerInfo peerInfo, final ChannelHandlerContext ctx) { - InetSocketAddress remoteAddress = ((InetSocketAddress) ctx.channel().remoteAddress()); + final InetSocketAddress remoteAddress = ((InetSocketAddress) ctx.channel().remoteAddress()); + int port = peerInfo.getPort(); + if (port == 0) { + // Most peers advertise a port of "0", just set a default best guess in this case + port = EnodeURL.DEFAULT_LISTENING_PORT; + } return DefaultPeer.fromEnodeURL( EnodeURL.builder() .nodeId(peerInfo.getNodeId()) .ipAddress(remoteAddress.getAddress()) - .listeningPort(peerInfo.getPort()) + .listeningPort(port) .build()); } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/Peer.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/Peer.java index 863e1e9f07..fda98361e7 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/Peer.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/Peer.java @@ -27,7 +27,7 @@ public interface Peer extends PeerId { * @return The generated peer ID. */ static BytesValue randomId() { - final byte[] id = new byte[64]; + final byte[] id = new byte[EnodeURL.NODE_ID_SIZE]; SecureRandomProvider.publicSecureRandom().nextBytes(id); return BytesValue.wrap(id); } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java index f7bd0daf56..0630d444ec 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java @@ -77,6 +77,12 @@ public List getCapabilities() { return capabilities; } + /** + * This value is meant to represent the port at which a peer is listening for connections. + * However, most peers actually advertise a port of "0" so this value is not reliable. + * + * @return (Unreliable) The tcp port on which the peer is listening for connections + */ public int getPort() { return port; } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramerTest.java index 2188bb05e2..e588b0248e 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramerTest.java @@ -172,6 +172,59 @@ public void decode_handlesHello() throws ExecutionException, InterruptedExceptio assertThat(out.size()).isEqualTo(1); } + @Test + public void decode_handlesHelloFromPeerWithAdvertisedPortOf0() + throws ExecutionException, InterruptedException { + ChannelFuture future = NettyMocks.channelFuture(false); + when(channel.closeFuture()).thenReturn(future); + + final Peer peer = createRemotePeer(); + final PeerInfo remotePeerInfo = + new PeerInfo( + peerInfo.getVersion(), + peerInfo.getClientId(), + peerInfo.getCapabilities(), + 0, + peer.getId()); + final DeFramer deFramer = createDeFramer(null); + + HelloMessage helloMessage = HelloMessage.create(remotePeerInfo); + ByteBuf data = Unpooled.wrappedBuffer(helloMessage.getData().extractArray()); + when(framer.deframe(eq(data))) + .thenReturn(new RawMessage(helloMessage.getCode(), helloMessage.getData())) + .thenReturn(null); + List out = new ArrayList<>(); + deFramer.decode(ctx, data, out); + + assertThat(connectFuture).isDone(); + assertThat(connectFuture).isNotCompletedExceptionally(); + PeerConnection peerConnection = connectFuture.get(); + assertThat(peerConnection.getPeerInfo()).isEqualTo(remotePeerInfo); + assertThat(out).isEmpty(); + + final EnodeURL expectedEnode = + EnodeURL.builder() + .ipAddress(remoteAddress.getAddress()) + .nodeId(peer.getId()) + // Listening port should be replaced with default port + .listeningPort(EnodeURL.DEFAULT_LISTENING_PORT) + .build(); + assertThat(peerConnection.getPeer().getEnodeURL()).isEqualTo(expectedEnode); + + // Next phase of pipeline should be setup + verify(pipeline, times(1)).addLast(any()); + + // Next message should be pushed out + PingMessage nextMessage = PingMessage.get(); + ByteBuf nextData = Unpooled.wrappedBuffer(nextMessage.getData().extractArray()); + when(framer.deframe(eq(nextData))) + .thenReturn(new RawMessage(nextMessage.getCode(), nextMessage.getData())) + .thenReturn(null); + verify(pipeline, times(1)).addLast(any()); + deFramer.decode(ctx, nextData, out); + assertThat(out.size()).isEqualTo(1); + } + @Test public void decode_handlesUnexpectedPeerId() { ChannelFuture future = NettyMocks.channelFuture(false); diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java index 98e189223f..9bbcd26be1 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/DefaultCommandValues.java @@ -57,7 +57,6 @@ public interface DefaultCommandValues { SyncMode DEFAULT_SYNC_MODE = SyncMode.FULL; int FAST_SYNC_MAX_WAIT_TIME = 0; int FAST_SYNC_MIN_PEER_COUNT = 5; - int P2P_PORT = 30303; int DEFAULT_MAX_PEERS = 25; static Path getDefaultPantheonDataPath(final Object command) { diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index c5523c22b7..e009e9135f 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -240,7 +240,7 @@ void setBootnodes(final List values) { paramLabel = MANDATORY_PORT_FORMAT_HELP, description = "Port on which to listen for p2p communication (default: ${DEFAULT-VALUE})", arity = "1") - private final Integer p2pPort = P2P_PORT; + private final Integer p2pPort = EnodeURL.DEFAULT_LISTENING_PORT; @Option( names = {"--network-id"}, diff --git a/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java b/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java index fa08ac32bd..33467a07bb 100644 --- a/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java +++ b/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java @@ -29,7 +29,8 @@ public class EnodeURL { - private static final int NODE_ID_SIZE = 64; + public static final int DEFAULT_LISTENING_PORT = 30303; + public static final int NODE_ID_SIZE = 64; private static final Pattern DISCPORT_QUERY_STRING_REGEX = Pattern.compile("^discport=([0-9]{1,5})$"); private static final Pattern NODE_ID_PATTERN = Pattern.compile("^[0-9a-fA-F]{128}$");