Skip to content

fix(net): reject inconsistent terminal sync replies #7079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
317787106 wants to merge 6 commits into
base: release_v4.8.3
Choose a base branch
from
+173 −61
Open

fix(net): reject inconsistent terminal sync replies #7079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a8d9cfd
fix(net): reject inconsistent sync replies and retry another peer
317787106 Nov 2, 2026
6c90efc
revert(net): remove proactive sync peer retries
317787106 Nov 3, 2026
daa5bd4
refactor(net): clarify advertised inventory block number
317787106 Nov 3, 2026
3916a58
format the code
317787106 Nov 3, 2026
722061c
adjust executuion timeout of VerifyTransferProof from 80ms to 200ms
317787106 Nov 3, 2026
9b52e54
fix(net): shorten isolation recovery delay
317787106 Nov 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions framework/src/main/java/org/tron/core/net/messagehandler/ChainInventoryMsgHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import org.tron.core.exception.P2pException.TypeEnum;
import org.tron.core.net.TronNetDelegate;
import org.tron.core.net.message.TronMessage;
import org.tron.core.net.message.handshake.HelloMessage;
import org.tron.core.net.message.sync.ChainInventoryMessage;
import org.tron.core.net.peer.PeerConnection;
import org.tron.core.net.peer.TronState;
import org.tron.core.net.service.sync.SyncService;
import org.tron.protos.Protocol.Inventory.InventoryType;

@Slf4j(topic = "net")
@Component
Expand Down Expand Up @@ -95,6 +97,7 @@ public void processMessage(PeerConnection peer, TronMessage msg) throws P2pExcep
}

private void check(PeerConnection peer, ChainInventoryMessage msg) throws P2pException {
HelloMessage hello = peer.getHelloMessageReceive();
Pair<Deque<BlockId>, Long> requested = peer.getSyncChainRequested();
if (requested == null) {
throw new P2pException(TypeEnum.BAD_MESSAGE, "not send syncBlockChainMsg");
Expand Down Expand Up @@ -147,6 +150,39 @@ private void check(PeerConnection peer, ChainInventoryMessage msg) throws P2pExc
+ ", futureMaxNum: " + maxFutureNum);
}
}

