From 9b6221e0e13aedd69a8ce9b7209f7a1a0e3333ac Mon Sep 17 00:00:00 2001 From: wb Date: Tue, 20 Oct 2026 14:09:56 +0800 Subject: [PATCH 1/6] fix(net): restore transaction revalidation and sanitize relay data Track permission-update owners when blocks are rolled back so cached transaction verification is invalidated. Clear transaction results before caching relay messages and omit full hash lists when transaction inventory is rate limited. --- .../main/java/org/tron/core/db/Manager.java | 8 +++-- .../tron/core/net/P2pEventHandlerImpl.java | 3 -- .../tron/core/net/service/adv/AdvService.java | 2 ++ .../java/org/tron/core/db/ManagerTest.java | 33 +++++++++++++++++++ .../TransactionsMsgHandlerTest.java | 31 +++++++++++++++++ .../core/net/services/AdvServiceTest.java | 13 ++++++-- 6 files changed, 83 insertions(+), 7 deletions(-) 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 0efa48dc20a..ea1102e0a46 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -999,8 +999,12 @@ public void eraseBlock() { khaosDb.pop(); revokingStore.fastPop(); logger.info("End to erase block: {}.", oldHeadBlock); - oldHeadBlock.getTransactions().forEach(tc -> - poppedTransactions.add(new TransactionCapsule(tc.getInstance()))); + oldHeadBlock.getTransactions().forEach(tc -> { + if (isMultiSignTransaction(tc.getInstance())) { + ownerAddressSet.add(ByteArray.toHexString(tc.getOwnerAddress())); + } + poppedTransactions.add(new TransactionCapsule(tc.getInstance())); + }); Metrics.gaugeInc(MetricKeys.Gauge.MANAGER_QUEUE, oldHeadBlock.getTransactions().size(), MetricLabels.Gauge.QUEUE_POPPED); diff --git a/framework/src/main/java/org/tron/core/net/P2pEventHandlerImpl.java b/framework/src/main/java/org/tron/core/net/P2pEventHandlerImpl.java index 67751230367..50fcafbf16a 100644 --- a/framework/src/main/java/org/tron/core/net/P2pEventHandlerImpl.java +++ b/framework/src/main/java/org/tron/core/net/P2pEventHandlerImpl.java @@ -233,9 +233,6 @@ private boolean checkInvRateLimit(PeerConnection peer, InventoryMessage msg) if (count + currentSize > maxCountIn10s) { logger.warn("Drop TRX inv from {}, window:{}, cur:{}, max:{}", peer.getInetAddress(), count, currentSize, maxCountIn10s); - if (Args.getInstance().isOpenPrintLog()) { - logger.warn("[overload] Drop tx list: {}", msg.getHashList()); - } return false; } } else if (invType == InventoryType.BLOCK) { diff --git a/framework/src/main/java/org/tron/core/net/service/adv/AdvService.java b/framework/src/main/java/org/tron/core/net/service/adv/AdvService.java index 2241ce8d1ce..f3de5607a17 100644 --- a/framework/src/main/java/org/tron/core/net/service/adv/AdvService.java +++ b/framework/src/main/java/org/tron/core/net/service/adv/AdvService.java @@ -169,6 +169,7 @@ public int fastBroadcastTransaction(TransactionMessage msg) { Item item = new Item(msg.getMessageId(), InventoryType.TRX); trxCount.add(); + msg.getTransactionCapsule().resetResult(); trxCache.put(item, new TransactionMessage(msg.getTransactionCapsule().getInstance())); List list = new ArrayList<>(); @@ -216,6 +217,7 @@ public void broadcast(Message msg) { TransactionMessage trxMsg = (TransactionMessage) msg; item = new Item(trxMsg.getMessageId(), InventoryType.TRX); trxCount.add(); + trxMsg.getTransactionCapsule().resetResult(); trxCache.put(item, new TransactionMessage(trxMsg.getTransactionCapsule().getInstance())); } else { logger.error("Adv item is neither block nor trx, type: {}", msg.getType()); diff --git a/framework/src/test/java/org/tron/core/db/ManagerTest.java b/framework/src/test/java/org/tron/core/db/ManagerTest.java index 804a29bab63..be6b4cab750 100755 --- a/framework/src/test/java/org/tron/core/db/ManagerTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerTest.java @@ -928,6 +928,39 @@ public void getVerifyTxsSkipsBlockWhenPermissionTxAlreadyConsumed() throws Excep } } + @Test + public void eraseBlockMarksPermissionUpdateOwnerForVerification() throws Exception { + byte[] owner = ByteArray.fromHexString(Wallet.getAddressPreFixString() + + "548794500882809695A8A687866E76D4271A1ABC"); + AccountContract.AccountPermissionUpdateContract contract = + AccountContract.AccountPermissionUpdateContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom(owner)) + .build(); + TransactionCapsule permissionUpdate = new TransactionCapsule(contract, + ContractType.AccountPermissionUpdateContract); + BlockCapsule rolledBackBlock = new BlockCapsule(1, Sha256Hash.ZERO_HASH.getByteString(), 0, + Arrays.asList(permissionUpdate.getInstance())); + rolledBackBlock.setMerkleRoot(); + + Manager manager = new Manager(); + ChainBaseManager mockedChainManager = mock(ChainBaseManager.class); + DynamicPropertiesStore mockedDynamicPropertiesStore = mock(DynamicPropertiesStore.class); + when(mockedChainManager.getDynamicPropertiesStore()).thenReturn(mockedDynamicPropertiesStore); + when(mockedDynamicPropertiesStore.getLatestBlockHeaderHash()) + .thenReturn(Sha256Hash.wrap(new byte[32])); + when(mockedChainManager.getBlockById(any(Sha256Hash.class))).thenReturn(rolledBackBlock); + ReflectUtils.setFieldValue(manager, "chainBaseManager", mockedChainManager); + ReflectUtils.setFieldValue(manager, "khaosDb", mock(KhaosDatabase.class)); + ReflectUtils.setFieldValue(manager, "revokingStore", mock(RevokingDatabase.class)); + + manager.eraseBlock(); + + @SuppressWarnings("unchecked") + Set ownerAddressSet = + (Set) ReflectUtils.getFieldObject(manager, "ownerAddressSet"); + Assert.assertTrue(ownerAddressSet.contains(ByteArray.toHexString(owner))); + } + @Test public void doNotSwitch() throws ValidateSignatureException, ContractValidateException, ContractExeException, diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java index d7a6155d968..a694e8c7aed 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java @@ -188,6 +188,37 @@ public void testProcessMessageAfterClose() throws Exception { } } + @Test + public void testResultClearedBeforeSmartContractQueued() throws Exception { + TransactionsMsgHandler handler = new TransactionsMsgHandler(); + try { + PeerConnection peer = Mockito.mock(PeerConnection.class); + Protocol.Transaction trx = TvmTestUtils.generateTriggerSmartContractAndGetTransaction( + ByteArray.fromHexString("121212a9cf"), + ByteArray.fromHexString("121212a9cf"), + ByteArray.fromHexString("123456"), + 100, 100000000, 0, 0) + .toBuilder() + .addRet(Protocol.Transaction.Result.newBuilder().setFee(123L).build()) + .build(); + TransactionsMessage msg = new TransactionsMessage(Collections.singletonList(trx)); + stubAdvInvRequest(peer, msg); + + handler.processMessage(peer, msg); + + Field queueField = TransactionsMsgHandler.class.getDeclaredField("smartContractQueue"); + queueField.setAccessible(true); + BlockingQueue smartContractQueue = (BlockingQueue) queueField.get(handler); + TransactionsMsgHandler.TrxEvent event = + (TransactionsMsgHandler.TrxEvent) smartContractQueue.poll(); + Assert.assertNotNull(event); + Assert.assertEquals(0, + event.getMsg().getTransactionCapsule().getInstance().getRetCount()); + } finally { + handler.close(); + } + } + @Test public void testRejectedExecution() throws Exception { TransactionsMsgHandler handler = new TransactionsMsgHandler(); diff --git a/framework/src/test/java/org/tron/core/net/services/AdvServiceTest.java b/framework/src/test/java/org/tron/core/net/services/AdvServiceTest.java index 808c5eb09e0..856f9fb01cf 100644 --- a/framework/src/test/java/org/tron/core/net/services/AdvServiceTest.java +++ b/framework/src/test/java/org/tron/core/net/services/AdvServiceTest.java @@ -128,12 +128,21 @@ private void testTrxBroadcast() { .setRawData( Protocol.Transaction.raw.newBuilder() .setRefBlockNum(1) - .setExpiration(System.currentTimeMillis() + 3000).build()).build(); + .setExpiration(System.currentTimeMillis() + 3000).build()) + .addRet(Protocol.Transaction.Result.newBuilder().setFee(123L).build()) + .build(); CommonParameter.getInstance().setValidContractProtoThreadNum(1); TransactionMessage msg = new TransactionMessage(trx); service.broadcast(msg); Item item = new Item(msg.getMessageId(), InventoryType.TRX); - Assert.assertNotNull(service.getMessage(item)); + TransactionMessage cached = (TransactionMessage) service.getMessage(item); + Assert.assertNotNull(cached); + Assert.assertEquals(0, msg.getTransactionCapsule().getInstance().getRetCount()); + Assert.assertEquals(0, cached.getTransactionCapsule().getInstance().getRetCount()); + + TransactionMessage fastMsg = new TransactionMessage(trx); + service.fastBroadcastTransaction(fastMsg); + Assert.assertEquals(0, fastMsg.getTransactionCapsule().getInstance().getRetCount()); Protocol.Transaction expiredTrx = Protocol.Transaction.newBuilder() .setRawData( From 9126dfdfb83f14473b64029ac214ccd957e2e9f1 Mon Sep 17 00:00:00 2001 From: wb Date: Tue, 20 Oct 2026 18:17:01 +0800 Subject: [PATCH 2/6] fix(net): recover sync requests after block processing failures Release the global sync request on every block processing failure so another peer can supply the block without waiting for cache expiration. Disconnect only the sender and preserve other peers' queued blocks and sync progress. Reject advertised blocks whose witness is outside the active set instead of silently dropping them. Keep advertisement recovery on the sync path and retain log-only handling for failures during advertised block application. Unify block application errors as BAD_BLOCK while retaining their causes. Log P2P failure details without a stack trace and keep stack traces for unexpected sync failures. --- .../org/tron/core/net/TronNetDelegate.java | 15 +- .../net/messagehandler/BlockMsgHandler.java | 9 +- .../core/net/service/sync/SyncService.java | 29 +- .../tron/core/net/TronNetDelegateTest.java | 58 ++++ .../BlockAlreadyKnownCounterTest.java | 3 - .../messagehandler/BlockMsgHandlerTest.java | 1 - .../net/services/BlockFetchFailureTest.java | 292 ++++++++++++++++++ 7 files changed, 367 insertions(+), 40 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java diff --git a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java index 9d15886f3ab..1d7f6a86294 100644 --- a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java +++ b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java @@ -1,7 +1,6 @@ package org.tron.core.net; import static org.tron.core.config.Parameter.ChainConstant.BLOCK_PRODUCED_INTERVAL; -import static org.tron.core.exception.BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; @@ -312,12 +311,7 @@ public void processBlock(BlockCapsule block, boolean isSync) throws P2pException | EventBloomException e) { metricsService.failProcessBlock(block.getNum(), e.getMessage()); logger.error("Process block failed, {}, reason: {}", blockId.getString(), e.getMessage()); - if (e instanceof BadBlockException - && ((BadBlockException) e).getType().equals(CALC_MERKLE_ROOT_FAILED)) { - throw new P2pException(TypeEnum.BLOCK_MERKLE_INVALID, e); - } else { - throw new P2pException(TypeEnum.BAD_BLOCK, e); - } + throw new P2pException(TypeEnum.BAD_BLOCK, e); } } } @@ -356,7 +350,7 @@ public void validSignature(BlockCapsule block) throws P2pException { } } - public boolean validBlock(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, @@ -368,7 +362,10 @@ public boolean validBlock(BlockCapsule block) throws P2pException { throw new P2pException(TypeEnum.BLOCK_MERKLE_INVALID, e.getMessage()); } validSignature(block); - return witnessScheduleStore.getActiveWitnesses().contains(block.getWitnessAddress()); + if (!witnessScheduleStore.getActiveWitnesses().contains(block.getWitnessAddress())) { + throw new P2pException(TypeEnum.BAD_BLOCK, + "Block witness is not active: " + block.getBlockId().getString()); + } } public PbftSignCapsule getBlockPbftCommitData(long blockNum) { 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 b1631979913..b92765ff815 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 @@ -134,14 +134,7 @@ private void check(PeerConnection peer, BlockMessage msg) throws P2pException { private void processBlock(PeerConnection peer, BlockCapsule block) throws P2pException { BlockId blockId = block.getBlockId(); - boolean flag = tronNetDelegate.validBlock(block); - if (!flag) { - logger.warn("Receive a bad block from {}, {}, {}", - peer.getInetSocketAddress(), blockId.getString(), - Hex.toHexString(block.getWitnessAddress().toByteArray())); - return; - } - + tronNetDelegate.validBlock(block); if (!tronNetDelegate.containBlock(block.getParentBlockId())) { logger.warn("Get unlink block {} from {}, head is {}", blockId.getString(), peer.getInetAddress(), tronNetDelegate.getHeadBlockId().getString()); diff --git a/framework/src/main/java/org/tron/core/net/service/sync/SyncService.java b/framework/src/main/java/org/tron/core/net/service/sync/SyncService.java index 75d1ec9f4b5..b8ccb6daf59 100644 --- a/framework/src/main/java/org/tron/core/net/service/sync/SyncService.java +++ b/framework/src/main/java/org/tron/core/net/service/sync/SyncService.java @@ -339,26 +339,21 @@ private synchronized void handleSyncBlock() { } private void processSyncBlock(BlockCapsule block, PeerConnection peerConnection) { - boolean flag = true; - boolean attackFlag = false; BlockId blockId = block.getBlockId(); try { tronNetDelegate.validSignature(block); tronNetDelegate.processBlock(block, true); peerConnection.setBlockRcvTime(System.currentTimeMillis()); pbftDataSyncHandler.processPBFTCommitData(block); - } catch (P2pException p2pException) { - logger.error("Process sync block {} failed, type: {}", - blockId.getString(), p2pException.getType()); - attackFlag = p2pException.getType().equals(TypeEnum.BLOCK_SIGN_INVALID) - || p2pException.getType().equals(TypeEnum.BLOCK_MERKLE_INVALID); - flag = false; } catch (Exception e) { - logger.error("Process sync block {} failed", blockId.getString(), e); - flag = false; - } - - if (attackFlag) { + if (e instanceof P2pException) { + logger.error("Process sync block {} from {} failed, type: {}, reason: {}", + blockId.getString(), peerConnection.getInetSocketAddress(), + ((P2pException) e).getType(), e.getMessage()); + } else { + logger.error("Process sync block {} from {} failed", + blockId.getString(), peerConnection.getInetSocketAddress(), e); + } invalid(blockId, peerConnection); peerConnection.disconnect(ReasonCode.BAD_BLOCK); return; @@ -369,12 +364,8 @@ private void processSyncBlock(BlockCapsule block, PeerConnection peerConnection) if (blockId.equals(bid)) { peer.setBlockBothHave(blockId); peer.getSyncBlockToFetch().remove(bid); - if (flag) { - if (peer.getSyncBlockToFetch().isEmpty() && peer.isFetchAble()) { - syncNext(peer); - } - } else { - peer.disconnect(ReasonCode.BAD_BLOCK); + if (peer.getSyncBlockToFetch().isEmpty() && peer.isFetchAble()) { + syncNext(peer); } } } diff --git a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java index 38738f78a6d..9cf78f88204 100644 --- a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java +++ b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java @@ -4,6 +4,7 @@ import com.google.protobuf.ByteString; import java.lang.reflect.Field; +import java.util.Collections; import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; @@ -17,9 +18,13 @@ import org.tron.core.capsule.TransactionCapsule; import org.tron.core.config.args.Args; import org.tron.core.db.Manager; +import org.tron.core.exception.BadBlockException; import org.tron.core.exception.P2pException; import org.tron.core.exception.P2pException.TypeEnum; +import org.tron.core.exception.ValidateScheduleException; +import org.tron.core.metrics.MetricsService; import org.tron.core.store.DynamicPropertiesStore; +import org.tron.core.store.WitnessScheduleStore; import org.tron.protos.Protocol.Transaction.Contract.ContractType; import org.tron.protos.contract.BalanceContract.TransferContract; @@ -186,4 +191,57 @@ public void testValidBlockMerkleRoot() throws Exception { Assert.assertEquals(TypeEnum.BLOCK_MERKLE_INVALID, e.getType()); } } + + @Test + public void testValidBlockRejectsInactiveWitness() throws Exception { + TronNetDelegate delegate = Mockito.spy(new TronNetDelegate()); + WitnessScheduleStore schedule = mock(WitnessScheduleStore.class); + setField(delegate, "witnessScheduleStore", schedule); + BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, + ByteString.copyFromUtf8("witness")); + block.setMerkleRoot(); + Mockito.doNothing().when(delegate).validSignature(block); + Mockito.when(schedule.getActiveWitnesses()).thenReturn(Collections.emptyList()); + try { + delegate.validBlock(block); + Assert.fail("An inactive witness must be rejected instead of silently dropped"); + } catch (P2pException e) { + Assert.assertEquals(TypeEnum.BAD_BLOCK, e.getType()); + } + Mockito.when(schedule.getActiveWitnesses()) + .thenReturn(Collections.singletonList(block.getWitnessAddress())); + delegate.validBlock(block); + } + + @Test + public void testProcessBlockWrapsScheduleFailureWithCause() throws Exception { + assertProcessBlockFailure(new ValidateScheduleException("invalid scheduled witness")); + } + + @Test + public void testProcessBlockWrapsMerkleFailureWithCause() throws Exception { + assertProcessBlockFailure(new BadBlockException( + BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED, "invalid merkle root")); + } + + private void assertProcessBlockFailure(Exception failure) throws Exception { + TronNetDelegate delegate = new TronNetDelegate(); + Manager manager = mock(Manager.class); + ChainBaseManager chainManager = mock(ChainBaseManager.class); + setField(delegate, "dbManager", manager); + setField(delegate, "chainBaseManager", chainManager); + setField(delegate, "metricsService", mock(MetricsService.class)); + Mockito.when(manager.getBlockedTimer()).thenReturn(new ThreadLocal<>()); + Mockito.when(chainManager.getHeadBlockId()) + .thenReturn(new BlockCapsule.BlockId(Sha256Hash.ZERO_HASH, 0)); + BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, ByteString.EMPTY); + Mockito.doThrow(failure).when(manager).pushBlock(block); + try { + delegate.processBlock(block, true); + Assert.fail("Expected block validation failure"); + } catch (P2pException e) { + Assert.assertEquals(TypeEnum.BAD_BLOCK, e.getType()); + Assert.assertSame(failure, e.getCause()); + } + } } diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/BlockAlreadyKnownCounterTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/BlockAlreadyKnownCounterTest.java index 64e77d77bba..b2afbafda7d 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/BlockAlreadyKnownCounterTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/BlockAlreadyKnownCounterTest.java @@ -90,7 +90,6 @@ public void tearDown() { @Test public void testMatchedRequestAlreadyKnownBlockIncrements() throws P2pException { when(delegate.containBlock(any(BlockId.class))).thenReturn(true); - when(delegate.validBlock(any(BlockCapsule.class))).thenReturn(true); when(delegate.getHeadBlockId()).thenReturn(new BlockId(Sha256Hash.ZERO_HASH, HEAD_NUM)); BlockMessage msg = newBlockMessage(1); @@ -116,7 +115,6 @@ public void testMatchedRequestUnknownForkBelowHeadNotIncremented() throws P2pExc assertNotEquals(msg.getBlockId(), msg.getBlockCapsule().getParentBlockId()); when(delegate.containBlock(msg.getBlockId())).thenReturn(false); when(delegate.containBlock(msg.getBlockCapsule().getParentBlockId())).thenReturn(true); - when(delegate.validBlock(any(BlockCapsule.class))).thenReturn(true); when(delegate.getHeadBlockId()).thenReturn(new BlockId(Sha256Hash.ZERO_HASH, HEAD_NUM)); double before = sample(); @@ -150,7 +148,6 @@ public void testMatchedRequestHeadDuplicateIncrements() throws P2pException { // re-delivery of the current head block id counts: the id is exactly known even // though it is not below head. when(delegate.containBlock(any(BlockId.class))).thenReturn(true); - when(delegate.validBlock(any(BlockCapsule.class))).thenReturn(true); when(delegate.getHeadBlockId()).thenReturn(new BlockId(Sha256Hash.ZERO_HASH, HEAD_NUM)); when(delegate.getActivePeer()).thenReturn(Collections.emptyList()); diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java index 82ea2b6cb57..65397e84da1 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java @@ -152,7 +152,6 @@ public void testProcessBlock() { .put(new Item(blockCapsule0.getBlockId(), InventoryType.BLOCK), System.currentTimeMillis()); - Mockito.doReturn(true).when(tronNetDelegate).validBlock(any(BlockCapsule.class)); Mockito.doReturn(true).when(tronNetDelegate).containBlock(any(BlockId.class)); Mockito.doReturn(blockCapsule0.getBlockId()).when(tronNetDelegate).getHeadBlockId(); Mockito.doNothing().when(tronNetDelegate).processBlock(any(BlockCapsule.class), anyBoolean()); diff --git a/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java b/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java new file mode 100644 index 00000000000..1ef73de9a99 --- /dev/null +++ b/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java @@ -0,0 +1,292 @@ +package org.tron.core.net.services; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.common.cache.Cache; +import com.google.protobuf.ByteString; +import java.lang.reflect.Method; +import java.net.InetSocketAddress; +import java.util.Arrays; +import java.util.Collections; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.tron.common.TestConstants; +import org.tron.common.utils.ReflectUtils; +import org.tron.common.utils.Sha256Hash; +import org.tron.core.capsule.BlockCapsule; +import org.tron.core.capsule.BlockCapsule.BlockId; +import org.tron.core.config.args.Args; +import org.tron.core.exception.BadBlockException; +import org.tron.core.exception.P2pException; +import org.tron.core.exception.P2pException.TypeEnum; +import org.tron.core.exception.ValidateScheduleException; +import org.tron.core.net.P2pEventHandlerImpl; +import org.tron.core.net.TronNetDelegate; +import org.tron.core.net.message.TronMessage; +import org.tron.core.net.message.adv.BlockMessage; +import org.tron.core.net.message.adv.FetchInvDataMessage; +import org.tron.core.net.messagehandler.BlockMsgHandler; +import org.tron.core.net.peer.Item; +import org.tron.core.net.peer.PeerConnection; +import org.tron.core.net.service.adv.AdvService; +import org.tron.core.net.service.fetchblock.FetchBlockService; +import org.tron.core.net.service.sync.SyncService; +import org.tron.p2p.connection.Channel; +import org.tron.protos.Protocol.Inventory.InventoryType; +import org.tron.protos.Protocol.ReasonCode; + +public class BlockFetchFailureTest { + + @ClassRule + public static final TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @BeforeClass + public static void init() throws Exception { + Args.setParam(new String[]{"-d", temporaryFolder.newFolder().toString()}, + TestConstants.TEST_CONF); + } + + @AfterClass + public static void destroy() { + Args.clearParam(); + } + + @Test + public void invalidAdvertisedSignatureDoesNotRetryAnotherPeer() throws Exception { + assertAdvertisedBlockDoesNotRetry(TypeEnum.BLOCK_SIGN_INVALID); + } + + @Test + public void invalidAdvertisedMerkleRootDoesNotRetryAnotherPeer() throws Exception { + assertAdvertisedBlockDoesNotRetry(TypeEnum.BLOCK_MERKLE_INVALID); + } + + @Test + public void inactiveAdvertisedWitnessIsBannedWithoutRetryingAnotherPeer() throws Exception { + assertAdvertisedBlockDoesNotRetry(TypeEnum.BAD_BLOCK); + } + + @Test + public void advertisedScheduleFailureDoesNotDisconnectRelayingPeers() throws Exception { + TronNetDelegate delegate = mock(TronNetDelegate.class); + FetchBlockService fetch = mock(FetchBlockService.class); + AdvService adv = new AdvService(); + ReflectUtils.setFieldValue(adv, "tronNetDelegate", delegate); + ReflectUtils.setFieldValue(adv, "fetchBlockService", fetch); + BlockMsgHandler handler = new BlockMsgHandler(); + ReflectUtils.setFieldValue(handler, "tronNetDelegate", delegate); + ReflectUtils.setFieldValue(handler, "fetchBlockService", fetch); + ReflectUtils.setFieldValue(handler, "advService", adv); + PeerConnection sender = newPeer(10001); + PeerConnection other = newPeer(10002); + BlockCapsule block = newBlock(); + Item item = new Item(block.getBlockId(), InventoryType.BLOCK); + when(delegate.getHeadBlockId()).thenReturn(new BlockId(Sha256Hash.ZERO_HASH, 0)); + when(delegate.containBlock(block.getParentBlockId())).thenReturn(true); + when(delegate.getActivePeer()).thenReturn(Collections.singletonList(sender)); + sender.getAdvInvReceive().put(item, System.currentTimeMillis()); + other.getAdvInvReceive().put(item, System.currentTimeMillis()); + P2pException failure = new P2pException(TypeEnum.BAD_BLOCK, + new ValidateScheduleException("wrong slot")); + doThrow(failure).when(delegate).processBlock(any(BlockCapsule.class), anyBoolean()); + try { + Assert.assertTrue(adv.addInv(item)); + when(delegate.getActivePeer()).thenReturn(Arrays.asList(sender, other)); + handler.processMessage(sender, new BlockMessage(block)); + verify(delegate).processBlock(block, false); + verify(sender, never()).disconnect(any()); + Assert.assertFalse(sender.getAdvInvRequest().containsKey(item)); + verify(other, never()).disconnect(any()); + Assert.assertNotEquals(block.getBlockId(), other.getBlockBothHave()); + Assert.assertFalse(other.getAdvInvRequest().containsKey(item)); + verify(other, never()).sendMessage(any(FetchInvDataMessage.class)); + } finally { + adv.close(); + } + } + + private void assertAdvertisedBlockDoesNotRetry(TypeEnum type) throws Exception { + TronNetDelegate delegate = mock(TronNetDelegate.class); + FetchBlockService fetch = mock(FetchBlockService.class); + AdvService adv = new AdvService(); + ReflectUtils.setFieldValue(adv, "tronNetDelegate", delegate); + ReflectUtils.setFieldValue(adv, "fetchBlockService", fetch); + BlockMsgHandler handler = new BlockMsgHandler(); + ReflectUtils.setFieldValue(handler, "tronNetDelegate", delegate); + ReflectUtils.setFieldValue(handler, "fetchBlockService", fetch); + PeerConnection sender = newPeer(10001); + PeerConnection other = newPeer(10002); + BlockCapsule block = newBlock(); + Item item = new Item(block.getBlockId(), InventoryType.BLOCK); + when(delegate.getHeadBlockId()).thenReturn(new BlockId(Sha256Hash.ZERO_HASH, 0)); + when(delegate.getActivePeer()).thenReturn(Collections.singletonList(sender)); + sender.getAdvInvReceive().put(item, System.currentTimeMillis()); + other.getAdvInvReceive().put(item, System.currentTimeMillis()); + P2pException failure = new P2pException(type, "invalid block"); + doThrow(failure).when(delegate).validBlock(any(BlockCapsule.class)); + try { + Assert.assertTrue(adv.addInv(item)); + Assert.assertTrue(sender.getAdvInvRequest().containsKey(item)); + try { + handler.processMessage(sender, new BlockMessage(block)); + Assert.fail("Invalid block must be rejected"); + } catch (P2pException e) { + Assert.assertSame(failure, e); + Method exceptionHandler = P2pEventHandlerImpl.class.getDeclaredMethod( + "processException", PeerConnection.class, TronMessage.class, Exception.class); + exceptionHandler.setAccessible(true); + exceptionHandler.invoke(new P2pEventHandlerImpl(), sender, new BlockMessage(block), e); + verify(sender).disconnect(ReasonCode.BAD_BLOCK); + } + Assert.assertFalse("The failed advertisement must not be retried on disconnect", + sender.getAdvInvRequest().containsKey(item)); + when(delegate.getActivePeer()).thenReturn(Collections.singletonList(other)); + adv.onDisconnect(sender); + Assert.assertFalse(other.getAdvInvRequest().containsKey(item)); + verify(other, never()).sendMessage(any(FetchInvDataMessage.class)); + verify(other, never()).disconnect(any()); + Assert.assertFalse("Leave recovery to sync instead of accepting the same INV again", + adv.addInv(item)); + } finally { + adv.close(); + } + } + + @Test + public void scheduleFailureReleasesSyncRequestWithoutDisconnectingOtherPeers() throws Exception { + assertSyncFailure(new P2pException(TypeEnum.BAD_BLOCK, + new ValidateScheduleException("validateWitnessSchedule error")), ReasonCode.BAD_BLOCK); + } + + @Test + public void invalidSignatureReleasesSyncRequestWithoutDisconnectingOtherPeers() throws Exception { + assertSyncFailure(new P2pException(TypeEnum.BLOCK_SIGN_INVALID, "signature"), + ReasonCode.BAD_BLOCK); + } + + @Test + public void invalidMerkleRootReleasesSyncRequestAndPreservesOtherPeers() throws Exception { + assertSyncFailure(new P2pException(TypeEnum.BAD_BLOCK, new BadBlockException( + BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED, "merkle")), + ReasonCode.BAD_BLOCK); + } + + @Test + public void invalidBlockReleasesSyncRequestWithoutDisconnectingOtherPeers() throws Exception { + assertSyncFailure(new P2pException(TypeEnum.BAD_BLOCK, "invalid transaction"), + ReasonCode.BAD_BLOCK); + } + + @Test + public void localFailureReleasesSyncRequestAndDisconnectsOnlySender() throws Exception { + assertSyncFailure(new IllegalStateException("local processing failure"), ReasonCode.BAD_BLOCK); + } + + @Test + public void unlinkedBlockReleasesSyncRequestAndDisconnectsOnlySender() throws Exception { + assertSyncFailure(new P2pException(TypeEnum.UNLINK_BLOCK, "missing parent"), + ReasonCode.BAD_BLOCK); + } + + @Test + public void syncResponseDoesNotCheckCurrentActiveWitnesses() throws Exception { + BlockMsgHandler handler = new BlockMsgHandler(); + TronNetDelegate delegate = mock(TronNetDelegate.class); + SyncService sync = mock(SyncService.class); + ReflectUtils.setFieldValue(handler, "tronNetDelegate", delegate); + ReflectUtils.setFieldValue(handler, "syncService", sync); + PeerConnection sender = newPeer(10001); + BlockMessage message = new BlockMessage(newBlock()); + sender.getSyncBlockRequested().put(message.getBlockId(), System.currentTimeMillis()); + + handler.processMessage(sender, message); + + verify(delegate, never()).validBlock(any()); + verify(sync).processBlock(sender, message); + Assert.assertFalse(sender.getSyncBlockRequested().containsKey(message.getBlockId())); + Assert.assertTrue(sender.getSyncBlockInProcess().contains(message.getBlockId())); + } + + @SuppressWarnings("unchecked") + private void assertSyncFailure(Exception failure, ReasonCode reason) throws Exception { + SyncService sync = new SyncService(); + TronNetDelegate delegate = mock(TronNetDelegate.class); + ReflectUtils.setFieldValue(sync, "tronNetDelegate", delegate); + PeerConnection sender = newPeer(10001); + PeerConnection other = newPeer(10002); + BlockCapsule block = newBlock(); + BlockId id = block.getBlockId(); + BlockId previous = new BlockId(Sha256Hash.ZERO_HASH, 0); + sender.getSyncBlockToFetch().add(id); + other.getSyncBlockToFetch().add(id); + sender.setBlockBothHave(previous); + other.setBlockBothHave(previous); + sender.getSyncBlockInProcess().add(id); + other.setFetchAble(true); + when(delegate.getActivePeer()).thenReturn(Arrays.asList(sender, other)); + when(delegate.getBlockLock()).thenReturn(new Object()); + when(delegate.getSolidBlockId()).thenReturn(previous); + if (failure instanceof P2pException + && ((P2pException) failure).getType() == TypeEnum.BLOCK_SIGN_INVALID) { + doThrow(failure).when(delegate).validSignature(any(BlockCapsule.class)); + } else { + doThrow(failure).when(delegate).processBlock(any(BlockCapsule.class), anyBoolean()); + } + Cache requests = + (Cache) ReflectUtils.getFieldObject(sync, "requestBlockIds"); + requests.put(id, sender); + try { + sync.processBlock(sender, new BlockMessage(block)); + invoke(sync, "handleSyncBlock"); + sync.onDisconnect(sender); + Assert.assertNull("A failed response must release the global request", + requests.getIfPresent(id)); + Assert.assertEquals(previous, other.getBlockBothHave()); + Assert.assertEquals(id, other.getSyncBlockToFetch().peek()); + Assert.assertFalse(sender.getSyncBlockInProcess().contains(id)); + verify(sender).disconnect(reason); + verify(other, never()).disconnect(any()); + when(delegate.getActivePeer()).thenReturn(Collections.singletonList(other)); + invoke(sync, "startFetchSyncBlock"); + Assert.assertSame(other, requests.getIfPresent(id)); + Assert.assertTrue(other.getSyncBlockRequested().containsKey(id)); + verify(other).sendMessage(any(FetchInvDataMessage.class)); + } finally { + sync.close(); + } + } + + private static BlockCapsule newBlock() { + return new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, ByteString.EMPTY); + } + + private static PeerConnection newPeer(int port) { + PeerConnection peer = spy(new PeerConnection()); + Channel channel = mock(Channel.class); + InetSocketAddress address = new InetSocketAddress("127.0.0.1", port); + when(channel.getInetSocketAddress()).thenReturn(address); + when(channel.getInetAddress()).thenReturn(address.getAddress()); + peer.setChannel(channel); + doNothing().when(peer).sendMessage(any()); + doNothing().when(peer).disconnect(any()); + return peer; + } + + private static void invoke(SyncService service, String name) throws Exception { + Method method = SyncService.class.getDeclaredMethod(name); + method.setAccessible(true); + method.invoke(service); + } +} From fdf2ccc4953c94ac4218b4f7c41289216bc46b2f Mon Sep 17 00:00:00 2001 From: wb Date: Wed, 21 Oct 2026 15:50:28 +0800 Subject: [PATCH 3/6] fix(net): preserve pending transaction results during relay Clear results only on the transaction copy cached for relay. Keep the original capsule unchanged so broadcasting cannot erase pending execution results or race with block assembly. --- .../tron/core/net/service/adv/AdvService.java | 8 +++--- .../core/net/services/AdvServiceTest.java | 25 +++++++++++++++---- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/framework/src/main/java/org/tron/core/net/service/adv/AdvService.java b/framework/src/main/java/org/tron/core/net/service/adv/AdvService.java index f3de5607a17..ba3538f673f 100644 --- a/framework/src/main/java/org/tron/core/net/service/adv/AdvService.java +++ b/framework/src/main/java/org/tron/core/net/service/adv/AdvService.java @@ -169,8 +169,8 @@ public int fastBroadcastTransaction(TransactionMessage msg) { Item item = new Item(msg.getMessageId(), InventoryType.TRX); trxCount.add(); - msg.getTransactionCapsule().resetResult(); - trxCache.put(item, new TransactionMessage(msg.getTransactionCapsule().getInstance())); + trxCache.put(item, new TransactionMessage( + msg.getTransactionCapsule().getInstance().toBuilder().clearRet().build())); List list = new ArrayList<>(); list.add(msg.getMessageId()); @@ -217,8 +217,8 @@ public void broadcast(Message msg) { TransactionMessage trxMsg = (TransactionMessage) msg; item = new Item(trxMsg.getMessageId(), InventoryType.TRX); trxCount.add(); - trxMsg.getTransactionCapsule().resetResult(); - trxCache.put(item, new TransactionMessage(trxMsg.getTransactionCapsule().getInstance())); + trxCache.put(item, new TransactionMessage( + trxMsg.getTransactionCapsule().getInstance().toBuilder().clearRet().build())); } else { logger.error("Adv item is neither block nor trx, type: {}", msg.getType()); return; diff --git a/framework/src/test/java/org/tron/core/net/services/AdvServiceTest.java b/framework/src/test/java/org/tron/core/net/services/AdvServiceTest.java index 856f9fb01cf..26864a806b9 100644 --- a/framework/src/test/java/org/tron/core/net/services/AdvServiceTest.java +++ b/framework/src/test/java/org/tron/core/net/services/AdvServiceTest.java @@ -69,7 +69,7 @@ public static void after() { } @Test - public void test() { + public void test() throws IOException { testAddInv(); testBroadcast(); testTrxBroadcast(); @@ -123,7 +123,7 @@ private void testBroadcast() { Assert.assertNotNull(peer.getAdvInvSpread().getIfPresent(item)); } - private void testTrxBroadcast() { + private void testTrxBroadcast() throws IOException { Protocol.Transaction trx = Protocol.Transaction.newBuilder() .setRawData( Protocol.Transaction.raw.newBuilder() @@ -137,12 +137,13 @@ private void testTrxBroadcast() { Item item = new Item(msg.getMessageId(), InventoryType.TRX); TransactionMessage cached = (TransactionMessage) service.getMessage(item); Assert.assertNotNull(cached); - Assert.assertEquals(0, msg.getTransactionCapsule().getInstance().getRetCount()); - Assert.assertEquals(0, cached.getTransactionCapsule().getInstance().getRetCount()); + assertRelayPreservesOriginal(trx, msg, cached); TransactionMessage fastMsg = new TransactionMessage(trx); service.fastBroadcastTransaction(fastMsg); - Assert.assertEquals(0, fastMsg.getTransactionCapsule().getInstance().getRetCount()); + TransactionMessage fastCached = (TransactionMessage) service.getMessage(item); + Assert.assertNotSame(cached, fastCached); + assertRelayPreservesOriginal(trx, fastMsg, fastCached); Protocol.Transaction expiredTrx = Protocol.Transaction.newBuilder() .setRawData( @@ -157,6 +158,20 @@ private void testTrxBroadcast() { Assert.assertNull(service.getMessage(item1)); } + private void assertRelayPreservesOriginal(Protocol.Transaction original, + TransactionMessage message, TransactionMessage cached) throws IOException { + Assert.assertNotNull(cached); + Assert.assertEquals(original, message.getTransactionCapsule().getInstance()); + Assert.assertArrayEquals(original.toByteArray(), message.getData()); + Assert.assertNotSame(message.getTransactionCapsule(), cached.getTransactionCapsule()); + Protocol.Transaction relay = Protocol.Transaction.parseFrom(cached.getData()); + Assert.assertEquals(0, relay.getRetCount()); + Assert.assertEquals(original.getRawData(), relay.getRawData()); + Assert.assertEquals(original.getSignatureList(), relay.getSignatureList()); + Assert.assertEquals(relay, cached.getTransactionCapsule().getInstance()); + Assert.assertEquals(message.getMessageId(), cached.getMessageId()); + } + @Test public void sameAddressPeersReceiveBroadcastAndDisconnectRetryIndependently() { InetSocketAddress address = new InetSocketAddress("127.0.0.2", 10001); From 342130074a3801ab8516214ae0c72f248397ac62 Mon Sep 17 00:00:00 2001 From: wb Date: Fri, 23 Oct 2026 16:45:19 +0800 Subject: [PATCH 4/6] fix(net): validate block transactions before propagation Reject transactions with a contract count other than one before relaying received blocks. Apply shared timestamp, transaction structure, Merkle root and witness signature checks to both advertised and sync blocks, while keeping current active-witness checks specific to advertised blocks. Validate Merkle roots at network admission and remove the redundant Manager check and capsule validation cache. Keep the existing signature failure types, messages and causes. --- .../org/tron/core/capsule/BlockCapsule.java | 5 - .../main/java/org/tron/core/db/Manager.java | 4 +- .../org/tron/core/net/TronNetDelegate.java | 39 +++-- .../net/messagehandler/BlockMsgHandler.java | 2 +- .../core/net/service/sync/SyncService.java | 2 +- .../tron/core/capsule/BlockCapsuleTest.java | 25 ++- .../java/org/tron/core/db/ManagerTest.java | 35 ++-- .../tron/core/net/TronNetDelegateTest.java | 155 ++++++++++++++++-- .../messagehandler/BlockMsgHandlerTest.java | 82 ++++++++- .../net/services/BlockFetchFailureTest.java | 128 +++++++++++++-- 10 files changed, 410 insertions(+), 67 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 096f295493a..471f4bc27a1 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java @@ -53,7 +53,6 @@ public class BlockCapsule implements ProtoCapsule { public boolean generatedByMyself = false; - private volatile boolean merkleValidated = false; @Getter @Setter private TransactionRetCapsule result; @@ -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() { 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 20fb38a15b5..d476ae73cfd 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -1223,7 +1223,8 @@ public List getVerifyTxs(BlockCapsule block) { } /** - * save a block. + * Save a block. Untrusted callers must first use network prevalidation to check its timestamp, + * transaction structure, Merkle root and block signature. */ public void pushBlock(final BlockCapsule block) throws ValidateSignatureException, ContractValidateException, ContractExeException, @@ -1265,7 +1266,6 @@ public void pushBlock(final BlockCapsule block) try (PendingManager pm = new PendingManager(this)) { if (!block.generatedByMyself) { - block.validateMerkleRoot(); consensus.receiveBlock(block); } diff --git a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java index 223ebe02a00..531e32c5b82 100644 --- a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java +++ b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java @@ -61,6 +61,7 @@ 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 @@ -337,35 +338,43 @@ public boolean pushTransaction(TransactionCapsule trx) throws P2pException { } } - public void validSignature(BlockCapsule block) throws P2pException { - boolean flag; - try { - flag = block.validateSignature(dbManager.getDynamicPropertiesStore(), - dbManager.getAccountStore()); - } catch (Exception e) { - throw new P2pException(TypeEnum.BLOCK_SIGN_INVALID, e); - } - if (!flag) { - throw new P2pException(TypeEnum.BLOCK_SIGN_INVALID, "valid signature failed."); - } - } - 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()); + "time:" + time + ",block time:" + block.getTimeStamp()); + } + for (Transaction transaction : block.getInstance().getTransactionsList()) { + int contractCount = transaction.getRawData().getContractCount(); + if (contractCount != 1) { + throw new P2pException(TypeEnum.BAD_BLOCK, + "Invalid contract count: " + contractCount + + ", block: " + block.getBlockId().getString()); + } } try { block.validateMerkleRoot(); } catch (BadBlockException e) { throw new P2pException(TypeEnum.BLOCK_MERKLE_INVALID, e.getMessage()); } - validSignature(block); + boolean signatureValid; + try { + signatureValid = block.validateSignature(dbManager.getDynamicPropertiesStore(), + dbManager.getAccountStore()); + } catch (Exception e) { + throw new P2pException(TypeEnum.BLOCK_SIGN_INVALID, e); + } + if (!signatureValid) { + throw new P2pException(TypeEnum.BLOCK_SIGN_INVALID, "valid signature failed."); + } + } + + public void validAdvBlock(BlockCapsule block) throws P2pException { if (!witnessScheduleStore.getActiveWitnesses().contains(block.getWitnessAddress())) { throw new P2pException(TypeEnum.BAD_BLOCK, "Block witness is not active: " + block.getBlockId().getString()); } + validBlock(block); } public PbftSignCapsule getBlockPbftCommitData(long blockNum) { 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 b92765ff815..94c7bcb5b9e 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 @@ -134,7 +134,7 @@ private void check(PeerConnection peer, BlockMessage msg) throws P2pException { private void processBlock(PeerConnection peer, BlockCapsule block) throws P2pException { BlockId blockId = block.getBlockId(); - tronNetDelegate.validBlock(block); + tronNetDelegate.validAdvBlock(block); if (!tronNetDelegate.containBlock(block.getParentBlockId())) { logger.warn("Get unlink block {} from {}, head is {}", blockId.getString(), peer.getInetAddress(), tronNetDelegate.getHeadBlockId().getString()); diff --git a/framework/src/main/java/org/tron/core/net/service/sync/SyncService.java b/framework/src/main/java/org/tron/core/net/service/sync/SyncService.java index b8ccb6daf59..8ccfb1e6886 100644 --- a/framework/src/main/java/org/tron/core/net/service/sync/SyncService.java +++ b/framework/src/main/java/org/tron/core/net/service/sync/SyncService.java @@ -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); diff --git a/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java b/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java index 2f46fcccb9d..c06e9d1ef83 100644 --- a/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java +++ b/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java @@ -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 @@ -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 diff --git a/framework/src/test/java/org/tron/core/db/ManagerTest.java b/framework/src/test/java/org/tron/core/db/ManagerTest.java index 4741c13640f..18ede91c0e4 100755 --- a/framework/src/test/java/org/tron/core/db/ManagerTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerTest.java @@ -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; @@ -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; @@ -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 diff --git a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java index 9cf78f88204..91b4f774eb3 100644 --- a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java +++ b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java @@ -22,9 +22,12 @@ import org.tron.core.exception.P2pException; import org.tron.core.exception.P2pException.TypeEnum; import org.tron.core.exception.ValidateScheduleException; +import org.tron.core.exception.ValidateSignatureException; import org.tron.core.metrics.MetricsService; +import org.tron.core.store.AccountStore; import org.tron.core.store.DynamicPropertiesStore; import org.tron.core.store.WitnessScheduleStore; +import org.tron.protos.Protocol.Transaction; import org.tron.protos.Protocol.Transaction.Contract.ContractType; import org.tron.protos.contract.BalanceContract.TransferContract; @@ -180,7 +183,6 @@ public void testValidBlockMerkleRoot() throws Exception { block.addTransaction( new TransactionCapsule(transferContract, ContractType.TransferContract)); - // Wrap in a fresh BlockCapsule so the merkleValidated flag is reset. BlockCapsule tampered = new BlockCapsule(block.getInstance()); TronNetDelegate tronNetDelegate = new TronNetDelegate(); @@ -193,24 +195,157 @@ public void testValidBlockMerkleRoot() throws Exception { } @Test - public void testValidBlockRejectsInactiveWitness() throws Exception { - TronNetDelegate delegate = Mockito.spy(new TronNetDelegate()); + public void testValidBlockRejectsInvalidContractCounts() throws Exception { + for (int count : new int[]{0, 2}) { + for (boolean laterTransaction : new boolean[]{false, true}) { + TronNetDelegate delegate = signatureDelegate(); + BlockCapsule block = Mockito.spy(new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, + ByteString.copyFromUtf8("witness"))); + if (laterTransaction) { + block.addTransaction(contractCountTransaction(1)); + } + block.addTransaction(contractCountTransaction(count)); + block.setMerkleRoot(); + Mockito.doReturn(true).when(block).validateSignature(Mockito.any(), Mockito.any()); + WitnessScheduleStore schedule = mock(WitnessScheduleStore.class); + Mockito.doReturn(Collections.singletonList(block.getWitnessAddress())) + .when(schedule).getActiveWitnesses(); + setField(delegate, "witnessScheduleStore", schedule); + + P2pException error = Assert.assertThrows(P2pException.class, + () -> delegate.validBlock(block)); + + Assert.assertEquals(TypeEnum.BAD_BLOCK, error.getType()); + Assert.assertTrue(error.getMessage().contains(block.getBlockId().getString())); + Assert.assertTrue(error.getMessage().contains("count: " + count)); + Mockito.verify(block, Mockito.never()).validateSignature(Mockito.any(), Mockito.any()); + } + } + } + + @Test + public void testValidBlockAcceptsEmptyAndSingleContractTransactions() throws Exception { + for (int transactions : new int[]{0, 1, 2}) { + TronNetDelegate delegate = signatureDelegate(); + BlockCapsule block = Mockito.spy(new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, + ByteString.copyFromUtf8("witness"))); + for (int i = 0; i < transactions; i++) { + block.addTransaction(contractCountTransaction(1)); + } + block.setMerkleRoot(); + Mockito.doReturn(true).when(block).validateSignature(Mockito.any(), Mockito.any()); + WitnessScheduleStore schedule = mock(WitnessScheduleStore.class); + Mockito.doReturn(Collections.singletonList(block.getWitnessAddress())) + .when(schedule).getActiveWitnesses(); + setField(delegate, "witnessScheduleStore", schedule); + + delegate.validBlock(block); + + Mockito.verify(block).validateSignature(Mockito.any(), Mockito.any()); + } + } + + private TransactionCapsule contractCountTransaction(int count) { + Transaction.raw.Builder raw = Transaction.raw.newBuilder(); + for (int i = 0; i < count; i++) { + raw.addContract(Transaction.Contract.newBuilder().setType(ContractType.TransferContract)); + } + return new TransactionCapsule(Transaction.newBuilder().setRawData(raw).build()); + } + + @Test + public void testValidAdvBlockRejectsInactiveWitness() throws Exception { + TronNetDelegate delegate = signatureDelegate(); WitnessScheduleStore schedule = mock(WitnessScheduleStore.class); setField(delegate, "witnessScheduleStore", schedule); - BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, - ByteString.copyFromUtf8("witness")); + BlockCapsule block = Mockito.spy(new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, + ByteString.copyFromUtf8("witness"))); block.setMerkleRoot(); - Mockito.doNothing().when(delegate).validSignature(block); + Mockito.doReturn(true).when(block).validateSignature(Mockito.any(), Mockito.any()); Mockito.when(schedule.getActiveWitnesses()).thenReturn(Collections.emptyList()); + delegate.validBlock(block); + Mockito.clearInvocations(delegate, block); try { - delegate.validBlock(block); + delegate.validAdvBlock(block); Assert.fail("An inactive witness must be rejected instead of silently dropped"); } catch (P2pException e) { Assert.assertEquals(TypeEnum.BAD_BLOCK, e.getType()); } - Mockito.when(schedule.getActiveWitnesses()) - .thenReturn(Collections.singletonList(block.getWitnessAddress())); - delegate.validBlock(block); + Mockito.verify(delegate, Mockito.never()).validBlock(block); + Mockito.verify(block, Mockito.never()).validateSignature(Mockito.any(), Mockito.any()); + Mockito.doReturn(Collections.singletonList(block.getWitnessAddress())) + .when(schedule).getActiveWitnesses(); + delegate.validAdvBlock(block); + Mockito.verify(delegate).validBlock(block); + Mockito.verify(block).validateSignature(Mockito.any(), Mockito.any()); + } + + @Test + public void testFutureTimeCutoffAppliesToCommonAndAdvertisedBlocks() throws Exception { + TronNetDelegate delegate = signatureDelegate(); + BlockCapsule block = Mockito.spy(new BlockCapsule(1, Sha256Hash.ZERO_HASH, + System.currentTimeMillis() + 60000, ByteString.copyFromUtf8("witness"))); + block.setMerkleRoot(); + Mockito.doReturn(true).when(block).validateSignature(Mockito.any(), Mockito.any()); + WitnessScheduleStore schedule = mock(WitnessScheduleStore.class); + Mockito.doReturn(Collections.singletonList(block.getWitnessAddress())) + .when(schedule).getActiveWitnesses(); + setField(delegate, "witnessScheduleStore", schedule); + + P2pException commonError = Assert.assertThrows(P2pException.class, + () -> delegate.validBlock(block)); + P2pException advertisedError = Assert.assertThrows(P2pException.class, + () -> delegate.validAdvBlock(block)); + + Assert.assertEquals(TypeEnum.BAD_BLOCK, commonError.getType()); + Assert.assertEquals(TypeEnum.BAD_BLOCK, advertisedError.getType()); + Assert.assertTrue(commonError.getMessage().contains("block time:")); + Assert.assertTrue(advertisedError.getMessage().contains("block time:")); + Mockito.verify(block, Mockito.never()).validateSignature(Mockito.any(), Mockito.any()); + } + + @Test + public void testValidBlockRejectsFalseSignature() throws Exception { + TronNetDelegate delegate = signatureDelegate(); + BlockCapsule block = Mockito.spy(new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, + ByteString.copyFromUtf8("witness"))); + block.setMerkleRoot(); + Mockito.doReturn(false).when(block).validateSignature(Mockito.any(), Mockito.any()); + + P2pException error = Assert.assertThrows(P2pException.class, + () -> delegate.validBlock(block)); + + Assert.assertEquals(TypeEnum.BLOCK_SIGN_INVALID, error.getType()); + Assert.assertEquals("valid signature failed.", error.getMessage()); + } + + @Test + public void testValidBlockWrapsSignatureFailureWithCause() throws Exception { + for (Exception failure : new Exception[]{new ValidateSignatureException("invalid signature"), + new IllegalStateException("signature validation unavailable")}) { + TronNetDelegate delegate = signatureDelegate(); + BlockCapsule block = Mockito.spy(new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, + ByteString.copyFromUtf8("witness"))); + block.setMerkleRoot(); + Mockito.doThrow(failure).when(block).validateSignature(Mockito.any(), Mockito.any()); + + P2pException error = Assert.assertThrows(P2pException.class, + () -> delegate.validBlock(block)); + + Assert.assertEquals(TypeEnum.BLOCK_SIGN_INVALID, error.getType()); + Assert.assertSame(failure, error.getCause()); + Assert.assertEquals(failure.toString(), error.getMessage()); + } + } + + private TronNetDelegate signatureDelegate() throws Exception { + Manager manager = mock(Manager.class); + Mockito.when(manager.getDynamicPropertiesStore()) + .thenReturn(mock(DynamicPropertiesStore.class)); + Mockito.when(manager.getAccountStore()).thenReturn(mock(AccountStore.class)); + TronNetDelegate delegate = Mockito.spy(new TronNetDelegate()); + setField(delegate, "dbManager", manager); + return delegate; } @Test diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java index c37ebe60690..f98db06ce89 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java @@ -42,6 +42,7 @@ import org.tron.core.net.peer.PeerConnection; import org.tron.core.net.service.adv.AdvService; import org.tron.core.net.service.fetchblock.FetchBlockService; +import org.tron.core.net.service.relay.RelayService; import org.tron.core.net.service.sync.SyncService; import org.tron.core.services.WitnessProductBlockService; import org.tron.core.store.AccountStore; @@ -235,6 +236,83 @@ public void testInvalidWitnessIngress() throws Exception { } } + @Test + public void testInvalidContractCountRejectedBeforeBroadcast() throws Exception { + for (boolean fastForward : new boolean[]{false, true}) { + for (int count : new int[]{0, 2}) { + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, 1234, + ByteString.copyFrom(key.getAddress())); + block.addTransaction(new TransactionCapsule(Transaction.newBuilder() + .setRawData(Transaction.raw.newBuilder().addContract( + Transaction.Contract.newBuilder() + .setType(Transaction.Contract.ContractType.TransferContract))).build())); + Transaction.raw.Builder raw = Transaction.raw.newBuilder().setTimestamp(1235); + for (int i = 0; i < count; i++) { + raw.addContract(Transaction.Contract.newBuilder() + .setType(Transaction.Contract.ContractType.TransferContract)); + } + block.addTransaction(new TransactionCapsule( + Transaction.newBuilder().setRawData(raw).build())); + block.setMerkleRoot(); + block.sign(key.getPrivKeyBytes()); + TronNetDelegate delegate = witnessDelegate(false, block.getWitnessAddress()); + Manager manager = (Manager) ReflectUtils.getFieldObject(delegate, "dbManager"); + Assert.assertTrue(block.validateSignature(manager.getDynamicPropertiesStore(), + manager.getAccountStore())); + AdvService adv = Mockito.mock(AdvService.class); + RelayService relay = Mockito.mock(RelayService.class); + BlockMsgHandler ingress = witnessHandler(delegate, adv, Mockito.mock(SyncService.class)); + ReflectUtils.setFieldValue(ingress, "fastForward", fastForward); + ReflectUtils.setFieldValue(ingress, "relayService", relay); + BlockMessage message = new BlockMessage(block); + PeerConnection sender = witnessPeer(); + sender.getAdvInvRequest().put(new Item(message.getBlockId(), InventoryType.BLOCK), 0L); + + P2pException error = Assert.assertThrows(P2pException.class, + () -> ingress.processMessage(sender, message)); + + Assert.assertEquals(P2pException.TypeEnum.BAD_BLOCK, error.getType()); + Mockito.verify(adv, Mockito.never()).broadcast(any()); + Mockito.verify(relay, Mockito.never()).broadcast(any()); + Mockito.verify(delegate, Mockito.never()).processBlock(any(BlockCapsule.class), + anyBoolean()); + } + } + } + + @Test + public void testInvalidMerkleRootRejectedBeforeBroadcast() throws Exception { + for (boolean fastForward : new boolean[]{false, true}) { + BlockCapsule block = signedWitnessBlock(); + block.addTransaction(new TransactionCapsule(Transaction.newBuilder() + .setRawData(Transaction.raw.newBuilder().setTimestamp(1235).addContract( + Transaction.Contract.newBuilder() + .setType(Transaction.Contract.ContractType.TransferContract))).build())); + TronNetDelegate delegate = witnessDelegate(false, block.getWitnessAddress()); + Manager manager = (Manager) ReflectUtils.getFieldObject(delegate, "dbManager"); + Assert.assertTrue(block.validateSignature(manager.getDynamicPropertiesStore(), + manager.getAccountStore())); + AdvService adv = Mockito.mock(AdvService.class); + RelayService relay = Mockito.mock(RelayService.class); + BlockMsgHandler ingress = witnessHandler(delegate, adv, Mockito.mock(SyncService.class)); + ReflectUtils.setFieldValue(ingress, "fastForward", fastForward); + ReflectUtils.setFieldValue(ingress, "relayService", relay); + BlockMessage message = new BlockMessage(block); + PeerConnection sender = witnessPeer(); + sender.getAdvInvRequest().put(new Item(message.getBlockId(), InventoryType.BLOCK), 0L); + + P2pException error = Assert.assertThrows(P2pException.class, + () -> ingress.processMessage(sender, message)); + + Assert.assertEquals(P2pException.TypeEnum.BLOCK_MERKLE_INVALID, error.getType()); + Mockito.verify(adv, Mockito.never()).broadcast(any()); + Mockito.verify(relay, Mockito.never()).broadcast(any()); + Mockito.verify(delegate, Mockito.never()).processBlock(any(BlockCapsule.class), + anyBoolean()); + } + } + @Test public void testPaddedWitnessSyncHandoff() throws Exception { BlockCapsule canonical = signedWitnessBlock(); @@ -263,7 +341,9 @@ private BlockCapsule signedWitnessBlock() { ByteString.copyFrom(key.getAddress())); // A padded transaction signature must survive witness normalization, including sync handoff. Transaction transaction = Transaction.newBuilder() - .setRawData(Transaction.raw.newBuilder().setTimestamp(1234)) + .setRawData(Transaction.raw.newBuilder().setTimestamp(1234) + .addContract(Transaction.Contract.newBuilder() + .setType(Transaction.Contract.ContractType.TransferContract))) .addSignature(ByteString.copyFrom(new byte[68])).build(); block.addTransaction(new TransactionCapsule(transaction)); block.setMerkleRoot(); diff --git a/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java b/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java index 1ef73de9a99..dad02bcceaf 100644 --- a/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java +++ b/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java @@ -2,7 +2,9 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -13,6 +15,7 @@ import com.google.common.cache.Cache; import com.google.protobuf.ByteString; import java.lang.reflect.Method; +import java.math.BigInteger; import java.net.InetSocketAddress; import java.util.Arrays; import java.util.Collections; @@ -23,12 +26,14 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.tron.common.TestConstants; +import org.tron.common.crypto.ECKey; import org.tron.common.utils.ReflectUtils; import org.tron.common.utils.Sha256Hash; import org.tron.core.capsule.BlockCapsule; import org.tron.core.capsule.BlockCapsule.BlockId; +import org.tron.core.capsule.TransactionCapsule; import org.tron.core.config.args.Args; -import org.tron.core.exception.BadBlockException; +import org.tron.core.db.Manager; import org.tron.core.exception.P2pException; import org.tron.core.exception.P2pException.TypeEnum; import org.tron.core.exception.ValidateScheduleException; @@ -38,14 +43,19 @@ import org.tron.core.net.message.adv.BlockMessage; import org.tron.core.net.message.adv.FetchInvDataMessage; import org.tron.core.net.messagehandler.BlockMsgHandler; +import org.tron.core.net.messagehandler.PbftDataSyncHandler; import org.tron.core.net.peer.Item; import org.tron.core.net.peer.PeerConnection; import org.tron.core.net.service.adv.AdvService; import org.tron.core.net.service.fetchblock.FetchBlockService; import org.tron.core.net.service.sync.SyncService; +import org.tron.core.store.AccountStore; +import org.tron.core.store.DynamicPropertiesStore; +import org.tron.core.store.WitnessScheduleStore; import org.tron.p2p.connection.Channel; import org.tron.protos.Protocol.Inventory.InventoryType; import org.tron.protos.Protocol.ReasonCode; +import org.tron.protos.Protocol.Transaction; public class BlockFetchFailureTest { @@ -135,7 +145,7 @@ private void assertAdvertisedBlockDoesNotRetry(TypeEnum type) throws Exception { sender.getAdvInvReceive().put(item, System.currentTimeMillis()); other.getAdvInvReceive().put(item, System.currentTimeMillis()); P2pException failure = new P2pException(type, "invalid block"); - doThrow(failure).when(delegate).validBlock(any(BlockCapsule.class)); + doThrow(failure).when(delegate).validAdvBlock(any(BlockCapsule.class)); try { Assert.assertTrue(adv.addInv(item)); Assert.assertTrue(sender.getAdvInvRequest().containsKey(item)); @@ -144,6 +154,8 @@ private void assertAdvertisedBlockDoesNotRetry(TypeEnum type) throws Exception { Assert.fail("Invalid block must be rejected"); } catch (P2pException e) { Assert.assertSame(failure, e); + verify(delegate).validAdvBlock(block); + verify(delegate, never()).processBlock(any(), anyBoolean()); Method exceptionHandler = P2pEventHandlerImpl.class.getDeclaredMethod( "processException", PeerConnection.class, TronMessage.class, Exception.class); exceptionHandler.setAccessible(true); @@ -173,14 +185,13 @@ public void scheduleFailureReleasesSyncRequestWithoutDisconnectingOtherPeers() t @Test public void invalidSignatureReleasesSyncRequestWithoutDisconnectingOtherPeers() throws Exception { assertSyncFailure(new P2pException(TypeEnum.BLOCK_SIGN_INVALID, "signature"), - ReasonCode.BAD_BLOCK); + ReasonCode.BAD_BLOCK, true); } @Test public void invalidMerkleRootReleasesSyncRequestAndPreservesOtherPeers() throws Exception { - assertSyncFailure(new P2pException(TypeEnum.BAD_BLOCK, new BadBlockException( - BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED, "merkle")), - ReasonCode.BAD_BLOCK); + assertSyncFailure(new P2pException(TypeEnum.BLOCK_MERKLE_INVALID, "merkle"), + ReasonCode.BAD_BLOCK, true); } @Test @@ -214,16 +225,23 @@ public void syncResponseDoesNotCheckCurrentActiveWitnesses() throws Exception { handler.processMessage(sender, message); verify(delegate, never()).validBlock(any()); + verify(delegate, never()).validAdvBlock(any()); verify(sync).processBlock(sender, message); Assert.assertFalse(sender.getSyncBlockRequested().containsKey(message.getBlockId())); Assert.assertTrue(sender.getSyncBlockInProcess().contains(message.getBlockId())); } - @SuppressWarnings("unchecked") private void assertSyncFailure(Exception failure, ReasonCode reason) throws Exception { + assertSyncFailure(failure, reason, false); + } + + @SuppressWarnings("unchecked") + private void assertSyncFailure(Exception failure, ReasonCode reason, boolean prevalidation) + throws Exception { SyncService sync = new SyncService(); TronNetDelegate delegate = mock(TronNetDelegate.class); ReflectUtils.setFieldValue(sync, "tronNetDelegate", delegate); + ReflectUtils.setFieldValue(sync, "pbftDataSyncHandler", mock(PbftDataSyncHandler.class)); PeerConnection sender = newPeer(10001); PeerConnection other = newPeer(10002); BlockCapsule block = newBlock(); @@ -238,9 +256,8 @@ private void assertSyncFailure(Exception failure, ReasonCode reason) throws Exce when(delegate.getActivePeer()).thenReturn(Arrays.asList(sender, other)); when(delegate.getBlockLock()).thenReturn(new Object()); when(delegate.getSolidBlockId()).thenReturn(previous); - if (failure instanceof P2pException - && ((P2pException) failure).getType() == TypeEnum.BLOCK_SIGN_INVALID) { - doThrow(failure).when(delegate).validSignature(any(BlockCapsule.class)); + if (prevalidation) { + doThrow(failure).when(delegate).validBlock(any(BlockCapsule.class)); } else { doThrow(failure).when(delegate).processBlock(any(BlockCapsule.class), anyBoolean()); } @@ -250,6 +267,13 @@ private void assertSyncFailure(Exception failure, ReasonCode reason) throws Exce try { sync.processBlock(sender, new BlockMessage(block)); invoke(sync, "handleSyncBlock"); + verify(delegate).validBlock(any(BlockCapsule.class)); + verify(delegate, never()).validAdvBlock(any()); + if (prevalidation) { + verify(delegate, never()).processBlock(any(), anyBoolean()); + } else { + verify(delegate).processBlock(any(BlockCapsule.class), eq(true)); + } sync.onDisconnect(sender); Assert.assertNull("A failed response must release the global request", requests.getIfPresent(id)); @@ -268,6 +292,90 @@ private void assertSyncFailure(Exception failure, ReasonCode reason) throws Exce } } + @Test + public void syncRejectsInvalidContractCountsBeforeProcessing() throws Exception { + for (int count : new int[]{0, 2}) { + assertRealSyncValidation(count, false); + } + } + + @Test + public void syncAcceptsHistoricalWitnessOutsideCurrentSchedule() throws Exception { + assertRealSyncValidation(1, true); + } + + @Test + public void syncRejectsInvalidMerkleRootBeforeProcessing() throws Exception { + assertRealSyncValidation(1, false, true); + } + + private void assertRealSyncValidation(int count, boolean accepted) throws Exception { + assertRealSyncValidation(count, accepted, false); + } + + private void assertRealSyncValidation(int count, boolean accepted, boolean staleRoot) + throws Exception { + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, 1234, + ByteString.copyFrom(key.getAddress())); + Transaction.raw.Builder raw = Transaction.raw.newBuilder(); + for (int i = 0; i < count; i++) { + raw.addContract(Transaction.Contract.newBuilder() + .setType(Transaction.Contract.ContractType.TransferContract)); + } + block.addTransaction(new TransactionCapsule(Transaction.newBuilder().setRawData(raw).build())); + block.setMerkleRoot(); + if (staleRoot) { + block.addTransaction(new TransactionCapsule(Transaction.newBuilder() + .setRawData(Transaction.raw.newBuilder().setTimestamp(1).addContract( + Transaction.Contract.newBuilder() + .setType(Transaction.Contract.ContractType.TransferContract))).build())); + } + block.sign(key.getPrivKeyBytes()); + block = spy(block); + DynamicPropertiesStore properties = mock(DynamicPropertiesStore.class); + Manager manager = mock(Manager.class); + when(manager.getDynamicPropertiesStore()).thenReturn(properties); + when(manager.getAccountStore()).thenReturn(mock(AccountStore.class)); + WitnessScheduleStore schedule = mock(WitnessScheduleStore.class); + when(schedule.getActiveWitnesses()).thenReturn(Collections.emptyList()); + TronNetDelegate delegate = spy(new TronNetDelegate()); + ReflectUtils.setFieldValue(delegate, "dbManager", manager); + ReflectUtils.setFieldValue(delegate, "witnessScheduleStore", schedule); + doReturn(Collections.emptyList()).when(delegate).getActivePeer(); + doNothing().when(delegate).processBlock(any(), anyBoolean()); + Assert.assertTrue(block.validateSignature(properties, manager.getAccountStore())); + SyncService sync = new SyncService(); + PbftDataSyncHandler pbft = mock(PbftDataSyncHandler.class); + ReflectUtils.setFieldValue(sync, "tronNetDelegate", delegate); + ReflectUtils.setFieldValue(sync, "pbftDataSyncHandler", pbft); + PeerConnection sender = newPeer(10001); + try { + Method process = SyncService.class.getDeclaredMethod("processSyncBlock", + BlockCapsule.class, PeerConnection.class); + process.setAccessible(true); + process.invoke(sync, block, sender); + + verify(delegate).validBlock(block); + verify(delegate, never()).validAdvBlock(any()); + verify(schedule, never()).getActiveWitnesses(); + if (accepted) { + verify(delegate).processBlock(block, true); + verify(pbft).processPBFTCommitData(block); + verify(sender, never()).disconnect(any()); + } else { + if (staleRoot) { + verify(block).validateSignature(properties, manager.getAccountStore()); + } + verify(delegate, never()).processBlock(any(), anyBoolean()); + verify(pbft, never()).processPBFTCommitData(any()); + verify(sender).disconnect(ReasonCode.BAD_BLOCK); + } + } finally { + sync.close(); + } + } + private static BlockCapsule newBlock() { return new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, ByteString.EMPTY); } From 9b3ab8740767e9d7ca220f07b25fe4eb1e49f244 Mon Sep 17 00:00:00 2001 From: wb Date: Tue, 27 Oct 2026 14:54:46 +0800 Subject: [PATCH 5/6] fix(net): reject block transactions smaller than 80 bytes Check the full serialized transaction size before Merkle validation for advertised and sync blocks. Exclude short transaction encodings from the 64-byte internal-node hash input domain. --- .../org/tron/core/net/TronNetDelegate.java | 9 +++ .../tron/core/net/TronNetDelegateTest.java | 81 ++++++++++++++++++- .../messagehandler/BlockMsgHandlerTest.java | 45 ++++++++++- .../net/services/BlockFetchFailureTest.java | 18 ++++- 4 files changed, 148 insertions(+), 5 deletions(-) diff --git a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java index 82be302cc9b..2837b374fc2 100644 --- a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java +++ b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java @@ -68,6 +68,9 @@ @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; @@ -365,6 +368,12 @@ public void validBlock(BlockCapsule block) throws P2pException { "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(); diff --git a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java index 72fc1fef17f..49dd10b403c 100644 --- a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java +++ b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java @@ -32,6 +32,7 @@ import org.tron.protos.Protocol.Transaction; import org.tron.protos.Protocol.Transaction.Contract.ContractType; import org.tron.protos.contract.BalanceContract.TransferContract; +import org.tron.protos.contract.ShieldContract.ShieldedTransferContract; public class TronNetDelegateTest { @@ -286,6 +287,83 @@ public void testValidAdvBlockAcceptsActiveWitnessWithoutChangingHistory() throws Assert.assertEquals(86_400_000L, peer.getLastInactiveWitnessBlockTime()); } + @Test + public void testValidBlockRejectsTransactionsBelow80Bytes() throws Exception { + for (int size : new int[]{64, 79}) { + for (boolean advertised : new boolean[]{false, true}) { + TronNetDelegate delegate = signatureDelegate(); + BlockCapsule block = blockWithTransaction(sizedTransaction(size)); + WitnessScheduleStore schedule = mock(WitnessScheduleStore.class); + Mockito.doReturn(Collections.singletonList(block.getWitnessAddress())) + .when(schedule).getActiveWitnesses(); + setField(delegate, "witnessScheduleStore", schedule); + + P2pException error = Assert.assertThrows(P2pException.class, () -> { + if (advertised) { + delegate.validAdvBlock(new PeerConnection(), block); + } else { + delegate.validBlock(block); + } + }); + + Assert.assertEquals(TypeEnum.BAD_BLOCK, error.getType()); + Assert.assertTrue(error.getMessage().contains("size: " + size)); + Mockito.verify(block, Mockito.never()).validateMerkleRoot(); + Mockito.verify(block, Mockito.never()).validateSignature(Mockito.any(), Mockito.any()); + } + } + } + + @Test + public void testValidBlockAccepts80ByteBoundary() throws Exception { + for (int size : new int[]{80, 81}) { + BlockCapsule block = blockWithTransaction(sizedTransaction(size)); + signatureDelegate().validBlock(block); + Mockito.verify(block).validateMerkleRoot(); + Mockito.verify(block).validateSignature(Mockito.any(), Mockito.any()); + } + } + + @Test + public void testValidBlockSizeIncludesSignaturesAndResults() throws Exception { + Transaction small = sizedTransaction(78); + for (Transaction transaction : new Transaction[]{ + small.toBuilder().addSignature(ByteString.copyFrom(new byte[1])).build(), + small.toBuilder().addRet(Transaction.Result.newBuilder().setFee(1)).build()}) { + Assert.assertTrue(transaction.getRawData().getSerializedSize() < 80); + Assert.assertTrue(transaction.getSerializedSize() >= 80); + signatureDelegate().validBlock(blockWithTransaction(transaction)); + } + } + + @Test + public void testValidBlockAcceptsShieldedTransactionWithoutOuterSignature() throws Exception { + Transaction transaction = new TransactionCapsule(ShieldedTransferContract.newBuilder() + .setBindingSignature(ByteString.copyFrom(new byte[64])).build(), + ContractType.ShieldedTransferContract).getInstance(); + Assert.assertEquals(0, transaction.getSignatureCount()); + Assert.assertTrue(transaction.getSerializedSize() >= 80); + signatureDelegate().validBlock(blockWithTransaction(transaction)); + } + + private Transaction sizedTransaction(int size) { + Transaction transaction = Transaction.newBuilder().setRawData(Transaction.raw.newBuilder() + .addContract(Transaction.Contract.newBuilder().setType(ContractType.TransferContract)) + .setData(ByteString.copyFrom(new byte[size - 8]))).build(); + Assert.assertEquals(size, transaction.getSerializedSize()); + Assert.assertEquals(size, transaction.toByteArray().length); + return transaction; + } + + private BlockCapsule blockWithTransaction(Transaction transaction) throws Exception { + BlockCapsule block = Mockito.spy(new BlockCapsule(1, Sha256Hash.ZERO_HASH, 0, + ByteString.copyFromUtf8("witness"))); + block.addTransaction(new TransactionCapsule(transaction)); + block.setMerkleRoot(); + Mockito.doReturn(true).when(block).validateSignature(Mockito.any(), Mockito.any()); + return block; + } + @Test public void testValidBlockRejectsInvalidContractCounts() throws Exception { for (int count : new int[]{0, 2}) { @@ -338,7 +416,8 @@ public void testValidBlockAcceptsEmptyAndSingleContractTransactions() throws Exc } private TransactionCapsule contractCountTransaction(int count) { - Transaction.raw.Builder raw = Transaction.raw.newBuilder(); + Transaction.raw.Builder raw = Transaction.raw.newBuilder() + .setData(ByteString.copyFrom(new byte[80])); for (int i = 0; i < count; i++) { raw.addContract(Transaction.Contract.newBuilder().setType(ContractType.TransferContract)); } diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java index 1947ff7932a..389a8d869af 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java @@ -245,7 +245,8 @@ public void testInvalidContractCountRejectedBeforeBroadcast() throws Exception { BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, 1234, ByteString.copyFrom(key.getAddress())); block.addTransaction(new TransactionCapsule(Transaction.newBuilder() - .setRawData(Transaction.raw.newBuilder().addContract( + .setRawData(Transaction.raw.newBuilder() + .setData(ByteString.copyFrom(new byte[80])).addContract( Transaction.Contract.newBuilder() .setType(Transaction.Contract.ContractType.TransferContract))).build())); Transaction.raw.Builder raw = Transaction.raw.newBuilder().setTimestamp(1235); @@ -274,6 +275,44 @@ public void testInvalidContractCountRejectedBeforeBroadcast() throws Exception { () -> ingress.processMessage(sender, message)); Assert.assertEquals(P2pException.TypeEnum.BAD_BLOCK, error.getType()); + Assert.assertTrue(error.getMessage().contains("count: " + count)); + Mockito.verify(adv, Mockito.never()).broadcast(any()); + Mockito.verify(relay, Mockito.never()).broadcast(any()); + Mockito.verify(delegate, Mockito.never()).processBlock(any(BlockCapsule.class), + anyBoolean()); + } + } + } + + @Test + public void testUndersizedLaterTransactionRejectedBeforeBroadcast() throws Exception { + for (boolean fastForward : new boolean[]{false, true}) { + for (int size : new int[]{64, 79}) { + BlockCapsule block = signedWitnessBlock(); + Transaction transaction = Transaction.newBuilder() + .setRawData(Transaction.raw.newBuilder() + .setData(ByteString.copyFrom(new byte[size - 8])).addContract( + Transaction.Contract.newBuilder() + .setType(Transaction.Contract.ContractType.TransferContract))).build(); + Assert.assertEquals(size, transaction.getSerializedSize()); + block.addTransaction(new TransactionCapsule(transaction)); + block.setMerkleRoot(); + block.sign(ECKey.fromPrivate(BigInteger.TEN).getPrivKeyBytes()); + TronNetDelegate delegate = witnessDelegate(false, block.getWitnessAddress()); + AdvService adv = Mockito.mock(AdvService.class); + RelayService relay = Mockito.mock(RelayService.class); + BlockMsgHandler ingress = witnessHandler(delegate, adv, Mockito.mock(SyncService.class)); + ReflectUtils.setFieldValue(ingress, "fastForward", fastForward); + ReflectUtils.setFieldValue(ingress, "relayService", relay); + BlockMessage message = new BlockMessage(block); + PeerConnection sender = witnessPeer(); + sender.getAdvInvRequest().put(new Item(message.getBlockId(), InventoryType.BLOCK), 0L); + + P2pException error = Assert.assertThrows(P2pException.class, + () -> ingress.processMessage(sender, message)); + + Assert.assertEquals(P2pException.TypeEnum.BAD_BLOCK, error.getType()); + Assert.assertTrue(error.getMessage().contains("size: " + size)); Mockito.verify(adv, Mockito.never()).broadcast(any()); Mockito.verify(relay, Mockito.never()).broadcast(any()); Mockito.verify(delegate, Mockito.never()).processBlock(any(BlockCapsule.class), @@ -287,7 +326,8 @@ public void testInvalidMerkleRootRejectedBeforeBroadcast() throws Exception { for (boolean fastForward : new boolean[]{false, true}) { BlockCapsule block = signedWitnessBlock(); block.addTransaction(new TransactionCapsule(Transaction.newBuilder() - .setRawData(Transaction.raw.newBuilder().setTimestamp(1235).addContract( + .setRawData(Transaction.raw.newBuilder().setTimestamp(1235) + .setData(ByteString.copyFrom(new byte[80])).addContract( Transaction.Contract.newBuilder() .setType(Transaction.Contract.ContractType.TransferContract))).build())); TronNetDelegate delegate = witnessDelegate(false, block.getWitnessAddress()); @@ -343,6 +383,7 @@ private BlockCapsule signedWitnessBlock() { // A padded transaction signature must survive witness normalization, including sync handoff. Transaction transaction = Transaction.newBuilder() .setRawData(Transaction.raw.newBuilder().setTimestamp(1234) + .setData(ByteString.copyFrom(new byte[80])) .addContract(Transaction.Contract.newBuilder() .setType(Transaction.Contract.ContractType.TransferContract))) .addSignature(ByteString.copyFrom(new byte[68])).build(); diff --git a/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java b/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java index 743da4c75a4..85216d805c0 100644 --- a/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java +++ b/framework/src/test/java/org/tron/core/net/services/BlockFetchFailureTest.java @@ -492,16 +492,29 @@ public void syncRejectsInvalidMerkleRootBeforeProcessing() throws Exception { assertRealSyncValidation(1, false, true); } + @Test + public void syncRejectsUndersizedTransactionsBeforeProcessing() throws Exception { + for (int dataSize : new int[]{56, 71}) { + assertRealSyncValidation(1, false, false, dataSize); + } + } + private void assertRealSyncValidation(int count, boolean accepted) throws Exception { assertRealSyncValidation(count, accepted, false); } private void assertRealSyncValidation(int count, boolean accepted, boolean staleRoot) throws Exception { + assertRealSyncValidation(count, accepted, staleRoot, 80); + } + + private void assertRealSyncValidation(int count, boolean accepted, boolean staleRoot, + int dataSize) throws Exception { ECKey key = ECKey.fromPrivate(BigInteger.TEN); BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, 1234, ByteString.copyFrom(key.getAddress())); - Transaction.raw.Builder raw = Transaction.raw.newBuilder(); + Transaction.raw.Builder raw = Transaction.raw.newBuilder() + .setData(ByteString.copyFrom(new byte[dataSize])); for (int i = 0; i < count; i++) { raw.addContract(Transaction.Contract.newBuilder() .setType(Transaction.Contract.ContractType.TransferContract)); @@ -510,7 +523,8 @@ private void assertRealSyncValidation(int count, boolean accepted, boolean stale block.setMerkleRoot(); if (staleRoot) { block.addTransaction(new TransactionCapsule(Transaction.newBuilder() - .setRawData(Transaction.raw.newBuilder().setTimestamp(1).addContract( + .setRawData(Transaction.raw.newBuilder().setTimestamp(1) + .setData(ByteString.copyFrom(new byte[80])).addContract( Transaction.Contract.newBuilder() .setType(Transaction.Contract.ContractType.TransferContract))).build())); } From 35bd037ab63f5fded64ef9c32f08df2878670fa6 Mon Sep 17 00:00:00 2001 From: wb Date: Tue, 27 Oct 2026 16:49:01 +0800 Subject: [PATCH 6/6] fix(net): keep block timestamp checks at message ingress Rely on BlockMsgHandler to check timestamps for advertised and sync blocks. Remove the duplicate one-second wall-clock rejection from shared block validation. --- .../org/tron/core/net/TronNetDelegate.java | 7 ----- .../tron/core/net/TronNetDelegateTest.java | 17 +++++------- .../messagehandler/BlockMsgHandlerTest.java | 26 +++++++++++++++++++ 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java index 2837b374fc2..2948862b181 100644 --- a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java +++ b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java @@ -93,8 +93,6 @@ public class TronNetDelegate { private int blockIdCacheSize = 100; - private long timeout = 1000; - @Getter // for test private volatile boolean hitDown = false; @@ -356,11 +354,6 @@ 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(); if (contractCount != 1) { diff --git a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java index 49dd10b403c..38c0b38f04b 100644 --- a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java +++ b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java @@ -425,7 +425,7 @@ private TransactionCapsule contractCountTransaction(int count) { } @Test - public void testFutureTimeCutoffAppliesToCommonAndAdvertisedBlocks() throws Exception { + public void testCommonAndAdvertisedValidationDoNotRepeatIngressClockCheck() throws Exception { TronNetDelegate delegate = signatureDelegate(); BlockCapsule block = Mockito.spy(new BlockCapsule(1, Sha256Hash.ZERO_HASH, System.currentTimeMillis() + 60000, ByteString.copyFromUtf8("witness"))); @@ -436,16 +436,11 @@ public void testFutureTimeCutoffAppliesToCommonAndAdvertisedBlocks() throws Exce .when(schedule).getActiveWitnesses(); setField(delegate, "witnessScheduleStore", schedule); - P2pException commonError = Assert.assertThrows(P2pException.class, - () -> delegate.validBlock(block)); - P2pException advertisedError = Assert.assertThrows(P2pException.class, - () -> delegate.validAdvBlock(new PeerConnection(), block)); - - Assert.assertEquals(TypeEnum.BAD_BLOCK, commonError.getType()); - Assert.assertEquals(TypeEnum.BAD_BLOCK, advertisedError.getType()); - Assert.assertTrue(commonError.getMessage().contains("block time:")); - Assert.assertTrue(advertisedError.getMessage().contains("block time:")); - Mockito.verify(block, Mockito.never()).validateSignature(Mockito.any(), Mockito.any()); + delegate.validBlock(block); + Assert.assertTrue(delegate.validAdvBlock(new PeerConnection(), block)); + + Mockito.verify(block, Mockito.times(2)).validateMerkleRoot(); + Mockito.verify(block, Mockito.times(2)).validateSignature(Mockito.any(), Mockito.any()); } @Test diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java index 389a8d869af..78521764e9a 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java @@ -237,6 +237,32 @@ public void testInvalidWitnessIngress() throws Exception { } } + @Test + public void testFutureBlockRejectedBeforeAdvertisedOrSyncProcessing() throws Exception { + for (boolean syncResponse : new boolean[]{false, true}) { + BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, + System.currentTimeMillis() + 60000, ByteString.EMPTY); + BlockMessage message = new BlockMessage(block); + PeerConnection sender = witnessPeer(); + if (syncResponse) { + sender.getSyncBlockRequested().put(message.getBlockId(), 0L); + } else { + sender.getAdvInvRequest().put(new Item(message.getBlockId(), InventoryType.BLOCK), 0L); + } + TronNetDelegate delegate = Mockito.mock(TronNetDelegate.class); + AdvService adv = Mockito.mock(AdvService.class); + SyncService sync = Mockito.mock(SyncService.class); + BlockMsgHandler ingress = witnessHandler(delegate, adv, sync); + + P2pException error = Assert.assertThrows(P2pException.class, + () -> ingress.processMessage(sender, message)); + + Assert.assertEquals(P2pException.TypeEnum.BAD_MESSAGE, error.getType()); + Assert.assertEquals("block time error", error.getMessage()); + Mockito.verifyNoInteractions(delegate, adv, sync); + } + } + @Test public void testInvalidContractCountRejectedBeforeBroadcast() throws Exception { for (boolean fastForward : new boolean[]{false, true}) {