Skip to content

feat(net): add P2P message deduplication and length validation#6712

Open
xxo1shine wants to merge 1 commit intotronprotocol:developfrom
xxo1shine:feature/p2p-msg-validation-1
Open

feat(net): add P2P message deduplication and length validation#6712
xxo1shine wants to merge 1 commit intotronprotocol:developfrom
xxo1shine:feature/p2p-msg-validation-1

Conversation

@xxo1shine
Copy link
Copy Markdown
Collaborator

@xxo1shine xxo1shine commented Apr 28, 2026

What does this PR do?

Add P2P message deduplication and length validation:

  • FetchInvDataMsgHandler: reject messages with duplicate hashes
  • TransactionsMsgHandler: reject messages with duplicate transactions
  • SyncBlockChainMsgHandler: reject blockIds list exceeding 30 entries
  • Add MAX_SYNC_CHAIN_IDS = 30 constant to NetConstants
  • Add unit tests covering duplicate rejection and boundary values

All violations throw P2pException(BAD_MESSAGE), triggering peer disconnect via existing P2pEventHandlerImpl error path.

Fixes #6667

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

- FetchInvDataMsgHandler: reject messages with duplicate hashes
- TransactionsMsgHandler: reject messages with duplicate transactions
- SyncBlockChainMsgHandler: reject blockIds list exceeding 30 entries
- Add MAX_SYNC_CHAIN_IDS = 30 constant to NetConstants
- Add unit tests covering duplicate rejection and boundary values

All violations throw P2pException(BAD_MESSAGE), triggering peer disconnect via existing P2pEventHandlerImpl error path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot requested a review from 317787106 April 28, 2026 07:30
@halibobo1205 halibobo1205 added the topic:net p2p net work, synchronization label Apr 28, 2026
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.2 milestone Apr 28, 2026
public static final int MSG_CACHE_DURATION_IN_BLOCKS = 5;
public static final int MAX_BLOCK_FETCH_PER_PEER = 100;
public static final int MAX_TRX_FETCH_PER_PEER = 1000;
public static final int MAX_SYNC_CHAIN_IDS = 30;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice to have a named constant here instead of a magic number! 👍

Quick question — how was 30 chosen? Observed from real traffic, or estimated from getBlockChainSummary()'s log-step algorithm?

@lvs0075 lvs0075 requested a review from waynercheung April 30, 2026 04:22
Copy link
Copy Markdown
Collaborator

@waynercheung waynercheung left a comment

Choose a reason for hiding this comment

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

LGTM

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

Labels

topic:net p2p net work, synchronization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add deduplication and length checks for p2p messages

5 participants