From a6aec846d7d60cddbd84875592fd0d5c2e65b16b Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Thu, 13 Jan 2022 17:24:18 +1000 Subject: [PATCH] Fix sub-protocols extending Eth protocol sending messages with null capability (#3273) This fixes an issue with the Istanbul protocol which extends that Eth protocol where devp2p messages were being trying to be sent with a null capability Signed-off-by: Jason Frame --- .../besu/ethereum/eth/manager/EthPeer.java | 28 ++++++++----------- .../ethereum/eth/manager/EthPeerTest.java | 10 ++----- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java index 0fba01f68b..5aedb769c1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java @@ -138,23 +138,21 @@ public class EthPeer { getAgreedCapabilities().stream().anyMatch(EthProtocol::isEth66Compatible); // eth protocol requestManagers.put( - EthProtocol.NAME, + protocolName, Map.ofEntries( Map.entry( EthPV62.GET_BLOCK_HEADERS, - new RequestManager(this, supportsRequestId, EthProtocol.NAME)), + new RequestManager(this, supportsRequestId, protocolName)), Map.entry( EthPV62.GET_BLOCK_BODIES, - new RequestManager(this, supportsRequestId, EthProtocol.NAME)), + new RequestManager(this, supportsRequestId, protocolName)), Map.entry( - EthPV63.GET_RECEIPTS, - new RequestManager(this, supportsRequestId, EthProtocol.NAME)), + EthPV63.GET_RECEIPTS, new RequestManager(this, supportsRequestId, protocolName)), Map.entry( - EthPV63.GET_NODE_DATA, - new RequestManager(this, supportsRequestId, EthProtocol.NAME)), + EthPV63.GET_NODE_DATA, new RequestManager(this, supportsRequestId, protocolName)), Map.entry( EthPV65.GET_POOLED_TRANSACTIONS, - new RequestManager(this, supportsRequestId, EthProtocol.NAME)))); + new RequestManager(this, supportsRequestId, protocolName)))); } private void initSnapRequestManagers() { @@ -256,7 +254,7 @@ public class EthPeer { final GetBlockHeadersMessage message = GetBlockHeadersMessage.create(hash, maxHeaders, skip, reverse); final RequestManager requestManager = - requestManagers.get(EthProtocol.NAME).get(EthPV62.GET_BLOCK_HEADERS); + requestManagers.get(protocolName).get(EthPV62.GET_BLOCK_HEADERS); return sendRequest(requestManager, message); } @@ -265,34 +263,32 @@ public class EthPeer { throws PeerNotConnected { final GetBlockHeadersMessage message = GetBlockHeadersMessage.create(blockNumber, maxHeaders, skip, reverse); - return sendRequest( - requestManagers.get(EthProtocol.NAME).get(EthPV62.GET_BLOCK_HEADERS), message); + return sendRequest(requestManagers.get(protocolName).get(EthPV62.GET_BLOCK_HEADERS), message); } public RequestManager.ResponseStream getBodies(final List blockHashes) throws PeerNotConnected { final GetBlockBodiesMessage message = GetBlockBodiesMessage.create(blockHashes); - return sendRequest( - requestManagers.get(EthProtocol.NAME).get(EthPV62.GET_BLOCK_BODIES), message); + return sendRequest(requestManagers.get(protocolName).get(EthPV62.GET_BLOCK_BODIES), message); } public RequestManager.ResponseStream getReceipts(final List blockHashes) throws PeerNotConnected { final GetReceiptsMessage message = GetReceiptsMessage.create(blockHashes); - return sendRequest(requestManagers.get(EthProtocol.NAME).get(EthPV63.GET_RECEIPTS), message); + return sendRequest(requestManagers.get(protocolName).get(EthPV63.GET_RECEIPTS), message); } public RequestManager.ResponseStream getNodeData(final Iterable nodeHashes) throws PeerNotConnected { final GetNodeDataMessage message = GetNodeDataMessage.create(nodeHashes); - return sendRequest(requestManagers.get(EthProtocol.NAME).get(EthPV63.GET_NODE_DATA), message); + return sendRequest(requestManagers.get(protocolName).get(EthPV63.GET_NODE_DATA), message); } public RequestManager.ResponseStream getPooledTransactions(final List hashes) throws PeerNotConnected { final GetPooledTransactionsMessage message = GetPooledTransactionsMessage.create(hashes); return sendRequest( - requestManagers.get(EthProtocol.NAME).get(EthPV65.GET_POOLED_TRANSACTIONS), message); + requestManagers.get(protocolName).get(EthPV65.GET_POOLED_TRANSACTIONS), message); } public RequestManager.ResponseStream getSnapAccountRange(final GetAccountRangeMessage message) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeerTest.java index 8e40307b92..8ed9c715bc 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeerTest.java @@ -27,7 +27,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.hyperledger.besu.ethereum.core.BlockDataGenerator; -import org.hyperledger.besu.ethereum.eth.EthProtocol; import org.hyperledger.besu.ethereum.eth.messages.BlockBodiesMessage; import org.hyperledger.besu.ethereum.eth.messages.BlockHeadersMessage; import org.hyperledger.besu.ethereum.eth.messages.NodeDataMessage; @@ -437,13 +436,10 @@ public class EthPeerTest { final List permissioningProviders) { final PeerConnection peerConnection = mock(PeerConnection.class); final Consumer onPeerReady = (peer) -> {}; + // Use a non-eth protocol name to ensure that EthPeer with sub-protocols such as Istanbul + // that extend the sub-protocol work correctly return new EthPeer( - peerConnection, - EthProtocol.NAME, - onPeerReady, - peerValidators, - clock, - permissioningProviders); + peerConnection, "foo", onPeerReady, peerValidators, clock, permissioningProviders); } @FunctionalInterface