if (msg.getRemainNum() == 0) {
BlockId lastBlockId = blockIds.get(blockIds.size() - 1);
long lastNum = lastBlockId.getNum();
long helloHeadNum = hello.getHeadBlockId().getNum();
if (lastNum < helloHeadNum) {

@xxo1shine xxo1shine Nov 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also treats a temporary inconsistency caused by a legitimate fork switch as a sync failure. For example, a peer can advertise height 200 in HELLO, switch forks and temporarily roll back to 198, then be disconnected for returning lastNum=198, remainNum=0. A fork switch interleaved with summary generation can produce a similar result.

Although the PR description and tests explicitly accept this behavior, recovery still requires the connection cooldown, reconnection and a new handshake. Please consider bounded renegotiation before disconnecting a peer for a persistent inconsistency, and add coverage for an honest peer recovering synchronization after a fork switch.

@317787106 317787106 Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. We agree that a legitimate fork switch can temporarily make the current head inconsistent with the earlier HELLO snapshot, and that interleaving a fork switch with summary generation can produce a similar response. This PR intentionally accepts a recoverable disconnect in these cases; the check is not intended to establish that the peer is malicious.

For the HELLO-triggered initial synchronization case, the peer advertised a higher head, so we attempted to synchronize from it. If a concurrent fork switch makes that attempt fail the consistency checks, we consider it acceptable to abandon this connection’s initial synchronization attempt. More precisely, this is a post-handshake initialization failure, since the HELLO exchange itself has already completed.

With other available peers, the node does not need to wait for this particular peer’s cooldown, reconnection, and fresh handshake before making synchronization progress. SYNC_FAIL follows the ordinary disconnect/cooldown path rather than requesting the longer penalty associated with BAD_PROTOCOL or BAD_BLOCK. We therefore accept the cost of temporarily losing this candidate synchronization source. The same policy applies when an existing connection encounters this inconsistency during a later synchronization attempt.

Bounded renegotiation could preserve some honest connections, but it would introduce retry accounting, timeout handling, and additional state transitions. Given the intended tradeoff—accepting occasional disconnects while avoiding repeated unproductive synchronization attempts—we do not consider same-connection renegotiation necessary for this PR.

The existing fork-switch test deliberately expects SYNC_FAIL to document this policy. Additional coverage showing that a fresh connection can synchronize after the fork has settled would be useful, but it can be added independently of bounded renegotiation.

// A fork rollback can temporarily lower the head; allow a fresh HELLO on reconnect.
throw new P2pException(TypeEnum.SYNC_FAILED,
"lastNum " + lastNum + " (remainNum=0) is below hello head " + helloHeadNum);
}

// A peer that has the terminal block in our summary should reply with that block alone.
if (blockIds.size() > 1 && requested.getKey().contains(lastBlockId)) {
// A concurrent fork switch can also produce this response; use a sync failure.
throw new P2pException(TypeEnum.SYNC_FAILED,
"terminal block already in summary: " + lastBlockId.getString());
}

long requestedAt = requested.getValue();
// Only use recent announcements received before this request, not a newer head
// announced while the peer was preparing its response. A concurrent fork switch also cause it
long maxAdvInvNum = peer.getAdvInvReceive().asMap().entrySet().stream()

@xxo1shine xxo1shine Nov 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .max() operation traverses the entire advInvReceive cache, including transaction announcements that are subsequently filtered out. The cache has a capacity of 20,000 entries, so a terminal reply can require scanning approximately twenty thousand entries.

Please consider maintaining recent BLOCK announcements separately, avoiding a scan of the entire mixed transaction/block cache just to check a small number of block announcements.

@317787106 317787106 Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The observation that .max() traverses the mixed cache is correct. However, I do not think this alone justifies introducing a separate BLOCK-announcement cache in this PR.

First, this operation is a single-pass, in-memory reduction with O(N) complexity, rather than a sorting operation. Transaction entries are rejected by the first type filter. Timestamp filtering and BlockId construction are performed only for the relevant BLOCK entries, with block-number extraction limited to announcements eligible for this request. The scan itself performs no database reads or network operations.

Second, 20,000 is the cache’s maximum capacity, not its actual entry count on every invocation. We should still account for a full cache, but the worst-case work remains a bounded linear traversal. Capacity establishes an upper bound; it does not demonstrate an unacceptable processing delay.

Third, the invocation frequency matters. This check executes only for terminal chain inventory replies (remain_num=0) associated with an outstanding synchronization request. It is not executed for every transaction announcement, every block announcement, or every normally received block. In normal steady-state operation, this is a low-frequency synchronization path rather than a per-message hot path.

The assessment also includes visiting entries outside the 15-second eligibility window. That window determines which announcements can constrain the response; it does not remove those entries from the cache or eliminate their traversal cost. The justification is the combination of bounded work, linear complexity, and low invocation frequency.

Maintaining BLOCK announcements separately would add per-peer state and require coordinated updates, expiration, cleanup, and concurrency handling. It would also need to preserve the current semantics: considering recent announcements received before the particular synchronization request, while excluding later announcements. That is additional complexity for a performance benefit that has not yet been quantified.

This scan has a nonzero cost, but there are currently no measurements showing that it causes meaningful handler latency or allocation pressure. I therefore suggest retaining the existing implementation in this focused correctness fix. If profiling demonstrates a material bottleneck, a separate BLOCK-announcement structure can be evaluated as a follow-up optimization.

.filter(entry -> entry.getKey().getType() == InventoryType.BLOCK)
.filter(entry -> entry.getValue() <= requestedAt
&& entry.getValue() >= requestedAt - NetConstants.ADV_TIME_OUT)
.mapToLong(entry -> new BlockId(entry.getKey().getHash()).getNum())
.max()
.orElse(lastNum);
if (lastNum < maxAdvInvNum) {

@xxo1shine xxo1shine Nov 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing BlockMsgHandler broadcasts a block before calling tronNetDelegate.processBlock. A peer can therefore announce block 100, subsequently fail to apply it locally, and remain at canonical height 99.

If we initiate a sync request within 15 seconds of receiving that announcement, the peer can truthfully return lastNum=99, remainNum=0 and still trigger SYNC_FAILED here because it previously announced 100. An announcement establishes that the peer forwarded the block, not that its canonical chain successfully applied it.

Please avoid treating a terminal reply as a sync failure solely on the basis of the announced height, and add a regression test for broadcasting a block followed by a local application failure.

@317787106 317787106 Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We agree that BlockMsgHandler can broadcast a block before local application succeeds. Therefore, a previous announcement does not prove that the block was successfully applied to the peer’s canonical chain. An honest peer can encounter the scenario described here.

However, this check is intended as a defensive synchronization-source policy. It addresses a repeated no-progress cycle observed in a production FullNode synchronization-stall incident.

A malicious peer can exploit the following sequence:

  1. Advertise blocks and become a source from which the receiver fetches block data.
  2. Exploit an existing synchronization delay, or contribute to it by delaying a required parent block. Subsequently delivered higher blocks cannot be linked locally, causing the receiver to request synchronization.
  3. Answer SYNC_BLOCK_CHAIN with only the already-known block at the end of the receiver’s summary, together with remain_num=0. This supplies no missing block IDs, despite the peer having recently announced and delivered higher blocks.
  4. The existing receiver treats that singleton response as synchronization completion and resumes accepting announcements from the connection.
  5. Repeat the cycle with newer blocks. The connection remains eligible as a broadcast source while repeatedly failing to help recover the missing chain. This can prolong the stall until another peer provides useful synchronization data.

The following is a sanitized, condensed extract from the observed logs. Peer identities and hashes are removed, heights are expressed relative to the stalled local head H, and timestamps are relative to when H was processed. The original comparisons used complete hashes and the same connection lifetime.

T+00.000  LOCAL: head=H

T+04.099  Peer A -> LOCAL: INVENTORY, BLOCK=H+3
T+05.218  Peer A -> LOCAL: BLOCK, height=H+3
T+05.220  LOCAL -> Peer A: SYNC_BLOCK_CHAIN, summary tail=H
T+05.399  Peer A -> LOCAL: BLOCK_CHAIN_INVENTORY,
                         size=1, last=H, remain=0

T+07.077  Peer A -> LOCAL: INVENTORY, BLOCK=H+4
T+07.730  Peer A -> LOCAL: BLOCK, height=H+4
T+07.732  LOCAL -> Peer A: SYNC_BLOCK_CHAIN, summary tail=H
T+07.912  Peer A -> LOCAL: BLOCK_CHAIN_INVENTORY,
                         size=1, last=H, remain=0

... further repetitions ...

T+61.097  Peer A -> LOCAL: INVENTORY, BLOCK=H+22
T+61.465  Peer A -> LOCAL: BLOCK, height=H+22
T+61.467  LOCAL -> Peer A: SYNC_BLOCK_CHAIN, summary tail=H
T+61.647  Peer A -> LOCAL: BLOCK_CHAIN_INVENTORY,
                         size=1, last=H, remain=0

T+61.747  Peer B -> LOCAL: BLOCK_CHAIN_INVENTORY,
                         blocks=[H..H+22], remain=0
T+62.567  LOCAL -> Peer B: FETCH_INV_DATA, blocks=[H+1..H+22]
T+63.350  LOCAL: successfully processes H+1

This was a 63.350-second interval without advancement beyond H, despite continuing to receive newer block announcements and block bodies. Progress resumed after another peer supplied the missing range.

Across the inspected log file, we found 36 such replies from 10 peer endpoints, out of 80 received ChainInventory replies. All 36 were singleton replies echoing the exact requested summary-tail hash with remain_num=0. In every case, the same connection had announced a higher block 71–1505ms before the synchronization request and had already delivered its matching block body. The other 44 replies had no preceding same-connection BLOCK INV in the available log and were not comparable under this metric. These are reply counts, not 36 independent stalls.

These observations establish the repeated message pattern, not malicious intent. Even receiving the higher block body does not prove that the peer successfully applied it. The defensive decision does not require that proof: we temporarily stop relying on a source whose recent announcements and terminal synchronization response indicate inconsistent availability.

Consequently, an honest peer affected by a fork or application failure may also be disconnected. We intentionally accept that possibility, using SYNC_FAIL and the ordinary disconnect/cooldown path. With alternative peers available, temporarily losing one candidate source is an acceptable cost compared with continuing a synchronization cycle that repeatedly makes no progress.

The core tradeoff is to accept occasional false-positive disconnects to reduce the risk of sustained synchronization stalls. This check breaks continued reliance on that connection; it does not claim to identify malicious peers conclusively or guarantee immediate recovery.

@0xbigapple 0xbigapple Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the log extract, it makes the target clear. All 36 bad replies have the same shape: size=1, last=<summary tail>, remain=0, i.e. they hit the blockIdWeGet.size() == 1 && containBlock(...) shortcut in processMessage that marks the peer SYNC_COMPLETED. That shortcut is the only path where the harm in the PR description can happen; a multi-block terminal reply queues the missing ids into syncBlockToFetch and recovers via fetch / syncNext.

The three throws in check() are wider than that. A multi-block reply ending below the HELLO head or below a recent announcement (the exact [H, H+1, H+2] an honest peer returns while it is still applying H+3, since BlockMsgHandler only relays blocks whose parent it already holds) carries useful ids and now costs the connection, and the size() > 1 summary-membership check cannot match any of the 36 samples at all.

Suggest scoping the policy to the observed signature: throw SYNC_FAILED only when the reply is a singleton echo of the summary tail with remain_num=0 and the peer announced a higher block in the window before the request. Multi-block replies keep the existing fetch path. That covers 100% of the incident pattern without the honest-peer disconnects in this thread and in the L158 thread, and does not need bounded renegotiation.

@317787106 317787106 Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The broadcast-before-apply race is possible, but its practical likelihood needs to account for the complete message exchange. Receiving a BLOCK INV does not directly trigger synchronization. The receiver first fetches the block and discovers that its parent is missing:

Peer P                                      Local node W

Broadcast INV(B) -------------------------->
                 <-------------------------- FETCH_INV_DATA(B)
Send BLOCK(B) ----------------------------->
                                             Detect missing parent
                                             Build chain summary
                 <-------------------------- SYNC_BLOCK_CHAIN
Read current head
Generate ChainInventory ------------------->

From P broadcasting the inventory to P reading its head for the synchronization response, this involves approximately two network round trips, plus block transfer, message processing, summary generation, and any queueing:

T ≈ 2 × RTT + transfer / processing / queueing time

Using 300ms as an illustrative block execution time, an RTT of 150–180ms already accounts for approximately 300–360ms before those additional costs. Under normal, successful block processing, P will therefore often have applied the advertised block by the time it handles SYNC_BLOCK_CHAIN.

The incident logs also support this timing argument. Among the 36 previously documented contradictory replies:

  • In 25 cases (69.44%), the interval from the local node receiving the preceding BLOCK INV to sending SYNC_BLOCK_CHAIN already exceeded 300ms.
  • The median interval was 368.5ms.
  • These intervals exclude the initial INV delivery time and the subsequent delivery of SYNC_BLOCK_CHAIN to the responder.

Assuming execution starts promptly after broadcasting and successfully completes within 300ms, those 25 cases cannot be explained solely by the responder still executing that advertised block.

This does not eliminate the race for low-latency connections, unusually slow execution, application failures, or fork switches. However, the existence of the race alone does not establish that honest-peer disconnects would be frequent. In the observed deployment, the complete exchange often provided more time than the assumed block execution duration. We accept occasional defensive disconnects to reduce the risk of sustained synchronization stalls.

@317787106 317787106 Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also measured the actual local block-processing times from the production FullNode logs, covering approximately 9.5 hours.

The dataset contains 10,968 successfully processed broadcast blocks, excluding duplicate-block fast returns and other cases without actual block processing.

All durations below are in milliseconds:

Measurement Samples Mean Median P95 P99 Maximum ≤300ms
Manager.pushBlock processing 10,968 110.43 106 151 173 783 99.87%
“Ready to broadcast” → “Success process block” 10,968 112.69 108 154 176 786 99.87%
Same broadcast-to-success interval within the 10-minute incident window 168 151.83 147.5 181 531 568 98.81%

The broadcast-to-success measurement is particularly relevant here: it includes broadcast overhead and subsequent lock waiting, rather than measuring only transaction execution. Records were matched by thread and complete block ID.

These measurements make 300ms a conservative reference for ordinary broadcast-block processing on this FullNode, although longer processing times do occur.

For comparison, in 33 of the 36 contradictory-reply cases (91.67%), the local interval from receiving the preceding BLOCK INV to sending SYNC_BLOCK_CHAIN already exceeded the measured 176ms P99 broadcast-to-success interval. This excludes the initial INV delivery time and the subsequent delivery of the synchronization request to the responder.

These are local measurements, not direct measurements of the remote peers. Nevertheless, they support the timing argument: the complete INV → FETCH → BLOCK → SYNC exchange will often outlast ordinary block processing. The broadcast-before-apply race remains possible, but its existence alone does not establish that this check would frequently disconnect honest peers.

throw new P2pException(TypeEnum.SYNC_FAILED,
"lastNum " + lastNum + " (remainNum=0) is below advertised block " + maxAdvInvNum);
}
}
}

}
6 changes: 4 additions & 2 deletions framework/src/main/java/org/tron/core/net/service/effective/ResilienceService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ public class ResilienceService {

private static final long inactiveThreshold =
CommonParameter.getInstance().getInactiveThreshold() * 1000L;
public static final long blockNotChangeThreshold = 60 * 1000L;
// Leave room for local GC pauses and disk stalls; a shorter threshold can evict healthy peers.
public static final long blockNotChangeThreshold = 30 * 1000L;

//when node is isolated, retention percent peers will not be disconnected
public static final double retentionPercent = 0.8;
Expand Down Expand Up @@ -68,13 +69,14 @@ public void init() {
}
}, initialDelay, 10, TimeUnit.SECONDS);

