Skip to content

feat(net): normalize inbound messages#6797

Open
halibobo1205 wants to merge 2 commits into
tronprotocol:release_v4.8.2from
halibobo1205:fix/sanitize-inbound-protobuf-unknown-fields
Open

feat(net): normalize inbound messages#6797
halibobo1205 wants to merge 2 commits into
tronprotocol:release_v4.8.2from
halibobo1205:fix/sanitize-inbound-protobuf-unknown-fields

Conversation

@halibobo1205
Copy link
Copy Markdown
Collaborator

@halibobo1205 halibobo1205 commented May 22, 2026

Summary

Normalize inbound BlockMessage / TransactionMessage / TransactionsMessage on raw byte[] decode paths.

Sanitize is applied only on the byte[] constructors. Object-form constructors (BlockMessage(BlockCapsule), TransactionMessage(Transaction), TransactionsMessage(List<Transaction>)) intentionally keep existing behavior and preserve trusted local/store representations.

What is sanitized

  • BlockMessage(byte[])

    • Strips outer unknown fields from Block.
    • Strips outer unknown fields from BlockHeader.
    • Does not touch BlockHeader.raw_data.
    • Does not rewrite transactions inside the block.
  • TransactionMessage(byte[])

    • Strips top-level unknown fields from Transaction.
    • Does not touch Transaction.raw_data.
  • TransactionsMessage(byte[])

    • Strips wrapper-level unknown fields from Protocol.Transactions.
    • Strips top-level unknown fields from each nested Transaction.
    • Does not touch nested Transaction.raw_data.

This keeps inbound wire representation normalized while leaving all consensus-hashed / signed regions byte-identical.

Size cap

Each inbound constructor applies the same size cap already enforced downstream, but earlier in the pipeline:

  • BlockMessage: rejects raw data.length > BLOCK_SIZE + ONE_THOUSAND before protobuf parse.
  • TransactionMessage: parses the tx first, then applies a Manager.validateCommon-equivalent size check against TRANSACTION_MAX_BYTE_SIZE (500 KB).
  • TransactionsMessage: parses the wrapper first, then applies the same per-transaction size check to every nested tx.

Performance note

The common no-unknown-fields path reuses the original inbound bytes when parsed.getSerializedSize() == data.length, so sanitize does not rebuild or reserialize clean messages.

For transaction messages, the size check still performs the same kind of TransactionCapsule / serialized-size work that previously happened in Manager.validateCommon; this work is moved earlier so oversized padded txs are rejected before entering tx processing.

Compatibility

TL;DR

  • Normal business traffic is unchanged.
  • Block hash, transaction id, signature verification, merkle root, fork choice, and consensus validation are unchanged.
  • Sanitize never modifies BlockHeader.raw_data or Transaction.raw_data.
  • No consensus impact / no fork risk found.
  • Only invalid oversized or padded inbound payloads see behavior changes.

Throw site moved earlier

Path Before After
P2P inbound oversize block BlockMsgHandler.processMessage, after parse BlockMessage(byte[]), before parse
P2P inbound oversize tx Manager.validateCommon, during pushTransaction TransactionMessage(byte[]) / TransactionsMessage(byte[]), after protobuf parse but before handler processing
HTTP / gRPC oversize tx submit Manager.validateCommon, via Wallet.broadcastTransaction -> pushTransaction TransactionMessage(byte[]), after protobuf parse but before checkExpiration / pushTransaction

Exception type changes

Before After
Oversize block P2pException(BAD_MESSAGE) P2pException(BAD_MESSAGE) (unchanged)
Oversize tx TooBigTransactionException downstream, or P2pException(TRX_EXE_FAILED) after TronNetDelegate wrapping P2pException(BAD_MESSAGE) from the message constructor

