Improve logging of peer disconnects due to subprotocol triggered reason (#6886)

Signed-off-by: Jason Frame <jason.frame@consensys.net>
pull/6892/head
Jason Frame 7 months ago committed by GitHub
parent d9bb7a6022
commit 758adc7d74
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 14
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java
  2. 4
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/MergePeerFilter.java
  3. 3
      ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java
  4. 23
      ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java
  5. 10
      ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessageTest.java

@ -314,7 +314,8 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
.setMessage("Post-merge disconnect: peer still gossiping blocks {}") .setMessage("Post-merge disconnect: peer still gossiping blocks {}")
.addArgument(ethPeer::toString) .addArgument(ethPeer::toString)
.log(); .log();
handleDisconnect(ethPeer.getConnection(), DisconnectReason.SUBPROTOCOL_TRIGGERED, false); handleDisconnect(
ethPeer.getConnection(), DisconnectReason.SUBPROTOCOL_TRIGGERED_POW_BLOCKS, false);
return; return;
} }
} }
@ -441,7 +442,7 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
.addArgument(status::networkId) .addArgument(status::networkId)
.addArgument(() -> getPeerOrPeerId(peer)) .addArgument(() -> getPeerOrPeerId(peer))
.log(); .log();
peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED_MISMATCHED_NETWORK);
} else if (!forkIdManager.peerCheck(forkId) && status.protocolVersion() > 63) { } else if (!forkIdManager.peerCheck(forkId) && status.protocolVersion() > 63) {
LOG.atDebug() LOG.atDebug()
.setMessage("{} has matching network id ({}), but non-matching fork id: {}") .setMessage("{} has matching network id ({}), but non-matching fork id: {}")
@ -449,7 +450,7 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
.addArgument(networkId::toString) .addArgument(networkId::toString)
.addArgument(forkId) .addArgument(forkId)
.log(); .log();
peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED_MISMATCHED_FORKID);
} else if (forkIdManager.peerCheck(status.genesisHash())) { } else if (forkIdManager.peerCheck(status.genesisHash())) {
LOG.atDebug() LOG.atDebug()
.setMessage("{} has matching network id ({}), but non-matching genesis hash: {}") .setMessage("{} has matching network id ({}), but non-matching genesis hash: {}")
@ -457,14 +458,15 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
.addArgument(networkId::toString) .addArgument(networkId::toString)
.addArgument(status::genesisHash) .addArgument(status::genesisHash)
.log(); .log();
peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED_MISMATCHED_GENESIS_HASH);
} else if (mergePeerFilter.isPresent() } else if (mergePeerFilter.isPresent()
&& mergePeerFilter.get().disconnectIfPoW(status, peer)) { && mergePeerFilter.get().disconnectIfPoW(status, peer)) {
LOG.atDebug() LOG.atDebug()
.setMessage("Post-merge disconnect: peer still PoW {}") .setMessage("Post-merge disconnect: peer still PoW {}")
.addArgument(() -> getPeerOrPeerId(peer)) .addArgument(() -> getPeerOrPeerId(peer))
.log(); .log();
handleDisconnect(peer.getConnection(), DisconnectReason.SUBPROTOCOL_TRIGGERED, false); handleDisconnect(
peer.getConnection(), DisconnectReason.SUBPROTOCOL_TRIGGERED_POW_DIFFICULTY, false);
} else { } else {
LOG.atDebug() LOG.atDebug()
.setMessage("Received status message from {}: {} with connection {}") .setMessage("Received status message from {}: {} with connection {}")
@ -486,7 +488,7 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
.log(); .log();
// Parsing errors can happen when clients broadcast network ids outside the int range, // Parsing errors can happen when clients broadcast network ids outside the int range,
// So just disconnect with "subprotocol" error rather than "breach of protocol". // So just disconnect with "subprotocol" error rather than "breach of protocol".
peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED_UNPARSABLE_STATUS);
} }
} }

@ -47,7 +47,7 @@ public class MergePeerFilter implements MergeStateHandler, UnverifiedForkchoiceL
LOG.debug( LOG.debug(
"Disconnecting peer with difficulty {}, likely still on PoW chain", "Disconnecting peer with difficulty {}, likely still on PoW chain",
status.totalDifficulty()); status.totalDifficulty());
peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED_POW_DIFFICULTY);
return true; return true;
} else { } else {
return false; return false;
@ -61,7 +61,7 @@ public class MergePeerFilter implements MergeStateHandler, UnverifiedForkchoiceL
final int code = message.getData().getCode(); final int code = message.getData().getCode();
if (isFinalized() && (code == EthPV62.NEW_BLOCK || code == EthPV62.NEW_BLOCK_HASHES)) { if (isFinalized() && (code == EthPV62.NEW_BLOCK || code == EthPV62.NEW_BLOCK_HASHES)) {
LOG.debug("disconnecting peer for sending new blocks after transition to PoS"); LOG.debug("disconnecting peer for sending new blocks after transition to PoS");
peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED_POW_BLOCKS);
return true; return true;
} else { } else {
return false; return false;

@ -257,8 +257,7 @@ public final class EthProtocolManagerTest {
assertThat(workPeer.isDisconnected()).isTrue(); assertThat(workPeer.isDisconnected()).isTrue();
assertThat(workPeer.getDisconnectReason()).isPresent(); assertThat(workPeer.getDisconnectReason()).isPresent();
assertThat(workPeer.getDisconnectReason()) assertThat(workPeer.getDisconnectReason())
.get() .hasValue(DisconnectReason.SUBPROTOCOL_TRIGGERED_POW_DIFFICULTY);
.isEqualTo(DisconnectReason.SUBPROTOCOL_TRIGGERED);
assertThat(stakePeer.isDisconnected()).isFalse(); assertThat(stakePeer.isDisconnected()).isFalse();
} }
} }

