diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/Endpoint.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/Endpoint.java index a06519bc6d..41cec4ac5f 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/Endpoint.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/Endpoint.java @@ -14,7 +14,6 @@ */ package org.hyperledger.besu.ethereum.p2p.discovery; -import static com.google.common.base.Preconditions.checkArgument; import static org.hyperledger.besu.util.NetworkUtility.checkPort; import static org.hyperledger.besu.util.Preconditions.checkGuard; @@ -36,17 +35,15 @@ import org.apache.tuweni.bytes.Bytes; * used in various Discovery messages. */ public class Endpoint { - private final String host; + private final Optional host; private final int udpPort; private final Optional tcpPort; public Endpoint(final String host, final int udpPort, final Optional tcpPort) { - checkArgument( - host != null && InetAddresses.isInetAddress(host), "host requires a valid IP address"); checkPort(udpPort, "UDP"); tcpPort.ifPresent(port -> checkPort(port, "TCP")); - this.host = host; + this.host = Optional.ofNullable(host); this.udpPort = udpPort; this.tcpPort = tcpPort; } @@ -66,14 +63,14 @@ public class Endpoint { public EnodeURL toEnode(final Bytes nodeId) { return EnodeURLImpl.builder() .nodeId(nodeId) - .ipAddress(host) + .ipAddress(host.orElse("")) .listeningPort(tcpPort.orElse(udpPort)) .discoveryPort(udpPort) .build(); } public String getHost() { - return host; + return host.orElse(""); } public int getUdpPort() { @@ -144,7 +141,16 @@ public class Endpoint { * @param out The RLP output stream. */ public void encodeInline(final RLPOutput out) { - out.writeInetAddress(InetAddresses.forString(host)); + if (host.isPresent()) { + InetAddress hostAddress = InetAddresses.forString(host.get()); + if (hostAddress != null) { + out.writeInetAddress(hostAddress); + } else { // present, but not a parseable IP address + out.writeNull(); + } + } else { + out.writeNull(); + } out.writeIntScalar(udpPort); tcpPort.ifPresentOrElse(out::writeIntScalar, out::writeNull); } @@ -164,7 +170,13 @@ public class Endpoint { "Invalid number of components in RLP representation of an endpoint: expected 2 or 3 elements but got %s", fieldCount); - final InetAddress addr = in.readInetAddress(); + String hostAddress = null; + if (!in.nextIsNull()) { + InetAddress addr = in.readInetAddress(); + hostAddress = addr.getHostAddress(); + } else { + in.skipNext(); + } final int udpPort = in.readIntScalar(); // Some mainnet packets have been shown to either not have the TCP port field at all, @@ -177,7 +189,7 @@ public class Endpoint { tcpPort = Optional.of(in.readIntScalar()); } } - return new Endpoint(addr.getHostAddress(), udpPort, tcpPort); + return new Endpoint(hostAddress, udpPort, tcpPort); } /** diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java index 6d62f02b37..fa011453b2 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java @@ -232,7 +232,10 @@ public class VertxPeerDiscoveryAgent extends PeerDiscoveryAgent { handleIncomingPacket(endpoint, event.result()); } else { if (event.cause() instanceof PeerDiscoveryPacketDecodingException) { - LOG.debug("Discarding invalid peer discovery packet: {}", event.cause().getMessage()); + LOG.debug( + "Discarding invalid peer discovery packet: {}, {}", + event.cause().getMessage(), + event.cause()); } else { LOG.error("Encountered error while handling packet", event.cause()); } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/DevP2PException.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/DevP2PException.java new file mode 100644 index 0000000000..b5f80faa8e --- /dev/null +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/DevP2PException.java @@ -0,0 +1,22 @@ +/* + * Copyright ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.hyperledger.besu.ethereum.p2p.discovery.internal; + +public class DevP2PException extends RuntimeException { + public DevP2PException(final String reason) { + super(reason); + } +} diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index edbf7636d9..a18f68f8d4 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -467,7 +467,7 @@ public class PeerDiscoveryController { interaction -> { final PingPacketData data = PingPacketData.create( - localPeer.getEndpoint(), + Optional.of(localPeer.getEndpoint()), peer.getEndpoint(), localPeer.getNodeRecord().map(NodeRecord::getSeq).orElse(null)); createPacket( @@ -561,6 +561,7 @@ public class PeerDiscoveryController { private void respondToPing( final PingPacketData packetData, final Bytes pingHash, final DiscoveryPeer sender) { if (packetData.getExpiration() < Instant.now().getEpochSecond()) { + LOG.debug("ignoring expired PING"); return; } // We don't care about the `from` field of the ping, we pong to the `sender` diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PingPacketData.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PingPacketData.java index 5c7aa02655..dba2cafd8e 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PingPacketData.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PingPacketData.java @@ -24,6 +24,8 @@ import org.hyperledger.besu.ethereum.rlp.RLPOutput; import java.util.Optional; import org.apache.tuweni.units.bigints.UInt64; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class PingPacketData implements PacketData { @@ -41,6 +43,7 @@ public class PingPacketData implements PacketData { /* Current sequence number of the sending node’s record */ private final UInt64 enrSeq; + private static final Logger LOG = LoggerFactory.getLogger(PingPacketData.class); private PingPacketData( final Optional maybeFrom, @@ -56,40 +59,97 @@ public class PingPacketData implements PacketData { this.enrSeq = enrSeq; } - public static PingPacketData create(final Endpoint from, final Endpoint to, final UInt64 enrSeq) { + public static PingPacketData create( + final Optional from, final Endpoint to, final UInt64 enrSeq) { checkArgument( enrSeq != null && UInt64.ZERO.compareTo(enrSeq) < 0, "enrSeq cannot be null or negative"); return create(from, to, PacketData.defaultExpiration(), enrSeq); } static PingPacketData create( - final Endpoint from, final Endpoint to, final long expirationSec, final UInt64 enrSeq) { + final Optional from, + final Endpoint to, + final long expirationSec, + final UInt64 enrSeq) { checkArgument( enrSeq != null && UInt64.ZERO.compareTo(enrSeq) < 0, "enrSeq cannot be null or negative"); - return new PingPacketData(Optional.of(from), to, expirationSec, enrSeq); + return new PingPacketData(from, to, expirationSec, enrSeq); } public static PingPacketData readFrom(final RLPInput in) { in.enterList(); // The first element signifies the "version", but this value is ignored as of EIP-8 in.readBigIntegerScalar(); - final Optional from = Endpoint.maybeDecodeStandalone(in); - final Endpoint to = Endpoint.decodeStandalone(in); + Optional from = Optional.empty(); + Optional to = Optional.empty(); + if (in.nextIsList()) { + to = Endpoint.maybeDecodeStandalone(in); + // https://github.com/ethereum/devp2p/blob/master/discv4.md#ping-packet-0x01 + if (in.nextIsList()) { // if there are two, the first is the from address, next is the to + // address + from = to; + to = Endpoint.maybeDecodeStandalone(in); + } + } else { + throw new DevP2PException("missing address in ping packet"); + } final long expiration = in.readLongScalar(); UInt64 enrSeq = null; if (!in.isEndOfCurrentList()) { try { - enrSeq = UInt64.valueOf(in.readLongScalar()); + enrSeq = UInt64.valueOf(in.readBigIntegerScalar()); + LOG.debug("read PING enr as long scalar"); } catch (MalformedRLPInputException malformed) { + LOG.debug("failed to read PING enr as scalar, trying to read bytes instead"); enrSeq = UInt64.fromBytes(in.readBytes()); } } in.leaveListLenient(); + return new PingPacketData(from, to.get(), expiration, enrSeq); + } + + /** + * Used by test classes to read legacy encodes of Pings which used a byte array for the ENR field + * + * @deprecated Only to be used by internal tests to confirm backward compatibility. + * @param in input stream to read from + * @return PingPacketData parsed from input, using legacy encode. + */ + @Deprecated + public static PingPacketData legacyReadFrom(final RLPInput in) { // only for testing, do not use + in.enterList(); + // The first element signifies the "version", but this value is ignored as of EIP-8 + in.readBigIntegerScalar(); + final Optional from = Endpoint.maybeDecodeStandalone(in); + final Endpoint to = Endpoint.decodeStandalone(in); + final long expiration = in.readLongScalar(); + UInt64 enrSeq = null; + if (!in.isEndOfCurrentList()) { + enrSeq = UInt64.fromBytes(in.readBytes()); + } + in.leaveListLenient(); return new PingPacketData(from, to, expiration, enrSeq); } @Override public void writeTo(final RLPOutput out) { + out.startList(); + out.writeIntScalar(VERSION); + if (maybeFrom.isPresent()) { + maybeFrom.get().encodeStandalone(out); + } + to.encodeStandalone(out); + out.writeLongScalar(expiration); + out.writeBigIntegerScalar(enrSeq.toBigInteger()); + out.endList(); + } + + /** + * @deprecated Only to be used by internal tests to confirm backward compatibility. + * @param out stream to write to + */ + @Deprecated + public void legacyWriteTo(final RLPOutput out) { out.startList(); out.writeIntScalar(VERSION); maybeFrom @@ -100,7 +160,13 @@ public class PingPacketData implements PacketData { .encodeStandalone(out); to.encodeStandalone(out); out.writeLongScalar(expiration); - out.writeLongScalar(enrSeq.toLong()); + out.writeBytes( + getEnrSeq() + .orElseThrow( + () -> + new IllegalStateException( + "Attempting to serialize invalid PING packet. Missing 'enrSeq' field")) + .toBytes()); out.endList(); } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PongPacketData.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PongPacketData.java index f3d0d9b101..80ed430242 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PongPacketData.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PongPacketData.java @@ -23,6 +23,8 @@ import java.util.Optional; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.units.bigints.UInt64; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class PongPacketData implements PacketData { @@ -37,6 +39,7 @@ public class PongPacketData implements PacketData { /* Current sequence number of the sending node’s record */ private final UInt64 enrSeq; + private static final Logger LOG = LoggerFactory.getLogger(PongPacketData.class); private PongPacketData( final Endpoint to, final Bytes pingHash, final long expiration, final UInt64 enrSeq) { @@ -59,8 +62,10 @@ public class PongPacketData implements PacketData { UInt64 enrSeq = null; if (!in.isEndOfCurrentList()) { try { - enrSeq = UInt64.valueOf(in.readLongScalar()); + enrSeq = UInt64.valueOf(in.readBigIntegerScalar()); + LOG.debug("read PONG enr from scalar"); } catch (MalformedRLPInputException malformed) { + LOG.debug("failed to read PONG enr from scalar, trying as byte array"); enrSeq = UInt64.fromBytes(in.readBytes()); } } @@ -74,7 +79,48 @@ public class PongPacketData implements PacketData { to.encodeStandalone(out); out.writeBytes(pingHash); out.writeLongScalar(expiration); - out.writeLongScalar(enrSeq.toLong()); + out.writeBigIntegerScalar(enrSeq.toBigInteger()); + out.endList(); + } + + /** + * Used by test classes to read legacy encodes of Pongs which used a byte array for the ENR field + * + * @deprecated Only to be used by internal tests to confirm backward compatibility. + * @param in input stream being read from + * @return PongPacketData parsed from input, using legacy encode + */ + @Deprecated + public static PongPacketData legacyReadFrom(final RLPInput in) { + in.enterList(); + final Endpoint to = Endpoint.decodeStandalone(in); + final Bytes hash = in.readBytes(); + final long expiration = in.readLongScalar(); + UInt64 enrSeq = null; + if (!in.isEndOfCurrentList()) { + enrSeq = UInt64.fromBytes(in.readBytes()); + } + in.leaveListLenient(); + return new PongPacketData(to, hash, expiration, enrSeq); + } + + /** + * @deprecated Only to be used by internal tests to confirm backward compatibility. + * @param out output stream being written to + */ + @Deprecated + public void legacyWriteTo(final RLPOutput out) { + out.startList(); + to.encodeStandalone(out); + out.writeBytes(pingHash); + out.writeLongScalar(expiration); + out.writeBytes( + getEnrSeq() + .orElseThrow( + () -> + new IllegalStateException( + "Attempting to serialize invalid PONG packet. Missing 'enrSeq' field")) + .toBytes()); out.endList(); } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java index 0360985605..171e955614 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java @@ -97,7 +97,7 @@ public class PeerDiscoveryTestHelper { return Packet.create( PacketType.PING, PingPacketData.create( - fromAgent.getAdvertisedPeer().get().getEndpoint(), + Optional.of(fromAgent.getAdvertisedPeer().get().getEndpoint()), toAgent.getAdvertisedPeer().get().getEndpoint(), UInt64.ONE), fromAgent.getNodeKey()); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 7989beffbf..3d88d11fd2 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -124,7 +124,8 @@ public class PeerDiscoveryControllerTest { // Mock the creation of the PING packet, so that we can control the hash, // which gets validated when receiving the PONG. final PingPacketData mockPing = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet mockPacket = Packet.create(PacketType.PING, mockPing, nodeKeys.get(0)); mockPingPacketCreation(mockPacket); @@ -195,7 +196,8 @@ public class PeerDiscoveryControllerTest { // Mock the creation of the PING packet, so that we can control the hash, // which gets validated when receiving the PONG. final PingPacketData mockPing = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet mockPacket = Packet.create(PacketType.PING, mockPing, nodeKeys.get(0)); mockPingPacketCreation(mockPacket); @@ -249,7 +251,8 @@ public class PeerDiscoveryControllerTest { // Mock the creation of the PING packet, so that we can control the hash, // which gets validated when receiving the PONG. final PingPacketData mockPing = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet mockPacket = Packet.create(PacketType.PING, mockPing, nodeKeys.get(0)); mockPingPacketCreation(mockPacket); @@ -284,7 +287,8 @@ public class PeerDiscoveryControllerTest { // Setup ping to be sent to discoPeer final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = - PingPacketData.create(localEndpoint, discoPeer.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localEndpoint), discoPeer.getEndpoint(), UInt64.ONE); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(discoPeer, discoPeerPing); @@ -313,7 +317,7 @@ public class PeerDiscoveryControllerTest { final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = PingPacketData.create( - localEndpoint, + Optional.ofNullable(localEndpoint), discoPeer.getEndpoint(), Instant.now().getEpochSecond() - PacketData.DEFAULT_EXPIRATION_PERIOD_SEC, UInt64.ONE); @@ -344,7 +348,8 @@ public class PeerDiscoveryControllerTest { // Mock the creation of the PING packet, so that we can control the hash, which gets validated // when receiving the PONG. final PingPacketData mockPing = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet mockPacket = Packet.create(PacketType.PING, mockPing, nodeKeys.get(0)); mockPingPacketCreation(mockPacket); @@ -404,7 +409,8 @@ public class PeerDiscoveryControllerTest { // when // processing the PONG. final PingPacketData mockPing = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet mockPacket = Packet.create(PacketType.PING, mockPing, nodeKeys.get(0)); mockPingPacketCreation(mockPacket); @@ -453,7 +459,8 @@ public class PeerDiscoveryControllerTest { // when // processing the PONG. final PingPacketData mockPing = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet mockPacket = Packet.create(PacketType.PING, mockPing, nodeKeys.get(0)); mockPingPacketCreation(mockPacket); controller.setRetryDelayFunction((prev) -> 999999999L); @@ -519,7 +526,8 @@ public class PeerDiscoveryControllerTest { // Mock the creation of the PING packet, so that we can control the hash, which gets validated // when processing the PONG. final PingPacketData pingPacketData = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(pingPacket); @@ -663,7 +671,8 @@ public class PeerDiscoveryControllerTest { // Setup ping to be sent to discoPeer List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); PingPacketData pingPacketData = - PingPacketData.create(localEndpoint, discoPeer.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localEndpoint), discoPeer.getEndpoint(), UInt64.ONE); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(discoPeer, discoPeerPing); @@ -680,13 +689,17 @@ public class PeerDiscoveryControllerTest { // Setup ping to be sent to otherPeer after neighbors packet is received nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); - pingPacketData = PingPacketData.create(localEndpoint, otherPeer.getEndpoint(), UInt64.ONE); + pingPacketData = + PingPacketData.create( + Optional.ofNullable(localEndpoint), otherPeer.getEndpoint(), UInt64.ONE); final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(otherPeer, pingPacket); // Setup ping to be sent to otherPeer2 after neighbors packet is received nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); - pingPacketData = PingPacketData.create(localEndpoint, otherPeer2.getEndpoint(), UInt64.ONE); + pingPacketData = + PingPacketData.create( + Optional.ofNullable(localEndpoint), otherPeer2.getEndpoint(), UInt64.ONE); final Packet pingPacket2 = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(otherPeer2, pingPacket2); @@ -742,7 +755,8 @@ public class PeerDiscoveryControllerTest { // Setup ping to be sent to discoPeer List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); PingPacketData pingPacketData = - PingPacketData.create(localEndpoint, discoPeer.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localEndpoint), discoPeer.getEndpoint(), UInt64.ONE); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(discoPeer, discoPeerPing); @@ -758,13 +772,17 @@ public class PeerDiscoveryControllerTest { // Setup ping to be sent to otherPeer after neighbors packet is received nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); - pingPacketData = PingPacketData.create(localEndpoint, otherPeer.getEndpoint(), UInt64.ONE); + pingPacketData = + PingPacketData.create( + Optional.ofNullable(localEndpoint), otherPeer.getEndpoint(), UInt64.ONE); final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(otherPeer, pingPacket); // Setup ping to be sent to otherPeer2 after neighbors packet is received nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); - pingPacketData = PingPacketData.create(localEndpoint, otherPeer2.getEndpoint(), UInt64.ONE); + pingPacketData = + PingPacketData.create( + Optional.ofNullable(localEndpoint), otherPeer2.getEndpoint(), UInt64.ONE); final Packet pingPacket2 = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(otherPeer2, pingPacket2); @@ -797,7 +815,8 @@ public class PeerDiscoveryControllerTest { // Setup ping to be sent to discoPeer final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = - PingPacketData.create(localEndpoint, discoPeer.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localEndpoint), discoPeer.getEndpoint(), UInt64.ONE); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(discoPeer, discoPeerPing); @@ -837,7 +856,8 @@ public class PeerDiscoveryControllerTest { // Setup ping to be sent to discoPeer final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = - PingPacketData.create(localEndpoint, discoPeer.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localEndpoint), discoPeer.getEndpoint(), UInt64.ONE); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(discoPeer, discoPeerPing); @@ -876,7 +896,8 @@ public class PeerDiscoveryControllerTest { // Setup ping to be sent to discoPeer final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = - PingPacketData.create(localEndpoint, discoPeer.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localEndpoint), discoPeer.getEndpoint(), UInt64.ONE); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(discoPeer, discoPeerPing); @@ -919,7 +940,8 @@ public class PeerDiscoveryControllerTest { // Setup ping to be sent to discoPeer final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = - PingPacketData.create(localEndpoint, discoPeer.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localEndpoint), discoPeer.getEndpoint(), UInt64.ONE); final Packet discoPeerPing = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(discoPeer, discoPeerPing); @@ -948,7 +970,8 @@ public class PeerDiscoveryControllerTest { // Mock the creation of the PING packet to control hash for PONG. final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); @@ -987,7 +1010,8 @@ public class PeerDiscoveryControllerTest { // Mock the creation of PING packets to control hash PONG packets. final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); mockPingPacketCreation(pingPacket); @@ -1046,7 +1070,8 @@ public class PeerDiscoveryControllerTest { // Mock the creation of the PING packet to control hash for PONG. final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); @@ -1290,7 +1315,8 @@ public class PeerDiscoveryControllerTest { // Mock the creation of the PING packet to control hash for PONG. final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); final PingPacketData pingPacketData = - PingPacketData.create(localPeer.getEndpoint(), peers.get(0).getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet pingPacket = Packet.create(PacketType.PING, pingPacketData, nodeKeys.get(0)); final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); @@ -1348,7 +1374,8 @@ public class PeerDiscoveryControllerTest { final Packet packet = mock(Packet.class); final PingPacketData pingPacketData = - PingPacketData.create(from.getEndpoint(), to.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(from.getEndpoint()), to.getEndpoint(), UInt64.ONE); when(packet.getPacketData(any())).thenReturn(Optional.of(pingPacketData)); final Bytes id = from.getId(); when(packet.getNodeId()).thenReturn(id); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java index c1b7607271..e36f795e74 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java @@ -75,7 +75,8 @@ public class PeerDiscoveryTableRefreshTest { controller.start(); final PingPacketData mockPing = - PingPacketData.create(localPeer.getEndpoint(), remotePeer.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), remotePeer.getEndpoint(), UInt64.ONE); final Packet mockPingPacket = Packet.create(PacketType.PING, mockPing, localKeyPair); doAnswer( @@ -89,7 +90,8 @@ public class PeerDiscoveryTableRefreshTest { // Send a PING, so as to add a Peer in the controller. final PingPacketData ping = - PingPacketData.create(remotePeer.getEndpoint(), localPeer.getEndpoint(), UInt64.ONE); + PingPacketData.create( + Optional.ofNullable(remotePeer.getEndpoint()), localPeer.getEndpoint(), UInt64.ONE); final Packet pingPacket = Packet.create(PacketType.PING, ping, remoteKeyPair); controller.onMessage(pingPacket, remotePeer); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PingPacketDataTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PingPacketDataTest.java index bb2630f140..89afd235a3 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PingPacketDataTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PingPacketDataTest.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat; import org.hyperledger.besu.ethereum.p2p.discovery.Endpoint; import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput; import org.hyperledger.besu.ethereum.rlp.RLP; +import org.hyperledger.besu.ethereum.rlp.RLPOutput; import java.time.Instant; import java.util.Optional; @@ -36,7 +37,7 @@ public class PingPacketDataTest { final Endpoint from = new Endpoint("127.0.0.1", 30303, Optional.of(30303)); final Endpoint to = new Endpoint("127.0.0.2", 30303, Optional.empty()); final UInt64 enrSeq = UInt64.ONE; - final PingPacketData packet = PingPacketData.create(from, to, enrSeq); + final PingPacketData packet = PingPacketData.create(Optional.of(from), to, enrSeq); final Bytes serialized = RLP.encode(packet::writeTo); final PingPacketData deserialized = PingPacketData.readFrom(RLP.input(serialized)); @@ -74,6 +75,30 @@ public class PingPacketDataTest { assertThat(deserialized.getEnrSeq().get()).isEqualTo(enrSeq); } + @Test + public void handlesNullEnr() { + final int version = 4; + final Endpoint from = new Endpoint("127.0.0.1", 30303, Optional.of(30303)); + final Endpoint to = new Endpoint("127.0.0.2", 30303, Optional.empty()); + final long time = System.currentTimeMillis(); + + final BytesValueRLPOutput out = new BytesValueRLPOutput(); + out.startList(); + out.writeIntScalar(version); + from.encodeStandalone(out); + to.encodeStandalone(out); + out.writeLongScalar(time); + out.endList(); + + final Bytes serialized = out.encoded(); + final PingPacketData deserialized = PingPacketData.readFrom(RLP.input(serialized)); + + assertThat(deserialized.getFrom()).contains(from); + assertThat(deserialized.getTo()).isEqualTo(to); + assertThat(deserialized.getExpiration()).isEqualTo(time); + assertThat(deserialized.getEnrSeq().isPresent()).isFalse(); + } + @Test public void handlesLegacyENREncode() { final int version = 4; @@ -101,6 +126,81 @@ public class PingPacketDataTest { assertThat(deserialized.getEnrSeq().get()).isEqualTo(enrSeq); } + @Test + public void handleOptionalSourceIP() { + + final Endpoint to = new Endpoint("127.0.0.2", 30303, Optional.empty()); + final long time = System.currentTimeMillis(); + final UInt64 enrSeq = UInt64.ONE; + + final PingPacketData anon = PingPacketData.create(Optional.empty(), to, time, enrSeq); + + final BytesValueRLPOutput out = new BytesValueRLPOutput(); + anon.writeTo(out); + final Bytes serialized = out.encoded(); + System.out.println(serialized.toHexString()); + final PingPacketData deserialized = PingPacketData.readFrom(RLP.input(serialized)); + + assertThat(deserialized.getFrom()).isEmpty(); + assertThat(deserialized.getTo()).isEqualTo(to); + assertThat(deserialized.getExpiration()).isEqualTo(time); + assertThat(deserialized.getEnrSeq().isPresent()).isTrue(); + assertThat(deserialized.getEnrSeq().get()).isEqualTo(enrSeq); + } + + @Test + public void handleSourcePortNullHost() { + final Endpoint to = new Endpoint("127.0.0.2", 30303, Optional.empty()); + final BytesValueRLPOutput out = new BytesValueRLPOutput(); + final UInt64 enrSeq = UInt64.MAX_VALUE; + final long time = System.currentTimeMillis(); + + out.startList(); + out.writeIntScalar(4); + + ((RLPOutput) out).startList(); + out.writeNull(); + out.writeIntScalar(30303); + out.writeNull(); + ((RLPOutput) out).endList(); + + to.encodeStandalone(out); + out.writeLongScalar(time); + out.writeBigIntegerScalar(enrSeq.toBigInteger()); + out.endList(); + + final Bytes serialized = out.encoded(); + final PingPacketData deserialized = PingPacketData.readFrom(RLP.input(serialized)); + + assertThat(deserialized.getFrom()).isPresent(); + assertThat(deserialized.getFrom().get().getUdpPort()).isEqualTo(30303); + assertThat(deserialized.getFrom().get().getHost().isEmpty()).isTrue(); + assertThat(deserialized.getFrom().get().getTcpPort().isEmpty()).isTrue(); + + assertThat(deserialized.getTo()).isEqualTo(to); + assertThat(deserialized.getExpiration()).isEqualTo(time); + assertThat(deserialized.getEnrSeq().isPresent()).isTrue(); + assertThat(deserialized.getEnrSeq().get()).isEqualTo(enrSeq); + } + + @Test + public void legacyHandlesScalarEncode() { + final Endpoint from = new Endpoint("127.0.0.1", 30303, Optional.of(30303)); + final Endpoint to = new Endpoint("127.0.0.2", 30303, Optional.empty()); + final UInt64 enrSeq = UInt64.MAX_VALUE; + final PingPacketData ping = PingPacketData.create(Optional.of(from), to, enrSeq); + + final BytesValueRLPOutput out = new BytesValueRLPOutput(); + ping.writeTo(out); + + final PingPacketData legacyPing = PingPacketData.legacyReadFrom(RLP.input(out.encoded())); + + assertThat(legacyPing.getFrom().get()).isEqualTo(from); + assertThat(legacyPing.getTo()).isEqualTo(to); + assertThat(legacyPing.getEnrSeq().isPresent()).isTrue(); + assertThat(legacyPing.getEnrSeq().get()).isEqualTo(enrSeq); + } + @Test public void readFrom_withExtraFields() { final int version = 4; diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PongPacketDataTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PongPacketDataTest.java index d760ec26f4..0d13604a57 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PongPacketDataTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PongPacketDataTest.java @@ -96,6 +96,25 @@ public class PongPacketDataTest { assertThat(deserialized.getEnrSeq().get()).isEqualTo(enrSeq); } + @Test + public void legacyHandlesScalar() { + + final Endpoint to = new Endpoint("127.0.0.2", 30303, Optional.empty()); + final UInt64 enrSeq = UInt64.MAX_VALUE; + final Bytes32 hash = Bytes32.fromHexStringLenient("0x1234"); + final PongPacketData pong = PongPacketData.create(to, hash, enrSeq); + + final BytesValueRLPOutput out = new BytesValueRLPOutput(); + pong.writeTo(out); + + final PongPacketData legacyPong = PongPacketData.legacyReadFrom(RLP.input(out.encoded())); + + assertThat(legacyPong.getTo()).isEqualTo(to); + assertThat(legacyPong.getPingHash()).isEqualTo(hash); + assertThat(legacyPong.getEnrSeq().isPresent()).isTrue(); + assertThat(legacyPong.getEnrSeq().get()).isEqualTo(enrSeq); + } + @Test public void readFrom_withExtraFields() { final long time = System.currentTimeMillis();