From ee0eb8f1b072070d1604269f59584cdb31ea65eb Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Thu, 6 Aug 2026 15:56:15 +0800 Subject: [PATCH 1/5] fix: prevent receive description self-assignment --- build.gradle | 1 + .../java/org/tron/core/capsule/ReceiveDescriptionCapsule.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 65e72c0fb73..31bc4633812 100644 --- a/build.gradle +++ b/build.gradle @@ -132,6 +132,7 @@ subprojects { disableAllChecks = true excludedPaths = '.*/generated/.*' errorproneArgs.addAll([ + '-Xep:SelfAssignment:ERROR', '-Xep:StringCaseLocaleUsage:ERROR', '-Xep:StringCaseLocaleUsageMethodRef:ERROR', ]) diff --git a/framework/src/main/java/org/tron/core/capsule/ReceiveDescriptionCapsule.java b/framework/src/main/java/org/tron/core/capsule/ReceiveDescriptionCapsule.java index db9e8e80fa0..15438a630ca 100644 --- a/framework/src/main/java/org/tron/core/capsule/ReceiveDescriptionCapsule.java +++ b/framework/src/main/java/org/tron/core/capsule/ReceiveDescriptionCapsule.java @@ -14,7 +14,7 @@ public ReceiveDescriptionCapsule() { receiveDescription = ReceiveDescription.newBuilder().build(); } - public ReceiveDescriptionCapsule(final ReceiveDescription outputDescription) { + public ReceiveDescriptionCapsule(final ReceiveDescription receiveDescription) { this.receiveDescription = receiveDescription; } From c60d56566d4e316e0634b022c60e592944440c6f Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Fri, 29 May 2026 06:01:46 +0000 Subject: [PATCH 2/5] feat(errorprone): enforce no-java.lang.Math rule at compile time Replace the regex-based .github/workflows/math-check.yml scan with a custom ErrorProne BugChecker (ForbidJavaLangMath) in the :errorprone module. It resolves symbols on the type-attributed AST, so it catches every usage form (direct, fully-qualified, statically-imported, method references, field access) with no string/comment false positives. java.lang.StrictMath remains allowed. - add ForbidJavaLangMath BugChecker (auto-registered via @AutoService) - enable -Xep:ForbidJavaLangMath:ERROR in build.gradle - exempt the canonical x86 MathWrapper via @SuppressWarnings - delete .github/workflows/math-check.yml --- build.gradle | 1 + errorprone/build.gradle | 16 +++ .../java/errorprone/ForbidJavaLangMath.java | 119 ++++++++++++++++++ .../errorprone/ForbidJavaLangMathTest.java | 87 +++++++++++++ gradle/verification-metadata.xml | 105 ++++++++++++++++ .../x86/org/tron/common/math/MathWrapper.java | 1 + 6 files changed, 329 insertions(+) create mode 100644 errorprone/src/main/java/errorprone/ForbidJavaLangMath.java create mode 100644 errorprone/src/test/java/errorprone/ForbidJavaLangMathTest.java diff --git a/build.gradle b/build.gradle index 31bc4633812..f870f45df56 100644 --- a/build.gradle +++ b/build.gradle @@ -135,6 +135,7 @@ subprojects { '-Xep:SelfAssignment:ERROR', '-Xep:StringCaseLocaleUsage:ERROR', '-Xep:StringCaseLocaleUsageMethodRef:ERROR', + '-Xep:ForbidJavaLangMath:ERROR', ]) } } diff --git a/errorprone/build.gradle b/errorprone/build.gradle index f8a634b7edc..8357cecc991 100644 --- a/errorprone/build.gradle +++ b/errorprone/build.gradle @@ -9,5 +9,21 @@ if (!JavaVersion.current().isJava11Compatible()) { compileOnly "com.google.errorprone:error_prone_core:${errorproneVersion}" compileOnly "com.google.auto.service:auto-service:1.1.1" annotationProcessor "com.google.auto.service:auto-service:1.1.1" + testImplementation "com.google.errorprone:error_prone_test_helpers:${errorproneVersion}" + } + + tasks.withType(Test).configureEach { + jvmArgs( + '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED', + '--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED', + '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED', + '--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED', + '--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED', + '--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED', + '--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED', + '--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED', + '--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED', + '--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED' + ) } } diff --git a/errorprone/src/main/java/errorprone/ForbidJavaLangMath.java b/errorprone/src/main/java/errorprone/ForbidJavaLangMath.java new file mode 100644 index 00000000000..e3caf53898e --- /dev/null +++ b/errorprone/src/main/java/errorprone/ForbidJavaLangMath.java @@ -0,0 +1,119 @@ +package errorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MemberReferenceTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; +import com.sun.tools.javac.code.Symbol; + +/** + * Forbids any direct use of {@code java.lang.Math}. + * + *

{@code java.lang.Math} permits the JIT to use platform-specific intrinsics for some + * methods (notably the transcendental and floating-point ones), so results are not + * guaranteed to be bit-for-bit identical across CPUs / JVMs. In a consensus system that + * non-determinism can fork the chain. All math must therefore go through + * {@code org.tron.common.math.StrictMathWrapper}, which is backed by {@code java.lang.StrictMath} + * and produces reproducible results. + * + *

This checker replaces the previous regex-based {@code .github/workflows/math-check.yml} + * scan. It resolves symbols on the type-attributed AST, so it has no false positives from + * strings/comments or unrelated classes named {@code Math}, and it catches every usage form: + *

+ * + *

{@code java.lang.StrictMath} itself is intentionally allowed — it is the deterministic + * primitive that {@code StrictMathWrapper} and the deprecated {@code MathWrapper} are built on. + * Those wrappers legitimately call {@code java.lang.Math}/{@code StrictMath} and are exempted via + * {@code @SuppressWarnings("ForbidJavaLangMath")}. + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "ForbidJavaLangMath", + summary = "Direct use of java.lang.Math is forbidden: its results are not guaranteed to be " + + "bit-for-bit identical across platforms, which can break consensus. Use " + + "org.tron.common.math.StrictMathWrapper instead.", + severity = BugPattern.SeverityLevel.ERROR +) +public class ForbidJavaLangMath extends BugChecker + implements BugChecker.MethodInvocationTreeMatcher, + BugChecker.MemberReferenceTreeMatcher, + BugChecker.MemberSelectTreeMatcher, + BugChecker.IdentifierTreeMatcher { + + private static final String JAVA_LANG_MATH = "java.lang.Math"; + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + return flagIfMathMember(ASTHelpers.getSymbol(tree), tree); + } + + @Override + public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) { + return flagIfMathMember(ASTHelpers.getSymbol(tree), tree); + } + + @Override + public Description matchMemberSelect(MemberSelectTree tree, VisitorState state) { + // Type-level references such as the class literal `Math.class` resolve to the Math + // ClassSymbol rather than a member, and are a back door to java.lang.Math via reflection + // (e.g. Math.class.getMethod("sin").invoke(...)). Flag them explicitly. + if (tree.getIdentifier().contentEquals("class") + && isJavaLangMathClass(ASTHelpers.getSymbol(tree.getExpression()))) { + return describeMatch(tree); + } + // Method selects (Math.max) are already reported via matchMethodInvocation / + // matchMemberReference; only flag field selects (Math.PI, Math.E) here to avoid + // double-reporting the same usage. + Symbol sym = ASTHelpers.getSymbol(tree); + if (!(sym instanceof Symbol.VarSymbol)) { + return Description.NO_MATCH; + } + return flagIfMathMember(sym, tree); + } + + @Override + public Description matchIdentifier(IdentifierTree tree, VisitorState state) { + // Catches statically-imported constants referenced bare (e.g. `import static + // java.lang.Math.PI; ... double c = PI;`). Statically-imported *methods* are caught by + // matchMethodInvocation, so restrict this to fields to avoid double-reporting. + Symbol sym = ASTHelpers.getSymbol(tree); + if (!(sym instanceof Symbol.VarSymbol)) { + return Description.NO_MATCH; + } + return flagIfMathMember(sym, tree); + } + + private static boolean isJavaLangMathClass(Symbol sym) { + return sym instanceof Symbol.ClassSymbol + && ((Symbol.ClassSymbol) sym).getQualifiedName().contentEquals(JAVA_LANG_MATH); + } + + private Description flagIfMathMember(Symbol sym, Tree tree) { + if (sym == null) { + return Description.NO_MATCH; + } + Symbol.ClassSymbol enclosingClass = ASTHelpers.enclosingClass(sym); + if (enclosingClass == null) { + return Description.NO_MATCH; + } + if (enclosingClass.getQualifiedName().contentEquals(JAVA_LANG_MATH)) { + return describeMatch(tree); + } + return Description.NO_MATCH; + } +} diff --git a/errorprone/src/test/java/errorprone/ForbidJavaLangMathTest.java b/errorprone/src/test/java/errorprone/ForbidJavaLangMathTest.java new file mode 100644 index 00000000000..75aca5d8da9 --- /dev/null +++ b/errorprone/src/test/java/errorprone/ForbidJavaLangMathTest.java @@ -0,0 +1,87 @@ +package errorprone; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; + +public class ForbidJavaLangMathTest { + + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(ForbidJavaLangMath.class, getClass()); + + @Test + public void rejectsJavaLangMathUsage() { + compilationHelper + .addSourceLines( + "Test.java", + "import static java.lang.Math.E;", + "import static java.lang.Math.max;", + "import java.util.function.DoubleBinaryOperator;", + "import java.util.function.DoubleUnaryOperator;", + "class Test {", + " double directCall() {", + " // BUG: Diagnostic contains: Direct use of java.lang.Math is forbidden", + " return Math.sin(1.0);", + " }", + " double fullyQualifiedCall() {", + " // BUG: Diagnostic contains: Direct use of java.lang.Math is forbidden", + " return java.lang.Math.cos(1.0);", + " }", + " double staticallyImportedCall() {", + " // BUG: Diagnostic contains: Direct use of java.lang.Math is forbidden", + " return max(1.0, 2.0);", + " }", + " DoubleUnaryOperator methodReference() {", + " // BUG: Diagnostic contains: Direct use of java.lang.Math is forbidden", + " return Math::sin;", + " }", + " DoubleBinaryOperator fullyQualifiedMethodReference() {", + " // BUG: Diagnostic contains: Direct use of java.lang.Math is forbidden", + " return java.lang.Math::max;", + " }", + " double fieldAccess() {", + " // BUG: Diagnostic contains: Direct use of java.lang.Math is forbidden", + " return Math.PI;", + " }", + " double staticallyImportedField() {", + " // BUG: Diagnostic contains: Direct use of java.lang.Math is forbidden", + " return E;", + " }", + " Class classLiteral() {", + " // BUG: Diagnostic contains: Direct use of java.lang.Math is forbidden", + " return Math.class;", + " }", + "}") + .doTest(); + } + + @Test + public void allowsUnrelatedMathStrictMathAndSuppression() { + compilationHelper + .addSourceLines( + "Test.java", + "import java.util.function.DoubleUnaryOperator;", + "class Math {", + " static final double PI = 3.0;", + " static double sin(double value) { return value; }", + "}", + "class Test {", + " double unrelatedMath() {", + " DoubleUnaryOperator operator = Math::sin;", + " Class type = Math.class;", + " return operator.applyAsDouble(Math.PI);", + " }", + " double strictMath() {", + " return java.lang.StrictMath.sin(StrictMath.PI);", + " }", + " @SuppressWarnings(\"ForbidJavaLangMath\")", + " double suppressed() {", + " return java.lang.Math.sin(Math.PI);", + " }", + " String text() {", + " // Math.sin(1.0) in a comment is not a usage.", + " return \"java.lang.Math.cos(1.0)\";", + " }", + "}") + .doTest(); + } +} diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 6a3e641d5d6..4af7f38867e 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -387,6 +387,22 @@ + + + + + + + + + + + + + + + + @@ -395,6 +411,16 @@ + + + + + + + + + + @@ -552,6 +578,14 @@ + + + + + + + + @@ -762,6 +796,19 @@ + + + + + + + + + + + + + @@ -857,6 +904,27 @@ + + + + + + + + + + + + + + + + + + + + + @@ -2017,6 +2085,11 @@ + + + + + @@ -2237,6 +2310,14 @@ + + + + + + + + @@ -2245,6 +2326,22 @@ + + + + + + + + + + + + + + + + @@ -2495,6 +2592,14 @@ + + + + + + + + diff --git a/platform/src/main/java/x86/org/tron/common/math/MathWrapper.java b/platform/src/main/java/x86/org/tron/common/math/MathWrapper.java index 758a0f18370..4e5d0cf6717 100644 --- a/platform/src/main/java/x86/org/tron/common/math/MathWrapper.java +++ b/platform/src/main/java/x86/org/tron/common/math/MathWrapper.java @@ -6,6 +6,7 @@ * especially for floating-point calculations. */ @Deprecated +@SuppressWarnings("ForbidJavaLangMath") // canonical wrapper: deliberately delegates to java.lang.Math public class MathWrapper { public static double pow(double a, double b) { From b6d7bcfc0709ff3914aec12fb147c8f454eb74fa Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Thu, 6 Aug 2026 16:56:40 +0800 Subject: [PATCH 3/5] feat(errorprone): forbid floating-point BigDecimal constructors --- build.gradle | 1 + .../BigDecimalFloatingPointConstructor.java | 51 +++++++++++++++++ ...igDecimalFloatingPointConstructorTest.java | 57 +++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 errorprone/src/main/java/errorprone/BigDecimalFloatingPointConstructor.java create mode 100644 errorprone/src/test/java/errorprone/BigDecimalFloatingPointConstructorTest.java diff --git a/build.gradle b/build.gradle index f870f45df56..9f2b7e1e974 100644 --- a/build.gradle +++ b/build.gradle @@ -132,6 +132,7 @@ subprojects { disableAllChecks = true excludedPaths = '.*/generated/.*' errorproneArgs.addAll([ + '-Xep:BigDecimalFloatingPointConstructor:ERROR', '-Xep:SelfAssignment:ERROR', '-Xep:StringCaseLocaleUsage:ERROR', '-Xep:StringCaseLocaleUsageMethodRef:ERROR', diff --git a/errorprone/src/main/java/errorprone/BigDecimalFloatingPointConstructor.java b/errorprone/src/main/java/errorprone/BigDecimalFloatingPointConstructor.java new file mode 100644 index 00000000000..7c46be01265 --- /dev/null +++ b/errorprone/src/main/java/errorprone/BigDecimalFloatingPointConstructor.java @@ -0,0 +1,51 @@ +package errorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.NewClassTree; +import com.sun.tools.javac.code.Symbol; +import java.util.List; + +/** + * Prevents constructing {@link java.math.BigDecimal} from binary floating-point values. + * + *

