From d08229f2c0647ac86839a38ce73c4ec7dce66b04 Mon Sep 17 00:00:00 2001 From: Reynold Morel Date: Thu, 14 Nov 2024 18:05:36 -0400 Subject: [PATCH 1/7] Handling transaction revert in eth_estimateGas --- .../co/rsk/rpc/modules/eth/EthModule.java | 36 +++++++++++-------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java b/rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java index b7857351509..1af54573ebe 100644 --- a/rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java +++ b/rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java @@ -146,20 +146,7 @@ public String call(CallArgumentsParam argsParam, BlockIdentifierParam bnOrId) { } else { res = callConstant(args, block); } - if (res.isRevert()) { - Pair programRevert = decodeProgramRevert(res); - String revertReason = programRevert.getLeft(); - byte[] revertData = programRevert.getRight(); - if (revertData == null) { - throw RskJsonRpcRequestException.transactionRevertedExecutionError(); - } - - if (revertReason == null) { - throw RskJsonRpcRequestException.transactionRevertedExecutionError(revertData); - } - - throw RskJsonRpcRequestException.transactionRevertedExecutionError(revertReason, revertData); - } + handleTransactionRevert(res); hReturn = HexUtils.toUnformattedJsonHex(res.getHReturn()); return hReturn; @@ -193,6 +180,9 @@ public String estimateGas(CallArgumentsParam args, @Nonnull BlockIdentifierParam snapshot ); + ProgramResult res = executor.getResult(); + handleTransactionRevert(res); + estimation = internalEstimateGas(executor.getResult()); return estimation; @@ -357,4 +347,22 @@ private ProgramResult callConstantWithState(CallArguments args, Block executionB hexArgs.getFromAddress() ); } + + private void handleTransactionRevert(ProgramResult res) { + if (res.isRevert()) { + Pair programRevert = decodeProgramRevert(res); + String revertReason = programRevert.getLeft(); + byte[] revertData = programRevert.getRight(); + + if (revertData == null) { + throw RskJsonRpcRequestException.transactionRevertedExecutionError(); + } + + if (revertReason == null) { + throw RskJsonRpcRequestException.transactionRevertedExecutionError(revertData); + } + + throw RskJsonRpcRequestException.transactionRevertedExecutionError(revertReason, revertData); + } + } } From 5d0c30692dbd95550f8beb06a0e28329989e084d Mon Sep 17 00:00:00 2001 From: Nazaret Garcia Date: Tue, 29 Oct 2024 19:06:29 -0300 Subject: [PATCH 2/7] Updating smell test --- .github/workflows/build_and_test.yml | 36 ++++------------------------ build.gradle | 8 +++++++ 2 files changed, 13 insertions(+), 31 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 2601e4e3d7d..8eb14e3d014 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -101,40 +101,14 @@ jobs: - name: Run SonarQube analysis env: - GH_EVENT: ${{ github.event_name }} - GH_PR_NUMBER: ${{ github.event.pull_request.number }} - GH_PR_BASE_REF: ${{ github.base_ref }} - GH_PR_HEAD_REF: ${{ github.head_ref }} GH_REF: ${{ github.ref }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - IS_FORK: ${{ github.event.pull_request.head.repo.fork }} run: | - if [ "$GH_EVENT" = "pull_request" ]; then - if [ "$IS_FORK" != "true" ]; then - ./gradlew -Dorg.gradle.jvmargs=-Xmx5g sonarqube --no-daemon -x build -x test \ - -Dsonar.pullrequest.base="$GH_PR_BASE_REF" \ - -Dsonar.pullrequest.branch="$GH_PR_HEAD_REF" \ - -Dsonar.pullrequest.key="$GH_PR_NUMBER" \ - -Dsonar.organization=rsksmart \ - -Dsonar.projectKey=rskj \ - -Dsonar.host.url="https://sonarcloud.io" \ - -Dsonar.junit.reportPaths=rskj-core/build/test-results/ \ - -Dsonar.coverage.jacoco.xmlReportPaths=rskj-core/build/reports/jacoco/test/jacocoTestReport.xml \ - -Dsonar.token="$SONAR_TOKEN" - else - echo "Skipping SonarQube analysis for pull request from a forked repo." - fi - else - ./gradlew -Dorg.gradle.jvmargs=-Xmx5g sonarqube --no-daemon -x build -x test \ - -Dsonar.branch.name="$GH_REF" \ - -Dsonar.organization=rsksmart \ - -Dsonar.projectKey=rskj \ - -Dsonar.host.url="https://sonarcloud.io" \ - -Dsonar.junit.reportPaths=rskj-core/build/test-results/ \ - -Dsonar.coverage.jacoco.xmlReportPaths=rskj-core/build/reports/jacoco/test/jacocoTestReport.xml \ - -Dsonar.token="$SONAR_TOKEN" - fi - + ./gradlew -Dorg.gradle.jvmargs=-Xmx5g sonar --no-daemon -x build -x test \ + -Dsonar.branch.name="$GH_REF" \ + -Dsonar.junit.reportPaths=rskj-core/build/test-results/ \ + -Dsonar.coverage.jacoco.xmlReportPaths=rskj-core/build/reports/jacoco/test/jacocoTestReport.xml \ + -Dsonar.token="$SONAR_TOKEN" mining-tests: needs: build diff --git a/build.gradle b/build.gradle index 9a423cc072c..7047a1e84cd 100644 --- a/build.gradle +++ b/build.gradle @@ -2,6 +2,14 @@ plugins { id "org.sonarqube" version "5.1.0.4882" } +sonar { + properties { + property "sonar.projectKey", "rskj" + property "sonar.organization", "rsksmart" + property "sonar.host.url", "https://sonarcloud.io" + } +} + subprojects { def config = new ConfigSlurper().parse(file("$projectDir/src/main/resources/version.properties").toURI().toURL()) group = 'co.rsk' From 160bac9e8462db8fb941c4af1e3920f1d4bd38bf Mon Sep 17 00:00:00 2001 From: Nazaret Garcia Date: Tue, 29 Oct 2024 19:30:02 -0300 Subject: [PATCH 3/7] Adding tmp code --- .../java/co/rsk/rpc/modules/debug/DebugModuleImpl.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/rskj-core/src/main/java/co/rsk/rpc/modules/debug/DebugModuleImpl.java b/rskj-core/src/main/java/co/rsk/rpc/modules/debug/DebugModuleImpl.java index 62be8e6e55c..ffaa5a3278d 100644 --- a/rskj-core/src/main/java/co/rsk/rpc/modules/debug/DebugModuleImpl.java +++ b/rskj-core/src/main/java/co/rsk/rpc/modules/debug/DebugModuleImpl.java @@ -143,6 +143,16 @@ private JsonNode traceBlock(Block block, TraceOptions options) { return programTraceProcessor.getProgramTracesAsJsonNode(txHashes); } + private boolean testFunctionNotCovered() { + boolean var1 = false; + boolean var2 = true; + boolean var3 = false; + if (!var1 && var2 || !var3) { + return true; + } + return false; + } + private TraceOptions toTraceOptions(Map traceOptions) { TraceOptions options = new TraceOptions(traceOptions); From 21ef867dcdeb2c700be80b9c3620850bfa43d49e Mon Sep 17 00:00:00 2001 From: Nazaret Garcia Date: Sun, 10 Nov 2024 11:05:15 -0300 Subject: [PATCH 4/7] Adding pointers to PR --- .github/workflows/build_and_test.yml | 20 +++++++++++++++----- build.gradle | 2 ++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 8eb14e3d014..f7f7ede65e2 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -101,14 +101,24 @@ jobs: - name: Run SonarQube analysis env: + GH_EVENT: ${{ github.event_name }} + GH_PR_NUMBER: ${{ github.event.pull_request.number }} + GH_PR_BASE_REF: ${{ github.base_ref }} + GH_PR_HEAD_REF: ${{ github.head_ref }} GH_REF: ${{ github.ref }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} run: | - ./gradlew -Dorg.gradle.jvmargs=-Xmx5g sonar --no-daemon -x build -x test \ - -Dsonar.branch.name="$GH_REF" \ - -Dsonar.junit.reportPaths=rskj-core/build/test-results/ \ - -Dsonar.coverage.jacoco.xmlReportPaths=rskj-core/build/reports/jacoco/test/jacocoTestReport.xml \ - -Dsonar.token="$SONAR_TOKEN" + if [ "$GH_EVENT" = "pull_request" ]; then # If this is a PR, then add pointers to it + ./gradlew -Dorg.gradle.jvmargs=-Xmx5g sonar --no-daemon -x build -x test \ + -Dsonar.pullrequest.base="$GH_PR_BASE_REF" \ + -Dsonar.pullrequest.branch="$GH_PR_HEAD_REF" \ + -Dsonar.pullrequest.key="$GH_PR_NUMBER" \ + -Dsonar.token="$SONAR_TOKEN" + else + ./gradlew -Dorg.gradle.jvmargs=-Xmx5g sonar --no-daemon -x build -x test \ + -Dsonar.branch.name="$GH_REF" \ + -Dsonar.token="$SONAR_TOKEN" + fi mining-tests: needs: build diff --git a/build.gradle b/build.gradle index 7047a1e84cd..66ca08e5b93 100644 --- a/build.gradle +++ b/build.gradle @@ -7,6 +7,8 @@ sonar { property "sonar.projectKey", "rskj" property "sonar.organization", "rsksmart" property "sonar.host.url", "https://sonarcloud.io" + property "sonar.junit.reportPaths", "rskj-core/build/test-results/" + property "sonar.coverage.jacoco.xmlReportPaths", "rskj-core/build/reports/jacoco/test/jacocoTestReport.xml" } } From 93da37b429371eb8d4044d0f89ef6ca130b5ce63 Mon Sep 17 00:00:00 2001 From: Nazaret Garcia Date: Sun, 10 Nov 2024 11:33:24 -0300 Subject: [PATCH 5/7] Removing test function --- .../java/co/rsk/rpc/modules/debug/DebugModuleImpl.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/rpc/modules/debug/DebugModuleImpl.java b/rskj-core/src/main/java/co/rsk/rpc/modules/debug/DebugModuleImpl.java index ffaa5a3278d..62be8e6e55c 100644 --- a/rskj-core/src/main/java/co/rsk/rpc/modules/debug/DebugModuleImpl.java +++ b/rskj-core/src/main/java/co/rsk/rpc/modules/debug/DebugModuleImpl.java @@ -143,16 +143,6 @@ private JsonNode traceBlock(Block block, TraceOptions options) { return programTraceProcessor.getProgramTracesAsJsonNode(txHashes); } - private boolean testFunctionNotCovered() { - boolean var1 = false; - boolean var2 = true; - boolean var3 = false; - if (!var1 && var2 || !var3) { - return true; - } - return false; - } - private TraceOptions toTraceOptions(Map traceOptions) { TraceOptions options = new TraceOptions(traceOptions); From 2aa85a5b7d8f6d455787cd480dcfa7df906fd0e3 Mon Sep 17 00:00:00 2001 From: Nazaret Garcia Date: Mon, 11 Nov 2024 19:40:52 -0300 Subject: [PATCH 6/7] Updating conditionals --- .github/workflows/build_and_test.yml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index f7f7ede65e2..19e3fe6afd8 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -108,12 +108,16 @@ jobs: GH_REF: ${{ github.ref }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} run: | - if [ "$GH_EVENT" = "pull_request" ]; then # If this is a PR, then add pointers to it - ./gradlew -Dorg.gradle.jvmargs=-Xmx5g sonar --no-daemon -x build -x test \ - -Dsonar.pullrequest.base="$GH_PR_BASE_REF" \ - -Dsonar.pullrequest.branch="$GH_PR_HEAD_REF" \ - -Dsonar.pullrequest.key="$GH_PR_NUMBER" \ - -Dsonar.token="$SONAR_TOKEN" + if [ "$GH_EVENT" = "pull_request" ]; then + if [ "$SONAR_TOKEN" != "" ]; then + ./gradlew -Dorg.gradle.jvmargs=-Xmx5g sonar --no-daemon -x build -x test \ + -Dsonar.pullrequest.base="$GH_PR_BASE_REF" \ + -Dsonar.pullrequest.branch="$GH_PR_HEAD_REF" \ + -Dsonar.pullrequest.key="$GH_PR_NUMBER" \ + -Dsonar.token="$SONAR_TOKEN" + else + echo "Skipping SonarQube analysis." + fi else ./gradlew -Dorg.gradle.jvmargs=-Xmx5g sonar --no-daemon -x build -x test \ -Dsonar.branch.name="$GH_REF" \ From f79732dc06f9a3ff7dc2d044d9632a79d2fde02d Mon Sep 17 00:00:00 2001 From: Reynold Morel Date: Mon, 18 Nov 2024 08:53:26 -0400 Subject: [PATCH 7/7] Addressing comments --- .../co/rsk/rpc/modules/eth/EthModule.java | 6 +-- .../co/rsk/rpc/modules/eth/EthModuleTest.java | 43 +++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java b/rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java index 1af54573ebe..c2dcbb2d2b2 100644 --- a/rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java +++ b/rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java @@ -146,7 +146,7 @@ public String call(CallArgumentsParam argsParam, BlockIdentifierParam bnOrId) { } else { res = callConstant(args, block); } - handleTransactionRevert(res); + handleTransactionRevertIfHappens(res); hReturn = HexUtils.toUnformattedJsonHex(res.getHReturn()); return hReturn; @@ -181,7 +181,7 @@ public String estimateGas(CallArgumentsParam args, @Nonnull BlockIdentifierParam ); ProgramResult res = executor.getResult(); - handleTransactionRevert(res); + handleTransactionRevertIfHappens(res); estimation = internalEstimateGas(executor.getResult()); @@ -348,7 +348,7 @@ private ProgramResult callConstantWithState(CallArguments args, Block executionB ); } - private void handleTransactionRevert(ProgramResult res) { + private void handleTransactionRevertIfHappens(ProgramResult res) { if (res.isRevert()) { Pair programRevert = decodeProgramRevert(res); String revertReason = programRevert.getLeft(); diff --git a/rskj-core/src/test/java/co/rsk/rpc/modules/eth/EthModuleTest.java b/rskj-core/src/test/java/co/rsk/rpc/modules/eth/EthModuleTest.java index 77c4f73a2d2..07b1db1adad 100644 --- a/rskj-core/src/test/java/co/rsk/rpc/modules/eth/EthModuleTest.java +++ b/rskj-core/src/test/java/co/rsk/rpc/modules/eth/EthModuleTest.java @@ -605,6 +605,49 @@ void whenExecuteEstimateGasWithDataParameter_callExecutorWithData() { assertArrayEquals(HexUtils.strHexOrStrNumberToByteArray(args.getData()), dataCaptor.getValue()); } + @Test + void testwhenExecuteEstimateGasWithDataParameter_callExecutorWithData() { + CallArguments args = new CallArguments(); + Block block = mock(Block.class); + ExecutionBlockRetriever.Result blockResult = mock(ExecutionBlockRetriever.Result.class); + when(blockResult.getBlock()).thenReturn(block); + ExecutionBlockRetriever retriever = mock(ExecutionBlockRetriever.class); + when(retriever.retrieveExecutionBlock("latest")).thenReturn(blockResult); + Blockchain blockchain = mock(Blockchain.class); + + ProgramResult executorResult = mock(ProgramResult.class); + when(executorResult.isRevert()).thenReturn(true); + TransactionExecutor transactionExecutor = mock(TransactionExecutor.class); + when(transactionExecutor.getResult()) + .thenReturn(executorResult); + + ReversibleTransactionExecutor reversibleTransactionExecutor = mock(ReversibleTransactionExecutor.class); + when(reversibleTransactionExecutor.estimateGas(eq(block), any(), any(), any(), any(), any(), any(), any(), any())) + .thenReturn(transactionExecutor); + + EthModule eth = new EthModule( + null, + Constants.REGTEST_CHAIN_ID, + blockchain, + null, + reversibleTransactionExecutor, + retriever, + mock(RepositoryLocator.class), + null, + null, + new BridgeSupportFactory( + null, null, null, signatureCache), + config.getGasEstimationCap(), + config.getCallGasCap()); + + CallArgumentsParam callArgumentsParam = TransactionFactoryHelper.toCallArgumentsParam(args); + + RskJsonRpcRequestException exception = assertThrows(RskJsonRpcRequestException.class, () -> { + eth.estimateGas(callArgumentsParam, new BlockIdentifierParam("latest")); + }); + assertThat(exception.getMessage(), Matchers.containsString("transaction reverted")); + } + @Test void whenExecuteEstimateGasWithInputParameter_callExecutorWithInput() { CallArguments args = new CallArguments();