Repository navigation
Conversation
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR do?
Allow eligible peers with pending transaction requests to serve advertised blocks, and recover outstanding block fetches after a disconnect without blindly resending requests already pending elsewhere. Reject malformed block numbers and parent references before clearing fetch requests.
1. Pending TRX requests exclude otherwise eligible block providers
Bug and impact
Initial and backup fetching previously used
PeerConnection.isIdle(), which requires the entireadvInvRequestmap to be empty. A peer with pending TRX requests was excluded even if it had announced the missing block and had no block or sync work outstanding. If every available announcer had pending transactions, block fetching had to wait.Solution
Add
isBlockFetchIdle(): no outstanding BLOCK request, sync-block request, chain-summary request, or sync block being processed. Pending TRX requests are allowed.Initial and backup selection share
FetchBlockService.canFetchBlock(), which also requires a connected peer, both sync-direction flags cleared, and an announcement received withinADV_TIME_OUT.Keep the two source-ranking policies:
getFetchLatency()estimates, including eligible peers with pending TRX requests.Sort queued BLOCK items by height before reserving requests, because a reservation makes that source busy for subsequent block selection. Skip cached, stored, or already-requested blocks. Leave items without an eligible source queued; an unavailable earlier block does not prevent fetching an available later block. Transaction fetching retains its existing batching behavior.
Pending TRX requests alone do not establish spare connection capacity. Block and transaction responses can still queue on the same connection; this change removes an eligibility restriction without claiming a measured propagation-latency improvement.
2. Disconnect recovery can resend a block already pending elsewhere
Bug and impact
Disconnect recovery previously requeued an outstanding item whenever another peer had advertised it. It did not check whether the block had arrived, whether another connected peer already owed the same response, or whether retry tracking still pointed to the disconnected provider.
For example, if A and backup B both have a request for block H, disconnecting A can requeue H and cause an unnecessary request to another announcer despite B's pending response.
Solution
Snapshot outstanding BLOCK items before
PeerConnection.onDisconnect()clears the request map. Submit block recovery to the advertisement fetch executor, preserving the deferred-recovery approach introduced by tronprotocol/java-tron#7101. Disconnect callbacks do not wait for fetch processing or send replacement requests.On the fetch executor, clear tracking owned by the disconnected peer, even if its outstanding request has already been removed, then handle each snapshotted item:
When a pending request is eligible for
headNum + 1tracking,fetchBlock()reads its originaladvInvRequesttimestamp. Recovery therefore preserves the request's elapsed time and does not restart its deadline.Serialize tracking registration, completion, disconnect cleanup, and worker transitions. Reject stale worker snapshots and require a request still to exist before registering it, so recovery cannot restore completed work or let an old worker clear newly recovered tracking. Coordinate block queue recovery with normal scheduling through the advertisement lock. Executor shutdown rejection is tolerated so a late transport callback can still finish clearing peer state.
3. Malformed block headers must be rejected before fetch completion
Bug and impact
A response with an invalid block number or parent reference must be rejected while its outstanding request is still present. If the handler consumes the request first, disconnect recovery cannot identify the failed fetch and schedule a replacement from another announcer.
Solution
In
BlockMsgHandler.check(), reject the response withBAD_BLOCKand"block number does not follow parent"when any of these conditions holds:Sha256Hash.LENGTH(32 bytes).blockNum - 1.Run these checks before
sanitize(), advertised fetch completion, and sync-request removal or handoff. A structural rejection preserves the original advertised or sync request and its timestamp for disconnect cleanup.Apply structural validation to ordinary advertisement and sync responses as well as fast-forward and relay ingress. Fast-forward and relay peers retain their existing exemption from the matching-request check; they must still provide a structurally valid header.
Why are these changes required?
Block-source eligibility should distinguish pending transaction work from pending block or sync work. Disconnect recovery should account for data already available and requests already in flight before scheduling replacement work. These changes address ordinary busy connections, slow responses, and disconnects while keeping each surviving provider's original request deadline. Early header validation also keeps malformed responses from consuming the request state needed by disconnect recovery.
This PR has been tested by:
On ARM64 / JDK 17 at
5c6104071b, 94 targeted tests passed with no failures, errors, or skipped tests:BlockFetchSchedulingTestFetchBlockServiceTestAdvServiceTestBlockFetchFailureTestPeerDisconnectConcurrencyTestTransactionsMsgHandlerTestAt
d07048a634, 65 targeted tests passed with no failures, errors, or skipped tests:BlockMsgHandlerTestBlockAlreadyKnownCounterTestBlockFetchFailureTestBlockFetchSchedulingTestThe CI-scoped Checkstyle command, Math usage check, and
git diff --checkalso passed for this update.The focused Gradle run compiled production and test sources. The full repository test suite, x86/JDK 8 execution, and manual network testing were not run.
Follow up
Extra details
This PR extracts block-source eligibility, disconnect recovery, and block-number/parent-reference validation from tronprotocol/java-tron#7005, with the tracking guards needed for safe recovery. BLOCK INV activity accounting, block contribution attribution, and the persistent retry/two-request-cap design from the broader PR remain outside this change.
No wire-format, database-format, configuration, or dependency changes. The added APIs and collections are Java 8 compatible.