mirrors/bitcoinj
1
0
Fork 0
mirror of https://github.com/bitcoinj/bitcoinj.git synced 2025-02-24 06:47:54 +01:00
Code Issues Projects Releases Packages Wiki Activity

PeerGroup: Be more careful about selecting download peer.

Browse source 
We now select a (new) download peer only if there is a clear consensus about a common chain height between
connected peers. If there is a tie between two common heights, we stay safe and don't change anything.
This commit is contained in:
Andreas Schildbach 2019-12-08 11:16:21 +01:00
parent 924e525d52
commit c2e1383b58
2 changed files with 58 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
core/src/main/java/org/bitcoinj/core/PeerGroup.java
View file

@ -1493,6 +1493,8 @@ public class PeerGroup implements TransactionBroadcaster {
if (shouldDownloadChain) {
startBlockChainDownloadFromPeer(downloadPeer);
}
} else {
log.info("Not yet setting download peer because there is no clear candidate.");
}
}
// Make sure the peer knows how to upload transactions that are requested from us.
@ -2135,8 +2137,8 @@ public class PeerGroup implements TransactionBroadcaster {
}
/**
* Returns our peers most commonly reported chain height. If multiple heights are tied, the highest is returned.
* If no peers are connected, returns zero.
* Returns our peers most commonly reported chain height.
* If the most common heights are tied, or no peers are connected, returns {@code 0}.
*/
public int getMostCommonChainHeight() {
lock.lock();
@ -2149,7 +2151,7 @@ public class PeerGroup implements TransactionBroadcaster {
/**
* Returns most commonly reported chain height from the given list of {@link Peer}s.
* If multiple heights are tied, the highest is returned. If no peers are connected, returns zero.
* If the most common heights are tied, or no peers are connected, returns {@code 0}.
*/
public static int getMostCommonChainHeight(final List<Peer> peers) {
if (peers.isEmpty())
@ -2160,8 +2162,9 @@ public class PeerGroup implements TransactionBroadcaster {
}
private static class Pair implements Comparable<Pair> {
int item, count;
public Pair(int item, int count) { this.count = count; this.item = item; }
final int item;
int count = 0;
public Pair(int item) { this.item = item; }
// note that in this implementation compareTo() is not consistent with equals()
@Override public int compareTo(Pair o) { return -Integer.compare(count, o.count); }
}
@ -2172,24 +2175,25 @@ public class PeerGroup implements TransactionBroadcaster {
// This would be much easier in a functional language (or in Java 8).
items = Ordering.natural().reverse().sortedCopy(items);
LinkedList<Pair> pairs = new LinkedList<>();
pairs.add(new Pair(items.get(0), 0));
pairs.add(new Pair(items.get(0)));
for (int item : items) {
Pair pair = pairs.getLast();
if (pair.item != item)
pairs.add((pair = new Pair(item, 0)));
pairs.add((pair = new Pair(item)));
pair.count++;
}
// pairs now contains a uniqified list of the sorted inputs, with counts for how often that item appeared.
// Now sort by how frequently they occur, and pick the max of the most frequent.
// pairs now contains a uniquified list of the sorted inputs, with counts for how often that item appeared.
// Now sort by how frequently they occur, and pick the most frequent. If the first place is tied between two,
// don't pick any.
Collections.sort(pairs);
int maxCount = pairs.getFirst().count;
int maxItem = pairs.getFirst().item;
for (Pair pair : pairs) {
if (pair.count != maxCount)
break;
maxItem = Math.max(maxItem, pair.item);
}
return maxItem;
final Pair firstPair = pairs.get(0);
if (pairs.size() == 1)
return firstPair.item;
final Pair secondPair = pairs.get(1);
if (firstPair.count > secondPair.count)
return firstPair.item;
checkState(firstPair.count == secondPair.count);
return 0;
}
/**
@ -2204,13 +2208,18 @@ public class PeerGroup implements TransactionBroadcaster {
// - Randomly, to try and spread the load.
if (peers.isEmpty())
return null;
// Make sure we don't select a peer that is behind/synchronizing itself.
int mostCommonChainHeight = getMostCommonChainHeight(peers);
// Make sure we don't select a peer if there is no consensus about block height.
if (mostCommonChainHeight == 0)
return null;
// Make sure we don't select a peer that is behind/synchronizing itself or announces an unrealistic height.
List<Peer> candidates = new ArrayList<>();
for (Peer peer : peers) {
if (!peer.getPeerVersionMessage().hasBlockChain())
continue;
if (peer.getBestHeight() < mostCommonChainHeight)
final long peerHeight = peer.getBestHeight();
if (peerHeight < mostCommonChainHeight || peerHeight > mostCommonChainHeight + 1)
continue;
candidates.add(peer);
}

48
core/src/test/java/org/bitcoinj/core/PeerGroupTest.java
View file

@ -468,25 +468,35 @@ public class PeerGroupTest extends TestWithPeerGroup {
@Test
public void downloadPeerSelection() throws Exception {
peerGroup.start();
VersionMessage versionMessage2 = new VersionMessage(UNITTEST, 2);
versionMessage2.clientVersion = NetworkParameters.ProtocolVersion.BLOOM_FILTER.getBitcoinProtocolVersion();
versionMessage2.localServices = VersionMessage.NODE_NETWORK;
VersionMessage versionMessage3 = new VersionMessage(UNITTEST, 3);
versionMessage3.clientVersion = NetworkParameters.ProtocolVersion.BLOOM_FILTER.getBitcoinProtocolVersion();
versionMessage3.localServices = VersionMessage.NODE_NETWORK;
VersionMessage v1 = new VersionMessage(UNITTEST, 2);
v1.clientVersion = NetworkParameters.ProtocolVersion.BLOOM_FILTER.getBitcoinProtocolVersion();
v1.localServices = VersionMessage.NODE_NETWORK;
VersionMessage v2 = new VersionMessage(UNITTEST, 4);
v2.clientVersion = NetworkParameters.ProtocolVersion.BLOOM_FILTER.getBitcoinProtocolVersion();
v2.localServices = VersionMessage.NODE_NETWORK;
assertNull(peerGroup.getDownloadPeer());
Peer a = connectPeer(1, versionMessage2).peer;
Peer p1 = connectPeer(0, v1).peer;
assertEquals(2, peerGroup.getMostCommonChainHeight());
assertEquals(a, peerGroup.getDownloadPeer());
connectPeer(2, versionMessage2);
assertEquals(2, peerGroup.getMostCommonChainHeight());
assertEquals(a, peerGroup.getDownloadPeer()); // No change.
Peer c = connectPeer(3, versionMessage3).peer;
assertEquals(2, peerGroup.getMostCommonChainHeight());
assertEquals(a, peerGroup.getDownloadPeer()); // No change yet.
connectPeer(4, versionMessage3);
assertEquals(3, peerGroup.getMostCommonChainHeight());
assertEquals(a, peerGroup.getDownloadPeer()); // Still no change.
assertEquals(2, peerGroup.selectDownloadPeer(peerGroup.getConnectedPeers()).getBestHeight());
assertEquals(p1, peerGroup.getDownloadPeer());
connectPeer(1, v1);
assertEquals(2, peerGroup.getMostCommonChainHeight()); // No change.
assertEquals(2, peerGroup.selectDownloadPeer(peerGroup.getConnectedPeers()).getBestHeight());
Peer p2 = connectPeer(2, v2).peer;
assertEquals(2, peerGroup.getMostCommonChainHeight()); // No change yet.
assertEquals(2, peerGroup.selectDownloadPeer(peerGroup.getConnectedPeers()).getBestHeight());
connectPeer(3, v2);
assertEquals(0, peerGroup.getMostCommonChainHeight()); // Most common height tied between two...
assertNull(peerGroup.selectDownloadPeer(peerGroup.getConnectedPeers())); // ...so no peer would be selected.
connectPeer(4, v2);
assertEquals(4, peerGroup.getMostCommonChainHeight()); // Now we have a new winner...
assertEquals(4, peerGroup.selectDownloadPeer(peerGroup.getConnectedPeers()).getBestHeight());
assertEquals(p1, peerGroup.getDownloadPeer()); // ...but the download peer doesn't change unless it misbehaves.
// New peer with a higher protocol version but same chain height.
// TODO: When PeerGroup.selectDownloadPeer.PREFERRED_VERSION is not equal to vMinRequiredProtocolVersion,
@ -866,7 +876,9 @@ public class PeerGroupTest extends TestWithPeerGroup {
public void testMaxOfMostFreq() throws Exception {
assertEquals(0, PeerGroup.maxOfMostFreq(Collections.<Integer>emptyList()));
assertEquals(0, PeerGroup.maxOfMostFreq(Arrays.asList(0, 0, 1)));
assertEquals(2, PeerGroup.maxOfMostFreq(Arrays.asList(1, 1, 2, 2)));
assertEquals(3, PeerGroup.maxOfMostFreq(Arrays.asList(1, 3, 1, 2, 2, 3, 3)));
assertEquals(0, PeerGroup.maxOfMostFreq(Arrays.asList(1, 1, 2, 2)));
assertEquals(0, PeerGroup.maxOfMostFreq(Arrays.asList(-1, 1, 1, 2, 2)));
assertEquals(1, PeerGroup.maxOfMostFreq(Arrays.asList(1, 1, 2, 2, 1)));
assertEquals(-1, PeerGroup.maxOfMostFreq(Arrays.asList(-1, -1, 2, 2, -1)));
}