fix(net): correct sync completion and chain summary request timeouts #7000
+669
−43
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package org.tron.core.net; | ||
|
|
||
| import static org.mockito.ArgumentMatchers.any; | ||
| import static org.mockito.Mockito.doNothing; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.spy; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| import java.net.InetSocketAddress; | ||
| import org.tron.common.utils.ReflectUtils; | ||
| import org.tron.common.utils.Sha256Hash; | ||
| import org.tron.core.capsule.BlockCapsule.BlockId; | ||
| import org.tron.core.net.message.handshake.HelloMessage; | ||
| import org.tron.core.net.peer.PeerConnection; | ||
| import org.tron.p2p.connection.Channel; | ||
| import org.tron.protos.Protocol; | ||
|
|
||
| public final class PeerSyncTestSupport { | ||
|
|
||
| private PeerSyncTestSupport() { | ||
| } | ||
|
|
||
| public static BlockId blockId(long number) { | ||
| return new BlockId(Sha256Hash.ZERO_HASH, number); | ||
| } | ||
|
|
||
| public static HelloMessage helloMessage(long headNum) throws Exception { | ||
| return new HelloMessage(Protocol.HelloMessage.newBuilder() | ||
| .setHeadBlockId(Protocol.HelloMessage.BlockId.newBuilder() | ||
| .setHash(blockId(headNum).getByteString()).setNumber(headNum)) | ||
| .build().toByteArray()); | ||
| } | ||
|
|
||
| public static PeerConnection peer(int port) { | ||
| PeerConnection peer = spy(new PeerConnection()); | ||
| Channel channel = mock(Channel.class); | ||
| InetSocketAddress address = new InetSocketAddress("127.0.0.1", port); | ||
| when(channel.getInetSocketAddress()).thenReturn(address); | ||
| when(channel.getInetAddress()).thenReturn(address.getAddress()); | ||
| ReflectUtils.setFieldValue(peer, "channel", channel); | ||
| doNothing().when(peer).sendMessage(any()); | ||
| doNothing().when(peer).disconnect(any()); | ||
| return peer; | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[SHOULD] A single known block can still prematurely terminate synchronization despite being inconsistent with the peer’s advertised height.
If the peer advertises
head=200in HELLO and the local node sends summary[0,50,100], a response containing only[50]withremainNum=0still passescheck(). It then clearssyncChainRequested, setsSYNC_COMPLETED, and setsneedSyncFromPeer=false. WhenneedSyncFromUs=false,isSyncFinish()also becomestrue, so the connection immediately exits both the response-timeout check and the 30-second sync-no-progress check.Please handle terminal responses that are inconsistent with the height previously advertised by the peer, while preserving appropriate no-progress handling and compatibility with normal responses from honestly lagging peers. Simply rejecting responses that end before the last block in the summary is insufficient:
[100], remainNum=0produces the same result.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 4e46755.
check()now rejects single-block responses below the peer’s advertised HELLO head before clearing the pending request or changing synchronization state. Both[50], remainNum=0and[100], remainNum=0therefore trigger aSYNC_FAILdisconnect when HELLO advertised height 200.Normal responses from lagging peers remain accepted—for example, HELLO=50 followed by
[50], 0when our local head has advanced to 100. A response observed during a temporary rollback can also trigger disconnection; usingSYNC_FAILallows recovery through a fresh handshake without the one-hourBAD_PROTOCOLban.Regression tests cover rejection, unchanged synchronization state, the actual disconnect reason, and compatibility with lagging peers. All 58 related tests and both Checkstyle checks passed locally.