From 28d50bccf9bf5da17ca6e0d0be653cb14b7a8af0 Mon Sep 17 00:00:00 2001
From: CalDescent <caldescent@protonmail.com>
Date: Sun, 16 May 2021 09:15:37 +0100
Subject: [PATCH] Exclude peers if we don't have a complete set of their block
 summaries.

This tightens up the decision making by adding two requirements:

1. The peer must return the same number of summaries to the ones requested.
2. The peer must return a summary that matches its latest reported signature.

This ensures we are always making sync decisions based on accurate data, and removes peers that are currently mid re-org. This is probably more validation than is actually necessary, but it's best to be really thorough here so it is as optimized as possible.
---
 .../org/qortal/controller/Synchronizer.java   | 26 +++++++++++++++----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/qortal/controller/Synchronizer.java b/src/main/java/org/qortal/controller/Synchronizer.java
index 948e9785..8b566fdc 100644
--- a/src/main/java/org/qortal/controller/Synchronizer.java
+++ b/src/main/java/org/qortal/controller/Synchronizer.java
@@ -282,7 +282,9 @@ public class Synchronizer {
 							return peers;
 
 						// Count the number of blocks this peer has beyond our common block
-						final int peerHeight = peer.getChainTipData().getLastHeight();
+						final PeerChainTipData peerChainTipData = peer.getChainTipData();
+						final int peerHeight = peerChainTipData.getLastHeight();
+						final byte[] peerLastBlockSignature = peerChainTipData.getLastBlockSignature();
 						final int peerAdditionalBlocksAfterCommonBlock = peerHeight - commonBlockSummary.getHeight();
 						// Limit the number of blocks we are comparing. FUTURE: we could request more in batches, but there may not be a case when this is needed
 						int summariesRequired = Math.min(peerAdditionalBlocksAfterCommonBlock, MAXIMUM_REQUEST_SIZE);
@@ -302,15 +304,23 @@ public class Synchronizer {
 							if (summariesRequired > 0) {
 								LOGGER.trace(String.format("Requesting %d block summar%s from peer %s after common block %.8s. Peer height: %d", summariesRequired, (summariesRequired != 1 ? "ies" : "y"), peer, Base58.encode(commonBlockSummary.getSignature()), peerHeight));
 
-								List<BlockSummaryData> blockSummaries = this.getBlockSummaries(peer, commonBlockSummary.getSignature(), summariesRequired);
-								peer.getCommonBlockData().setBlockSummariesAfterCommonBlock(blockSummaries);
+								// Forget any cached summaries
+								peer.getCommonBlockData().setBlockSummariesAfterCommonBlock(null);
 
+								// Request new block summaries
+								List<BlockSummaryData> blockSummaries = this.getBlockSummaries(peer, commonBlockSummary.getSignature(), summariesRequired);
 								if (blockSummaries != null) {
 									LOGGER.trace(String.format("Peer %s returned %d block summar%s", peer, blockSummaries.size(), (blockSummaries.size() != 1 ? "ies" : "y")));
 
 									if (blockSummaries.size() < summariesRequired)
-										// This could mean that the peer has re-orged. But we still have the same common block, so it's safe to proceed with this set of signatures instead.
-										LOGGER.debug(String.format("Peer %s returned %d block summar%s instead of expected %d", peer, blockSummaries.size(), (blockSummaries.size() != 1 ? "ies" : "y"), summariesRequired));
+										// This could mean that the peer has re-orged. Exclude this peer until they return the summaries we expect.
+										LOGGER.debug(String.format("Peer %s returned %d block summar%s instead of expected %d - excluding them from this round", peer, blockSummaries.size(), (blockSummaries.size() != 1 ? "ies" : "y"), summariesRequired));
+									else if (blockSummaryWithSignature(peerLastBlockSignature, blockSummaries) == null)
+										// We don't have a block summary for the peer's reported chain tip, so should exclude it
+										LOGGER.debug(String.format("Peer %s didn't return a block summary with signature %.8s - excluding them from this round", peer, Base58.encode(peerLastBlockSignature)));
+									else
+										// All looks good, so store the retrieved block summaries in the peer's cache
+										peer.getCommonBlockData().setBlockSummariesAfterCommonBlock(blockSummaries);
 								}
 							} else {
 								// There are no block summaries after this common block
@@ -451,6 +461,12 @@ public class Synchronizer {
 		return minChainLength;
 	}
 
+	private BlockSummaryData blockSummaryWithSignature(byte[] signature, List<BlockSummaryData> blockSummaries) {
+		if (blockSummaries != null)
+			return blockSummaries.stream().filter(blockSummary -> Arrays.equals(blockSummary.getSignature(), signature)).findAny().orElse(null);
+		return null;
+	}
+
 
 	/**
 	 * Attempt to synchronize blockchain with peer.