Downstream effects:

  • P2P inbound oversize tx: previously Manager.validateCommon threw TooBigTransactionException, TronNetDelegate.pushTransaction wrapped it as P2pException(TRX_EXE_FAILED), and TransactionsMsgHandler logged it without disconnecting the peer. After this PR, the constructor throws P2pException(BAD_MESSAGE), which surfaces through TronMessageFactory.create into P2pEventHandlerImpl.processException; BAD_MESSAGE maps to ReasonCode.BAD_PROTOCOL, so the peer is disconnected. This aligns oversize tx handling with the existing oversize block behavior.
  • HTTP / gRPC oversize tx submit: Wallet.broadcastTransaction catches P2pException(BAD_MESSAGE) and maps it back to response_code.TOO_BIG_TRANSACTION_ERROR, so code-based callers keep the same response code. The response message string changes as described below.

Exception detail strings

Before After
Oversize block "block size over limit" "block size over limit"
Oversize tx "Too big transaction..." Manager-format messages "transaction size over limit"

For HTTP / gRPC oversize submissions, response.code remains TOO_BIG_TRANSACTION_ERROR; only response.message changes. Callers that string-match on the old Manager-format text may need adjustment.

Invalid compound payload ordering

Because oversized checks now happen earlier, invalid payloads that are both oversized and otherwise invalid may report the size error first. Examples:

  • Oversize + expired HTTP/gRPC tx now returns TOO_BIG_TRANSACTION_ERROR before expiration is checked.
  • Oversized TRXS payloads are rejected during message construction before duplicate/no-request/contract-count checks.

This only affects invalid oversized inputs. Normal, valid traffic is unchanged.

Log changes

  • Oversized blocks are rejected before parsing, so BlockMsgHandler no longer logs the block id for that case.
  • The failure is logged by P2pEventHandlerImpl.processException; because the message object was not constructed, the log may contain msg = null.
  • Oversized txs rejected by constructors no longer reach Manager.validateCommon, so Manager's old TooBigTransactionException logs do not fire for those inbound paths.

Stored / rebroadcast representation

After this PR, inbound byte-decoded messages that contain targeted unknown fields are stored/rebroadcast without those unknown fields. Pre-upgrade nodes may keep the original padded wire bytes.

The logical identifiers are unchanged because sanitize does not touch BlockHeader.raw_data or Transaction.raw_data. During a rolling upgrade, nodes may temporarily have different on-disk byte representations for the same logical block/tx, but the block id / tx id and consensus behavior remain the same.

@github-actions github-actions Bot requested a review from 317787106 May 22, 2026 11:17
@halibobo1205 halibobo1205 requested a review from xxo1shine May 22, 2026 11:18
@halibobo1205 halibobo1205 added the topic:net p2p net work, synchronization label May 22, 2026
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.2 milestone May 22, 2026
@halibobo1205 halibobo1205 changed the title feat(net): strip unknown protobuf fields on inbound messages feat(net):strip unknown protobuf fields on inbound messages May 22, 2026
@halibobo1205 halibobo1205 changed the title feat(net):strip unknown protobuf fields on inbound messages feat(net): strip unknown protobuf fields on inbound messages May 22, 2026
@halibobo1205 halibobo1205 changed the title feat(net): strip unknown protobuf fields on inbound messages feat(net): normalize inbound messages May 22, 2026
@halibobo1205 halibobo1205 changed the title feat(net): normalize inbound messages feat(net): normalize inbound messages May 22, 2026
@halibobo1205 halibobo1205 requested a review from waynercheung May 22, 2026 11:43
@halibobo1205 halibobo1205 force-pushed the fix/sanitize-inbound-protobuf-unknown-fields branch from 6cb7d58 to 656cb38 Compare May 23, 2026 07:35
- BlockMessage strips Block and BlockHeader outer unknown fields and enforces the existing BLOCK_SIZE + ONE_THOUSAND cap before parsing.
- TransactionMessage strips top-level transaction unknown fields and rejects oversized transactions with Manager.validateCommon-equivalent size checks after parsing.
- TransactionsMessage strips wrapper and nested transaction top-level unknown fields and applies the same per-transaction size check to each entry.
@halibobo1205 halibobo1205 force-pushed the fix/sanitize-inbound-protobuf-unknown-fields branch from 656cb38 to cdcceba Compare May 23, 2026 08:13
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.

1 participant