From 1dd07e7f60b1d85e1a35486c93237e1c131da9e4 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Fri, 22 May 2026 19:02:10 +0800 Subject: [PATCH 1/2] feat(net): normalize inbound messages --- .../org/tron/core/capsule/BlockCapsule.java | 20 ++ .../tron/core/capsule/TransactionCapsule.java | 9 + .../src/main/java/org/tron/core/Wallet.java | 2 +- .../main/java/org/tron/core/db/Manager.java | 3 + .../core/net/message/adv/BlockMessage.java | 5 + .../net/messagehandler/BlockMsgHandler.java | 2 + .../adv/SanitizeUnknownFieldsTest.java | 180 ++++++++++++++++++ 7 files changed, 220 insertions(+), 1 deletion(-) create mode 100644 framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java diff --git a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java index 34b7853d4d1..fcfd6526b26 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java @@ -21,6 +21,7 @@ import com.google.protobuf.ByteString; import com.google.protobuf.CodedInputStream; import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.UnknownFieldSet; import java.security.SignatureException; import java.util.ArrayList; import java.util.Arrays; @@ -328,6 +329,25 @@ public boolean hasWitnessSignature() { return !getInstance().getBlockHeader().getWitnessSignature().isEmpty(); } + public void sanitize() { + boolean blockHasUnknown = !this.block.getUnknownFields().asMap().isEmpty(); + boolean headerHasUnknown = !this.block.getBlockHeader().getUnknownFields().asMap().isEmpty(); + if (!blockHasUnknown && !headerHasUnknown) { + return; + } + UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance(); + Block.Builder builder = this.block.toBuilder(); + if (blockHasUnknown) { + builder.setUnknownFields(empty); + } + if (headerHasUnknown) { + builder.setBlockHeader(this.block.getBlockHeader().toBuilder() + .setUnknownFields(empty) + .build()); + } + this.block = builder.build(); + } + @Override public String toString() { StringBuilder toStringBuff = new StringBuilder(); diff --git a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java index bb4b70cde1b..bfd3ee305c5 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java @@ -28,6 +28,7 @@ import com.google.protobuf.GeneratedMessageV3; import com.google.protobuf.Internal; import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.UnknownFieldSet; import java.io.IOException; import java.security.SignatureException; import java.util.ArrayList; @@ -494,6 +495,14 @@ public static boolean validateSignature(Transaction transaction, return false; } + public void sanitize() { + if (!this.transaction.getUnknownFields().asMap().isEmpty()) { + this.transaction = transaction.toBuilder() + .setUnknownFields(UnknownFieldSet.getDefaultInstance()) + .build(); + } + } + public void resetResult() { if (this.getInstance().getRetCount() > 0) { this.transaction = this.getInstance().toBuilder().clearRet().build(); diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index 0482643d8d0..f7c2332303f 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -556,9 +556,9 @@ public GrpcAPI.Return broadcastTransaction(Transaction signedTransaction) { if (trx.getInstance().getRawData().getContractCount() == 0) { throw new ContractValidateException(ActuatorConstant.CONTRACT_NOT_EXIST); } - TransactionMessage message = new TransactionMessage(trx.getInstance().toByteArray()); trx.checkExpiration(chainBaseManager.getNextBlockSlotTime()); dbManager.pushTransaction(trx); + TransactionMessage message = new TransactionMessage(trx.getInstance().toByteArray()); int num = tronNetService.fastBroadcastTransaction(message); if (num == 0 && minEffectiveConnection != 0) { return builder.setResult(false).setCode(response_code.NOT_ENOUGH_EFFECTIVE_CONNECTION) diff --git a/framework/src/main/java/org/tron/core/db/Manager.java b/framework/src/main/java/org/tron/core/db/Manager.java index 3de0260b5c8..20cf5b98386 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -1534,6 +1534,9 @@ public TransactionInfo processTransaction(final TransactionCapsule trxCap, Block String.format(" %s transaction signature validate failed", txId)); } + if (!trxCap.isInBlock()) { + trxCap.sanitize(); + } TransactionTrace trace = new TransactionTrace(trxCap, StoreFactory.getInstance(), new RuntimeImpl()); trxCap.setTrxTrace(trace); diff --git a/framework/src/main/java/org/tron/core/net/message/adv/BlockMessage.java b/framework/src/main/java/org/tron/core/net/message/adv/BlockMessage.java index d5aad2cd5c4..cbf2d2eb506 100644 --- a/framework/src/main/java/org/tron/core/net/message/adv/BlockMessage.java +++ b/framework/src/main/java/org/tron/core/net/message/adv/BlockMessage.java @@ -28,6 +28,11 @@ public BlockMessage(BlockCapsule block) { this.block = block; } + public void sanitize() { + this.block.sanitize(); + this.data = this.block.getData(); + } + public BlockId getBlockId() { return getBlockCapsule().getBlockId(); } diff --git a/framework/src/main/java/org/tron/core/net/messagehandler/BlockMsgHandler.java b/framework/src/main/java/org/tron/core/net/messagehandler/BlockMsgHandler.java index 3b9e86d4791..452209d575f 100644 --- a/framework/src/main/java/org/tron/core/net/messagehandler/BlockMsgHandler.java +++ b/framework/src/main/java/org/tron/core/net/messagehandler/BlockMsgHandler.java @@ -77,6 +77,8 @@ public void processMessage(PeerConnection peer, TronMessage msg) throws P2pExcep check(peer, blockMessage); } + blockMessage.sanitize(); + if (peer.getSyncBlockRequested().containsKey(blockId)) { peer.getSyncBlockRequested().remove(blockId); peer.getSyncBlockInProcess().add(blockId); diff --git a/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java b/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java new file mode 100644 index 00000000000..63803257eb9 --- /dev/null +++ b/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java @@ -0,0 +1,180 @@ +package org.tron.core.net.message.adv; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +import com.google.protobuf.ByteString; +import com.google.protobuf.UnknownFieldSet; +import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.Mockito; +import org.tron.common.overlay.message.Message; +import org.tron.core.capsule.BlockCapsule; +import org.tron.core.capsule.TransactionCapsule; +import org.tron.core.store.DynamicPropertiesStore; +import org.tron.protos.Protocol.Block; +import org.tron.protos.Protocol.BlockHeader; +import org.tron.protos.Protocol.Transaction; + +/** + * Verifies the {@code sanitize()} helpers on {@link BlockCapsule}, + * {@link TransactionCapsule} and {@link BlockMessage}: they strip outer + * unknown protobuf fields while leaving every consensus-hashed / signed + * region byte-identical. + */ +public class SanitizeUnknownFieldsTest { + + private static final UnknownFieldSet PADDING = UnknownFieldSet.newBuilder() + .addField(99999, UnknownFieldSet.Field.newBuilder() + .addLengthDelimited(ByteString.copyFrom(new byte[1024])) + .build()) + .build(); + + @BeforeClass + public static void setUp() { + // BlockMessage(byte[]) calls Message.isFilter() which dereferences the + // static DynamicPropertiesStore. The mock's primitive-long getter returns + // 0L by default, so isFilter() returns false. + Message.setDynamicPropertiesStore(Mockito.mock(DynamicPropertiesStore.class)); + } + + private static BlockHeader.raw sampleRawHeader() { + return BlockHeader.raw.newBuilder() + .setNumber(100) + .setTimestamp(123456789L) + .build(); + } + + private static Block sampleBlock() { + return Block.newBuilder() + .setBlockHeader(BlockHeader.newBuilder().setRawData(sampleRawHeader()).build()) + .build(); + } + + private static Transaction sampleTransaction() { + return Transaction.newBuilder() + .setRawData(Transaction.raw.newBuilder().setTimestamp(123456789L).build()) + .build(); + } + + // ---- BlockCapsule.sanitize ---- + + @Test + public void blockCapsuleSanitizeStripsBlockLevelUnknownFields() { + Block padded = sampleBlock().toBuilder().setUnknownFields(PADDING).build(); + BlockCapsule capsule = new BlockCapsule(padded); + long originalSize = capsule.getData().length; + + capsule.sanitize(); + + assertTrue("Block-level unknown fields should be stripped", + capsule.getInstance().getUnknownFields().asMap().isEmpty()); + assertTrue("Sanitized capsule bytes should shrink", + capsule.getData().length < originalSize); + } + + @Test + public void blockCapsuleSanitizeStripsBlockHeaderOuterUnknownFields() { + BlockHeader paddedHeader = BlockHeader.newBuilder() + .setRawData(sampleRawHeader()) + .setUnknownFields(PADDING) + .build(); + Block padded = Block.newBuilder().setBlockHeader(paddedHeader).build(); + BlockCapsule capsule = new BlockCapsule(padded); + long originalSize = capsule.getData().length; + + capsule.sanitize(); + + assertTrue("BlockHeader outer unknown fields should be stripped", + capsule.getInstance().getBlockHeader().getUnknownFields().asMap().isEmpty()); + assertTrue(capsule.getData().length < originalSize); + } + + @Test + public void blockCapsuleSanitizePreservesBlockHeaderRawData() { + Block clean = sampleBlock(); + Block padded = clean.toBuilder().setUnknownFields(PADDING).build(); + BlockCapsule capsule = new BlockCapsule(padded); + + capsule.sanitize(); + + assertEquals("BlockHeader.raw_data must be byte-identical so block hash matches", + clean.getBlockHeader().getRawData(), + capsule.getInstance().getBlockHeader().getRawData()); + } + + @Test + public void blockCapsuleSanitizeIsNoOpOnCleanBlock() { + Block clean = sampleBlock(); + BlockCapsule capsule = new BlockCapsule(clean); + byte[] before = capsule.getData(); + + capsule.sanitize(); + + assertArrayEquals("Clean block should pass through unchanged", before, capsule.getData()); + } + + // ---- TransactionCapsule.sanitize ---- + + @Test + public void transactionCapsuleSanitizeStripsTopLevelUnknownFields() { + Transaction padded = sampleTransaction().toBuilder().setUnknownFields(PADDING).build(); + TransactionCapsule capsule = new TransactionCapsule(padded); + long originalSize = capsule.getData().length; + + capsule.sanitize(); + + assertTrue("Transaction-level unknown fields should be stripped", + capsule.getInstance().getUnknownFields().asMap().isEmpty()); + assertTrue(capsule.getData().length < originalSize); + } + + @Test + public void transactionCapsuleSanitizePreservesTransactionId() { + Transaction clean = sampleTransaction(); + Transaction padded = clean.toBuilder().setUnknownFields(PADDING).build(); + TransactionCapsule cleanCapsule = new TransactionCapsule(clean); + TransactionCapsule paddedCapsule = new TransactionCapsule(padded); + + paddedCapsule.sanitize(); + + assertEquals("Padding outside raw_data must not change the transaction id", + cleanCapsule.getTransactionId(), + paddedCapsule.getTransactionId()); + } + + @Test + public void transactionCapsuleSanitizeIsNoOpOnCleanTransaction() { + Transaction clean = sampleTransaction(); + TransactionCapsule capsule = new TransactionCapsule(clean); + byte[] before = capsule.getData(); + + capsule.sanitize(); + + assertArrayEquals(before, capsule.getData()); + } + + // ---- BlockMessage.sanitize ---- + + @Test + public void blockMessageSanitizeUpdatesBothCapsuleAndWireBytes() throws Exception { + Block padded = sampleBlock().toBuilder().setUnknownFields(PADDING).build(); + byte[] paddedBytes = padded.toByteArray(); + BlockMessage msg = new BlockMessage(paddedBytes); + assertArrayEquals("Constructor should not sanitize on its own", + paddedBytes, msg.getData()); + + msg.sanitize(); + + assertTrue("BlockCapsule should be sanitized", + msg.getBlockCapsule().getInstance().getUnknownFields().asMap().isEmpty()); + assertTrue("msg.data should also be rewritten to canonical bytes", + msg.getData().length < paddedBytes.length); + assertArrayEquals("msg.data should equal capsule.getData() after sanitize", + msg.getBlockCapsule().getData(), msg.getData()); + assertNotEquals("msg.data should no longer match the padded wire bytes", + paddedBytes.length, msg.getData().length); + } +} From 38c52982bae83a1d146f32b0ee245a92a5222f0b Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Mon, 25 May 2026 15:28:33 +0800 Subject: [PATCH 2/2] perf(net): skip wire-byte rewrite when sanitize is a no-op --- .../org/tron/core/capsule/BlockCapsule.java | 5 ++- .../tron/core/capsule/TransactionCapsule.java | 12 +++--- .../core/net/message/adv/BlockMessage.java | 5 ++- .../adv/SanitizeUnknownFieldsTest.java | 38 ++++++++++++++----- 4 files changed, 42 insertions(+), 18 deletions(-) diff --git a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java index fcfd6526b26..63acf64b64f 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java @@ -329,11 +329,11 @@ public boolean hasWitnessSignature() { return !getInstance().getBlockHeader().getWitnessSignature().isEmpty(); } - public void sanitize() { + public boolean sanitize() { boolean blockHasUnknown = !this.block.getUnknownFields().asMap().isEmpty(); boolean headerHasUnknown = !this.block.getBlockHeader().getUnknownFields().asMap().isEmpty(); if (!blockHasUnknown && !headerHasUnknown) { - return; + return false; } UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance(); Block.Builder builder = this.block.toBuilder(); @@ -346,6 +346,7 @@ public void sanitize() { .build()); } this.block = builder.build(); + return true; } @Override diff --git a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java index bfd3ee305c5..8724a688548 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java @@ -495,12 +495,14 @@ public static boolean validateSignature(Transaction transaction, return false; } - public void sanitize() { - if (!this.transaction.getUnknownFields().asMap().isEmpty()) { - this.transaction = transaction.toBuilder() - .setUnknownFields(UnknownFieldSet.getDefaultInstance()) - .build(); + public boolean sanitize() { + if (this.transaction.getUnknownFields().asMap().isEmpty()) { + return false; } + this.transaction = this.transaction.toBuilder() + .setUnknownFields(UnknownFieldSet.getDefaultInstance()) + .build(); + return true; } public void resetResult() { diff --git a/framework/src/main/java/org/tron/core/net/message/adv/BlockMessage.java b/framework/src/main/java/org/tron/core/net/message/adv/BlockMessage.java index cbf2d2eb506..99be34e1bf1 100644 --- a/framework/src/main/java/org/tron/core/net/message/adv/BlockMessage.java +++ b/framework/src/main/java/org/tron/core/net/message/adv/BlockMessage.java @@ -29,8 +29,9 @@ public BlockMessage(BlockCapsule block) { } public void sanitize() { - this.block.sanitize(); - this.data = this.block.getData(); + if (this.block.sanitize()) { + this.data = this.block.getData(); + } } public BlockId getBlockId() { diff --git a/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java b/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java index 63803257eb9..7d883b7207d 100644 --- a/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java +++ b/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java @@ -2,7 +2,9 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import com.google.protobuf.ByteString; @@ -67,7 +69,7 @@ public void blockCapsuleSanitizeStripsBlockLevelUnknownFields() { BlockCapsule capsule = new BlockCapsule(padded); long originalSize = capsule.getData().length; - capsule.sanitize(); + assertTrue("sanitize() should report it mutated the capsule", capsule.sanitize()); assertTrue("Block-level unknown fields should be stripped", capsule.getInstance().getUnknownFields().asMap().isEmpty()); @@ -85,7 +87,7 @@ public void blockCapsuleSanitizeStripsBlockHeaderOuterUnknownFields() { BlockCapsule capsule = new BlockCapsule(padded); long originalSize = capsule.getData().length; - capsule.sanitize(); + assertTrue("sanitize() should report it mutated the capsule", capsule.sanitize()); assertTrue("BlockHeader outer unknown fields should be stripped", capsule.getInstance().getBlockHeader().getUnknownFields().asMap().isEmpty()); @@ -109,11 +111,14 @@ public void blockCapsuleSanitizePreservesBlockHeaderRawData() { public void blockCapsuleSanitizeIsNoOpOnCleanBlock() { Block clean = sampleBlock(); BlockCapsule capsule = new BlockCapsule(clean); - byte[] before = capsule.getData(); + Block beforeInstance = capsule.getInstance(); + byte[] beforeData = capsule.getData(); - capsule.sanitize(); + assertFalse("sanitize() should report no-op on a clean block", capsule.sanitize()); - assertArrayEquals("Clean block should pass through unchanged", before, capsule.getData()); + assertSame("Underlying Block reference should not be rebuilt", + beforeInstance, capsule.getInstance()); + assertArrayEquals("Clean block should pass through unchanged", beforeData, capsule.getData()); } // ---- TransactionCapsule.sanitize ---- @@ -124,7 +129,7 @@ public void transactionCapsuleSanitizeStripsTopLevelUnknownFields() { TransactionCapsule capsule = new TransactionCapsule(padded); long originalSize = capsule.getData().length; - capsule.sanitize(); + assertTrue("sanitize() should report it mutated the capsule", capsule.sanitize()); assertTrue("Transaction-level unknown fields should be stripped", capsule.getInstance().getUnknownFields().asMap().isEmpty()); @@ -149,11 +154,14 @@ public void transactionCapsuleSanitizePreservesTransactionId() { public void transactionCapsuleSanitizeIsNoOpOnCleanTransaction() { Transaction clean = sampleTransaction(); TransactionCapsule capsule = new TransactionCapsule(clean); - byte[] before = capsule.getData(); + Transaction beforeInstance = capsule.getInstance(); + byte[] beforeData = capsule.getData(); - capsule.sanitize(); + assertFalse("sanitize() should report no-op on a clean transaction", capsule.sanitize()); - assertArrayEquals(before, capsule.getData()); + assertSame("Underlying Transaction reference should not be rebuilt", + beforeInstance, capsule.getInstance()); + assertArrayEquals(beforeData, capsule.getData()); } // ---- BlockMessage.sanitize ---- @@ -177,4 +185,16 @@ public void blockMessageSanitizeUpdatesBothCapsuleAndWireBytes() throws Exceptio assertNotEquals("msg.data should no longer match the padded wire bytes", paddedBytes.length, msg.getData().length); } + + @Test + public void blockMessageSanitizeSkipsDataRewriteOnCleanBlock() throws Exception { + byte[] cleanBytes = sampleBlock().toByteArray(); + BlockMessage msg = new BlockMessage(cleanBytes); + byte[] before = msg.getData(); + + msg.sanitize(); + + assertSame("msg.data should not be rewritten on the no-op path", + before, msg.getData()); + } }