Skip to content

refactor(net): truncate oversized signatures#34

Open
Federico2014 wants to merge 1 commit into
release_v4.8.2from
fix/net/truncate-oversized-signature
Open

refactor(net): truncate oversized signatures#34
Federico2014 wants to merge 1 commit into
release_v4.8.2from
fix/net/truncate-oversized-signature

Conversation

@Federico2014
Copy link
Copy Markdown
Owner

@Federico2014 Federico2014 commented May 26, 2026

What does this PR do?

Follow-up to tronprotocol#6782. For transaction signatures longer than 68 bytes, truncate the trailing bytes to exactly 68 instead of rejecting the transaction. Signatures shorter than 65 bytes are no longer rejected up front — they fall through to downstream signature verification, which still rejects them.

  • TransactionCapsule.sanitizeSignatures() truncates each oversized signature to 68 bytes; signatures already in range are untouched (no rebuild, no transaction-id change).
  • TransactionsMessage.sanitizeSignature() sanitizes every contained transaction and rewrites the wire bytes when anything changed.
  • Wallet.broadcastTransaction and TransactionsMsgHandler.check call sanitize first, then drop the now-redundant per-signature length validation.
  • RelayService (peer hello handshake) keeps the strict [65, 68] check inline; SignUtils.isValidLength is removed since no caller remains.

Why are these changes required?

The strict admission filter from tronprotocol#6782 also rejects historical transactions whose signatures legitimately carry trailing padding beyond 68 bytes. Truncating those bytes preserves the canonical 65-byte r/s/v while still admitting the transaction, matching the sanitize pattern introduced in tronprotocol#6797.

This PR has been tested by:

  • Unit Tests
    • new SanitizeSignaturesTest (capsule + message-level, 8 cases)
    • updated WalletMockTest.testBroadcastTxInvalidSigLength
    • updated TransactionsMsgHandlerTest.testInvalidSigLength
  • ./gradlew :framework:checkstyleMain :framework:checkstyleTest

Follow up

None.

Extra details

Peer hello handshake (RelayService) intentionally stays strict — hello signatures are not on the transaction admission path and have no historical padding to accommodate.


Summary by cubic

Truncate transaction signatures longer than 68 bytes at admission instead of rejecting them to restore compatibility with historical transactions. Upfront length checks are removed; undersized signatures still fail during downstream verification.

  • Refactors
    • Add sanitize in TransactionCapsule to trim each signature to 68 bytes; in-range signatures are untouched and txid stays the same.
    • Add sanitize in TransactionsMessage and rewrite wire bytes only when changed.
    • Call sanitize in Wallet.broadcastTransaction and TransactionsMsgHandler; remove redundant per-signature length checks.
    • Keep strict [65, 68] check in RelayService hello; remove SignUtils.isValidLength.

Written for commit 6ff34e6. Summary will update on new commits. Review in cubic

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: da6e7590-3d75-47f1-8b9b-8456fd75f3e3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/net/truncate-oversized-signature

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Federico2014 Federico2014 marked this pull request as ready for review May 26, 2026 13:13
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 9 files

Re-trigger cubic

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