@ -118,10 +118,17 @@ public final class DisconnectMessage extends AbstractMessageData {
UNEXPECTED_ID((byte) 0x09), UNEXPECTED_ID((byte) 0x09),
LOCAL_IDENTITY((byte) 0x0a), LOCAL_IDENTITY((byte) 0x0a),
TIMEOUT((byte) 0x0b), TIMEOUT((byte) 0x0b),
SUBPROTOCOL_TRIGGERED((byte) 0x10); SUBPROTOCOL_TRIGGERED((byte) 0x10),
SUBPROTOCOL_TRIGGERED_MISMATCHED_NETWORK((byte) 0x10, "Mismatched network id"),
SUBPROTOCOL_TRIGGERED_MISMATCHED_FORKID((byte) 0x10, "Mismatched fork id"),
SUBPROTOCOL_TRIGGERED_MISMATCHED_GENESIS_HASH((byte) 0x10, "Mismatched genesis hash"),
SUBPROTOCOL_TRIGGERED_UNPARSABLE_STATUS((byte) 0x10, "Unparsable status message"),
SUBPROTOCOL_TRIGGERED_POW_DIFFICULTY((byte) 0x10, "Peer has difficulty greater than POS TTD"),
SUBPROTOCOL_TRIGGERED_POW_BLOCKS((byte) 0x10, "Peer sent blocks after POS transition");
private static final DisconnectReason[] BY_ID; private static final DisconnectReason[] BY_ID;
private final Optional<Byte> code; private final Optional<Byte> code;
private final Optional<String> message;
static { static {
final int maxValue = final int maxValue =
@ -132,7 +139,7 @@ public final class DisconnectMessage extends AbstractMessageData {
.getAsInt(); .getAsInt();
BY_ID = new DisconnectReason[maxValue + 1]; BY_ID = new DisconnectReason[maxValue + 1];
Stream.of(DisconnectReason.values()) Stream.of(DisconnectReason.values())
.filter(r -> r.code.isPresent()) .filter(r -> r.code.isPresent() && r.message.isEmpty())
.forEach(r -> BY_ID[r.code.get()] = r); .forEach(r -> BY_ID[r.code.get()] = r);
} }
@ -146,15 +153,25 @@ public final class DisconnectMessage extends AbstractMessageData {
DisconnectReason(final Byte code) { DisconnectReason(final Byte code) {
this.code = Optional.ofNullable(code); this.code = Optional.ofNullable(code);
this.message = Optional.empty();
}
DisconnectReason(final Byte code, final String message) {
this.code = Optional.ofNullable(code);
this.message = Optional.of(message);
} }
public Bytes getValue() { public Bytes getValue() {
return code.map(Bytes::of).orElse(Bytes.EMPTY); return code.map(Bytes::of).orElse(Bytes.EMPTY);
} }
public String getMessage() {
return message.orElse("");
}
@Override @Override
public String toString() { public String toString() {
return getValue().toString() + " " + name(); return getValue().toString() + " " + name() + " " + getMessage();
} }
} }
} }

@ -89,4 +89,14 @@ public class DisconnectMessageTest {
assertThat(disconnectMessage.getReason()).isEqualTo(DisconnectReason.UNKNOWN); assertThat(disconnectMessage.getReason()).isEqualTo(DisconnectReason.UNKNOWN);
assertThat(disconnectMessage.getData().toString()).isEqualToIgnoringCase("0xC180"); assertThat(disconnectMessage.getData().toString()).isEqualToIgnoringCase("0xC180");
} }
@Test
public void readFromWithSubprotocolTriggeredUsesGenericReason() {
MessageData messageData =
new RawMessage(WireMessageCodes.DISCONNECT, Bytes.fromHexString("0xC110"));
DisconnectMessage disconnectMessage = DisconnectMessage.readFrom(messageData);
DisconnectReason reason = disconnectMessage.getReason();
assertThat(reason).isEqualTo(DisconnectReason.SUBPROTOCOL_TRIGGERED);
}
} }

Loading…
Cancel
Save