This checks the resolved constructor signature, so it also catches {@link Double} and + * {@link Float} arguments that javac unboxes to the {@code double} constructor. + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "BigDecimalFloatingPointConstructor", + summary = "Do not construct BigDecimal from a floating-point value. Use a decimal String, " + + "for example new BigDecimal(\"0.0001\").", + severity = BugPattern.SeverityLevel.ERROR +) +public class BigDecimalFloatingPointConstructor extends BugChecker + implements BugChecker.NewClassTreeMatcher { + + private static final String BIG_DECIMAL = "java.math.BigDecimal"; + + @Override + public Description matchNewClass(NewClassTree tree, VisitorState state) { + Symbol symbol = ASTHelpers.getSymbol(tree); + if (!(symbol instanceof Symbol.MethodSymbol)) { + return Description.NO_MATCH; + } + + Symbol.MethodSymbol constructor = (Symbol.MethodSymbol) symbol; + if (!constructor.owner.getQualifiedName().contentEquals(BIG_DECIMAL)) { + return Description.NO_MATCH; + } + + List parameters = constructor.getParameters(); + if (parameters.isEmpty() + || !ASTHelpers.isSameType(parameters.get(0).type, state.getSymtab().doubleType, state)) { + return Description.NO_MATCH; + } + + return describeMatch(tree); + } +} diff --git a/errorprone/src/test/java/errorprone/BigDecimalFloatingPointConstructorTest.java b/errorprone/src/test/java/errorprone/BigDecimalFloatingPointConstructorTest.java new file mode 100644 index 00000000000..ea63d8e2fb0 --- /dev/null +++ b/errorprone/src/test/java/errorprone/BigDecimalFloatingPointConstructorTest.java @@ -0,0 +1,57 @@ +package errorprone; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; + +public class BigDecimalFloatingPointConstructorTest { + + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(BigDecimalFloatingPointConstructor.class, getClass()); + + @Test + public void rejectsFloatingPointArguments() { + compilationHelper + .addSourceLines( + "Test.java", + "import java.math.BigDecimal;", + "import java.math.MathContext;", + "class Test {", + " void primitive(double doubleValue, float floatValue) {", + " // BUG: Diagnostic contains: Do not construct BigDecimal from a floating-point value", + " new BigDecimal(doubleValue);", + " // BUG: Diagnostic contains: Do not construct BigDecimal from a floating-point value", + " new BigDecimal(floatValue);", + " // BUG: Diagnostic contains: Do not construct BigDecimal from a floating-point value", + " new BigDecimal(0.0001);", + " // BUG: Diagnostic contains: Do not construct BigDecimal from a floating-point value", + " new BigDecimal(0.0001f, MathContext.DECIMAL64);", + " }", + " void boxed(Double doubleValue, Float floatValue) {", + " // BUG: Diagnostic contains: Do not construct BigDecimal from a floating-point value", + " new BigDecimal(doubleValue);", + " // BUG: Diagnostic contains: Do not construct BigDecimal from a floating-point value", + " new BigDecimal(floatValue, MathContext.DECIMAL64);", + " }", + "}") + .doTest(); + } + + @Test + public void allowsNonFloatingPointArguments() { + compilationHelper + .addSourceLines( + "Test.java", + "import java.math.BigDecimal;", + "import java.math.BigInteger;", + "class Test {", + " void safe(String value, BigInteger integer, long longValue) {", + " new BigDecimal(value);", + " new BigDecimal(\"0.0001\");", + " new BigDecimal(integer);", + " new BigDecimal(longValue);", + " BigDecimal.valueOf(0.0001);", + " }", + "}") + .doTest(); + } +} From 8e90ade2158ed4990503a469632b23479d339348 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Thu, 6 Aug 2026 18:14:01 +0800 Subject: [PATCH 4/5] feat(errorprone): enable production-only recommended checks --- .../tron/core/actuator/ActuatorFactory.java | 2 +- .../tron/core/vm/PrecompiledContracts.java | 10 +++---- .../org/tron/core/vm/program/Program.java | 2 ++ build.gradle | 30 +++++++++++++++++++ .../core/capsule/PedersenHashCapsule.java | 20 ------------- .../tron/core/capsule/TransactionCapsule.java | 10 ++++--- .../org/tron/core/db/BandwidthProcessor.java | 3 +- .../java/org/tron/common/utils/FileUtil.java | 2 +- .../main/java/org/tron/core/db/Manager.java | 4 +-- .../net/messagehandler/PbftMsgHandler.java | 4 +-- .../JsonRpcOnPBFTServlet.java | 4 +-- .../JsonRpcOnSolidityServlet.java | 4 +-- .../core/services/jsonrpc/JsonRpcApiUtil.java | 4 +-- 13 files changed, 56 insertions(+), 43 deletions(-) diff --git a/actuator/src/main/java/org/tron/core/actuator/ActuatorFactory.java b/actuator/src/main/java/org/tron/core/actuator/ActuatorFactory.java index 34031150ad1..2a890facc2c 100644 --- a/actuator/src/main/java/org/tron/core/actuator/ActuatorFactory.java +++ b/actuator/src/main/java/org/tron/core/actuator/ActuatorFactory.java @@ -40,7 +40,7 @@ public static List createActuator(TransactionCapsule transactionCapsul actuatorList .add(getActuatorByContract(contract, chainBaseManager, transactionCapsule)); } catch (IllegalAccessException | InstantiationException e) { - e.printStackTrace(); + logger.error("Failed to create actuator for contract {}.", contract.getType(), e); } }); return actuatorList; 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 3993e8ed835..f52bb519555 100644 --- a/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java +++ b/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java @@ -514,7 +514,7 @@ public long getEnergyForData(byte[] data) { if (data == null) { return 15; } - return 15L + (data.length + 31) / 32 * 3; + return 15L + (data.length + 31L) / 32 * 3; } @Override @@ -534,7 +534,7 @@ public long getEnergyForData(byte[] data) { if (data == null) { return 60; } - return 60L + (data.length + 31) / 32 * 12; + return 60L + (data.length + 31L) / 32 * 12; } @Override @@ -561,7 +561,7 @@ public long getEnergyForData(byte[] data) { if (data == null) { return 600; } - return 600L + (data.length + 31) / 32 * 120; + return 600L + (data.length + 31L) / 32 * 120; } @Override @@ -1043,7 +1043,7 @@ public static class ValidateMultiSign extends PrecompiledContract { @Override public long getEnergyForData(byte[] data) { - long cnt = (data.length / WORD_SIZE - 5) / 5; + long cnt = ((long) data.length / WORD_SIZE - ABI_HEADER_WORDS) / ABI_ITEM_WORDS; // one sign 1500, half of ecrecover return cnt * ENGERYPERSIGN; } @@ -1136,7 +1136,7 @@ public static class BatchValidateSign extends PrecompiledContract { @Override public long getEnergyForData(byte[] data) { - long cnt = (data.length / WORD_SIZE - 5) / 6; + long cnt = ((long) data.length / WORD_SIZE - ABI_HEADER_WORDS) / ABI_ITEM_WORDS; // one sign 1500, half of ecrecover return cnt * ENGERYPERSIGN; } diff --git a/actuator/src/main/java/org/tron/core/vm/program/Program.java b/actuator/src/main/java/org/tron/core/vm/program/Program.java index 590859a9fef..776d6b65984 100644 --- a/actuator/src/main/java/org/tron/core/vm/program/Program.java +++ b/actuator/src/main/java/org/tron/core/vm/program/Program.java @@ -637,6 +637,8 @@ private long transferFrozenV2BalanceToInheritor(byte[] ownerAddr, byte[] inherit case TRON_POWER: inheritorCapsule.addFrozenForTronPowerV2(freezeV2.getAmount()); break; + case UNRECOGNIZED: + break; } }); diff --git a/build.gradle b/build.gradle index 9f2b7e1e974..65665acb75a 100644 --- a/build.gradle +++ b/build.gradle @@ -125,18 +125,48 @@ subprojects { errorprone "com.google.errorprone:error_prone_core:${errorproneVersion}" errorprone rootProject.project(':errorprone') } + // Keep test compilation free of Error Prone so existing test code does not block builds. tasks.withType(JavaCompile).configureEach { + options.errorprone.enabled = false + } + tasks.named(sourceSets.main.compileJavaTaskName, JavaCompile).configure { options.errorprone { enabled = true disableWarningsInGeneratedCode = true disableAllChecks = true excludedPaths = '.*/generated/.*' errorproneArgs.addAll([ + // Project-specific checks. '-Xep:BigDecimalFloatingPointConstructor:ERROR', '-Xep:SelfAssignment:ERROR', '-Xep:StringCaseLocaleUsage:ERROR', '-Xep:StringCaseLocaleUsageMethodRef:ERROR', '-Xep:ForbidJavaLangMath:ERROR', + + // High-signal checks. + '-Xep:ArrayEquals:ERROR', + '-Xep:ArrayHashCode:ERROR', + '-Xep:ArrayToString:ERROR', + '-Xep:ArraysAsListPrimitiveArray:ERROR', + '-Xep:BadShiftAmount:ERROR', + '-Xep:CollectionIncompatibleType:ERROR', + '-Xep:ConstantOverflow:ERROR', + '-Xep:EqualsHashCode:ERROR', + '-Xep:EqualsIncompatibleType:ERROR', + '-Xep:FloatingPointLiteralPrecision:ERROR', + '-Xep:GuardedBy:ERROR', + '-Xep:JavaTimeDefaultTimeZone:ERROR', + '-Xep:MathRoundIntLong:ERROR', + '-Xep:NonAtomicVolatileUpdate:ERROR', + '-Xep:ThreadJoinLoop:ERROR', + '-Xep:UnicodeEscape:ERROR', + '-Xep:XorPower:ERROR', + + // Small production baseline fixed in this change. + '-Xep:IntLongMath:ERROR', + '-Xep:LockNotBeforeTry:ERROR', + '-Xep:MissingCasesInEnumSwitch:ERROR', + '-Xep:CatchAndPrintStackTrace:ERROR', ]) } } diff --git a/chainbase/src/main/java/org/tron/core/capsule/PedersenHashCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/PedersenHashCapsule.java index 137675bb822..9dcb812d54d 100644 --- a/chainbase/src/main/java/org/tron/core/capsule/PedersenHashCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/PedersenHashCapsule.java @@ -55,26 +55,6 @@ public static PedersenHashCapsule uncommitted() throws ZksnarkException { return compressCapsule; } - public static void main(String[] args) { - try { - byte[] a = - ByteArray - .fromHexString("05655316a07e6ec8c9769af54ef98b30667bfb6302b32987d552227dae86a087"); - byte[] b = - ByteArray - .fromHexString("06041357de59ba64959d1b60f93de24dfe5ea1e26ed9e8a73d35b225a1845ba7"); - - PedersenHash sa = PedersenHash.newBuilder().setContent(ByteString.copyFrom(a)).build(); - PedersenHash sb = PedersenHash.newBuilder().setContent(ByteString.copyFrom(b)).build(); - - PedersenHash result = combine(sa, sb, 25).getInstance(); - // 61a50a5540b4944da27cbd9b3d6ec39234ba229d2c461f4d719bc136573bf45b - System.out.println(ByteArray.toHexString(result.getContent().toByteArray())); - } catch (ZksnarkException e) { - e.printStackTrace(); - } - } - public ByteString getContent() { return this.pedersenHash.getContent(); } diff --git a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java index b3f560541cf..2658a8fd906 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java @@ -786,8 +786,10 @@ public String toString() { getInstance().getRawData().getContractList().forEach(contract -> { toStringBuff.append("[" + i + "] ").append("type: ").append(contract.getType()) .append("\n"); - toStringBuff.append("from address=").append(getOwner(contract)).append("\n"); - toStringBuff.append("to address=").append(getToAddress(contract)).append("\n"); + toStringBuff.append("from address=") + .append(ByteArray.toHexString(getOwner(contract))).append("\n"); + toStringBuff.append("to address=") + .append(ByteArray.toHexString(getToAddress(contract))).append("\n"); if (contract.getType().equals(ContractType.TransferContract)) { TransferContract transferContract; try { @@ -796,7 +798,7 @@ public String toString() { toStringBuff.append("transfer amount=").append(transferContract.getAmount()) .append("\n"); } catch (InvalidProtocolBufferException e) { - e.printStackTrace(); + logger.debug("Failed to unpack transfer contract.", e); } } else if (contract.getType().equals(ContractType.TransferAssetContract)) { TransferAssetContract transferAssetContract; @@ -808,7 +810,7 @@ public String toString() { toStringBuff.append("transfer amount=").append(transferAssetContract.getAmount()) .append("\n"); } catch (InvalidProtocolBufferException e) { - e.printStackTrace(); + logger.debug("Failed to unpack transfer asset contract.", e); } } if (this.transaction.getSignatureList().size() >= i.get() + 1) { diff --git a/chainbase/src/main/java/org/tron/core/db/BandwidthProcessor.java b/chainbase/src/main/java/org/tron/core/db/BandwidthProcessor.java index ece16b25819..22427890511 100644 --- a/chainbase/src/main/java/org/tron/core/db/BandwidthProcessor.java +++ b/chainbase/src/main/java/org/tron/core/db/BandwidthProcessor.java @@ -141,7 +141,7 @@ public void consume(TransactionCapsule trx, TransactionTrace trace) long maxCreateAccountTxSize = dynamicPropertiesStore.getMaxCreateAccountTxSize(); int signatureCount = trx.getInstance().getSignatureCount(); long createAccountBytesSize = trx.getInstance().toBuilder().clearRet() - .build().getSerializedSize() - (signatureCount * PER_SIGN_LENGTH); + .build().getSerializedSize() - ((long) signatureCount * PER_SIGN_LENGTH); if (createAccountBytesSize > maxCreateAccountTxSize) { throw new TooBigTransactionException(String.format( "Too big new account transaction, TxId %s, the size is %d bytes, maxTxSize %d", @@ -548,4 +548,3 @@ private boolean useFreeNet(AccountCapsule accountCapsule, long bytes, long now) } - diff --git a/common/src/main/java/org/tron/common/utils/FileUtil.java b/common/src/main/java/org/tron/common/utils/FileUtil.java index 8031c764019..2de08cc1da7 100644 --- a/common/src/main/java/org/tron/common/utils/FileUtil.java +++ b/common/src/main/java/org/tron/common/utils/FileUtil.java @@ -102,7 +102,7 @@ public static int readData(String filePath, char[] buf) { try (BufferedReader bufRead = new BufferedReader(new FileReader(file))) { len = bufRead.read(buf, 0, buf.length); } catch (IOException ex) { - ex.printStackTrace(); + logger.warn("Failed to read data from file.", ex); return 0; } return len; 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 9d7a7c979b9..3785b02dbb2 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -1078,7 +1078,7 @@ private void applyBlock(BlockCapsule block, List txs) revokingStore.setMaxFlushCount(maxFlushCount); if (Args.getInstance().getShutdownBlockTime() != null && Args.getInstance().getShutdownBlockTime().getNextValidTimeAfter( - new Date(block.getTimeStamp() - maxFlushCount * 1000 * 3L)) + new Date(block.getTimeStamp() - maxFlushCount * 1000L * 3)) .compareTo(new Date(block.getTimeStamp())) <= 0) { revokingStore.setMaxFlushCount(SnapshotManager.DEFAULT_MIN_FLUSH_COUNT); } @@ -2552,7 +2552,7 @@ public Collection getTxListFromPending() { } public long getPendingSize() { - long value = getPendingTransactions().size() + getRePushTransactions().size() + long value = (long) getPendingTransactions().size() + getRePushTransactions().size() + getPoppedTransactions().size(); return value; } diff --git a/framework/src/main/java/org/tron/core/net/messagehandler/PbftMsgHandler.java b/framework/src/main/java/org/tron/core/net/messagehandler/PbftMsgHandler.java index d086cc28b6c..f76161d2ad5 100644 --- a/framework/src/main/java/org/tron/core/net/messagehandler/PbftMsgHandler.java +++ b/framework/src/main/java/org/tron/core/net/messagehandler/PbftMsgHandler.java @@ -53,8 +53,8 @@ public void processMessage(PeerConnection peer, PbftMessage msg) throws Exceptio msg.analyzeSignature(); String key = buildKey(msg); Lock lock = striped.get(key); + lock.lock(); try { - lock.lock(); if (msgCache.getIfPresent(key) != null) { return; } @@ -79,4 +79,4 @@ private String buildKey(PbftBaseMessage msg) { return msg.getKey() + msg.getPbftMessage().getRawData().getMsgType().toString(); } -} \ No newline at end of file +} diff --git a/framework/src/main/java/org/tron/core/services/interfaceJsonRpcOnPBFT/JsonRpcOnPBFTServlet.java b/framework/src/main/java/org/tron/core/services/interfaceJsonRpcOnPBFT/JsonRpcOnPBFTServlet.java index bcc383e55db..e672e78741f 100644 --- a/framework/src/main/java/org/tron/core/services/interfaceJsonRpcOnPBFT/JsonRpcOnPBFTServlet.java +++ b/framework/src/main/java/org/tron/core/services/interfaceJsonRpcOnPBFT/JsonRpcOnPBFTServlet.java @@ -21,9 +21,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) try { super.doPost(request, response); } catch (IOException e) { - e.printStackTrace(); + logger.error("Failed to process JSON-RPC request.", e); throw new RuntimeException(e); } }); } -} \ No newline at end of file +} diff --git a/framework/src/main/java/org/tron/core/services/interfaceJsonRpcOnSolidity/JsonRpcOnSolidityServlet.java b/framework/src/main/java/org/tron/core/services/interfaceJsonRpcOnSolidity/JsonRpcOnSolidityServlet.java index a20c7906ad0..66deed0dc72 100644 --- a/framework/src/main/java/org/tron/core/services/interfaceJsonRpcOnSolidity/JsonRpcOnSolidityServlet.java +++ b/framework/src/main/java/org/tron/core/services/interfaceJsonRpcOnSolidity/JsonRpcOnSolidityServlet.java @@ -21,9 +21,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) try { super.doPost(request, response); } catch (IOException e) { - e.printStackTrace(); + logger.error("Failed to process JSON-RPC request.", e); throw new RuntimeException(e); } }); } -} \ No newline at end of file +} diff --git a/framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcApiUtil.java b/framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcApiUtil.java index f4bba9fbf37..623c9b8e091 100644 --- a/framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcApiUtil.java +++ b/framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcApiUtil.java @@ -209,7 +209,7 @@ public static List getTo(Transaction transaction) { } return list; } catch (Exception ex) { - ex.printStackTrace(); + logger.warn("Failed to extract transaction addresses.", ex); } return list; } @@ -361,7 +361,7 @@ public static long getAmountFromTransactionInfo(String hash, ContractType contra logger.warn("Exception happens when get amount from transactionInfo. Exception = [{}]", Throwables.getStackTraceAsString(e)); } catch (Throwable t) { - t.printStackTrace(); + logger.warn("Unexpected error when getting amount from transaction info.", t); } return amount; } From cd313fb62ca8660f782e365dd1cc2aebaa016b55 Mon Sep 17 00:00:00 2001 From: halibobo1205 Date: Mon, 10 Aug 2026 20:16:56 +0800 Subject: [PATCH 5/5] feat(errorprone): enforce comparator contracts --- build.gradle | 1 + .../org/tron/core/store/AssetIssueStore.java | 12 +- .../org/tron/core/store/ExchangeStore.java | 7 +- .../org/tron/core/store/ProposalStore.java | 18 +- errorprone/build.gradle | 1 + .../ComparatorNeverReturnsZero.java | 172 ++++++++++++ .../ComparatorNeverReturnsZeroTest.java | 244 ++++++++++++++++++ .../tron/core/db/AssetIssueStoreSortTest.java | 53 ++++ ...EnergyPriceHistoryEqualExpirationTest.java | 57 ++++ .../tron/core/db/ExchangeStoreSortTest.java | 74 ++++++ .../tron/core/db/ProposalStoreSortTest.java | 98 +++++++ 11 files changed, 719 insertions(+), 18 deletions(-) create mode 100644 errorprone/src/main/java/errorprone/ComparatorNeverReturnsZero.java create mode 100644 errorprone/src/test/java/errorprone/ComparatorNeverReturnsZeroTest.java create mode 100644 framework/src/test/java/org/tron/core/db/AssetIssueStoreSortTest.java create mode 100644 framework/src/test/java/org/tron/core/db/EnergyPriceHistoryEqualExpirationTest.java create mode 100644 framework/src/test/java/org/tron/core/db/ExchangeStoreSortTest.java create mode 100644 framework/src/test/java/org/tron/core/db/ProposalStoreSortTest.java diff --git a/build.gradle b/build.gradle index 65665acb75a..8955c1f1402 100644 --- a/build.gradle +++ b/build.gradle @@ -142,6 +142,7 @@ subprojects { '-Xep:StringCaseLocaleUsage:ERROR', '-Xep:StringCaseLocaleUsageMethodRef:ERROR', '-Xep:ForbidJavaLangMath:ERROR', + '-Xep:ComparatorNeverReturnsZero:ERROR', // High-signal checks. '-Xep:ArrayEquals:ERROR', diff --git a/chainbase/src/main/java/org/tron/core/store/AssetIssueStore.java b/chainbase/src/main/java/org/tron/core/store/AssetIssueStore.java index 4f69a6c3c66..1f68cf26783 100644 --- a/chainbase/src/main/java/org/tron/core/store/AssetIssueStore.java +++ b/chainbase/src/main/java/org/tron/core/store/AssetIssueStore.java @@ -3,6 +3,8 @@ import static org.tron.common.utils.Commons.ASSET_ISSUE_COUNT_LIMIT_MAX; import com.google.common.collect.Streams; +import com.google.protobuf.ByteString; +import java.util.Comparator; import java.util.List; import java.util.Map.Entry; import java.util.stream.Collectors; @@ -46,12 +48,10 @@ private List getAssetIssuesPaginated(List if (assetIssueList.size() <= offset) { return null; } - assetIssueList.sort((o1, o2) -> { - if (o1.getName() != o2.getName()) { - return o1.getName().toStringUtf8().compareTo(o2.getName().toStringUtf8()); - } - return Long.compare(o1.getOrder(), o2.getOrder()); - }); + assetIssueList.sort( + Comparator.comparing(AssetIssueCapsule::getName, + ByteString.unsignedLexicographicalComparator()) + .thenComparingLong(AssetIssueCapsule::getOrder)); limit = limit > ASSET_ISSUE_COUNT_LIMIT_MAX ? ASSET_ISSUE_COUNT_LIMIT_MAX : limit; long end = offset + limit; end = end > assetIssueList.size() ? assetIssueList.size() : end; diff --git a/chainbase/src/main/java/org/tron/core/store/ExchangeStore.java b/chainbase/src/main/java/org/tron/core/store/ExchangeStore.java index 0cbd1958485..7162dbf7c48 100644 --- a/chainbase/src/main/java/org/tron/core/store/ExchangeStore.java +++ b/chainbase/src/main/java/org/tron/core/store/ExchangeStore.java @@ -1,6 +1,7 @@ package org.tron.core.store; import com.google.common.collect.Streams; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -31,9 +32,7 @@ public ExchangeCapsule get(byte[] key) throws ItemNotFoundException { public List getAllExchanges() { return Streams.stream(iterator()) .map(Map.Entry::getValue) - .sorted( - (ExchangeCapsule a, ExchangeCapsule b) -> a.getCreateTime() <= b.getCreateTime() ? 1 - : -1) + .sorted(Comparator.comparingLong(ExchangeCapsule::getCreateTime).reversed()) .collect(Collectors.toList()); } -} \ No newline at end of file +} diff --git a/chainbase/src/main/java/org/tron/core/store/ProposalStore.java b/chainbase/src/main/java/org/tron/core/store/ProposalStore.java index 3d39b717cfc..798e7f3b08c 100644 --- a/chainbase/src/main/java/org/tron/core/store/ProposalStore.java +++ b/chainbase/src/main/java/org/tron/core/store/ProposalStore.java @@ -1,6 +1,7 @@ package org.tron.core.store; import com.google.common.collect.Streams; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -32,23 +33,24 @@ public ProposalCapsule get(byte[] key) throws ItemNotFoundException { public List getAllProposals() { return Streams.stream(iterator()) .map(Map.Entry::getValue) - .sorted( - (ProposalCapsule a, ProposalCapsule b) -> a.getCreateTime() <= b.getCreateTime() ? 1 - : -1) + .sorted(Comparator.comparingLong(ProposalCapsule::getCreateTime).reversed()) .collect(Collectors.toList()); } /** - * note: return in asc order by expired time + * Returns proposals in ascending expiration order, ties broken by descending proposal ID. + * + *

The descending-id tie-break preserves the execution order for equal-expiration proposals: + * live execution applies the highest id first and the lowest id last (final value), and the + * energy/bandwidth price-history loaders rebuild from the tail, so the lowest id must be last. */ public List getSpecifiedProposals(State state, long code) { return Streams.stream(iterator()) .map(Map.Entry::getValue) .filter(proposalCapsule -> proposalCapsule.getState().equals(state)) .filter(proposalCapsule -> proposalCapsule.getParameters().containsKey(code)) - .sorted( - (ProposalCapsule a, ProposalCapsule b) -> a.getExpirationTime() > b.getExpirationTime() - ? 1 : -1) + .sorted(Comparator.comparingLong(ProposalCapsule::getExpirationTime) + .thenComparing(Comparator.comparingLong(ProposalCapsule::getID).reversed())) .collect(Collectors.toList()); } -} \ No newline at end of file +} diff --git a/errorprone/build.gradle b/errorprone/build.gradle index 8357cecc991..9505136b482 100644 --- a/errorprone/build.gradle +++ b/errorprone/build.gradle @@ -2,6 +2,7 @@ if (!JavaVersion.current().isJava11Compatible()) { // ErrorProne core requires JDK 11+; skip this module on JDK 8 tasks.withType(JavaCompile).configureEach { enabled = false } tasks.withType(Jar).configureEach { enabled = false } + tasks.withType(Test).configureEach { enabled = false } } else { dependencies { compileOnly "com.google.errorprone:error_prone_annotations:${errorproneVersion}" diff --git a/errorprone/src/main/java/errorprone/ComparatorNeverReturnsZero.java b/errorprone/src/main/java/errorprone/ComparatorNeverReturnsZero.java new file mode 100644 index 00000000000..cb9929d6222 --- /dev/null +++ b/errorprone/src/main/java/errorprone/ComparatorNeverReturnsZero.java @@ -0,0 +1,172 @@ +package errorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.Matchers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.BlockTree; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.ConditionalExpressionTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.LambdaExpressionTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.ParenthesizedTree; +import com.sun.source.tree.ReturnTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.UnaryTree; +import com.sun.source.util.TreeScanner; +import com.sun.tools.javac.code.Symbol; +import java.util.ArrayList; +import java.util.List; + +/** + * Flags {@link java.util.Comparator}/{@link java.lang.Comparable} implementations that can never + * return {@code 0} (e.g. {@code a <= b ? 1 : -1}). + * + *

Such a comparator violates the general contract: when two elements compare "equal" it still + * returns a non-zero value, breaking antisymmetry. At runtime {@code List.sort} (TimSort) throws + * {@code IllegalArgumentException: Comparison method violates its general contract!} once the list + * is large enough and contains equal keys; otherwise it silently produces an undefined order. + * + *

The built-in ErrorProne {@code ComparisonContractViolated} checker only inspects + * {@code compare}/{@code compareTo} method declarations, so it cannot see lambda + * comparators such as {@code list.sort((a, b) -> a.t() <= b.t() ? 1 : -1)}. This checker covers + * both the lambda form and the method-declaration form. + * + *

This is a deliberately conservative syntactic check: it only flags a body whose every + * return is a non-zero {@code int} constant (a literal, or a ternary of such constants). It does no + * data-flow analysis, so a value laundered through a variable + * ({@code int r = c ? -1 : 1; return r;}) is not flagged. + * + *

Fix by returning {@code 0} on equality, e.g. {@code Long.compare(a, b)} or + * {@code Comparator.comparingLong(X::t)} (append {@code .reversed()} for descending order). + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "ComparatorNeverReturnsZero", + summary = "Comparator/compareTo can never return 0 (e.g. `? 1 : -1`), violating the comparison " + + "contract; List.sort may throw IllegalArgumentException. Return 0 on equality, e.g. " + + "Long.compare(a, b) or Comparator.comparingLong(...).", + severity = BugPattern.SeverityLevel.ERROR) +public class ComparatorNeverReturnsZero extends BugChecker + implements BugChecker.LambdaExpressionTreeMatcher, BugChecker.MethodTreeMatcher { + + private static final Matcher IS_COMPARATOR = + Matchers.isSubtypeOf("java.util.Comparator"); + + @Override + public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) { + // Only comparator lambdas: the target functional interface is java.util.Comparator. + // (Comparable is not a functional interface, so it can never be a lambda.) + if (!IS_COMPARATOR.matches(tree, state)) { + return Description.NO_MATCH; + } + return neverReturnsZero(tree.getBody()) ? describeMatch(tree) : Description.NO_MATCH; + } + + @Override + public Description matchMethod(MethodTree tree, VisitorState state) { + if (tree.getBody() == null || !isComparatorMethod(tree, state)) { + return Description.NO_MATCH; + } + return neverReturnsZero(tree.getBody()) ? describeMatch(tree) : Description.NO_MATCH; + } + + /** + * True if the comparator body ({@code (a,b) -> expr}, {@code (a,b) -> {..}} or a method block) + * provably yields a non-zero {@code int} constant on every path. Conservative: any path whose + * value cannot be proven non-zero (a method call, a subtraction, a plain {@code 0}, ...) makes + * this return {@code false}, so legitimate comparators such as {@code Long.compare(a, b)} are + * never flagged. + */ + private static boolean neverReturnsZero(Tree body) { + if (body instanceof ExpressionTree) { + return alwaysNonZero((ExpressionTree) body); + } + if (body instanceof BlockTree) { + List returns = new ArrayList<>(); + new ReturnCollector().scan(body, returns); + if (returns.isEmpty()) { + return false; + } + for (ReturnTree r : returns) { + if (r.getExpression() == null || !alwaysNonZero(r.getExpression())) { + return false; + } + } + return true; + } + return false; + } + + /** True if {@code e} is provably a non-zero {@code int} constant on every branch. */ + private static boolean alwaysNonZero(ExpressionTree e) { + e = stripParens(e); + Integer c = ASTHelpers.constValue(e, Integer.class); + if (c != null) { + return c != 0; + } + // Fall back for `-1` / `+1` in case constant folding did not run. + if (e instanceof UnaryTree + && (e.getKind() == Tree.Kind.UNARY_MINUS || e.getKind() == Tree.Kind.UNARY_PLUS)) { + Integer operand = + ASTHelpers.constValue(stripParens(((UnaryTree) e).getExpression()), Integer.class); + return operand != null && operand != 0; + } + if (e instanceof ConditionalExpressionTree) { + ConditionalExpressionTree cond = (ConditionalExpressionTree) e; + return alwaysNonZero(cond.getTrueExpression()) && alwaysNonZero(cond.getFalseExpression()); + } + return false; + } + + private static ExpressionTree stripParens(ExpressionTree e) { + while (e instanceof ParenthesizedTree) { + e = ((ParenthesizedTree) e).getExpression(); + } + return e; + } + + /** + * True only for methods that genuinely override {@code Comparator.compare} or + * {@code Comparable.compareTo}. Matching by name + arity alone would wrongly flag same-named + * overloads (e.g. {@code compareTo(long)}) and static/private helpers; those override nothing, so + * {@link ASTHelpers#findSuperMethods} returns no comparator super-method for them. + */ + private static boolean isComparatorMethod(MethodTree tree, VisitorState state) { + Symbol.MethodSymbol sym = ASTHelpers.getSymbol(tree); + if (sym == null) { + return false; + } + for (Symbol.MethodSymbol superMethod : ASTHelpers.findSuperMethods(sym, state.getTypes())) { + String ownerName = superMethod.owner.getQualifiedName().toString(); + if (ownerName.equals("java.util.Comparator") || ownerName.equals("java.lang.Comparable")) { + return true; + } + } + return false; + } + + /** Collects return statements owned by this method/lambda, not by nested functions. */ + private static final class ReturnCollector extends TreeScanner> { + @Override + public Void visitReturn(ReturnTree node, List returns) { + returns.add(node); + return super.visitReturn(node, returns); + } + + @Override + public Void visitLambdaExpression(LambdaExpressionTree node, List returns) { + return null; // a nested lambda's returns are its own + } + + @Override + public Void visitClass(ClassTree node, List returns) { + return null; // a nested / anonymous class's returns are its own + } + } +} diff --git a/errorprone/src/test/java/errorprone/ComparatorNeverReturnsZeroTest.java b/errorprone/src/test/java/errorprone/ComparatorNeverReturnsZeroTest.java new file mode 100644 index 00000000000..95e461b0618 --- /dev/null +++ b/errorprone/src/test/java/errorprone/ComparatorNeverReturnsZeroTest.java @@ -0,0 +1,244 @@ +package errorprone; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; + +/** Tests for {@link ComparatorNeverReturnsZero}. */ +public class ComparatorNeverReturnsZeroTest { + + private final CompilationTestHelper helper = + CompilationTestHelper.newInstance(ComparatorNeverReturnsZero.class, getClass()); + + // ---------- positive: must be flagged ---------- + + @Test + public void lambdaExpression_leEver1Else1_flagged() { + helper + .addSourceLines( + "Test.java", + "import java.util.List;", + "class Test {", + " void f(List xs) {", + " // BUG: Diagnostic contains: ComparatorNeverReturnsZero", + " xs.sort((a, b) -> a <= b ? 1 : -1);", + " }", + "}") + .doTest(); + } + + @Test + public void lambdaExpression_gtThen1Else1_flagged() { + helper + .addSourceLines( + "Test.java", + "import java.util.List;", + "class Test {", + " void f(List xs) {", + " // BUG: Diagnostic contains: ComparatorNeverReturnsZero", + " xs.sort((a, b) -> a > b ? 1 : -1);", + " }", + "}") + .doTest(); + } + + @Test + public void lambdaBlock_allReturnsNonZero_flagged() { + helper + .addSourceLines( + "Test.java", + "import java.util.Comparator;", + "class Test {", + " Comparator c() {", + " // BUG: Diagnostic contains: ComparatorNeverReturnsZero", + " return (a, b) -> {", + " if (a <= b) {", + " return 1;", + " }", + " return -1;", + " };", + " }", + "}") + .doTest(); + } + + @Test + public void anonymousComparator_compare_flagged() { + helper + .addSourceLines( + "Test.java", + "import java.util.Comparator;", + "class Test {", + " Comparator c() {", + " return new Comparator() {", + " // BUG: Diagnostic contains: ComparatorNeverReturnsZero", + " public int compare(Long a, Long b) {", + " return a <= b ? 1 : -1;", + " }", + " };", + " }", + "}") + .doTest(); + } + + @Test + public void namedComparable_compareTo_flagged() { + helper + .addSourceLines( + "Test.java", + "class Test implements Comparable {", + " long t;", + " // BUG: Diagnostic contains: ComparatorNeverReturnsZero", + " public int compareTo(Test o) {", + " return this.t < o.t ? -1 : 1;", + " }", + "}") + .doTest(); + } + + // ---------- negative: must NOT be flagged ---------- + + @Test + public void lambdaLongCompare_ok() { + helper + .addSourceLines( + "Test.java", + "import java.util.List;", + "class Test {", + " void f(List xs) {", + " xs.sort((a, b) -> Long.compare(a, b));", + " }", + "}") + .doTest(); + } + + @Test + public void lambdaComparingLong_ok() { + helper + .addSourceLines( + "Test.java", + "import java.util.Comparator;", + "import java.util.List;", + "class Test {", + " void f(List xs) {", + " xs.sort(Comparator.comparingLong((Test t) -> t.t).reversed());", + " }", + " long t;", + "}") + .doTest(); + } + + @Test + public void ternaryCanReturnZero_ok() { + helper + .addSourceLines( + "Test.java", + "import java.util.List;", + "class Test {", + " void f(List xs) {", + " xs.sort((a, b) -> a <= b ? 1 : 0);", // can yield 0 -> not a violation shape + " }", + "}") + .doTest(); + } + + @Test + public void ceilingDivisionNotAComparator_ok() { + helper + .addSourceLines( + "Test.java", + "class Test {", + " long ceil(long numerator, long denominator) {", + " return (numerator / denominator) + ((numerator % denominator) > 0 ? 1 : 0);", + " }", + "}") + .doTest(); + } + + @Test + public void compareToUsingLongCompare_ok() { + helper + .addSourceLines( + "Test.java", + "class Test implements Comparable {", + " long t;", + " public int compareTo(Test o) {", + " return Long.compare(this.t, o.t);", + " }", + "}") + .doTest(); + } + + @Test + public void compareToWithNullGuardAndRealCompare_ok() { + // Mirrors DataWord.compareTo: one constant return (-1 for null) plus a non-constant return. + helper + .addSourceLines( + "Test.java", + "class Test implements Comparable {", + " long t;", + " public int compareTo(Test o) {", + " if (o == null) {", + " return -1;", + " }", + " return Long.compare(this.t, o.t);", + " }", + "}") + .doTest(); + } + + @Test + public void overloadedCompareToNotAnOverride_ok() { + // compareTo(long) is an overload, not an override of Comparable.compareTo(Test); must not flag. + helper + .addSourceLines( + "Test.java", + "class Test implements Comparable {", + " long t;", + " public int compareTo(Test o) {", + " return Long.compare(this.t, o.t);", + " }", + " int compareTo(long value) {", + " return value < 0 ? -1 : 1;", + " }", + "}") + .doTest(); + } + + @Test + public void twoArgStaticCompareOverloadInComparator_ok() { + // static compare(long,long) is a 2-arg overload inside a Comparator class but overrides + // nothing. The old name+arity+owner rule would flag it, the override-based rule must not. + helper + .addSourceLines( + "Test.java", + "import java.util.Comparator;", + "class Test implements Comparator {", + " public int compare(Long a, Long b) {", + " return Long.compare(a, b);", + " }", + " static int compare(long a, long b) {", + " return a < b ? -1 : 1;", + " }", + "}") + .doTest(); + } + + @Test + public void twoArgPrivateCompareOverloadInComparator_ok() { + // private compare(long,long) is a 2-arg overload inside a Comparator class but overrides + // nothing. The old name+arity+owner rule would flag it, the override-based rule must not. + helper + .addSourceLines( + "Test.java", + "import java.util.Comparator;", + "class Test implements Comparator {", + " public int compare(Long a, Long b) {", + " return Long.compare(a, b);", + " }", + " private int compare(long a, long b) {", + " return a < b ? -1 : 1;", + " }", + "}") + .doTest(); + } +} diff --git a/framework/src/test/java/org/tron/core/db/AssetIssueStoreSortTest.java b/framework/src/test/java/org/tron/core/db/AssetIssueStoreSortTest.java new file mode 100644 index 00000000000..6f026a4bc0c --- /dev/null +++ b/framework/src/test/java/org/tron/core/db/AssetIssueStoreSortTest.java @@ -0,0 +1,53 @@ +package org.tron.core.db; + +import static java.util.stream.Collectors.toList; +import static org.junit.Assert.assertEquals; + +import com.google.protobuf.ByteString; +import java.util.Arrays; +import java.util.List; +import javax.annotation.Resource; +import org.junit.Test; +import org.tron.common.BaseTest; +import org.tron.common.TestConstants; +import org.tron.core.capsule.AssetIssueCapsule; +import org.tron.core.config.args.Args; +import org.tron.core.store.AssetIssueStore; +import org.tron.protos.contract.AssetIssueContractOuterClass.AssetIssueContract; + +/** + * Sort-behaviour regression test for {@link AssetIssueStore#getAssetIssuesPaginated(long, long)}. + * Guards the fix that replaced the {@code ByteString} reference-comparison comparator (whose + * secondary {@code order} branch was dead code) with a name-then-order comparator built from + * {@code Comparator.comparing(..., unsignedLexicographicalComparator()).thenComparingLong(...)}. + */ +public class AssetIssueStoreSortTest extends BaseTest { + + @Resource + private AssetIssueStore assetIssueStore; + + static { + Args.setParam(new String[] {"--output-directory", dbPath()}, TestConstants.TEST_CONF); + } + + private void put(String id, String name, long order) { + AssetIssueContract contract = AssetIssueContract.newBuilder() + .setId(id) + .setName(ByteString.copyFromUtf8(name)) + .setOrder(order) + .build(); + assetIssueStore.put(id.getBytes(), new AssetIssueCapsule(contract)); + } + + @Test + public void paginated_ordersByNameThenOrder() { + put("2001", "BBB", 0L); + put("2002", "AAA", 5L); // same name as 2003, larger order -> must sort after it + put("2003", "AAA", 0L); + List ordered = assetIssueStore.getAssetIssuesPaginated(0, 100).stream() + .map(a -> a.getName().toStringUtf8() + "/" + a.getOrder()) + .collect(toList()); + // name ascending, then order ascending within the same name + assertEquals(Arrays.asList("AAA/0", "AAA/5", "BBB/0"), ordered); + } +} diff --git a/framework/src/test/java/org/tron/core/db/EnergyPriceHistoryEqualExpirationTest.java b/framework/src/test/java/org/tron/core/db/EnergyPriceHistoryEqualExpirationTest.java new file mode 100644 index 00000000000..579ef6088dc --- /dev/null +++ b/framework/src/test/java/org/tron/core/db/EnergyPriceHistoryEqualExpirationTest.java @@ -0,0 +1,57 @@ +package org.tron.core.db; + +import static org.tron.core.utils.ProposalUtil.ProposalType.ENERGY_FEE; + +import org.junit.Assert; +import org.junit.Test; +import org.tron.common.BaseTest; +import org.tron.common.TestConstants; +import org.tron.core.capsule.ProposalCapsule; +import org.tron.core.config.args.Args; +import org.tron.core.db.api.EnergyPriceHistoryLoader; +import org.tron.core.services.jsonrpc.JsonRpcApiUtil; +import org.tron.protos.Protocol.Proposal; +import org.tron.protos.Protocol.Proposal.State; + +/** + * End-to-end regression test for equal-expiration proposal ordering. When two ENERGY_FEE proposals + * share an expiration time, the rebuilt price history must resolve to the lowest-id (final) value, + * matching live execution order. Guards the descending-id tie-break in + * {@link org.tron.core.store.ProposalStore#getSpecifiedProposals}. + */ +public class EnergyPriceHistoryEqualExpirationTest extends BaseTest { + + static { + Args.setParam(new String[] {"--output-directory", dbPath()}, TestConstants.TEST_CONF); + } + + private void initProposal(long code, long timestamp, long price, State state) { + long id = chainBaseManager.getDynamicPropertiesStore().getLatestProposalNum() + 1; + Proposal proposal = Proposal.newBuilder() + .putParameters(code, price) + .setExpirationTime(timestamp) + .setState(state) + .setProposalId(id) + .build(); + ProposalCapsule capsule = new ProposalCapsule(proposal); + chainBaseManager.getProposalStore().put(capsule.createDbKey(), capsule); + chainBaseManager.getDynamicPropertiesStore().saveLatestProposalNum(id); + } + + @Test + public void rebuildsLowestIdValueForEqualExpiration() { + long expiration = 1600000000000L; + long lowIdPrice = 15; + long highIdPrice = 99; + initProposal(ENERGY_FEE.getCode(), expiration, lowIdPrice, State.APPROVED); // id N (lower) + initProposal(ENERGY_FEE.getCode(), expiration, highIdPrice, State.APPROVED); // id N+1 (higher) + + EnergyPriceHistoryLoader loader = new EnergyPriceHistoryLoader(chainBaseManager); + loader.getEnergyProposals(); + String history = loader.parseProposalsToStr(); + + // Live execution applies highest id first and lowest id last, so the lowest id is the final + // value. The tail-first parseEnergyFee must resolve a post-expiration timestamp to that price. + Assert.assertEquals(lowIdPrice, JsonRpcApiUtil.parseEnergyFee(expiration + 1, history)); + } +} diff --git a/framework/src/test/java/org/tron/core/db/ExchangeStoreSortTest.java b/framework/src/test/java/org/tron/core/db/ExchangeStoreSortTest.java new file mode 100644 index 00000000000..63984bc5aee --- /dev/null +++ b/framework/src/test/java/org/tron/core/db/ExchangeStoreSortTest.java @@ -0,0 +1,74 @@ +package org.tron.core.db; + +import static java.util.stream.Collectors.toList; +import static org.junit.Assert.assertEquals; + +import com.google.protobuf.ByteString; +import java.util.Arrays; +import java.util.List; +import javax.annotation.Resource; +import org.junit.Test; +import org.tron.common.BaseTest; +import org.tron.common.TestConstants; +import org.tron.core.capsule.ExchangeCapsule; +import org.tron.core.config.args.Args; +import org.tron.core.store.ExchangeStore; +import org.tron.protos.Protocol; + +/** + * Sort-behaviour regression tests for {@link ExchangeStore#getAllExchanges()}. Guards the fix that + * replaced the never-return-0 comparator (`? 1 : -1`) with contract-safe + * {@code Comparator.comparingLong(...).reversed()} (createTime descending). + * + *

Each test uses a disjoint exchange-id range and asserts only on its own ids. + */ +public class ExchangeStoreSortTest extends BaseTest { + + @Resource + private ExchangeStore exchangeStore; + + static { + Args.setParam(new String[] {"--output-directory", dbPath()}, TestConstants.TEST_CONF); + } + + private void put(long id, long createTime) { + Protocol.Exchange exchange = Protocol.Exchange.newBuilder() + .setExchangeId(id) + .setCreateTime(createTime) + .setCreatorAddress(ByteString.copyFromUtf8("Address" + id)) + .build(); + ExchangeCapsule capsule = new ExchangeCapsule(exchange); + exchangeStore.put(capsule.createDbKey(), capsule); + } + + private List idsInRange(List list, long lo, long hi) { + return list.stream() + .map(ExchangeCapsule::getID) + .filter(id -> id >= lo && id <= hi) + .collect(toList()); + } + + @Test + public void getAllExchanges_ordersByCreateTimeDescending() { + put(101, 100L); + put(102, 300L); + put(103, 200L); + // newest (largest createTime) first + assertEquals(Arrays.asList(102L, 103L, 101L), + idsInRange(exchangeStore.getAllExchanges(), 101, 103)); + } + + @Test + public void getAllExchanges_manyEqualCreateTime_returnsAllWithoutThrowing() { + long lo = 1000; + // >= 32 elements exercises TimSort's merge path. All-equal keys form one run (no merge), so + // this does NOT reproduce the old never-return-0 throw -- that form is caught at compile time + // by ComparatorNeverReturnsZero. Here we only lock the runtime invariant: every equal-time + // exchange is returned, no exception. + int n = 40; + for (int i = 0; i < n; i++) { + put(lo + i, 500L); + } + assertEquals(n, idsInRange(exchangeStore.getAllExchanges(), lo, lo + n - 1).size()); + } +} diff --git a/framework/src/test/java/org/tron/core/db/ProposalStoreSortTest.java b/framework/src/test/java/org/tron/core/db/ProposalStoreSortTest.java new file mode 100644 index 00000000000..7b94fa86aa2 --- /dev/null +++ b/framework/src/test/java/org/tron/core/db/ProposalStoreSortTest.java @@ -0,0 +1,98 @@ +package org.tron.core.db; + +import static java.util.stream.Collectors.toList; +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.List; +import javax.annotation.Resource; +import org.junit.Test; +import org.tron.common.BaseTest; +import org.tron.common.TestConstants; +import org.tron.core.capsule.ProposalCapsule; +import org.tron.core.config.args.Args; +import org.tron.core.store.ProposalStore; +import org.tron.protos.Protocol.Proposal; +import org.tron.protos.Protocol.Proposal.State; + +/** + * Sort-behaviour regression tests for {@link ProposalStore}. Guards the fix that replaced the + * never-return-0 comparators (`? 1 : -1`) with contract-safe {@code Comparator.comparingLong}. + * + *

Each test uses a disjoint proposal-id range and asserts only on its own ids, so the tests are + * independent of each other despite BaseTest sharing one DB per class. + */ +public class ProposalStoreSortTest extends BaseTest { + + @Resource + private ProposalStore proposalStore; + + static { + Args.setParam(new String[] {"--output-directory", dbPath()}, TestConstants.TEST_CONF); + } + + private void put(long id, long createTime, long expirationTime, State state, int paramKey) { + Proposal proposal = Proposal.newBuilder() + .setProposalId(id) + .setCreateTime(createTime) + .setExpirationTime(expirationTime) + .setState(state) + .putParameters(paramKey, 1L) + .build(); + proposalStore.put(Long.toString(id).getBytes(), new ProposalCapsule(proposal)); + } + + private List idsInRange(List list, long lo, long hi) { + return list.stream() + .map(ProposalCapsule::getID) + .filter(id -> id >= lo && id <= hi) + .collect(toList()); + } + + @Test + public void getAllProposals_ordersByCreateTimeDescending() { + put(101, 100L, 0L, State.PENDING, 1); + put(102, 300L, 0L, State.PENDING, 1); + put(103, 200L, 0L, State.PENDING, 1); + // newest (largest createTime) first + assertEquals(Arrays.asList(102L, 103L, 101L), + idsInRange(proposalStore.getAllProposals(), 101, 103)); + } + + @Test + public void getSpecifiedProposals_ordersByExpirationAscending() { + put(201, 0L, 300L, State.PENDING, 7); + put(202, 0L, 100L, State.PENDING, 7); + put(203, 0L, 200L, State.PENDING, 7); + // soonest expiration first + assertEquals(Arrays.asList(202L, 203L, 201L), + idsInRange(proposalStore.getSpecifiedProposals(State.PENDING, 7), 201, 203)); + } + + @Test + public void getSpecifiedProposals_equalExpiration_breaksTiesByIdDescending() { + // Equal-expiration proposals must be returned highest-id-first. Live execution applies the + // highest id first and the lowest id last (final value), and the price-history loaders rebuild + // from the tail -- so the lowest id must be last. The pre-fix ascending-id order inverted this + // and reconstructed the wrong energy/bandwidth price. This test fails on that broken order. + put(401, 0L, 500L, State.APPROVED, 9); + put(402, 0L, 500L, State.APPROVED, 9); + put(403, 0L, 500L, State.APPROVED, 9); + assertEquals(Arrays.asList(403L, 402L, 401L), + idsInRange(proposalStore.getSpecifiedProposals(State.APPROVED, 9), 401, 403)); + } + + @Test + public void getAllProposals_manyEqualCreateTime_returnsAllWithoutThrowing() { + long lo = 1000; + // >= 32 elements exercises TimSort's merge path. All-equal keys form one run (no merge), so + // this does NOT reproduce the old never-return-0 throw -- that form is caught at compile time + // by ComparatorNeverReturnsZero. Here we only lock the runtime invariant: every equal-time + // proposal is returned, no exception. + int n = 40; + for (int i = 0; i < n; i++) { + put(lo + i, 500L, 0L, State.PENDING, 1); + } + assertEquals(n, idsInRange(proposalStore.getAllProposals(), lo, lo + n - 1).size()); + } +}