// Limit polling delay after the no-progress threshold is reached.
executor.scheduleWithFixedDelay(() -> {
try {
disconnectIsolated2();
} catch (Exception e) {
logger.error("DisconnectIsolated node failed", e);
}
}, initialDelay, 30, TimeUnit.SECONDS);
}, initialDelay, 10, TimeUnit.SECONDS);
}

private void disconnectRandom() {
Expand Down
2 changes: 1 addition & 1 deletion framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsVerifyProofTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3699,7 +3699,7 @@ public void verifyBurnWrongHash() throws ZksnarkException {

private Pair<Boolean, byte[]> verifyTransfer(byte[] input) {
transferContract.getEnergyForData(input);
transferContract.setVmShouldEndInUs(System.nanoTime() / 1000 + 80 * 1000);
transferContract.setVmShouldEndInUs(System.nanoTime() / 1000 + 200 * 1000);
return transferContract.execute(input);
}

Expand Down
2 changes: 1 addition & 1 deletion framework/src/test/java/org/tron/core/ShieldedTRC20BuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2376,7 +2376,7 @@ private byte[] abiEncodeForBurn(ShieldedTRC20Parameters params, long value) {

private Pair<Boolean, byte[]> verifyTransfer(byte[] input) {
transferContract.getEnergyForData(input);
transferContract.setVmShouldEndInUs(System.nanoTime() / 1000 + 80 * 1000);
transferContract.setVmShouldEndInUs(System.nanoTime() / 1000 + 200 * 1000);
Pair<Boolean, byte[]> ret = transferContract.execute(input);
return ret;
}
Expand Down
188 changes: 131 additions & 57 deletions framework/src/test/java/org/tron/core/net/messagehandler/ChainInventoryMsgHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.tron.core.net.service.adv.AdvService;
import org.tron.core.net.service.sync.SyncService;
import org.tron.protos.Protocol.Inventory.InventoryType;
import org.tron.protos.Protocol.ReasonCode;

public class ChainInventoryMsgHandlerTest {

Expand Down Expand Up @@ -111,6 +112,7 @@ public void testQueueChangedDuringProcessing() throws Exception {
BlockId next = new BlockId(Sha256Hash.ZERO_HASH, 1);
PeerConnection peer = Mockito.mock(PeerConnection.class);
Mockito.when(peer.getHelloMessageReceive()).thenReturn(helloMessage(0));
Mockito.doReturn(this.peer.getAdvInvReceive()).when(peer).getAdvInvReceive();
Deque<BlockId> toFetch = Mockito.mock(Deque.class);
LinkedList<BlockId> requested = new LinkedList<>();
requested.add(parent);
Expand Down Expand Up @@ -175,82 +177,154 @@ public void testKnownSummaryBlockCompletesDownloadAndPreservesUploadDirection()
}

@Test
public void testSingleBlockBelowHelloHeadUsesLocalBlockAvailability() throws Exception {
public void testSingleBlockBelowHelloHeadIsRejected() throws Exception {
for (long height : new long[]{50, 100, 199}) {
for (boolean needSyncFromUs : new boolean[]{false, true}) {
resetFixture();
peer.setHelloMessageReceive(helloMessage(200));
peer.setNeedSyncFromUs(needSyncFromUs);
request(blockId(0), blockId(50), blockId(100), blockId(199));
if (height > 100) {
peer.getSyncBlockToFetch().add(blockId(height));
}

respond(0, blockId(height));

Assert.assertEquals(needSyncFromUs, peer.isNeedSyncFromUs());
if (height <= 100) {
assertDownloadCompleted();
Assert.assertEquals(!needSyncFromUs, peer.isSyncFinish());
} else {
Assert.assertTrue(peer.isNeedSyncFromPeer());
Assert.assertEquals(Collections.singletonList(blockId(height)),
new ArrayList<>(peer.getSyncBlockToFetch()));
verify(sync).setFetchFlag(true);
verify(sync, never()).syncNext(any());
}
verify(peer, never()).disconnect(any());
assertRejected(new ChainInventoryMessage(Collections.singletonList(blockId(height)), 0L),
TypeEnum.SYNC_FAILED);
Assert.assertFalse(peer.isSyncFinish());
}
}
}

@Test
public void testTerminalMultiBlockBelowHelloHeadIsAccepted() throws Exception {
public void testTerminalResponseBelowPriorBlockInventoryIsRejected() throws Exception {
for (List<BlockId> ids : Arrays.asList(Collections.singletonList(blockId(100)),
Arrays.asList(blockId(99), blockId(100)),
Arrays.asList(blockId(100), blockId(101)))) {
resetFixture();
request(blockId(0), blockId(50), ids.get(0));
peer.getAdvInvReceive().put(new Item(blockId(102), InventoryType.BLOCK),
peer.getSyncChainRequested().getValue() - 1);

P2pException exception = assertRejected(new ChainInventoryMessage(ids, 0L),
TypeEnum.SYNC_FAILED);

Assert.assertEquals("lastNum " + ids.get(ids.size() - 1).getNum()
+ " (remainNum=0) is below advertised block 102", exception.getMessage());
}
}

@Test
public void testTerminalInventoryContradictionDisconnectsWithSyncFail() throws Exception {
peer.getAdvInvReceive().put(new Item(blockId(102), InventoryType.BLOCK),
peer.getSyncChainRequested().getValue() - 1);
Pair<Deque<BlockId>, Long> requested = peer.getSyncChainRequested();
P2pEventHandlerImpl events = new P2pEventHandlerImpl();
ReflectUtils.setFieldValue(events, "chainInventoryMsgHandler", handler);
ChainInventoryMessage response = new ChainInventoryMessage(
Collections.singletonList(blockId(100)), 0L);

ReflectUtils.invokeMethod(events, "processMessage",
new Class<?>[]{PeerConnection.class, byte[].class}, peer, response.getSendBytes());

verify(peer).disconnect(ReasonCode.SYNC_FAIL);
verify(peer, never()).disconnect(ReasonCode.BAD_PROTOCOL);
Assert.assertSame(requested, peer.getSyncChainRequested());
Assert.assertTrue(peer.isNeedSyncFromPeer());
Assert.assertEquals(TronState.SYNCING, peer.getTronState());
verify(sync, never()).syncNext(any());
verify(sync, never()).setFetchFlag(true);
}

@Test
public void testTerminalResponseReachesPriorAdvertisedBlock() throws Exception {
peer.getAdvInvReceive().put(new Item(blockId(101), InventoryType.BLOCK),
peer.getSyncChainRequested().getValue() - 1);

respond(0, blockId(100), blockId(101));

Assert.assertTrue(peer.isNeedSyncFromPeer());
Assert.assertEquals(Collections.singletonList(blockId(101)),
new ArrayList<>(peer.getSyncBlockToFetch()));
verify(sync).setFetchFlag(true);
verify(peer, never()).disconnect(any());
}

@Test
public void testTerminalResponseIgnoresInventoryAfterRequest() throws Exception {
peer.getAdvInvReceive().put(new Item(blockId(102), InventoryType.BLOCK),
peer.getSyncChainRequested().getValue() + 1);

respond(0, blockId(100));

assertDownloadCompleted();
verify(peer, never()).disconnect(any());
}

@Test
public void testTerminalResponseIgnoresStaleBlockInventory() throws Exception {
peer.getAdvInvReceive().put(new Item(blockId(102), InventoryType.BLOCK),
peer.getSyncChainRequested().getValue() - NetConstants.ADV_TIME_OUT - 1);

respond(0, blockId(100));

assertDownloadCompleted();
verify(peer, never()).disconnect(any());
}

@Test
public void testTerminalResponseIgnoresTransactionInventory() throws Exception {
peer.getAdvInvReceive().put(new Item(blockId(102), InventoryType.TRX),
peer.getSyncChainRequested().getValue() - 1);

respond(0, blockId(100));

assertDownloadCompleted();
verify(peer, never()).disconnect(any());
}

@Test
public void testPagedResponseCanEndBelowPriorAdvertisedBlock() throws Exception {
List<BlockId> ids = range(100, (int) NetConstants.SYNC_FETCH_BATCH_NUM + 1);
peer.getAdvInvReceive().put(new Item(blockId(ids.get(ids.size() - 1).getNum() + 10),
InventoryType.BLOCK), peer.getSyncChainRequested().getValue() - 1);

handler.processMessage(peer, new ChainInventoryMessage(ids, 10L));

Assert.assertTrue(peer.isNeedSyncFromPeer());
Assert.assertEquals(10, peer.getRemainNum());
verify(sync).setFetchFlag(true);
verify(peer, never()).disconnect(any());
}

@Test
public void testTerminalMultiBlockBelowHelloHeadIsRejected() throws Exception {
for (int size : new int[]{2, 3, (int) NetConstants.SYNC_FETCH_BATCH_NUM + 1}) {
resetFixture();
List<BlockId> ids = range(50, size);
peer.setHelloMessageReceive(helloMessage(ids.get(ids.size() - 1).getNum() + 1));
peer.setBlockBothHave(blockId(50));
peer.getSyncBlockToFetch().add(blockId(101));

handler.processMessage(peer, new ChainInventoryMessage(ids, 0L));

List<BlockId> pending = new ArrayList<>(ids);
pending.removeIf(id -> id.getNum() <= 100);
Assert.assertEquals(pending, new ArrayList<>(peer.getSyncBlockToFetch()));
Assert.assertTrue(peer.isNeedSyncFromPeer());
Assert.assertNull(peer.getSyncChainRequested());
Assert.assertEquals(0, peer.getRemainNum());
if (pending.isEmpty()) {
verify(sync).syncNext(peer);
} else {
verify(sync).setFetchFlag(true);
}
verify(peer, never()).disconnect(any());
assertRejected(new ChainInventoryMessage(ids, 0L), TypeEnum.SYNC_FAILED);
}
}

@Test
public void testTerminalResponseBelowHelloHeadDoesNotDisconnect() throws Exception {
public void testTerminalResponseBelowHelloHeadDisconnectsWithSyncFail() throws Exception {
for (int size : new int[]{1, 2, 3}) {
resetFixture();
peer.setHelloMessageReceive(helloMessage(200));
Pair<Deque<BlockId>, Long> requested = peer.getSyncChainRequested();
P2pEventHandlerImpl events = new P2pEventHandlerImpl();
ReflectUtils.setFieldValue(events, "chainInventoryMsgHandler", handler);
ChainInventoryMessage response = new ChainInventoryMessage(range(50, size), 0L);

ReflectUtils.invokeMethod(events, "processMessage",
new Class<?>[]{PeerConnection.class, byte[].class}, peer, response.getSendBytes());

verify(peer, never()).disconnect(any());
Assert.assertNull(peer.getSyncChainRequested());
if (size == 1) {
assertDownloadCompleted();
} else {
Assert.assertTrue(peer.isNeedSyncFromPeer());
Assert.assertEquals(TronState.SYNCING, peer.getTronState());
verify(sync).syncNext(peer);
}
verify(peer).disconnect(ReasonCode.SYNC_FAIL);
verify(peer, never()).disconnect(ReasonCode.BAD_PROTOCOL);
Assert.assertSame(requested, peer.getSyncChainRequested());
Assert.assertTrue(peer.isNeedSyncFromPeer());
Assert.assertEquals(TronState.SYNCING, peer.getTronState());
verify(sync, never()).syncNext(any());
verify(sync, never()).setFetchFlag(true);
}
}
Expand Down Expand Up @@ -479,24 +553,26 @@ public void testKnownMultiBlockResponseClearsRemoteUploadState() throws Exceptio
}

@Test
public void testRepeatedKnownTerminalResponseRequestsAnotherSummary() throws Exception {
public void testRepeatedKnownTerminalResponseIsRejected() throws Exception {
peer.setHelloMessageReceive(helloMessage(52));
respond(0, blockId(50), blockId(51), blockId(52));
verify(sync).syncNext(peer);

request(blockId(0), blockId(50), blockId(52));
respond(0, blockId(50), blockId(51), blockId(52));
Pair<Deque<BlockId>, Long> requested = peer.getSyncChainRequested();
long updateTime = peer.getBlockBothHaveUpdateTime();
P2pException exception = Assert.assertThrows(P2pException.class,
() -> respond(0, blockId(50), blockId(51), blockId(52)));

Assert.assertNull(peer.getSyncChainRequested());
Assert.assertEquals(blockId(52), peer.getBlockBothHave());
Assert.assertTrue(peer.getSyncBlockToFetch().isEmpty());
Assert.assertEquals(TypeEnum.SYNC_FAILED, exception.getType());
Assert.assertSame(requested, peer.getSyncChainRequested());
Assert.assertEquals(updateTime, peer.getBlockBothHaveUpdateTime());
Assert.assertTrue(peer.isNeedSyncFromPeer());
verify(sync, times(2)).syncNext(peer);
verify(peer, never()).disconnect(any());
verify(sync).syncNext(peer);
}

@Test
public void testReorgDuringSummaryResponseRequestsAnotherSummary() throws Exception {
public void testReorgDuringSummaryResponseDisconnectsWithSyncFail() throws Exception {
request(blockId(0), blockId(51), blockId(76), blockId(89), blockId(95), blockId(98),
blockId(100));
AtomicBoolean forkSwitched = new AtomicBoolean();
Expand Down Expand Up @@ -527,18 +603,16 @@ public void testReorgDuringSummaryResponseRequestsAnotherSummary() throws Except
Assert.assertEquals(Arrays.asList(blockId(98), blockId(99), blockId(100)),
response.getBlockIds());
Assert.assertEquals(Long.valueOf(0), response.getRemainNum());
P2pException exception = assertRejected(response, TypeEnum.SYNC_FAILED);
Assert.assertTrue(exception.getMessage().startsWith("terminal block already in summary:"));

P2pEventHandlerImpl events = new P2pEventHandlerImpl();
ReflectUtils.setFieldValue(events, "chainInventoryMsgHandler", handler);
ReflectUtils.invokeMethod(events, "processMessage",
new Class<?>[]{PeerConnection.class, byte[].class}, peer, response.getSendBytes());

verify(peer, never()).disconnect(any());
Assert.assertNull(peer.getSyncChainRequested());
Assert.assertEquals(blockId(100), peer.getBlockBothHave());
Assert.assertTrue(peer.getSyncBlockToFetch().isEmpty());
Assert.assertTrue(peer.isNeedSyncFromPeer());
verify(sync).syncNext(peer);
verify(peer).disconnect(ReasonCode.SYNC_FAIL);
verify(peer, never()).disconnect(ReasonCode.BAD_PROTOCOL);
}

@Test
Expand Down
Loading