From f174e2350e7fa46220afef873f9c397037bcf60d Mon Sep 17 00:00:00 2001 From: Andreas Schildbach Date: Mon, 3 Apr 2023 12:12:38 +0200 Subject: [PATCH] VersionMessage: inline helpers `isWitnessSupported()`, `hasBlockChain()` and `hasLimitedBlockChain()` Our API is now fluent enough that we don't need them any more. --- .../src/main/java/org/bitcoinj/core/Peer.java | 15 ++++++----- .../java/org/bitcoinj/core/PeerGroup.java | 4 +-- .../org/bitcoinj/core/VersionMessage.java | 27 +++++++------------ .../bitcoinj/core/BitcoindComparisonTool.java | 2 +- .../testing/TestWithNetworkConnections.java | 3 ++- .../bitcoinj/testing/TestWithPeerGroup.java | 4 +-- 6 files changed, 24 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Peer.java b/core/src/main/java/org/bitcoinj/core/Peer.java index fcf4f25df..e5ead5752 100644 --- a/core/src/main/java/org/bitcoinj/core/Peer.java +++ b/core/src/main/java/org/bitcoinj/core/Peer.java @@ -522,21 +522,22 @@ public class Peer extends PeerSocketHandler { // mode nodes because we can't download the data from them we need to find/verify transactions. Some bogus // implementations claim to have a block chain in their services field but then report a height of zero, filter // them out here. - if (!peerVersionMessage.hasLimitedBlockChain() || + Services services = peerVersionMessage.services(); + if (!(services.has(Services.NODE_NETWORK) || services.has(Services.NODE_NETWORK_LIMITED)) || (!params.allowEmptyPeerChain() && peerVersionMessage.bestHeight == 0)) { // Shut down the channel gracefully. log.info("{}: Peer does not have at least a recent part of the block chain.", this); close(); return; } - if (!peerVersionMessage.localServices.has(requiredServices)) { + if (!services.has(requiredServices)) { log.info("{}: Peer doesn't support these required services: {}", this, Services.of(requiredServices & ~peerVersionMessage.localServices.bits()).toString()); // Shut down the channel gracefully. close(); return; } - if (peerVersionMessage.localServices.has(Services.NODE_BITCOIN_CASH)) { + if (services.has(Services.NODE_BITCOIN_CASH)) { log.info("{}: Peer follows an incompatible block chain.", this); // Shut down the channel gracefully. close(); @@ -911,7 +912,7 @@ public class Peer extends PeerSocketHandler { private GetDataMessage buildMultiTransactionDataMessage(Set txIds) { GetDataMessage getdata = new GetDataMessage(); txIds.forEach(txId -> - getdata.addTransaction(txId, vPeerVersionMessage.isWitnessSupported())); + getdata.addTransaction(txId, vPeerVersionMessage.services().has(Services.NODE_WITNESS))); return getdata; } @@ -1183,7 +1184,7 @@ public class Peer extends PeerSocketHandler { } else { if (log.isDebugEnabled()) log.debug("{}: getdata on tx {}", getAddress(), item.hash); - getdata.addTransaction(item.hash, vPeerVersionMessage.isWitnessSupported()); + getdata.addTransaction(item.hash, vPeerVersionMessage.services().has(Services.NODE_WITNESS)); if (pendingTxDownloads.size() > PENDING_TX_DOWNLOADS_LIMIT) { log.info("{}: Too many pending transactions, disconnecting", this); close(); @@ -1228,7 +1229,7 @@ public class Peer extends PeerSocketHandler { getdata.addFilteredBlock(item.hash); pingAfterGetData = true; } else { - getdata.addBlock(item.hash, vPeerVersionMessage.isWitnessSupported()); + getdata.addBlock(item.hash, vPeerVersionMessage.services().has(Services.NODE_WITNESS)); } pendingBlockDownloads.add(item.hash); } @@ -1284,7 +1285,7 @@ public class Peer extends PeerSocketHandler { // TODO: Unit test this method. log.info("Request to fetch peer mempool tx {}", hash); GetDataMessage getdata = new GetDataMessage(); - getdata.addTransaction(hash, vPeerVersionMessage.isWitnessSupported()); + getdata.addTransaction(hash, vPeerVersionMessage.services().has(Services.NODE_WITNESS)); return ListenableCompletableFuture.of(sendSingleGetData(getdata)); } diff --git a/core/src/main/java/org/bitcoinj/core/PeerGroup.java b/core/src/main/java/org/bitcoinj/core/PeerGroup.java index f42ec3c8a..1fc4461b3 100644 --- a/core/src/main/java/org/bitcoinj/core/PeerGroup.java +++ b/core/src/main/java/org/bitcoinj/core/PeerGroup.java @@ -2377,9 +2377,9 @@ public class PeerGroup implements TransactionBroadcaster { final VersionMessage versionMessage = peer.getPeerVersionMessage(); if (versionMessage.clientVersion < MINIMUM_VERSION) continue; - if (!versionMessage.hasBlockChain()) + if (!versionMessage.services().has(Services.NODE_NETWORK)) continue; - if (!versionMessage.isWitnessSupported()) + if (!versionMessage.services().has(Services.NODE_WITNESS)) continue; final long peerHeight = peer.getBestHeight(); if (peerHeight < mostCommonChainHeight || peerHeight > mostCommonChainHeight + 1) diff --git a/core/src/main/java/org/bitcoinj/core/VersionMessage.java b/core/src/main/java/org/bitcoinj/core/VersionMessage.java index 49212f4d6..1ed875449 100644 --- a/core/src/main/java/org/bitcoinj/core/VersionMessage.java +++ b/core/src/main/java/org/bitcoinj/core/VersionMessage.java @@ -113,6 +113,15 @@ public class VersionMessage extends Message { relayTxesBeforeFilter = true; } + /** + * Get the service bitfield that represents the node services being provided. + * + * @return service bitfield + */ + public Services services() { + return localServices; + } + @Override protected void parse(ByteBuffer payload) throws BufferUnderflowException, ProtocolException { clientVersion = (int) ByteUtils.readUint32(payload); @@ -260,22 +269,4 @@ public class VersionMessage extends Message { return true; return false; } - - /** Returns true if a peer can be asked for blocks and transactions including witness data. */ - public boolean isWitnessSupported() { - return localServices.has(Services.NODE_WITNESS); - } - - /** - * Returns true if the version message indicates the sender has a full copy of the block chain, or false if it's - * running in client mode (only has the headers). - */ - public boolean hasBlockChain() { - return localServices.has(Services.NODE_NETWORK); - } - - /** Returns true if the peer has at least the last two days worth of blockchain (BIP159). */ - public boolean hasLimitedBlockChain() { - return hasBlockChain() || localServices.has(Services.NODE_NETWORK_LIMITED); - } } diff --git a/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java b/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java index 7b3b34916..b1aecfaa0 100644 --- a/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java +++ b/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java @@ -94,7 +94,7 @@ public class BitcoindComparisonTool { ver.localServices = Services.of(Services.NODE_NETWORK); final Peer bitcoind = new Peer(PARAMS, ver, PeerAddress.localhost(PARAMS), new BlockChain(PARAMS, new MemoryBlockStore(PARAMS.getGenesisBlock()))); - checkState(bitcoind.getVersionMessage().hasBlockChain()); + checkState(bitcoind.getVersionMessage().services().has(Services.NODE_NETWORK)); final BlockWrapper currentBlock = new BlockWrapper(); diff --git a/integration-test/src/test/java/org/bitcoinj/testing/TestWithNetworkConnections.java b/integration-test/src/test/java/org/bitcoinj/testing/TestWithNetworkConnections.java index 64b200ba0..f2117884e 100644 --- a/integration-test/src/test/java/org/bitcoinj/testing/TestWithNetworkConnections.java +++ b/integration-test/src/test/java/org/bitcoinj/testing/TestWithNetworkConnections.java @@ -27,6 +27,7 @@ import org.bitcoinj.core.NetworkParameters; import org.bitcoinj.core.Peer; import org.bitcoinj.core.Ping; import org.bitcoinj.core.Pong; +import org.bitcoinj.core.Services; import org.bitcoinj.core.VersionAck; import org.bitcoinj.core.VersionMessage; import org.bitcoinj.core.listeners.PreMessageReceivedEventListener; @@ -168,7 +169,7 @@ public class TestWithNetworkConnections { } protected InboundMessageQueuer connect(Peer peer, VersionMessage versionMessage) throws Exception { - checkArgument(versionMessage.hasBlockChain()); + checkArgument(versionMessage.services().has(Services.NODE_NETWORK)); final AtomicBoolean doneConnecting = new AtomicBoolean(false); final Thread thisThread = Thread.currentThread(); peer.addDisconnectedEventListener((p, peerCount) -> { diff --git a/integration-test/src/test/java/org/bitcoinj/testing/TestWithPeerGroup.java b/integration-test/src/test/java/org/bitcoinj/testing/TestWithPeerGroup.java index 5e718bc30..6c758ae11 100644 --- a/integration-test/src/test/java/org/bitcoinj/testing/TestWithPeerGroup.java +++ b/integration-test/src/test/java/org/bitcoinj/testing/TestWithPeerGroup.java @@ -146,7 +146,7 @@ public class TestWithPeerGroup extends TestWithNetworkConnections { } protected InboundMessageQueuer connectPeer(int id, VersionMessage versionMessage) throws Exception { - checkArgument(versionMessage.hasBlockChain()); + checkArgument(versionMessage.services().has(Services.NODE_NETWORK)); InboundMessageQueuer writeTarget = connectPeerWithoutVersionExchange(id); // Complete handshake with the peer - send/receive version(ack)s, receive bloom filter writeTarget.sendMessage(versionMessage); @@ -163,7 +163,7 @@ public class TestWithPeerGroup extends TestWithNetworkConnections { // handle peer discovered by PeerGroup protected InboundMessageQueuer handleConnectToPeer(int id, VersionMessage versionMessage) throws Exception { InboundMessageQueuer writeTarget = newPeerWriteTargetQueue.take(); - checkArgument(versionMessage.hasBlockChain()); + checkArgument(versionMessage.services().has(Services.NODE_NETWORK)); // Complete handshake with the peer - send/receive version(ack)s, receive bloom filter writeTarget.sendMessage(versionMessage); writeTarget.sendMessage(new VersionAck());