Skip to content

fix(net): validate block transactions before propagation #7066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lvs0075 merged 9 commits into from
Oct 27, 2026
Merged

fix(net): validate block transactions before propagation #7066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9b6221e
fix(net): restore transaction revalidation and sanitize relay data
Oct 20, 2026
9126dfd
fix(net): recover sync requests after block processing failures
Oct 20, 2026
fdf2ccc
fix(net): preserve pending transaction results during relay
Oct 21, 2026
d24fa79
chore(net): merge release_v4.8.3 into ww
Oct 23, 2026
2b1a063
chore(db): merge release_v4.8.3 into ww
Oct 23, 2026
3421300
fix(net): validate block transactions before propagation
Oct 23, 2026
d3328c9
chore(net): merge latest release_v4.8.3
Oct 27, 2026
9b3ab87
fix(net): reject block transactions smaller than 80 bytes
Oct 27, 2026
35bd037
fix(net): keep block timestamp checks at message ingress
Oct 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
public class BlockCapsule implements ProtoCapsule<Block> {

public boolean generatedByMyself = false;
private volatile boolean merkleValidated = false;
@Getter
@Setter
private TransactionRetCapsule result;
Expand Down Expand Up @@ -233,16 +232,12 @@ public Sha256Hash calcMerkleRoot() {
}

public void validateMerkleRoot() throws BadBlockException {
if (merkleValidated) {
return;
}
Sha256Hash actual = calcMerkleRoot();
if (!actual.equals(getMerkleRoot())) {
throw new BadBlockException(CALC_MERKLE_ROOT_FAILED,
String.format("merkle root mismatch for block %d: expected %s, actual %s",
getNum(), getMerkleRoot(), actual));
}
merkleValidated = true;
}

public void setMerkleRoot() {
Expand Down
4 changes: 2 additions & 2 deletions framework/src/main/java/org/tron/core/db/Manager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,8 @@ public List<TransactionCapsule> getVerifyTxs(BlockCapsule block) {
}

/**
* save a block.
* Save a block. Untrusted callers must first use network prevalidation to check its timestamp,

@bladehan1 bladehan1 Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[DISCUSS] Confirm the Merkle-validation boundary for future callers

This removes the Merkle-root check from pushBlock and makes network prevalidation a caller prerequisite. The current advertised and sync paths both call validBlock, so I did not find a current bypass; however, a future direct caller that misses this Javadoc contract would no longer get the previous defense-in-depth check.

Suggestion: Keep the prevalidation requirement explicit for all new entry points, and consider retaining a core Merkle check or introducing a validated-block abstraction if that caller contract cannot be enforced.

@xxo1shine xxo1shine Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is an intentional change in validation responsibility: the advertised-block path calls validAdvBlock -> validBlock, and the sync path calls validBlock directly before entering pushBlock. The duplicate Merkle validation in the core layer was therefore removed.

I agree with the reminder about future callers. The Javadoc for pushBlock explicitly requires untrusted blocks to undergo prevalidation, and new entry points must respect that prerequisite as well. No further code changes are planned for this comment.

* transaction structure, Merkle root and block signature.
*/
public void pushBlock(final BlockCapsule block)
throws ValidateSignatureException, ContractValidateException, ContractExeException,
Expand Down Expand Up @@ -1265,7 +1266,6 @@ public void pushBlock(final BlockCapsule block)
try (PendingManager pm = new PendingManager(this)) {

if (!block.generatedByMyself) {
block.validateMerkleRoot();
consensus.receiveBlock(block);
}

Expand Down
23 changes: 17 additions & 6 deletions framework/src/main/java/org/tron/core/net/TronNetDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,15 @@
import org.tron.core.net.peer.PeerConnection;
import org.tron.core.store.WitnessScheduleStore;
import org.tron.protos.Protocol.Inventory.InventoryType;
import org.tron.protos.Protocol.Transaction;

@Slf4j(topic = "net")
@Component
public class TronNetDelegate {

// Keep Merkle leaf encodings larger than the 64-byte internal-node input.
private static final int MIN_TRANSACTION_SIZE = 80;

@Autowired
private Manager dbManager;

Expand All @@ -89,8 +93,6 @@ public class TronNetDelegate {

private int blockIdCacheSize = 100;

private long timeout = 1000;

@Getter // for test
private volatile boolean hitDown = false;

Expand Down Expand Up @@ -352,10 +354,19 @@ public void validSignature(BlockCapsule block) throws P2pException {
}

public void validBlock(BlockCapsule block) throws P2pException {
long time = System.currentTimeMillis();
if (block.getTimeStamp() - time > timeout) {
throw new P2pException(TypeEnum.BAD_BLOCK,
"time:" + time + ",block time:" + block.getTimeStamp());
for (Transaction transaction : block.getInstance().getTransactionsList()) {
int contractCount = transaction.getRawData().getContractCount();

@Federico2014 Federico2014 Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[SHOULD] Please also check each transaction’s full serialized size here, rejecting transactions smaller than 65 bytes or larger than Constant.TRANSACTION_MAX_BYTE_SIZE before propagation.

Merkle leaves and internal nodes use the same hash function without domain separation. A 64-byte transaction can match the concatenation of two 32-byte child hashes, allowing different transaction lists to share the same Merkle root. Checking contract count alone does not rule this out.

Please add boundary tests and verify compatibility with historical blocks and shielded transactions.

@xxo1shine xxo1shine Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Implemented in 9b3ab87. MIN_TRANSACTION_SIZE = 80 now rejects transactions whose full Transaction.getSerializedSize() is below 80 bytes, before Merkle validation for both advertised and sync blocks.

Tests cover 64/79-byte rejection, 80/81-byte acceptance, signatures and ret counting toward the full size, a synthetic shielded transaction without an outer signature, and rejection before broadcast, fast-forwarding, or sync processing. All 62 targeted tests, the build with tests excluded, and Checkstyle passed.

This change adds the lower bound only; upper-bound validation is unchanged. A full historical-chain replay has not been performed.

if (contractCount != 1) {
throw new P2pException(TypeEnum.BAD_BLOCK,
"Invalid contract count: " + contractCount
+ ", block: " + block.getBlockId().getString());
}
int transactionSize = transaction.getSerializedSize();
if (transactionSize < MIN_TRANSACTION_SIZE) {
throw new P2pException(TypeEnum.BAD_BLOCK,
"Invalid transaction size: " + transactionSize
+ ", block: " + block.getBlockId().getString());
}
}
try {
block.validateMerkleRoot();
Expand Down
2 changes: 1 addition & 1 deletion framework/src/main/java/org/tron/core/net/service/sync/SyncService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ private synchronized void handleSyncBlock() {
private void processSyncBlock(BlockCapsule block, PeerConnection peerConnection) {
BlockId blockId = block.getBlockId();
try {
tronNetDelegate.validSignature(block);
tronNetDelegate.validBlock(block);
tronNetDelegate.processBlock(block, true);
peerConnection.setBlockRcvTime(System.currentTimeMillis());
pbftDataSyncHandler.processPBFTCommitData(block);
Expand Down
25 changes: 21 additions & 4 deletions framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,13 @@ public void testValidateMerkleRoot() throws Exception {
1234,
ByteString.copyFrom("1234567".getBytes()));

local.addTransaction(new TransactionCapsule(
TransferContract.newBuilder().setAmount(1).build(), ContractType.TransferContract));

// valid block: setMerkleRoot then validate — should not throw
local.setMerkleRoot();
local.validateMerkleRoot(); // no exception

// flag is set — second call must be a no-op (no recomputation)
local.validateMerkleRoot(); // still no exception

// tamper with a transaction to break merkle
Expand All @@ -124,14 +126,29 @@ public void testValidateMerkleRoot() throws Exception {
new TransactionCapsule(transferContract, ContractType.TransferContract));
// merkle root was set before adding the tx, so it is now stale/invalid

BlockCapsule tampered = new BlockCapsule(local.getInstance());
// tampered has no merkleValidated flag set
try {
tampered.validateMerkleRoot();
local.validateMerkleRoot();
Assert.fail("Expected BadBlockException for merkle mismatch");
} catch (BadBlockException e) {
Assert.assertTrue(e.getMessage().contains("merkle"));
}
local.setMerkleRoot();
local.validateMerkleRoot();
}

@Test
public void testAddAllTransactionsMakesMerkleRootStale() throws Exception {
BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, 1234, ByteString.EMPTY);
block.setMerkleRoot();
block.validateMerkleRoot();
TransactionCapsule transaction = new TransactionCapsule(
TransferContract.newBuilder().setAmount(999).build(), ContractType.TransferContract);

block.addAllTransactions(Arrays.asList(transaction));

Assert.assertThrows(BadBlockException.class, block::validateMerkleRoot);
block.setMerkleRoot();
block.validateMerkleRoot();
}

/* @Test
Expand Down
35 changes: 17 additions & 18 deletions framework/src/test/java/org/tron/core/db/ManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.tron.common.utils.Commons.adjustAssetBalanceV2;
import static org.tron.common.utils.Commons.adjustTotalShieldedPoolValue;
import static org.tron.common.utils.Commons.getExchangeStoreFinal;
import static org.tron.common.utils.StringUtil.encode58Check;
import static org.tron.core.exception.BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED;
import static org.tron.protos.Protocol.Transaction.Result.contractResult.SUCCESS;

import com.beust.jcommander.internal.Lists;
Expand Down Expand Up @@ -103,6 +103,7 @@
import org.tron.core.exception.ValidateScheduleException;
import org.tron.core.exception.ValidateSignatureException;
import org.tron.core.exception.ZksnarkException;
import org.tron.core.net.TronNetDelegate;
import org.tron.core.store.AccountStore;
import org.tron.core.store.CodeStore;
import org.tron.core.store.DynamicPropertiesStore;
Expand Down Expand Up @@ -503,23 +504,21 @@ public void adjustAssetBalanceV2Test() {
}

@Test
public void pushBlockInvalidMerkelRoot() {
Transaction trx = Transaction.newBuilder().build();
TransactionCapsule moreTrans = new TransactionCapsule(trx);
blockCapsule2.addTransaction(moreTrans); // add one more transaction will change merkroot
blockCapsule2.sign(ByteArray.fromHexString(Args.getLocalWitnesses()
.getPrivateKey()));
try {
dbManager.pushBlock(blockCapsule2);
Assert.assertTrue(false);
} catch (BadBlockException e) {
Assert.assertTrue(e instanceof BadBlockException);
Assert.assertTrue(e.getType().equals(CALC_MERKLE_ROOT_FAILED));
Assert.assertTrue(e.getMessage().startsWith(
"merkle root mismatch for block " + blockCapsule2.getNum() + ":"));
} catch (Exception e) {
Assert.assertFalse(e instanceof Exception);
}
public void pushPrevalidatedBlockDoesNotRepeatMerkleValidation() throws Exception {
byte[] witnessKey = ByteArray.fromHexString(Args.getLocalWitnesses().getPrivateKey());
BlockCapsule block = new BlockCapsule(1, chainManager.getGenesisBlockId(), 1,
ByteString.copyFrom(ECKey.fromPrivate(witnessKey).getAddress()));
block.setMerkleRoot();
block.sign(witnessKey);
BlockCapsule received = spy(block);

context.getBean(TronNetDelegate.class).validBlock(received);
dbManager.pushBlock(received);

Assert.assertEquals(received.getBlockId(), chainManager.getHeadBlockId());
Assert.assertEquals(received.getInstance(),
chainManager.getBlockStore().get(received.getBlockId().getBytes()).getInstance());
verify(received, times(1)).validateMerkleRoot();
}

@Test
Expand Down
Loading
Loading