Skip to content

fix(net): improve block source selection and disconnect recovery - #7106

Open
317787106 wants to merge 2 commits into
tronprotocol:release_v4.8.3from
317787106:fix/block-fetch-disconnect
Open

317787106 wants to merge 2 commits into
tronprotocol:release_v4.8.3from
317787106:fix/block-fetch-disconnect

Conversation

@317787106

@317787106 317787106 commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

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 entire advInvRequest map 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 within ADV_TIME_OUT.

Keep the two source-ranking policies:

  • Initial fetching prefers fully idle eligible announcers, then eligible announcers with pending TRX requests. Within each group, prefer smaller batches assigned during the current scheduling pass.
  • Backup fetching continues to prefer lower 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:

Remaining state Recovery
Block is in the advertisement cache or local storage Skip another fetch
Another connected peer has the same BLOCK request pending Reuse the pending request without resending
Another connected peer has announced the block Requeue it for normal eligibility-based scheduling
No source remains Remove the queued item and invalidate its deduplication entry so a future announcement can enqueue it

When a pending request is eligible for headNum + 1 tracking, fetchBlock() reads its original advInvRequest timestamp. 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 with BAD_BLOCK and "block number does not follow parent" when any of these conditions holds:

  • The block number is nonpositive.
  • The raw parent hash is not exactly Sha256Hash.LENGTH (32 bytes).
  • The block number encoded in the parent hash is not 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:

Test class Tests Main coverage
BlockFetchSchedulingTest 20 Initial TRX fallback, idle-source preference, backup latency ranking, eligibility, block ordering, TRX batching, pending-request reuse, original timestamps, cache/storage skips, request snapshots, stale workers, and shutdown cleanup
FetchBlockServiceTest 12 Existing latency estimation, source selection, and timeout behavior
AdvServiceTest 3 Broadcast/fetch behavior and concurrent disconnect cleanup
BlockFetchFailureTest 26 Existing advertised/sync failure handling, signature/Merkle rejection, and inactive-witness behavior
PeerDisconnectConcurrencyTest 5 Disconnect, handshake, timeout, and deferred-fetch concurrency regressions
TransactionsMsgHandlerTest 28 Existing transaction admission and disconnect recovery

At d07048a634, 65 targeted tests passed with no failures, errors, or skipped tests:

Test class Tests Main coverage
BlockMsgHandlerTest 15 Nonpositive/mismatched block numbers; empty, short, and oversized parent hashes; advertisement/sync request preservation; rejection before sanitization; fast-forward/relay rejection and valid unrequested ingress
BlockAlreadyKnownCounterTest 4 Existing duplicate-counter behavior with structurally valid parent references
BlockFetchFailureTest 26 Existing advertised/sync failure and disconnect behavior
BlockFetchSchedulingTest 20 Source eligibility and disconnect-recovery regressions

The CI-scoped Checkstyle command, Math usage check, and git diff --check also 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

  • Evaluate backlog throughput with few eligible sources: reserving one BLOCK request makes that peer unavailable for another initial block assignment until completion.
  • Continued retry tracking after the short timeout or a backup dispatch remains separate work. This PR retains the release baseline's tracking-clear behavior, latency-switch conditions, and queue-expiration policy.
  • Already-sent future-height requests are not automatically promoted when the head advances. Atomicity across request completion, block validation/cache insertion, and disconnect recovery also remains follow-up work.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant