From fa175467095756508aa3b3112408312adbc80177 Mon Sep 17 00:00:00 2001 From: Asuka Date: Wed, 6 May 2026 16:37:43 +0800 Subject: [PATCH 1/9] feat(vm): canonicalize calldata length for validateMultiSign / batchValidateSign under Osaka Reject calldata that doesn't fit the (words - H) / I shape (H=5, I=5/6) inside execute(); rejected inputs return Pair.of(false, EMPTY_BYTE_ARRAY). getEnergyForData unchanged. --- .../tron/core/vm/PrecompiledContracts.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java b/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java index 634f7f2d3d1..a31945eacd1 100644 --- a/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java +++ b/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java @@ -414,6 +414,14 @@ private static byte[] extractBytes(byte[] data, int offset, int len) { return Arrays.copyOfRange(data, offset, offset + len); } + private static boolean isValidAbiEncoding(byte[] data, int headerWords, int itemWords) { + if (data == null || data.length % WORD_SIZE != 0) { + return false; + } + int tail = data.length - headerWords * WORD_SIZE; + return tail >= 0 && tail % (itemWords * WORD_SIZE) == 0; + } + public abstract static class PrecompiledContract { protected static final byte[] DATA_FALSE = new byte[WORD_SIZE]; @@ -938,6 +946,8 @@ public static class ValidateMultiSign extends PrecompiledContract { private static final int ENGERYPERSIGN = 1500; private static final int MAX_SIZE = 5; + private static final int ABI_HEADER_WORDS = 5; + private static final int ABI_ITEM_WORDS = 5; @Override @@ -949,6 +959,10 @@ public long getEnergyForData(byte[] data) { @Override public Pair execute(byte[] rawData) { + if (VMConfig.allowTvmOsaka() + && !isValidAbiEncoding(rawData, ABI_HEADER_WORDS, ABI_ITEM_WORDS)) { + return Pair.of(false, EMPTY_BYTE_ARRAY); + } DataWord[] words = DataWord.parseArray(rawData); byte[] address = words[0].toTronAddress(); int permissionId = words[1].intValueSafe(); @@ -1021,6 +1035,8 @@ public static class BatchValidateSign extends PrecompiledContract { private static final String workersName = "validate-sign-contract"; private static final int ENGERYPERSIGN = 1500; private static final int MAX_SIZE = 16; + private static final int ABI_HEADER_WORDS = 5; + private static final int ABI_ITEM_WORDS = 6; static { workers = ExecutorServiceManager.newFixedThreadPool(workersName, @@ -1048,6 +1064,10 @@ public Pair execute(byte[] data) { private Pair doExecute(byte[] data) throws InterruptedException, ExecutionException { + if (VMConfig.allowTvmOsaka() + && !isValidAbiEncoding(data, ABI_HEADER_WORDS, ABI_ITEM_WORDS)) { + return Pair.of(false, EMPTY_BYTE_ARRAY); + } DataWord[] words = DataWord.parseArray(data); byte[] hash = words[0].getData(); From a5f1718cba6cc466132655a0ab93da10ddc52e8e Mon Sep 17 00:00:00 2001 From: Asuka Date: Wed, 6 May 2026 16:37:43 +0800 Subject: [PATCH 2/9] test(vm): cover TIP-854 calldata canonicalization for sign-validation precompiles Add Osaka-gated rejection cases (mis-aligned, short head, bad tail, null) and a Program#callToPrecompiledAddress integration test pinning outer-frame containment. --- .../vm/BatchValidateSignContractTest.java | 82 ++++++++++++++ .../common/runtime/vm/OperationsTest.java | 43 ++++++++ .../vm/ValidateMultiSignContractTest.java | 104 ++++++++++++++++++ 3 files changed, 229 insertions(+) diff --git a/framework/src/test/java/org/tron/common/runtime/vm/BatchValidateSignContractTest.java b/framework/src/test/java/org/tron/common/runtime/vm/BatchValidateSignContractTest.java index c18eb396546..8849e114c94 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/BatchValidateSignContractTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/BatchValidateSignContractTest.java @@ -10,6 +10,7 @@ import org.junit.Test; import org.tron.common.crypto.ECKey; import org.tron.common.crypto.Hash; +import org.tron.common.utils.ByteUtil; import org.tron.common.utils.StringUtil; import org.tron.common.utils.client.utils.AbiUtil; import org.tron.core.db.TransactionTrace; @@ -130,6 +131,87 @@ public void correctionTest() { System.gc(); // force triggering full gc to avoid timeout for next test } + // TIP-854: after activation, batchValidateSign (H=5, I=6) must reject calldata + // whose byte length is incompatible with the (words - 5) / 6 shape the per-call + // energy formula already assumes, returning (false, empty). The guard lives in + // doExecute(); the outer try/catch does not mask it because the guard does not + // throw (pure arithmetic + a static getter). + @Test + public void testTip854RejectsMalformedCalldata() { + contract.setVmShouldEndInUs(System.nanoTime() / 1000 + 2_000_000); + VMConfig.initAllowTvmOsaka(1); + try { + // Bucket 1: 32-aligned head + sub-word trailing bytes (r=1, r=31). + for (int r : new int[]{1, 31}) { + byte[] data = new byte[(5 + 6) * 32 + r]; + Pair ret = contract.execute(data); + Assert.assertFalse("non-32-aligned len=" + data.length, ret.getLeft()); + Assert.assertSame(ByteUtil.EMPTY_BYTE_ARRAY, ret.getRight()); + } + // Bucket 2: fewer than the static head's 5 words. + for (int bytes : new int[]{0, 32, 64, 96, 128}) { + Pair ret = contract.execute(new byte[bytes]); + Assert.assertFalse("len=" + bytes + " < 5 words", ret.getLeft()); + Assert.assertSame(ByteUtil.EMPTY_BYTE_ARRAY, ret.getRight()); + } + // Bucket 3: 32-aligned but tail not a multiple of I=6 words (k = 1..5). + for (int k = 1; k <= 5; k++) { + byte[] data = new byte[(5 + k) * 32]; + Pair ret = contract.execute(data); + Assert.assertFalse("aligned bad-tail k=" + k, ret.getLeft()); + Assert.assertSame(ByteUtil.EMPTY_BYTE_ARRAY, ret.getRight()); + } + // Null calldata: explicit spec clause. + Pair ret = contract.execute(null); + Assert.assertFalse("null calldata", ret.getLeft()); + Assert.assertSame(ByteUtil.EMPTY_BYTE_ARRAY, ret.getRight()); + } finally { + VMConfig.initAllowTvmOsaka(0); + } + System.gc(); + } + + // TIP-854 Compatibility: for canonically-shaped calldata — all 65-byte real + // signatures so each bytes[i] encodes in exactly 4 words (1 length + 3 content) + // — total length equals 5*32 + 6*32*N, so pre- and post-activation must be + // observationally identical. + @Test + public void testTip854CanonicalInputUnchanged() { + contract.setConstantCall(true); + List signatures = new ArrayList<>(); + List addresses = new ArrayList<>(); + byte[] hash = Hash.sha3(longData); + for (int i = 0; i < 8; i++) { + ECKey key = new ECKey(); + signatures.add(Hex.toHexString(key.sign(hash).toByteArray())); + addresses.add(StringUtil.encode58Check(key.getAddress())); + } + + VMConfig.initAllowTvmOsaka(0); + Pair pre = validateMultiSign(hash, signatures, addresses); + VMConfig.initAllowTvmOsaka(1); + try { + Pair post = validateMultiSign(hash, signatures, addresses); + Assert.assertEquals(pre.getLeft(), post.getLeft()); + Assert.assertArrayEquals(pre.getValue(), post.getValue()); + } finally { + VMConfig.initAllowTvmOsaka(0); + } + System.gc(); + } + + // TIP-854: before activation the guard is not consulted. Malformed calldata + // that would raise inside doExecute gets collapsed to (true, 32-byte zero) by + // the outer catch — this is the legacy behaviour and must be preserved. + @Test + public void testTip854PreActivationNoOp() { + VMConfig.initAllowTvmOsaka(0); + contract.setVmShouldEndInUs(System.nanoTime() / 1000 + 2_000_000); + Pair ret = contract.execute(new byte[(5 + 1) * 32]); + Assert.assertTrue("pre-activation must not take the new reject path", ret.getLeft()); + Assert.assertEquals(32, ret.getRight().length); + } + Pair validateMultiSign(byte[] hash, List signatures, List addresses) { List parameters = Arrays.asList("0x" + Hex.toHexString(hash), signatures, addresses); diff --git a/framework/src/test/java/org/tron/common/runtime/vm/OperationsTest.java b/framework/src/test/java/org/tron/common/runtime/vm/OperationsTest.java index 583b0131942..8b405c41234 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/OperationsTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/OperationsTest.java @@ -786,6 +786,49 @@ Op.CALL, new DataWord(10000), VMConfig.initAllowTvmSelfdestructRestriction(0); } + // TIP-854 outer-frame containment: a CALL to validateMultiSign or + // batchValidateSign with malformed calldata must (a) push 0 onto the outer + // stack, (b) leave the outer frame free of any propagated exception, and + // (c) allow the outer frame to continue executing afterwards. + @Test + public void testTip854OuterFrameContainment() throws ContractValidateException { + byte prePrefixByte = DecodeUtil.addressPreFixByte; + DecodeUtil.addressPreFixByte = Constant.ADD_PRE_FIX_BYTE_MAINNET; + VMConfig.initAllowTvmOsaka(1); + try { + for (PrecompiledContracts.PrecompiledContract contract : + new PrecompiledContracts.PrecompiledContract[]{ + new PrecompiledContracts.ValidateMultiSign(), + new PrecompiledContracts.BatchValidateSign()}) { + invoke = new ProgramInvokeMockImpl(); + InternalTransaction interTrx = new InternalTransaction( + Protocol.Transaction.getDefaultInstance(), + InternalTransaction.TrxType.TRX_UNKNOWN_TYPE); + program = new Program(new byte[0], new byte[0], invoke, interTrx); + // inDataSize=0 ⇒ data=[] ⇒ fewer than H=5 head words ⇒ guard rejects. + MessageCall messageCall = new MessageCall( + Op.CALL, new DataWord(10000), + DataWord.ZERO(), DataWord.ZERO(), + DataWord.ZERO(), DataWord.ZERO(), + DataWord.ZERO(), DataWord.ZERO(), + DataWord.ZERO(), false); + program.callToPrecompiledAddress(messageCall, contract); + + Assert.assertNull(contract.getClass().getSimpleName() + + ": outer frame must not inherit an exception", + program.getResult().getException()); + Assert.assertEquals(contract.getClass().getSimpleName() + ": inner CALL pushes 0", + DataWord.ZERO(), program.getStack().pop()); + // Outer frame continues: another stack op works without throwing. + program.stackPush(new DataWord(1)); + Assert.assertEquals(new DataWord(1), program.getStack().pop()); + } + } finally { + VMConfig.initAllowTvmOsaka(0); + DecodeUtil.addressPreFixByte = prePrefixByte; + } + } + @Test public void testOtherOperations() throws ContractValidateException { invoke = new ProgramInvokeMockImpl(); diff --git a/framework/src/test/java/org/tron/common/runtime/vm/ValidateMultiSignContractTest.java b/framework/src/test/java/org/tron/common/runtime/vm/ValidateMultiSignContractTest.java index 518d42041ee..d7ccab73bd9 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/ValidateMultiSignContractTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/ValidateMultiSignContractTest.java @@ -155,6 +155,110 @@ public void testDifferentCase() { } + // TIP-854: after activation, validateMultiSign (H=5, I=5) must reject calldata + // whose byte length is incompatible with the (words - 5) / 5 shape the per-call + // energy formula already assumes, returning (false, empty). + @Test + public void testTip854RejectsMalformedCalldata() { + VMConfig.initAllowTvmOsaka(1); + try { + // Bucket 1: 32-aligned head + sub-word trailing bytes (r=1, r=31). + for (int r : new int[]{1, 31}) { + byte[] data = new byte[(5 + 5) * 32 + r]; + Pair ret = contract.execute(data); + Assert.assertFalse("non-32-aligned len=" + data.length, ret.getLeft()); + Assert.assertSame(ByteUtil.EMPTY_BYTE_ARRAY, ret.getRight()); + } + // Bucket 2: fewer than the static head's 5 words. + for (int bytes : new int[]{0, 32, 64, 96, 128}) { + Pair ret = contract.execute(new byte[bytes]); + Assert.assertFalse("len=" + bytes + " < 5 words", ret.getLeft()); + Assert.assertSame(ByteUtil.EMPTY_BYTE_ARRAY, ret.getRight()); + } + // Bucket 3: 32-aligned but tail not a multiple of I=5 words (k = 1..4). + for (int k = 1; k <= 4; k++) { + byte[] data = new byte[(5 + k) * 32]; + Pair ret = contract.execute(data); + Assert.assertFalse("aligned bad-tail k=" + k, ret.getLeft()); + Assert.assertSame(ByteUtil.EMPTY_BYTE_ARRAY, ret.getRight()); + } + // Null calldata: explicit spec clause. + Pair ret = contract.execute(null); + Assert.assertFalse("null calldata", ret.getLeft()); + Assert.assertSame(ByteUtil.EMPTY_BYTE_ARRAY, ret.getRight()); + } finally { + VMConfig.initAllowTvmOsaka(0); + } + } + + // TIP-854 Compatibility: for canonically-shaped calldata (real 65-byte sigs, + // total length == 5*32 + 5*32*N), behaviour must be identical pre- vs + // post-activation — the guard is a no-op for well-formed inputs. + @Test + public void testTip854CanonicalInputUnchanged() { + ECKey key = new ECKey(); + AccountCapsule toAccount = new AccountCapsule(ByteString.copyFrom(key.getAddress()), + Protocol.AccountType.Normal, + System.currentTimeMillis(), true, dbManager.getDynamicPropertiesStore()); + ECKey key1 = new ECKey(); + ECKey key2 = new ECKey(); + Protocol.Permission activePermission = + Protocol.Permission.newBuilder() + .setType(Protocol.Permission.PermissionType.Active) + .setId(2) + .setPermissionName("active") + .setThreshold(2) + .setOperations(ByteString.copyFrom(ByteArray + .fromHexString("0000000000000000000000000000000000000000000000000000000000000000"))) + .addKeys(Protocol.Key.newBuilder().setAddress(ByteString.copyFrom(key1.getAddress())) + .setWeight(1).build()) + .addKeys(Protocol.Key.newBuilder().setAddress(ByteString.copyFrom(key2.getAddress())) + .setWeight(1).build()) + .build(); + toAccount.updatePermissions(toAccount.getPermissionById(0), null, + Collections.singletonList(activePermission)); + dbManager.getAccountStore().put(key.getAddress(), toAccount); + + byte[] data = Sha256Hash.hash(CommonParameter.getInstance().isECKeyCryptoEngine(), longData); + byte[] merged = ByteUtil.merge(key.getAddress(), ByteArray.fromInt(2), data); + byte[] toSign = Sha256Hash.hash(CommonParameter.getInstance().isECKeyCryptoEngine(), merged); + List signs = new ArrayList<>(); + signs.add(Hex.toHexString(key1.sign(toSign).toByteArray())); + signs.add(Hex.toHexString(key2.sign(toSign).toByteArray())); + + VMConfig.initAllowTvmOsaka(0); + Pair pre = + validateMultiSign(StringUtil.encode58Check(key.getAddress()), 2, data, signs); + VMConfig.initAllowTvmOsaka(1); + try { + Pair post = + validateMultiSign(StringUtil.encode58Check(key.getAddress()), 2, data, signs); + Assert.assertEquals(pre.getLeft(), post.getLeft()); + Assert.assertArrayEquals(pre.getValue(), post.getValue()); + Assert.assertArrayEquals(DataWord.ONE().getData(), post.getValue()); + } finally { + VMConfig.initAllowTvmOsaka(0); + } + } + + // TIP-854: before activation, malformed calldata reaches the legacy decoder. + // Assert the guard is not taken — this precompile has no outer catch, so a + // too-short input raises inside the decoder; that is the documented + // pre-activation failure mode the TIP explicitly preserves. + @Test + public void testTip854PreActivationNoOp() { + VMConfig.initAllowTvmOsaka(0); + contract.setRepository(RepositoryImpl.createRoot(StoreFactory.getInstance())); + try { + Pair ret = contract.execute(new byte[(5 + 1) * 32]); + // If the decoder happened to handle it without raising, we must not have + // taken the post-activation reject path (false, empty). + Assert.assertNotSame(ByteUtil.EMPTY_BYTE_ARRAY, ret.getRight()); + } catch (RuntimeException expectedLegacyBehaviour) { + // Pre-activation: decoder may throw — this is the existing behaviour. + } + } + Pair validateMultiSign(String address, int permissionId, byte[] hash, List signatures) { List parameters = Arrays From 0f9fc764bfde3a1c4d8def6bbcb9a53d5a76a618 Mon Sep 17 00:00:00 2001 From: Asuka Date: Thu, 7 May 2026 19:13:47 +0800 Subject: [PATCH 3/9] fix(vm): reject header-only calldata in canonical ABI check --- .../main/java/org/tron/core/vm/PrecompiledContracts.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java b/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java index a31945eacd1..19cae17e00a 100644 --- a/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java +++ b/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java @@ -3,6 +3,8 @@ import static java.util.Arrays.copyOfRange; import static org.tron.common.math.Maths.max; import static org.tron.common.math.Maths.min; +import static org.tron.common.math.StrictMathWrapper.multiplyExact; +import static org.tron.common.math.StrictMathWrapper.subtractExact; import static org.tron.common.runtime.vm.DataWord.WORD_SIZE; import static org.tron.common.utils.BIUtil.addSafely; import static org.tron.common.utils.BIUtil.isLessThan; @@ -418,8 +420,8 @@ private static boolean isValidAbiEncoding(byte[] data, int headerWords, int item if (data == null || data.length % WORD_SIZE != 0) { return false; } - int tail = data.length - headerWords * WORD_SIZE; - return tail >= 0 && tail % (itemWords * WORD_SIZE) == 0; + long tail = subtractExact(data.length, multiplyExact(headerWords, WORD_SIZE)); + return tail > 0 && tail % multiplyExact(itemWords, WORD_SIZE) == 0; } public abstract static class PrecompiledContract { From 3c26b260fd37ae82de1c0568b4552b11ecc2b1bd Mon Sep 17 00:00:00 2001 From: Asuka Date: Fri, 8 May 2026 10:43:28 +0800 Subject: [PATCH 4/9] feat(vm): add node-level vm.constantCallTimeoutMs for constant calls Operators may set any positive integer to extend the per-call deadline for the constant-call APIs (triggerconstantcontract, triggersmartcontract dispatched to view/pure functions, estimateenergy, eth_call, eth_estimateGas, and others). The configured value is used verbatim. Replaces unsafe use of --debug, which also extends block-processing. --- .../org/tron/core/actuator/VMActuator.java | 11 +++++++-- .../common/parameter/CommonParameter.java | 12 ++++++++++ .../org/tron/core/config/args/VmConfig.java | 24 +++++++++++++++++-- .../java/org/tron/core/config/args/Args.java | 1 + framework/src/main/resources/config.conf | 12 ++++++++++ 5 files changed, 56 insertions(+), 4 deletions(-) diff --git a/actuator/src/main/java/org/tron/core/actuator/VMActuator.java b/actuator/src/main/java/org/tron/core/actuator/VMActuator.java index 9da41574ff0..ff6a90f16f6 100644 --- a/actuator/src/main/java/org/tron/core/actuator/VMActuator.java +++ b/actuator/src/main/java/org/tron/core/actuator/VMActuator.java @@ -401,6 +401,10 @@ private void create() long maxCpuTimeOfOneTx = rootRepository.getDynamicPropertiesStore() .getMaxCpuTimeOfOneTx() * VMConstant.ONE_THOUSAND; long thisTxCPULimitInUs = (long) (maxCpuTimeOfOneTx * getCpuLimitInUsRatio()); + long constantCallTimeoutMs = CommonParameter.getInstance().getConstantCallTimeoutMs(); + if (isConstantCall && constantCallTimeoutMs > 0L) { + thisTxCPULimitInUs = constantCallTimeoutMs * VMConstant.ONE_THOUSAND; + } long vmStartInUs = System.nanoTime() / VMConstant.ONE_THOUSAND; long vmShouldEndInUs = vmStartInUs + thisTxCPULimitInUs; ProgramInvoke programInvoke = ProgramInvokeFactory @@ -514,8 +518,11 @@ private void call() long maxCpuTimeOfOneTx = rootRepository.getDynamicPropertiesStore() .getMaxCpuTimeOfOneTx() * VMConstant.ONE_THOUSAND; - long thisTxCPULimitInUs = - (long) (maxCpuTimeOfOneTx * getCpuLimitInUsRatio()); + long thisTxCPULimitInUs = (long) (maxCpuTimeOfOneTx * getCpuLimitInUsRatio()); + long constantCallTimeoutMs = CommonParameter.getInstance().getConstantCallTimeoutMs(); + if (isConstantCall && constantCallTimeoutMs > 0L) { + thisTxCPULimitInUs = constantCallTimeoutMs * VMConstant.ONE_THOUSAND; + } long vmStartInUs = System.nanoTime() / VMConstant.ONE_THOUSAND; long vmShouldEndInUs = vmStartInUs + thisTxCPULimitInUs; ProgramInvoke programInvoke = ProgramInvokeFactory diff --git a/common/src/main/java/org/tron/common/parameter/CommonParameter.java b/common/src/main/java/org/tron/common/parameter/CommonParameter.java index a73158a718a..30e26d89b26 100644 --- a/common/src/main/java/org/tron/common/parameter/CommonParameter.java +++ b/common/src/main/java/org/tron/common/parameter/CommonParameter.java @@ -58,6 +58,18 @@ public class CommonParameter { @Getter @Setter public double maxTimeRatio = calcMaxTimeRatio(); + /** + * Max TVM execution time (ms) for constant calls — covers + * triggerconstantcontract, triggersmartcontract dispatched to view/pure + * functions, estimateenergy, eth_call, eth_estimateGas, and any other + * RPC routed through Wallet#callConstantContract. 0 = use the same + * deadline as block processing (current behaviour). When operators set + * this in config the value must be positive; validated at config-load + * in VmConfig. + */ + @Getter + @Setter + public long constantCallTimeoutMs = 0L; @Getter @Setter public boolean saveInternalTx; diff --git a/common/src/main/java/org/tron/core/config/args/VmConfig.java b/common/src/main/java/org/tron/core/config/args/VmConfig.java index d583cf4c601..712e3375e1c 100644 --- a/common/src/main/java/org/tron/core/config/args/VmConfig.java +++ b/common/src/main/java/org/tron/core/config/args/VmConfig.java @@ -2,6 +2,7 @@ import com.typesafe.config.Config; import com.typesafe.config.ConfigBeanFactory; +import lombok.AccessLevel; import lombok.Getter; import lombok.Setter; import lombok.extern.slf4j.Slf4j; @@ -15,6 +16,8 @@ @Setter public class VmConfig { + private static final String CONSTANT_CALL_TIMEOUT_MS_KEY = "constantCallTimeoutMs"; + private boolean supportConstant = false; private long maxEnergyLimitForConstant = 100_000_000L; private int lruCacheSize = 500; @@ -27,6 +30,11 @@ public class VmConfig { private boolean saveInternalTx = false; private boolean saveFeaturedInternalTx = false; private boolean saveCancelAllUnfreezeV2Details = false; + // Excluded from ConfigBeanFactory binding (no setter): the property is + // intentionally absent from reference.conf so {@code Config#hasPath} alone + // signals operator opt-in. Bound manually in {@link #fromConfig}. + @Setter(AccessLevel.NONE) + private long constantCallTimeoutMs = 0L; /** * Create VmConfig from the "vm" section of the application config. @@ -36,11 +44,11 @@ public class VmConfig { public static VmConfig fromConfig(Config config) { Config vmSection = config.getConfig("vm"); VmConfig vmConfig = ConfigBeanFactory.create(vmSection, VmConfig.class); - vmConfig.postProcess(); + vmConfig.postProcess(vmSection); return vmConfig; } - private void postProcess() { + private void postProcess(Config vmSection) { // clamp maxEnergyLimitForConstant if (maxEnergyLimitForConstant < 3_000_000L) { maxEnergyLimitForConstant = 3_000_000L; @@ -60,5 +68,17 @@ private void postProcess() { logger.warn("Configuring [vm.saveCancelAllUnfreezeV2Details] won't work as " + "vm.saveInternalTx or vm.saveFeaturedInternalTx is off."); } + + // constantCallTimeoutMs is excluded from ConfigBeanFactory binding (no + // setter) and intentionally absent from reference.conf, so hasPath alone + // tells us whether the operator opted in. Only positive values are valid. + if (vmSection.hasPath(CONSTANT_CALL_TIMEOUT_MS_KEY)) { + long value = vmSection.getLong(CONSTANT_CALL_TIMEOUT_MS_KEY); + if (value <= 0L) { + throw new IllegalArgumentException( + "vm.constantCallTimeoutMs must be > 0 when configured, got " + value); + } + constantCallTimeoutMs = value; + } } } diff --git a/framework/src/main/java/org/tron/core/config/args/Args.java b/framework/src/main/java/org/tron/core/config/args/Args.java index f91c6a437ac..fb9a4dce835 100644 --- a/framework/src/main/java/org/tron/core/config/args/Args.java +++ b/framework/src/main/java/org/tron/core/config/args/Args.java @@ -212,6 +212,7 @@ private static void applyVmConfig(VmConfig vm) { PARAMETER.saveInternalTx = vm.isSaveInternalTx(); PARAMETER.saveFeaturedInternalTx = vm.isSaveFeaturedInternalTx(); PARAMETER.saveCancelAllUnfreezeV2Details = vm.isSaveCancelAllUnfreezeV2Details(); + PARAMETER.constantCallTimeoutMs = vm.getConstantCallTimeoutMs(); } // Old applyStorageConfig removed — merged into applyStorageConfig() diff --git a/framework/src/main/resources/config.conf b/framework/src/main/resources/config.conf index 369924074bc..2a0c21809af 100644 --- a/framework/src/main/resources/config.conf +++ b/framework/src/main/resources/config.conf @@ -710,6 +710,18 @@ vm = { # Indicates the max retry time for executing transaction in estimating energy. Default 3. # estimateEnergyMaxRetry = 3 + + # Max TVM execution time (ms) for constant calls — applies to + # triggerconstantcontract, triggersmartcontract dispatched to view/pure + # functions, estimateenergy, eth_call, eth_estimateGas, and any other RPC + # routed through the constant-call path. When set, must be a positive + # integer and is used verbatim as the per-call deadline (no clamp against + # the network's maxCpuTimeOfOneTx). Omit the property entirely to keep the + # default behaviour of sharing the block-processing deadline. Migration + # note: if previously running --debug to extend constant calls, switch to + # this option (--debug also extends block-processing, which is unsafe; see + # issue #6266). + # constantCallTimeoutMs = 100 } # These parameters are designed for private chain testing only and cannot be freely switched on or off in production systems. From 8b7901fdc1d3265cc13c5e7cbec49fc2510c71b3 Mon Sep 17 00:00:00 2001 From: Asuka Date: Fri, 8 May 2026 10:43:32 +0800 Subject: [PATCH 5/9] test(vm): cover constantCallTimeoutMs explicit-configure validation --- .../tron/core/config/args/VmConfigTest.java | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/common/src/test/java/org/tron/core/config/args/VmConfigTest.java b/common/src/test/java/org/tron/core/config/args/VmConfigTest.java index b134fe00c2b..f63b43a4bfc 100644 --- a/common/src/test/java/org/tron/core/config/args/VmConfigTest.java +++ b/common/src/test/java/org/tron/core/config/args/VmConfigTest.java @@ -88,4 +88,55 @@ public void testEstimateEnergyMaxRetryBoundaryValues() { assertEquals(3, VmConfig.fromConfig( withRef("vm { estimateEnergyMaxRetry = 3 }")).getEstimateEnergyMaxRetry()); } + + // =========================================================================== + // Constant-call timeout (issue #6681). The validation rule: any positive + // value is accepted, but zero/negative is rejected ONLY when the operator + // explicitly set the property in their config. Absence keeps the in-Java + // default (0L = "share the block-processing deadline"). + // =========================================================================== + + @Test + public void testConstantCallTimeoutDefaultWhenAbsent() { + // No path in the config, no entry in reference.conf -> default 0L kept, + // no validation triggered. + VmConfig vm = VmConfig.fromConfig(withRef()); + assertEquals(0L, vm.getConstantCallTimeoutMs()); + } + + @Test + public void testConstantCallTimeoutAcceptsAnyPositiveValue() { + assertEquals(1L, VmConfig.fromConfig( + withRef("vm { constantCallTimeoutMs = 1 }")).getConstantCallTimeoutMs()); + assertEquals(50L, VmConfig.fromConfig( + withRef("vm { constantCallTimeoutMs = 50 }")).getConstantCallTimeoutMs()); + assertEquals(500L, VmConfig.fromConfig( + withRef("vm { constantCallTimeoutMs = 500 }")).getConstantCallTimeoutMs()); + assertEquals(5_000L, VmConfig.fromConfig( + withRef("vm { constantCallTimeoutMs = 5000 }")).getConstantCallTimeoutMs()); + } + + @Test + public void testConstantCallTimeoutZeroRejectedWhenExplicitlyConfigured() { + // Operator wrote `= 0` in config -> treated as a misconfiguration even + // though it equals the in-Java default. Forces an explicit positive value. + try { + VmConfig.fromConfig(withRef("vm { constantCallTimeoutMs = 0 }")); + org.junit.Assert.fail("expected IllegalArgumentException for explicit 0"); + } catch (IllegalArgumentException ex) { + org.junit.Assert.assertTrue(ex.getMessage(), + ex.getMessage().contains("constantCallTimeoutMs")); + } + } + + @Test + public void testConstantCallTimeoutNegativeRejected() { + try { + VmConfig.fromConfig(withRef("vm { constantCallTimeoutMs = -1 }")); + org.junit.Assert.fail("expected IllegalArgumentException for negative ms"); + } catch (IllegalArgumentException ex) { + org.junit.Assert.assertTrue(ex.getMessage(), + ex.getMessage().contains("constantCallTimeoutMs")); + } + } } From 057bf4e21705ab96df80b3084d4bb9d38f29daa6 Mon Sep 17 00:00:00 2001 From: Asuka Date: Sat, 9 May 2026 10:45:03 +0800 Subject: [PATCH 6/9] fix(vm): harden constant call timeout deadline --- .../org/tron/core/actuator/VMActuator.java | 28 +++++++++---------- .../tron/core/actuator/VMActuatorTest.java | 23 +++++++++++++++ .../common/parameter/CommonParameter.java | 4 +-- .../org/tron/core/config/args/VmConfig.java | 12 ++++++-- .../tron/core/config/args/VmConfigTest.java | 19 +++++++++++-- framework/src/main/resources/config.conf | 12 ++++---- 6 files changed, 71 insertions(+), 27 deletions(-) create mode 100644 actuator/src/test/java/org/tron/core/actuator/VMActuatorTest.java diff --git a/actuator/src/main/java/org/tron/core/actuator/VMActuator.java b/actuator/src/main/java/org/tron/core/actuator/VMActuator.java index ff6a90f16f6..1b0e8a6637f 100644 --- a/actuator/src/main/java/org/tron/core/actuator/VMActuator.java +++ b/actuator/src/main/java/org/tron/core/actuator/VMActuator.java @@ -398,13 +398,9 @@ private void create() byte[] ops = newSmartContract.getBytecode().toByteArray(); rootInternalTx = new InternalTransaction(trx, trxType); - long maxCpuTimeOfOneTx = rootRepository.getDynamicPropertiesStore() - .getMaxCpuTimeOfOneTx() * VMConstant.ONE_THOUSAND; - long thisTxCPULimitInUs = (long) (maxCpuTimeOfOneTx * getCpuLimitInUsRatio()); - long constantCallTimeoutMs = CommonParameter.getInstance().getConstantCallTimeoutMs(); - if (isConstantCall && constantCallTimeoutMs > 0L) { - thisTxCPULimitInUs = constantCallTimeoutMs * VMConstant.ONE_THOUSAND; - } + long thisTxCPULimitInUs = calculateCpuLimitInUs(isConstantCall, + rootRepository.getDynamicPropertiesStore().getMaxCpuTimeOfOneTx(), + getCpuLimitInUsRatio(), CommonParameter.getInstance().getConstantCallTimeoutMs()); long vmStartInUs = System.nanoTime() / VMConstant.ONE_THOUSAND; long vmShouldEndInUs = vmStartInUs + thisTxCPULimitInUs; ProgramInvoke programInvoke = ProgramInvokeFactory @@ -516,13 +512,9 @@ private void call() energyLimit = getTotalEnergyLimit(creator, caller, contract, feeLimit, callValue); } - long maxCpuTimeOfOneTx = rootRepository.getDynamicPropertiesStore() - .getMaxCpuTimeOfOneTx() * VMConstant.ONE_THOUSAND; - long thisTxCPULimitInUs = (long) (maxCpuTimeOfOneTx * getCpuLimitInUsRatio()); - long constantCallTimeoutMs = CommonParameter.getInstance().getConstantCallTimeoutMs(); - if (isConstantCall && constantCallTimeoutMs > 0L) { - thisTxCPULimitInUs = constantCallTimeoutMs * VMConstant.ONE_THOUSAND; - } + long thisTxCPULimitInUs = calculateCpuLimitInUs(isConstantCall, + rootRepository.getDynamicPropertiesStore().getMaxCpuTimeOfOneTx(), + getCpuLimitInUsRatio(), CommonParameter.getInstance().getConstantCallTimeoutMs()); long vmStartInUs = System.nanoTime() / VMConstant.ONE_THOUSAND; long vmShouldEndInUs = vmStartInUs + thisTxCPULimitInUs; ProgramInvoke programInvoke = ProgramInvokeFactory @@ -699,6 +691,14 @@ private double getCpuLimitInUsRatio() { return cpuLimitRatio; } + static long calculateCpuLimitInUs(boolean isConstantCall, long maxCpuTimeOfOneTxMs, + double cpuLimitInUsRatio, long constantCallTimeoutMs) { + if (isConstantCall && constantCallTimeoutMs > 0L) { + return constantCallTimeoutMs * VMConstant.ONE_THOUSAND; + } + return (long) (maxCpuTimeOfOneTxMs * VMConstant.ONE_THOUSAND * cpuLimitInUsRatio); + } + public long getTotalEnergyLimitWithFixRatio(AccountCapsule creator, AccountCapsule caller, TriggerSmartContract contract, long feeLimit, long callValue) throws ContractValidateException { diff --git a/actuator/src/test/java/org/tron/core/actuator/VMActuatorTest.java b/actuator/src/test/java/org/tron/core/actuator/VMActuatorTest.java new file mode 100644 index 00000000000..240c606e2e9 --- /dev/null +++ b/actuator/src/test/java/org/tron/core/actuator/VMActuatorTest.java @@ -0,0 +1,23 @@ +package org.tron.core.actuator; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +public class VMActuatorTest { + + @Test + public void testConstantCallUsesConfiguredTimeoutVerbatim() { + assertEquals(123_000L, VMActuator.calculateCpuLimitInUs(true, 80L, 5.0, 123L)); + } + + @Test + public void testConstantCallWithoutConfiguredTimeoutUsesNetworkDeadline() { + assertEquals(400_000L, VMActuator.calculateCpuLimitInUs(true, 80L, 5.0, 0L)); + } + + @Test + public void testNonConstantCallIgnoresConfiguredTimeout() { + assertEquals(400_000L, VMActuator.calculateCpuLimitInUs(false, 80L, 5.0, 123L)); + } +} diff --git a/common/src/main/java/org/tron/common/parameter/CommonParameter.java b/common/src/main/java/org/tron/common/parameter/CommonParameter.java index 30e26d89b26..ab48e0c9442 100644 --- a/common/src/main/java/org/tron/common/parameter/CommonParameter.java +++ b/common/src/main/java/org/tron/common/parameter/CommonParameter.java @@ -64,8 +64,8 @@ public class CommonParameter { * functions, estimateenergy, eth_call, eth_estimateGas, and any other * RPC routed through Wallet#callConstantContract. 0 = use the same * deadline as block processing (current behaviour). When operators set - * this in config the value must be positive; validated at config-load - * in VmConfig. + * this in config the value must be positive and fit VM deadline conversion; + * validated at config-load in VmConfig. */ @Getter @Setter diff --git a/common/src/main/java/org/tron/core/config/args/VmConfig.java b/common/src/main/java/org/tron/core/config/args/VmConfig.java index 712e3375e1c..00ba85aa6cc 100644 --- a/common/src/main/java/org/tron/core/config/args/VmConfig.java +++ b/common/src/main/java/org/tron/core/config/args/VmConfig.java @@ -9,7 +9,8 @@ /** * VM configuration bean. Field names match config.conf keys under the "vm" section. - * Bound automatically via ConfigBeanFactory — no manual key constants needed. + * Most fields are bound automatically via ConfigBeanFactory; opt-in fields that + * must stay absent from reference.conf are bound manually after hasPath checks. */ @Slf4j @Getter @@ -17,6 +18,7 @@ public class VmConfig { private static final String CONSTANT_CALL_TIMEOUT_MS_KEY = "constantCallTimeoutMs"; + static final long MAX_CONSTANT_CALL_TIMEOUT_MS = Long.MAX_VALUE / 1_000L; private boolean supportConstant = false; private long maxEnergyLimitForConstant = 100_000_000L; @@ -71,13 +73,19 @@ private void postProcess(Config vmSection) { // constantCallTimeoutMs is excluded from ConfigBeanFactory binding (no // setter) and intentionally absent from reference.conf, so hasPath alone - // tells us whether the operator opted in. Only positive values are valid. + // tells us whether the operator opted in. Only positive values that can be + // safely converted to microseconds are valid. if (vmSection.hasPath(CONSTANT_CALL_TIMEOUT_MS_KEY)) { long value = vmSection.getLong(CONSTANT_CALL_TIMEOUT_MS_KEY); if (value <= 0L) { throw new IllegalArgumentException( "vm.constantCallTimeoutMs must be > 0 when configured, got " + value); } + if (value > MAX_CONSTANT_CALL_TIMEOUT_MS) { + throw new IllegalArgumentException( + "vm.constantCallTimeoutMs must be <= " + MAX_CONSTANT_CALL_TIMEOUT_MS + + " to fit VM deadline conversion, got " + value); + } constantCallTimeoutMs = value; } } diff --git a/common/src/test/java/org/tron/core/config/args/VmConfigTest.java b/common/src/test/java/org/tron/core/config/args/VmConfigTest.java index f63b43a4bfc..e406ef24e7b 100644 --- a/common/src/test/java/org/tron/core/config/args/VmConfigTest.java +++ b/common/src/test/java/org/tron/core/config/args/VmConfigTest.java @@ -91,9 +91,10 @@ public void testEstimateEnergyMaxRetryBoundaryValues() { // =========================================================================== // Constant-call timeout (issue #6681). The validation rule: any positive - // value is accepted, but zero/negative is rejected ONLY when the operator - // explicitly set the property in their config. Absence keeps the in-Java - // default (0L = "share the block-processing deadline"). + // value that fits VM deadline conversion is accepted, but zero/negative is + // rejected ONLY when the operator explicitly set the property in their + // config. Absence keeps the in-Java default (0L = "share the + // block-processing deadline"). // =========================================================================== @Test @@ -139,4 +140,16 @@ public void testConstantCallTimeoutNegativeRejected() { ex.getMessage().contains("constantCallTimeoutMs")); } } + + @Test + public void testConstantCallTimeoutOverflowRejected() { + long value = VmConfig.MAX_CONSTANT_CALL_TIMEOUT_MS + 1L; + try { + VmConfig.fromConfig(withRef("vm { constantCallTimeoutMs = " + value + " }")); + org.junit.Assert.fail("expected IllegalArgumentException for overflowing ms"); + } catch (IllegalArgumentException ex) { + org.junit.Assert.assertTrue(ex.getMessage(), + ex.getMessage().contains("deadline conversion")); + } + } } diff --git a/framework/src/main/resources/config.conf b/framework/src/main/resources/config.conf index 2a0c21809af..9c8ab69a9e5 100644 --- a/framework/src/main/resources/config.conf +++ b/framework/src/main/resources/config.conf @@ -715,12 +715,12 @@ vm = { # triggerconstantcontract, triggersmartcontract dispatched to view/pure # functions, estimateenergy, eth_call, eth_estimateGas, and any other RPC # routed through the constant-call path. When set, must be a positive - # integer and is used verbatim as the per-call deadline (no clamp against - # the network's maxCpuTimeOfOneTx). Omit the property entirely to keep the - # default behaviour of sharing the block-processing deadline. Migration - # note: if previously running --debug to extend constant calls, switch to - # this option (--debug also extends block-processing, which is unsafe; see - # issue #6266). + # integer that fits VM deadline conversion and is used verbatim as the + # per-call deadline (no clamp against the network's maxCpuTimeOfOneTx). + # Omit the property entirely to keep the default behaviour of sharing the + # block-processing deadline. Migration note: if previously running --debug + # to extend constant calls, switch to this option (--debug also extends + # block-processing, which is unsafe; see issue #6266). # constantCallTimeoutMs = 100 } From b1522fa3734efb996a8bb781e237cb4fea052238 Mon Sep 17 00:00:00 2001 From: xxo1_shine Date: Sat, 9 May 2026 10:50:52 +0800 Subject: [PATCH 7/9] feat(net): optimize transaction rate limiting with accurate cache size check (#6714) 1. Add Manager.getCachedTransactionSize() = pushTransactionQueue + pendingTransactions + rePushTransactions to expose the true cached transaction count across all three queues. 2. Fix isTooManyPending() to include pushTransactionQueue, which was previously omitted, causing the pending threshold to be underestimated. 3. Update TransactionsMsgHandler.isBusy() to factor in the Manager cache size via TronNetDelegate.getCachedTransactionSize(), so the node stops accepting TRX INV messages when the full pipeline is busy. 4. Make the busy threshold configurable via node.maxTrxCacheSize (default: 50000), replacing the hardcoded MAX_TRX_SIZE constant. --- .../common/parameter/CommonParameter.java | 3 ++ .../org/tron/core/config/args/NodeConfig.java | 6 +++ common/src/main/resources/reference.conf | 2 + .../java/org/tron/core/config/args/Args.java | 1 + .../main/java/org/tron/core/db/Manager.java | 8 ++- .../org/tron/core/net/TronNetDelegate.java | 4 ++ .../TransactionsMsgHandler.java | 7 +-- framework/src/main/resources/config.conf | 4 ++ .../java/org/tron/core/db/ManagerTest.java | 51 +++++++++++++++++++ .../TransactionsMsgHandlerTest.java | 26 +++++++++- 10 files changed, 105 insertions(+), 7 deletions(-) diff --git a/common/src/main/java/org/tron/common/parameter/CommonParameter.java b/common/src/main/java/org/tron/common/parameter/CommonParameter.java index 0028a5d50d0..1d59e3a9d10 100644 --- a/common/src/main/java/org/tron/common/parameter/CommonParameter.java +++ b/common/src/main/java/org/tron/common/parameter/CommonParameter.java @@ -492,6 +492,9 @@ public class CommonParameter { public long pendingTransactionTimeout; @Getter @Setter + public int maxTrxCacheSize; + @Getter + @Setter public boolean nodeMetricsEnable = false; @Getter @Setter diff --git a/common/src/main/java/org/tron/core/config/args/NodeConfig.java b/common/src/main/java/org/tron/core/config/args/NodeConfig.java index 620152a907a..653fac2eb2c 100644 --- a/common/src/main/java/org/tron/core/config/args/NodeConfig.java +++ b/common/src/main/java/org/tron/core/config/args/NodeConfig.java @@ -85,6 +85,7 @@ public class NodeConfig { private ChannelConfig channel = new ChannelConfig(); private int maxTransactionPendingSize = 2000; private long pendingTransactionTimeout = 60000; + private int maxTrxCacheSize = 50_000; private int agreeNodeCount = 0; private boolean openHistoryQueryWhenLiteFN = false; private boolean unsolidifiedBlockCheck = false; @@ -498,6 +499,11 @@ private void postProcess() { if (dynamicConfig.checkInterval <= 0) { dynamicConfig.checkInterval = 600; } + + // maxTrxCacheSize: minimum 2000 + if (maxTrxCacheSize < 2000) { + maxTrxCacheSize = 2000; + } } // =========================================================================== diff --git a/common/src/main/resources/reference.conf b/common/src/main/resources/reference.conf index 63e5d86a4af..67343b9b75a 100644 --- a/common/src/main/resources/reference.conf +++ b/common/src/main/resources/reference.conf @@ -346,6 +346,8 @@ node { receiveTcpMinDataLength = 2048 maxTransactionPendingSize = 2000 pendingTransactionTimeout = 60000 + # total cached trx across handler queues + pending + rePush + maxTrxCacheSize = 50000 # Consensus agreement agreeNodeCount = 0 diff --git a/framework/src/main/java/org/tron/core/config/args/Args.java b/framework/src/main/java/org/tron/core/config/args/Args.java index 652f37a90db..350f4455b4a 100644 --- a/framework/src/main/java/org/tron/core/config/args/Args.java +++ b/framework/src/main/java/org/tron/core/config/args/Args.java @@ -625,6 +625,7 @@ private static void applyNodeConfig(NodeConfig nc) { PARAMETER.maxTransactionPendingSize = nc.getMaxTransactionPendingSize(); PARAMETER.pendingTransactionTimeout = nc.getPendingTransactionTimeout(); + PARAMETER.maxTrxCacheSize = nc.getMaxTrxCacheSize(); PARAMETER.validContractProtoThreadNum = nc.getValidContractProtoThreads(); 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 2c188c90b30..d6ce53299f1 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -2085,9 +2085,13 @@ public NullifierStore getNullifierStore() { return chainBaseManager.getNullifierStore(); } + public int getCachedTransactionSize() { + return pushTransactionQueue.size() + getPendingTransactions().size() + + getRePushTransactions().size(); + } + public boolean isTooManyPending() { - return getPendingTransactions().size() + getRePushTransactions().size() - > maxTransactionPendingSize; + return getCachedTransactionSize() > maxTransactionPendingSize; } private void preValidateTransactionSign(List txs) 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 100bad179bf..9a53172a806 100644 --- a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java +++ b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java @@ -384,4 +384,8 @@ public boolean isBlockUnsolidified() { return headNum - solidNum >= maxUnsolidifiedBlocks; } + public int getCachedTransactionSize() { + return dbManager.getCachedTransactionSize(); + } + } diff --git a/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java b/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java index d6bd439d7ff..02a1983c260 100644 --- a/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java +++ b/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java @@ -32,7 +32,6 @@ @Component public class TransactionsMsgHandler implements TronMsgHandler { - private static int MAX_TRX_SIZE = 50_000; private static int MAX_SMART_CONTRACT_SUBMIT_SIZE = 100; @Autowired private TronNetDelegate tronNetDelegate; @@ -41,7 +40,8 @@ public class TransactionsMsgHandler implements TronMsgHandler { @Autowired private ChainBaseManager chainBaseManager; - private BlockingQueue smartContractQueue = new LinkedBlockingQueue(MAX_TRX_SIZE); + private BlockingQueue smartContractQueue = new LinkedBlockingQueue( + Args.getInstance().getMaxTrxCacheSize()); private BlockingQueue queue = new LinkedBlockingQueue(); @@ -71,7 +71,8 @@ public void close() { } public boolean isBusy() { - return queue.size() + smartContractQueue.size() > MAX_TRX_SIZE; + return queue.size() + smartContractQueue.size() + + tronNetDelegate.getCachedTransactionSize() > Args.getInstance().getMaxTrxCacheSize(); } @Override diff --git a/framework/src/main/resources/config.conf b/framework/src/main/resources/config.conf index 6c8f2082301..07f5974a27d 100644 --- a/framework/src/main/resources/config.conf +++ b/framework/src/main/resources/config.conf @@ -160,6 +160,10 @@ node { # Range: [50, 2000], default: 500 # maxPendingBlockSize = 500 + # Maximum total number of cached transactions (handler queues + pending + rePush). + # When exceeded, the node stops accepting TRX INV messages from peers. + # maxTrxCacheSize = 50000 + # Number of validate sign thread, default availableProcessors # validateSignThreadNum = 16 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 a0522417c59..211bddd1c07 100755 --- a/framework/src/test/java/org/tron/core/db/ManagerTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerTest.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -1419,6 +1420,56 @@ public void testClearSolidityContractTriggerCache() throws Exception { } } + @Test + public void testGetCachedTransactionSize() throws Exception { + BlockingQueue pushQ = new LinkedBlockingQueue<>(); + pushQ.add(new TransactionCapsule(Protocol.Transaction.getDefaultInstance())); + Field pushField = Manager.class.getDeclaredField("pushTransactionQueue"); + pushField.setAccessible(true); + pushField.set(dbManager, pushQ); + + dbManager.getPendingTransactions().clear(); + dbManager.getPendingTransactions().add( + new TransactionCapsule(Protocol.Transaction.getDefaultInstance())); + dbManager.getPendingTransactions().add( + new TransactionCapsule(Protocol.Transaction.getDefaultInstance())); + + dbManager.getRePushTransactions().clear(); + + // 1 (push) + 2 (pending) + 0 (rePush) = 3 + Assert.assertEquals(3, dbManager.getCachedTransactionSize()); + + // cleanup + pushQ.clear(); + dbManager.getPendingTransactions().clear(); + } + + @Test + public void testIsTooManyPendingIncludesPushQueue() throws Exception { + int threshold = Args.getInstance().getMaxTransactionPendingSize(); + + BlockingQueue pushQ = new LinkedBlockingQueue<>(); + Field pushField = Manager.class.getDeclaredField("pushTransactionQueue"); + pushField.setAccessible(true); + pushField.set(dbManager, pushQ); + + dbManager.getPendingTransactions().clear(); + dbManager.getRePushTransactions().clear(); + + for (int i = 0; i < threshold; i++) { + dbManager.getPendingTransactions().add( + new TransactionCapsule(Protocol.Transaction.getDefaultInstance())); + } + Assert.assertFalse(dbManager.isTooManyPending()); + + pushQ.add(new TransactionCapsule(Protocol.Transaction.getDefaultInstance())); + Assert.assertTrue(dbManager.isTooManyPending()); + + // cleanup + dbManager.getPendingTransactions().clear(); + pushQ.clear(); + } + public void adjustBalance(AccountStore accountStore, byte[] accountAddress, long amount) throws BalanceInsufficientException { Commons.adjustBalance(accountStore, accountAddress, amount, 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 c14f9a9c86a..beeeecbca98 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 @@ -48,8 +48,6 @@ public static void init() { public void testProcessMessage() { TransactionsMsgHandler transactionsMsgHandler = new TransactionsMsgHandler(); try { - Assert.assertFalse(transactionsMsgHandler.isBusy()); - transactionsMsgHandler.init(); PeerConnection peer = Mockito.mock(PeerConnection.class); @@ -60,6 +58,8 @@ public void testProcessMessage() { field.setAccessible(true); field.set(transactionsMsgHandler, tronNetDelegate); + Assert.assertFalse(transactionsMsgHandler.isBusy()); + BalanceContract.TransferContract transferContract = BalanceContract.TransferContract .newBuilder() .setAmount(10) @@ -290,6 +290,28 @@ public void testHandleTransaction() throws Exception { } } + @Test + public void testIsBusyWithCachedTransactions() throws Exception { + TransactionsMsgHandler handler = new TransactionsMsgHandler(); + + int threshold = Args.getInstance().getMaxTrxCacheSize(); + TronNetDelegate tronNetDelegateMock = Mockito.mock(TronNetDelegate.class); + Field field = TransactionsMsgHandler.class.getDeclaredField("tronNetDelegate"); + field.setAccessible(true); + field.set(handler, tronNetDelegateMock); + + // queue and smartContractQueue are empty, but cached size > threshold + Mockito.when(tronNetDelegateMock.getCachedTransactionSize()).thenReturn(threshold + 1); + Assert.assertTrue(handler.isBusy()); + + // boundary: cached size == threshold, isBusy() uses strict >, so not busy + Mockito.when(tronNetDelegateMock.getCachedTransactionSize()).thenReturn(threshold); + Assert.assertFalse(handler.isBusy()); + + Mockito.when(tronNetDelegateMock.getCachedTransactionSize()).thenReturn(0); + Assert.assertFalse(handler.isBusy()); + } + class TrxEvent { @Getter From af8a84da5a57a06d60f716d35ba519af5d6e4962 Mon Sep 17 00:00:00 2001 From: xxo1_shine Date: Sat, 9 May 2026 12:10:04 +0800 Subject: [PATCH 8/9] feat(net): add P2P message deduplication and length validation (#6712) - FetchInvDataMsgHandler: reject messages with duplicate hashes - TransactionsMsgHandler: reject messages with duplicate transactions - SyncBlockChainMsgHandler: reject blockIds list exceeding 30 entries - Add MAX_SYNC_CHAIN_IDS = 30 constant to NetConstants - Add unit tests covering duplicate rejection and boundary values All violations throw P2pException(BAD_MESSAGE), triggering peer disconnect via existing P2pEventHandlerImpl error path. --- .../java/org/tron/core/config/Parameter.java | 1 + .../FetchInvDataMsgHandler.java | 7 +++ .../messagehandler/InventoryMsgHandler.java | 17 ++++-- .../SyncBlockChainMsgHandler.java | 6 +++ .../TransactionsMsgHandler.java | 15 +++++- .../FetchInvDataMsgHandlerTest.java | 35 ++++++++++++- .../InventoryMsgHandlerTest.java | 22 ++++++++ .../SyncBlockChainMsgHandlerTest.java | 52 +++++++++++++++++++ .../TransactionsMsgHandlerTest.java | 51 +++++++++++++++++- 9 files changed, 197 insertions(+), 9 deletions(-) diff --git a/common/src/main/java/org/tron/core/config/Parameter.java b/common/src/main/java/org/tron/core/config/Parameter.java index db88ab5e047..5349ef8d875 100644 --- a/common/src/main/java/org/tron/core/config/Parameter.java +++ b/common/src/main/java/org/tron/core/config/Parameter.java @@ -102,6 +102,7 @@ public class NetConstants { public static final int MSG_CACHE_DURATION_IN_BLOCKS = 5; public static final int MAX_BLOCK_FETCH_PER_PEER = 100; public static final int MAX_TRX_FETCH_PER_PEER = 1000; + public static final int MAX_SYNC_CHAIN_IDS = 30; } public class DatabaseConstants { diff --git a/framework/src/main/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandler.java b/framework/src/main/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandler.java index ecb7853ce6f..b1f26468081 100644 --- a/framework/src/main/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandler.java +++ b/framework/src/main/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandler.java @@ -3,6 +3,7 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.common.collect.Lists; +import java.util.HashSet; import java.util.List; import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; @@ -153,6 +154,12 @@ public boolean isAdvInv(PeerConnection peer, FetchInvDataMessage msg) { private void check(PeerConnection peer, FetchInvDataMessage fetchInvDataMsg, boolean isAdv) throws P2pException { + List hashList = fetchInvDataMsg.getHashList(); + if (hashList.size() != new HashSet<>(hashList).size()) { + throw new P2pException(TypeEnum.BAD_MESSAGE, + "FetchInvData contains duplicate hashes, size: " + hashList.size()); + } + MessageTypes type = fetchInvDataMsg.getInvMessageType(); if (type == MessageTypes.TRX) { diff --git a/framework/src/main/java/org/tron/core/net/messagehandler/InventoryMsgHandler.java b/framework/src/main/java/org/tron/core/net/messagehandler/InventoryMsgHandler.java index e8783b25e95..c92d53584a3 100644 --- a/framework/src/main/java/org/tron/core/net/messagehandler/InventoryMsgHandler.java +++ b/framework/src/main/java/org/tron/core/net/messagehandler/InventoryMsgHandler.java @@ -1,10 +1,14 @@ package org.tron.core.net.messagehandler; +import java.util.HashSet; +import java.util.List; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import org.tron.common.utils.Sha256Hash; import org.tron.core.config.args.Args; +import org.tron.core.exception.P2pException; +import org.tron.core.exception.P2pException.TypeEnum; import org.tron.core.net.TronNetDelegate; import org.tron.core.net.message.TronMessage; import org.tron.core.net.message.adv.InventoryMessage; @@ -27,7 +31,7 @@ public class InventoryMsgHandler implements TronMsgHandler { private TransactionsMsgHandler transactionsMsgHandler; @Override - public void processMessage(PeerConnection peer, TronMessage msg) { + public void processMessage(PeerConnection peer, TronMessage msg) throws P2pException { InventoryMessage inventoryMessage = (InventoryMessage) msg; InventoryType type = inventoryMessage.getInventoryType(); @@ -45,10 +49,17 @@ public void processMessage(PeerConnection peer, TronMessage msg) { } } - private boolean check(PeerConnection peer, InventoryMessage inventoryMessage) { + private boolean check(PeerConnection peer, InventoryMessage inventoryMessage) + throws P2pException { + + List hashList = inventoryMessage.getHashList(); + if (hashList.size() != new HashSet<>(hashList).size()) { + throw new P2pException(TypeEnum.BAD_MESSAGE, + "Inventory contains duplicate hashes, size: " + hashList.size()); + } InventoryType type = inventoryMessage.getInventoryType(); - int size = inventoryMessage.getHashList().size(); + int size = hashList.size(); if (peer.isNeedSyncFromPeer() || peer.isNeedSyncFromUs()) { logger.warn("Drop inv: {} size: {} from Peer {}, syncFromUs: {}, syncFromPeer: {}", diff --git a/framework/src/main/java/org/tron/core/net/messagehandler/SyncBlockChainMsgHandler.java b/framework/src/main/java/org/tron/core/net/messagehandler/SyncBlockChainMsgHandler.java index 71d268b22bc..5c18e014978 100644 --- a/framework/src/main/java/org/tron/core/net/messagehandler/SyncBlockChainMsgHandler.java +++ b/framework/src/main/java/org/tron/core/net/messagehandler/SyncBlockChainMsgHandler.java @@ -71,6 +71,12 @@ private boolean check(PeerConnection peer, SyncBlockChainMessage msg) throws P2p throw new P2pException(TypeEnum.BAD_MESSAGE, "SyncBlockChain blockIds is empty"); } + if (blockIds.size() > NetConstants.MAX_SYNC_CHAIN_IDS) { + throw new P2pException(TypeEnum.BAD_MESSAGE, + "SyncBlockChain blockIds size " + blockIds.size() + + " exceeds limit " + NetConstants.MAX_SYNC_CHAIN_IDS); + } + BlockId firstId = blockIds.get(0); if (!tronNetDelegate.containBlockInMainChain(firstId)) { logger.warn("Sync message from peer {} without the first block: {}", diff --git a/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java b/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java index 02a1983c260..e153e21f331 100644 --- a/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java +++ b/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java @@ -1,5 +1,8 @@ package org.tron.core.net.messagehandler; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; @@ -11,6 +14,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import org.tron.common.es.ExecutorServiceManager; +import org.tron.common.utils.Sha256Hash; import org.tron.core.ChainBaseManager; import org.tron.core.config.args.Args; import org.tron.core.exception.P2pException; @@ -121,8 +125,15 @@ public void processMessage(PeerConnection peer, TronMessage msg) throws P2pExcep } private void check(PeerConnection peer, TransactionsMessage msg) throws P2pException { - for (Transaction trx : msg.getTransactions().getTransactionsList()) { - Item item = new Item(new TransactionMessage(trx).getMessageId(), InventoryType.TRX); + List list = msg.getTransactions().getTransactionsList(); + Set seen = new HashSet<>(list.size() * 2); + for (Transaction trx : list) { + Sha256Hash id = new TransactionMessage(trx).getMessageId(); + if (!seen.add(id)) { + throw new P2pException(TypeEnum.BAD_MESSAGE, + "TransactionsMessage contains duplicate transaction: " + id); + } + Item item = new Item(id, InventoryType.TRX); if (!peer.getAdvInvRequest().containsKey(item)) { throw new P2pException(TypeEnum.BAD_MESSAGE, "trx: " + msg.getMessageId() + " without request."); diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandlerTest.java index e05ee29d015..e8ec4257814 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandlerTest.java @@ -15,6 +15,7 @@ import org.tron.common.utils.Sha256Hash; import org.tron.core.capsule.BlockCapsule; import org.tron.core.config.Parameter; +import org.tron.core.exception.P2pException; import org.tron.core.net.P2pRateLimiter; import org.tron.core.net.TronNetDelegate; import org.tron.core.net.message.adv.BlockMessage; @@ -124,12 +125,42 @@ public void testSyncFetchCheck() { Assert.assertEquals("minBlockNum: 16000, blockNum: 10000", e2.getMessage()); } + @Test + public void testDuplicateHashRejected() throws Exception { + FetchInvDataMsgHandler handler = new FetchInvDataMsgHandler(); + PeerConnection peer = Mockito.mock(PeerConnection.class); + AdvService advService = Mockito.mock(AdvService.class); + TronNetDelegate tronNetDelegate = Mockito.mock(TronNetDelegate.class); + + ReflectUtils.setFieldValue(handler, "advService", advService); + ReflectUtils.setFieldValue(handler, "tronNetDelegate", tronNetDelegate); + + Sha256Hash hash = Sha256Hash.ZERO_HASH; + List hashList = new LinkedList<>(); + hashList.add(hash); + hashList.add(hash); // duplicate + + FetchInvDataMessage msg = new FetchInvDataMessage(hashList, + Protocol.Inventory.InventoryType.TRX); + + Cache advInvSpread = CacheBuilder.newBuilder() + .maximumSize(20000).expireAfterWrite(1, TimeUnit.HOURS).build(); + advInvSpread.put(new Item(hash, Protocol.Inventory.InventoryType.TRX), 1L); + Mockito.when(peer.getAdvInvSpread()).thenReturn(advInvSpread); + + try { + handler.processMessage(peer, msg); + Assert.fail("Expected P2pException for duplicate hash"); + } catch (P2pException e) { + Assert.assertEquals(P2pException.TypeEnum.BAD_MESSAGE, e.getType()); + } + } + @Test public void testRateLimiter() { - BlockCapsule.BlockId blockId = new BlockCapsule.BlockId(Sha256Hash.ZERO_HASH, 10000L); List blockIds = new LinkedList<>(); for (int i = 0; i <= 100; i++) { - blockIds.add(blockId); + blockIds.add(new BlockCapsule.BlockId(Sha256Hash.ZERO_HASH, (long) i)); } FetchInvDataMessage msg = new FetchInvDataMessage(blockIds, Protocol.Inventory.InventoryType.BLOCK); diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/InventoryMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/InventoryMsgHandlerTest.java index 1dbf7c7150f..3d24ff2a4bf 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/InventoryMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/InventoryMsgHandlerTest.java @@ -6,10 +6,14 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.util.ArrayList; +import java.util.Arrays; +import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; import org.tron.common.TestConstants; +import org.tron.common.utils.Sha256Hash; import org.tron.core.config.args.Args; +import org.tron.core.exception.P2pException; import org.tron.core.net.TronNetDelegate; import org.tron.core.net.message.adv.InventoryMessage; import org.tron.core.net.peer.PeerConnection; @@ -52,6 +56,24 @@ public void testProcessMessage() throws Exception { Mockito.verify(tronNetDelegate, Mockito.atLeastOnce()).isBlockUnsolidified(); } + @Test + public void testDuplicateHashesRejected() throws Exception { + InventoryMsgHandler handler = new InventoryMsgHandler(); + Args.setParam(new String[] {}, TestConstants.TEST_CONF); + + Sha256Hash hash = Sha256Hash.wrap(new byte[32]); + InventoryMessage msg = new InventoryMessage(Arrays.asList(hash, hash), InventoryType.TRX); + PeerConnection peer = new PeerConnection(); + peer.setChannel(getChannel("1.0.0.4", 1000)); + + try { + handler.processMessage(peer, msg); + Assert.fail("Expected P2pException for duplicate hashes"); + } catch (P2pException e) { + Assert.assertEquals(P2pException.TypeEnum.BAD_MESSAGE, e.getType()); + } + } + private Channel getChannel(String host, int port) throws Exception { Channel channel = new Channel(); InetSocketAddress inetSocketAddress = new InetSocketAddress(host, port); diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/SyncBlockChainMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/SyncBlockChainMsgHandlerTest.java index 7960ef190f1..08c5484880f 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/SyncBlockChainMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/SyncBlockChainMsgHandlerTest.java @@ -16,11 +16,13 @@ import org.junit.rules.TemporaryFolder; import org.tron.common.TestConstants; import org.tron.common.application.TronApplicationContext; +import org.tron.common.utils.Sha256Hash; import org.tron.core.capsule.BlockCapsule; import org.tron.core.capsule.BlockCapsule.BlockId; import org.tron.core.config.DefaultConfig; import org.tron.core.config.args.Args; import org.tron.core.exception.P2pException; +import org.tron.core.net.TronNetDelegate; import org.tron.core.net.message.sync.BlockInventoryMessage; import org.tron.core.net.message.sync.SyncBlockChainMessage; import org.tron.core.net.peer.PeerConnection; @@ -108,6 +110,56 @@ public void testProcessMessage() throws Exception { Assert.assertEquals(1, list.size()); } + @Test + public void testBlockIdsExceedsLimit() throws Exception { + List blockIds = new ArrayList<>(); + // genesis block as first (in main chain), then 30 more = 31 total → exceeds limit + BlockId genesis = context.getBean( + TronNetDelegate.class).getGenesisBlockId(); + blockIds.add(genesis); + for (int i = 1; i <= 30; i++) { + blockIds.add(new BlockId(Sha256Hash.ZERO_HASH, i)); + } + SyncBlockChainMessage msg = new SyncBlockChainMessage(blockIds); + + try { + Method checkMethod = SyncBlockChainMsgHandler.class + .getDeclaredMethod("check", PeerConnection.class, SyncBlockChainMessage.class); + checkMethod.setAccessible(true); + checkMethod.invoke(handler, peer, msg); + Assert.fail("Expected P2pException for oversized blockIds"); + } catch (InvocationTargetException e) { + Assert.assertTrue(e.getCause() instanceof P2pException); + Assert.assertEquals(P2pException.TypeEnum.BAD_MESSAGE, + ((P2pException) e.getCause()).getType()); + } + } + + @Test + public void testBlockIdsAtLimit() throws Exception { + List blockIds = new ArrayList<>(); + BlockId genesis = context.getBean( + TronNetDelegate.class).getGenesisBlockId(); + blockIds.add(genesis); + for (int i = 1; i < 30; i++) { + blockIds.add(new BlockId(Sha256Hash.ZERO_HASH, i)); + } + // exactly 30 → should not throw for length check + SyncBlockChainMessage msg = new SyncBlockChainMessage(blockIds); + + Method checkMethod = SyncBlockChainMsgHandler.class + .getDeclaredMethod("check", PeerConnection.class, SyncBlockChainMessage.class); + checkMethod.setAccessible(true); + // does not throw P2pException due to length (may return false for other checks — that's fine) + try { + checkMethod.invoke(handler, peer, msg); + } catch (InvocationTargetException e) { + Assert.assertFalse("Should not fail with BAD_MESSAGE for length at limit", + e.getCause() instanceof P2pException + && ((P2pException) e.getCause()).getMessage().contains("exceeds limit")); + } + } + @AfterClass public static void destroy() { for (PeerConnection p : PeerManager.getPeers()) { 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 beeeecbca98..abe69e76ff2 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 @@ -23,6 +23,7 @@ import org.tron.common.TestConstants; import org.tron.common.runtime.TvmTestUtils; import org.tron.common.utils.ByteArray; +import org.tron.common.utils.ReflectUtils; import org.tron.core.ChainBaseManager; import org.tron.core.config.args.Args; import org.tron.core.exception.P2pException; @@ -142,10 +143,10 @@ public void testProcessMessageAfterClose() throws Exception { TransactionsMsgHandler handler = new TransactionsMsgHandler(); handler.init(); handler.close(); - + PeerConnection peer = Mockito.mock(PeerConnection.class); TransactionsMessage msg = Mockito.mock(TransactionsMessage.class); - + handler.processMessage(peer, msg); Mockito.verify(msg, Mockito.never()).getTransactions(); @@ -290,6 +291,52 @@ public void testHandleTransaction() throws Exception { } } + @Test + public void testDuplicateTransactionRejected() throws Exception { + TransactionsMsgHandler handler = new TransactionsMsgHandler(); + handler.init(); + try { + PeerConnection peer = Mockito.mock(PeerConnection.class); + + // Build a transaction + BalanceContract.TransferContract transferContract = BalanceContract.TransferContract + .newBuilder() + .setAmount(10) + .setOwnerAddress(ByteString.copyFrom(ByteArray.fromHexString("121212a9cf"))) + .setToAddress(ByteString.copyFrom(ByteArray.fromHexString("232323a9cf"))) + .build(); + Protocol.Transaction trx = Protocol.Transaction.newBuilder() + .setRawData(Protocol.Transaction.raw.newBuilder() + .addContract(Protocol.Transaction.Contract.newBuilder() + .setType(Protocol.Transaction.Contract.ContractType.TransferContract) + .setParameter(Any.pack(transferContract)).build()) + .build()) + .build(); + + // Same trx twice → duplicate + Protocol.Transactions transactions = Protocol.Transactions.newBuilder() + .addTransactions(trx) + .addTransactions(trx) + .build(); + TransactionsMessage msg = new TransactionsMessage(transactions.getTransactionsList()); + + TransactionMessage trxMsg = new TransactionMessage(trx); + Item item = new Item(trxMsg.getMessageId(), Protocol.Inventory.InventoryType.TRX); + Map advInvRequest = new ConcurrentHashMap<>(); + advInvRequest.put(item, System.currentTimeMillis()); + Mockito.when(peer.getAdvInvRequest()).thenReturn(advInvRequest); + + try { + handler.processMessage(peer, msg); + Assert.fail("Expected P2pException for duplicate transaction"); + } catch (P2pException e) { + Assert.assertEquals(P2pException.TypeEnum.BAD_MESSAGE, e.getType()); + } + } finally { + handler.close(); + } + } + @Test public void testIsBusyWithCachedTransactions() throws Exception { TransactionsMsgHandler handler = new TransactionsMsgHandler(); From 4f41f26404b15f8a4cc7a6e8234ee73eb78244b2 Mon Sep 17 00:00:00 2001 From: xxo1_shine Date: Sat, 9 May 2026 12:13:59 +0800 Subject: [PATCH 9/9] fix(security): validate merkle root before broadcast and reset isVerified on permission revocation (#6716) --- .../org/tron/core/capsule/BlockCapsule.java | 17 ++++++++ .../main/java/org/tron/core/db/Manager.java | 14 ++++--- .../org/tron/core/net/TronNetDelegate.java | 5 +++ .../tron/core/capsule/BlockCapsuleTest.java | 38 +++++++++++++++++ .../org/tron/core/db/ManagerMockTest.java | 13 +++++- .../java/org/tron/core/db/ManagerTest.java | 33 ++++++++++++++- .../tron/core/net/TronNetDelegateTest.java | 41 +++++++++++++++++++ 7 files changed, 152 insertions(+), 9 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 01ff7fb5365..34b7853d4d1 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java @@ -15,6 +15,8 @@ package org.tron.core.capsule; +import static org.tron.core.exception.BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED; + import com.google.common.primitives.Longs; import com.google.protobuf.ByteString; import com.google.protobuf.CodedInputStream; @@ -37,6 +39,7 @@ import org.tron.common.utils.Time; import org.tron.core.capsule.utils.MerkleTree; import org.tron.core.config.Parameter.ChainConstant; +import org.tron.core.exception.BadBlockException; import org.tron.core.exception.BadItemException; import org.tron.core.exception.ValidateSignatureException; import org.tron.core.store.AccountStore; @@ -49,6 +52,7 @@ public class BlockCapsule implements ProtoCapsule { public boolean generatedByMyself = false; + private volatile boolean merkleValidated = false; @Getter @Setter private TransactionRetCapsule result; @@ -225,6 +229,19 @@ public Sha256Hash calcMerkleRoot() { return MerkleTree.getInstance().createTree(ids).getRoot().getHash(); } + 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() { BlockHeader.raw blockHeaderRaw = this.block.getBlockHeader().getRawData().toBuilder() 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 d6ce53299f1..1d3aec2554d 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -1307,12 +1307,7 @@ public void pushBlock(final BlockCapsule block) try (PendingManager pm = new PendingManager(this)) { if (!block.generatedByMyself) { - if (!block.calcMerkleRoot().equals(block.getMerkleRoot())) { - logger.warn("Num: {}, the merkle root doesn't match, expect is {} , actual is {}.", - block.getNum(), block.getMerkleRoot(), block.calcMerkleRoot()); - throw new BadBlockException(CALC_MERKLE_ROOT_FAILED, - String.format("The merkle hash is not validated for %d", block.getNum())); - } + block.validateMerkleRoot(); consensus.receiveBlock(block); } @@ -2130,6 +2125,13 @@ public void rePush(TransactionCapsule tx) { return; } + String ownerAddress = ByteArray.toHexString(tx.getOwnerAddress()); + synchronized (this) { + if (ownerAddressSet.contains(ownerAddress)) { + tx.setVerified(false); + } + } + try { this.pushTransaction(tx); } catch (ValidateSignatureException | ContractValidateException | ContractExeException 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 9a53172a806..f9214f99e04 100644 --- a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java +++ b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java @@ -347,6 +347,11 @@ public boolean validBlock(BlockCapsule block) throws P2pException { throw new P2pException(TypeEnum.BAD_BLOCK, "time:" + time + ",block time:" + block.getTimeStamp()); } + try { + block.validateMerkleRoot(); + } catch (BadBlockException e) { + throw new P2pException(TypeEnum.BLOCK_MERKLE_ERROR, e.getMessage()); + } validSignature(block); return witnessScheduleStore.getActiveWitnesses().contains(block.getWitnessAddress()); } 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 61790849b43..ca0844c2c16 100644 --- a/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java +++ b/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java @@ -19,6 +19,7 @@ import org.tron.common.utils.Sha256Hash; import org.tron.core.Wallet; import org.tron.core.config.args.Args; +import org.tron.core.exception.BadBlockException; import org.tron.core.exception.BadItemException; import org.tron.protos.Protocol.Transaction.Contract.ContractType; import org.tron.protos.contract.BalanceContract.TransferContract; @@ -85,6 +86,43 @@ public void testCalcMerkleRoot() throws Exception { logger.info("Transaction[O] Merkle Root : {}", blockCapsule0.getMerkleRoot().toString()); } + @Test + public void testValidateMerkleRoot() throws Exception { + // build a fresh local block so shared blockCapsule0 is not mutated + String parentHash = "9938a342238077182498b464ac0292229938a342238077182498b464ac029222"; + BlockCapsule local = new BlockCapsule(1, + Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString(parentHash))), + 1234, + ByteString.copyFrom("1234567".getBytes())); + + // 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 + TransferContract transferContract = TransferContract.newBuilder() + .setAmount(999L) + .setOwnerAddress(ByteString.copyFrom("0x0000000000000000000".getBytes())) + .setToAddress(ByteString.copyFrom(ByteArray.fromHexString( + Wallet.getAddressPreFixString() + "A389132D6639FBDA4FBC8B659264E6B7C90DB086"))) + .build(); + local.addTransaction( + 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(); + Assert.fail("Expected BadBlockException for merkle mismatch"); + } catch (BadBlockException e) { + Assert.assertTrue(e.getMessage().contains("merkle")); + } + } + /* @Test public void testAddTransaction() { TransactionCapsule transactionCapsule = new TransactionCapsule("123", 1L); diff --git a/framework/src/test/java/org/tron/core/db/ManagerMockTest.java b/framework/src/test/java/org/tron/core/db/ManagerMockTest.java index 1e4b9a037ac..e3de0441c97 100644 --- a/framework/src/test/java/org/tron/core/db/ManagerMockTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerMockTest.java @@ -380,7 +380,18 @@ public void testRePush() { @Test public void testRePush1() { Manager dbManager = spy(new Manager()); - Protocol.Transaction transaction = Protocol.Transaction.newBuilder().build(); + BalanceContract.TransferContract transferContract = + BalanceContract.TransferContract.newBuilder() + .setOwnerAddress(ByteString.copyFromUtf8("aaa")) + .setToAddress(ByteString.copyFromUtf8("bbb")) + .setAmount(1) + .build(); + Protocol.Transaction transaction = Protocol.Transaction.newBuilder() + .setRawData(Protocol.Transaction.raw.newBuilder() + .addContract(Protocol.Transaction.Contract.newBuilder() + .setParameter(Any.pack(transferContract)) + .setType(Protocol.Transaction.Contract.ContractType.TransferContract))) + .build(); TransactionCapsule trx = new TransactionCapsule(transaction); TransactionStore transactionStoreMock = mock(TransactionStore.class); 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 211bddd1c07..eb0f8a7d5bf 100755 --- a/framework/src/test/java/org/tron/core/db/ManagerTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerTest.java @@ -509,8 +509,8 @@ public void pushBlockInvalidMerkelRoot() { } catch (BadBlockException e) { Assert.assertTrue(e instanceof BadBlockException); Assert.assertTrue(e.getType().equals(CALC_MERKLE_ROOT_FAILED)); - Assert.assertEquals("The merkle hash is not validated for " - + blockCapsule2.getNum(), e.getMessage()); + Assert.assertTrue(e.getMessage().startsWith( + "merkle root mismatch for block " + blockCapsule2.getNum() + ":")); } catch (Exception e) { Assert.assertFalse(e instanceof Exception); } @@ -1420,6 +1420,35 @@ public void testClearSolidityContractTriggerCache() throws Exception { } } + @Test + public void testRePushResetsVerifiedOnOwnerAddressSetHit() throws Exception { + TransferContract transferContract = TransferContract.newBuilder() + .setAmount(1L) + .setOwnerAddress(ByteString.copyFrom( + ByteArray.fromHexString(Wallet.getAddressPreFixString() + + "548794500882809695A8A687866E76D4271A1ABC"))) + .setToAddress(ByteString.copyFrom( + ByteArray.fromHexString(Wallet.getAddressPreFixString() + + "A389132D6639FBDA4FBC8B659264E6B7C90DB086"))) + .build(); + TransactionCapsule tx = new TransactionCapsule(transferContract, ContractType.TransferContract); + tx.setVerified(true); // simulate mempool-cached state + + String ownerAddress = ByteArray.toHexString(tx.getOwnerAddress()); + + // Inject ownerAddress into ownerAddressSet via reflection + Set ownerAddressSet = + (Set) ReflectUtils.getFieldObject(dbManager, "ownerAddressSet"); + ownerAddressSet.add(ownerAddress); + + // rePush should reset isVerified to false before pushTransaction + dbManager.rePush(tx); + + // After rePush, isVerified must be false + Boolean verified = (Boolean) ReflectUtils.getFieldObject(tx, "isVerified"); + Assert.assertFalse(verified); + } + @Test public void testGetCachedTransactionSize() throws Exception { BlockingQueue pushQ = new LinkedBlockingQueue<>(); 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 30659bde5d3..3083d36425a 100644 --- a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java +++ b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java @@ -2,16 +2,24 @@ import static org.mockito.Mockito.mock; +import com.google.protobuf.ByteString; import java.lang.reflect.Field; import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; import org.tron.common.TestConstants; import org.tron.common.parameter.CommonParameter; +import org.tron.common.utils.ByteArray; import org.tron.common.utils.Sha256Hash; import org.tron.core.ChainBaseManager; +import org.tron.core.Wallet; import org.tron.core.capsule.BlockCapsule; +import org.tron.core.capsule.TransactionCapsule; import org.tron.core.config.args.Args; +import org.tron.core.exception.P2pException; +import org.tron.core.exception.P2pException.TypeEnum; +import org.tron.protos.Protocol.Transaction.Contract.ContractType; +import org.tron.protos.contract.BalanceContract.TransferContract; public class TronNetDelegateTest { @@ -49,4 +57,37 @@ public void test() throws Exception { Assert.assertTrue(!tronNetDelegate.isBlockUnsolidified()); } + + @Test + public void testValidBlockMerkleRoot() throws Exception { + Args.setParam(new String[] {}, TestConstants.TEST_CONF); + + String parentHash = "9938a342238077182498b464ac0292229938a342238077182498b464ac029222"; + BlockCapsule block = new BlockCapsule(1, + Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString(parentHash))), + System.currentTimeMillis(), + ByteString.copyFrom("witness".getBytes())); + block.setMerkleRoot(); + + // Add a transaction after setMerkleRoot, making the stored merkle root stale. + TransferContract transferContract = TransferContract.newBuilder() + .setAmount(1L) + .setOwnerAddress(ByteString.copyFrom("0x0000000000000000000".getBytes())) + .setToAddress(ByteString.copyFrom(ByteArray.fromHexString( + Wallet.getAddressPreFixString() + "A389132D6639FBDA4FBC8B659264E6B7C90DB086"))) + .build(); + 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(); + try { + tronNetDelegate.validBlock(tampered); + Assert.fail("Expected P2pException for tampered merkle root"); + } catch (P2pException e) { + Assert.assertEquals(TypeEnum.BLOCK_MERKLE_ERROR, e.getType()); + } + } }