From 61d6f54233373b1c324f3633656c377bba933e8e Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 19 Mar 2024 11:57:04 +0100 Subject: [PATCH] Improve linea_estimateGas error response Signed-off-by: Fabio Di Fabio --- .../acc/test/rpc/linea/EstimateGasTest.java | 66 ++++++++++++++ .../rpc/counters/GenerateCountersV0.java | 3 +- .../linea/rpc/linea/LineaEstimateGas.java | 86 ++++++++++++++----- .../GenerateConflatedTracesV0.java | 3 +- gradle.properties | 2 +- native/Dockerfile-win-dockcross | 5 +- native/wsl.sh | 15 +++- 7 files changed, 149 insertions(+), 31 deletions(-) diff --git a/acceptance-tests/src/test/java/linea/plugin/acc/test/rpc/linea/EstimateGasTest.java b/acceptance-tests/src/test/java/linea/plugin/acc/test/rpc/linea/EstimateGasTest.java index 1f4493a600..d27b0a2b01 100644 --- a/acceptance-tests/src/test/java/linea/plugin/acc/test/rpc/linea/EstimateGasTest.java +++ b/acceptance-tests/src/test/java/linea/plugin/acc/test/rpc/linea/EstimateGasTest.java @@ -18,8 +18,13 @@ import java.io.IOException; import java.math.BigInteger; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.Map; import linea.plugin.acc.test.LineaPluginTestBase; import linea.plugin.acc.test.TestCommandLineOptionsBuilder; @@ -32,12 +37,15 @@ import org.bouncycastle.crypto.digests.KeccakDigest; import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; import org.hyperledger.besu.tests.acceptance.dsl.account.Account; import org.hyperledger.besu.tests.acceptance.dsl.transaction.NodeRequests; import org.hyperledger.besu.tests.acceptance.dsl.transaction.Transaction; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.web3j.protocol.core.Request; +import org.web3j.protocol.core.Response; +import org.web3j.protocol.http.HttpService; public class EstimateGasTest extends LineaPluginTestBase { protected static final int VERIFICATION_GAS_COST = 1_200_000; @@ -183,6 +191,36 @@ public void lineaEstimateGasPriorityFeeMinGasPriceLowerBound() { assertMinGasPriceLowerBound(baseFee, estimatedMaxGasPrice); } + @Test + public void invalidParametersLineaEstimateGasRequestReturnErrorResponse() { + final var reqLinea = + new BadLineaEstimateGasRequest(Map.of("gasLimit", String.valueOf(Integer.MAX_VALUE))); + final var respLinea = reqLinea.execute(minerNode.nodeRequests()); + assertThat(respLinea.getCode()).isEqualTo(RpcErrorType.INVALID_PARAMS.getCode()); + assertThat(respLinea.getMessage()).isEqualTo(RpcErrorType.INVALID_PARAMS.getMessage()); + } + + @Test + public void parseErrorLineaEstimateGasRequestReturnErrorResponse() + throws IOException, InterruptedException { + final var httpService = (HttpService) minerNode.nodeRequests().getWeb3jService(); + final var httpClient = HttpClient.newHttpClient(); + final var badJsonRequest = + HttpRequest.newBuilder(URI.create(httpService.getUrl())) + .headers("Content-Type", "application/json") + .POST( + HttpRequest.BodyPublishers.ofString( + """ + {"jsonrpc":"2.0","method":"linea_estimateGas","params":[malformed json],"id":53} + """)) + .build(); + final var errorResponse = httpClient.send(badJsonRequest, HttpResponse.BodyHandlers.ofString()); + assertThat(errorResponse.body()) + .isEqualTo( + """ + {"jsonrpc":"2.0","id":null,"error":{"code":-32700,"message":"Parse error"}}"""); + } + protected void assertMinGasPriceLowerBound(final Wei baseFee, final Wei estimatedMaxGasPrice) { final var minGasPrice = minerNode.getMiningParameters().getMinTransactionGasPrice(); assertThat(estimatedMaxGasPrice).isEqualTo(minGasPrice); @@ -215,6 +253,34 @@ static class LineaEstimateGasResponse extends org.web3j.protocol.core.Response { + private final Map badCallParams; + + public BadLineaEstimateGasRequest(final Map badCallParams) { + this.badCallParams = badCallParams; + } + + @Override + public org.web3j.protocol.core.Response.Error execute(final NodeRequests nodeRequests) { + try { + return new Request<>( + "linea_estimateGas", + List.of(badCallParams), + nodeRequests.getWeb3jService(), + BadLineaEstimateGasResponse.class) + .send() + .getError(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + static class BadLineaEstimateGasResponse extends org.web3j.protocol.core.Response {} + + record Response(String gasLimit, String baseFeePerGas, String priorityFeePerGas) {} + } + static class RawEstimateGasRequest implements Transaction { private final CallParams callParams; diff --git a/arithmetization/src/main/java/net/consensys/linea/rpc/counters/GenerateCountersV0.java b/arithmetization/src/main/java/net/consensys/linea/rpc/counters/GenerateCountersV0.java index 916ea1fa93..98a770e4a3 100644 --- a/arithmetization/src/main/java/net/consensys/linea/rpc/counters/GenerateCountersV0.java +++ b/arithmetization/src/main/java/net/consensys/linea/rpc/counters/GenerateCountersV0.java @@ -22,6 +22,7 @@ import com.google.common.cache.CacheBuilder; import lombok.extern.slf4j.Slf4j; import net.consensys.linea.zktracer.ZkTracer; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; import org.hyperledger.besu.plugin.BesuContext; import org.hyperledger.besu.plugin.services.TraceService; import org.hyperledger.besu.plugin.services.exception.PluginRpcEndpointException; @@ -97,7 +98,7 @@ public Counters execute(final PluginRpcRequest request) { log.info("counters for {} returned in {}", requestedBlockNumber, sw); return r; } catch (Exception ex) { - throw new PluginRpcEndpointException(ex.getMessage()); + throw new PluginRpcEndpointException(RpcErrorType.PLUGIN_INTERNAL_ERROR, ex.getMessage()); } } diff --git a/arithmetization/src/main/java/net/consensys/linea/rpc/linea/LineaEstimateGas.java b/arithmetization/src/main/java/net/consensys/linea/rpc/linea/LineaEstimateGas.java index 23b214336e..409c37ec75 100644 --- a/arithmetization/src/main/java/net/consensys/linea/rpc/linea/LineaEstimateGas.java +++ b/arithmetization/src/main/java/net/consensys/linea/rpc/linea/LineaEstimateGas.java @@ -20,6 +20,7 @@ import java.math.BigDecimal; import java.math.BigInteger; import java.math.RoundingMode; +import java.util.Optional; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.annotations.VisibleForTesting; @@ -37,12 +38,15 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.exception.InvalidJsonRpcParameters; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonCallParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonRpcParameter; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.evm.tracing.EstimateGasOperationTracer; import org.hyperledger.besu.plugin.services.BesuConfiguration; import org.hyperledger.besu.plugin.services.BlockchainService; import org.hyperledger.besu.plugin.services.TransactionSimulationService; +import org.hyperledger.besu.plugin.services.exception.PluginRpcEndpointException; import org.hyperledger.besu.plugin.services.rpc.PluginRpcRequest; +import org.hyperledger.besu.plugin.services.rpc.RpcMethodError; @Slf4j public class LineaEstimateGas { @@ -117,7 +121,10 @@ public LineaEstimateGas.Response execute(final PluginRpcRequest request) { final Wei baseFee = blockchainService .getNextBlockBaseFee() - .orElseThrow(() -> new IllegalStateException("Not on a baseFee market")); + .orElseThrow( + () -> + new PluginRpcEndpointException( + RpcErrorType.INVALID_REQUEST, "Not on a baseFee market")); final Wei estimatedPriorityFee = getEstimatedPriorityFee(transaction, baseFee, minGasPrice, estimatedGasUsed); @@ -174,19 +181,32 @@ private Long estimateGasUsed( return maybeSimulationResults .map( r -> { - - // if the transaction is invalid or doesn't have enough gas with the max it never - // will! - if (r.isInvalid() || !r.isSuccessful()) { + // if the transaction is invalid or doesn't have enough gas with the max it never will + if (r.isInvalid()) { log.atDebug() - .setMessage("Invalid or unsuccessful transaction {}, reason {}") + .setMessage("Invalid transaction {}, reason {}") .addArgument(transaction::toTraceLog) .addArgument(r.result()) .log(); + throw new PluginRpcEndpointException( + new InvalidTransactionError(r.result().getInvalidReason())); + } + if (!r.isSuccessful()) { + log.atDebug() + .setMessage("Failed transaction {}, reason {}") + .addArgument(transaction::toTraceLog) + .addArgument(r.result()) + .log(); + r.getRevertReason() + .ifPresent( + rr -> { + throw new PluginRpcEndpointException( + RpcErrorType.REVERT_ERROR, rr.toHexString()); + }); final var invalidReason = r.result().getInvalidReason(); - throw new RuntimeException( - "Invalid or unsuccessful transaction" - + invalidReason.map(ir -> ", reason: " + ir).orElse("")); + throw new PluginRpcEndpointException( + RpcErrorType.PLUGIN_INTERNAL_ERROR, + "Failed transaction" + invalidReason.map(ir -> ", reason: " + ir).orElse("")); } final var lowGasEstimation = r.result().getEstimateGasUsedByTransaction(); @@ -200,8 +220,7 @@ private Long estimateGasUsed( return lowResult .map( lr -> { - // if with the low estimation gas is successful the return this - // estimation + // if with the low estimation gas is successful the return this estimation if (lr.isSuccessful()) { log.trace( "Low gas estimation {} successful, call params {}", @@ -232,11 +251,12 @@ private Long estimateGasUsed( if (binarySearchResult.isEmpty() || !binarySearchResult.get().isSuccessful()) { - low = mid; log.atTrace() .setMessage( - "Binary gas estimation search low={},med={},high={}, unsuccessful result {}, call params {}") - .addArgument(lowGasEstimation) + "Binary gas estimation search low={},mid={},high={}, unsuccessful result {}, call params {}") + .addArgument(low) + .addArgument(mid) + .addArgument(high) .addArgument( () -> binarySearchResult @@ -244,23 +264,29 @@ private Long estimateGasUsed( .orElse("empty")) .addArgument(callParameters) .log(); - + low = mid; } else { - high = mid; log.trace( - "Binary gas estimation search low={},med={},high={}, successful, call params {}", + "Binary gas estimation search low={},mid={},high={}, successful, call params {}", low, mid, high, callParameters); + high = mid; } } return high; } }) - .orElseThrow(); + .orElseThrow( + () -> + new PluginRpcEndpointException( + RpcErrorType.PLUGIN_INTERNAL_ERROR, "Empty result from simulation")); }) - .orElseThrow(); + .orElseThrow( + () -> + new PluginRpcEndpointException( + RpcErrorType.PLUGIN_INTERNAL_ERROR, "Empty result from simulation")); } private JsonCallParameter parseRequest(final Object[] params) { @@ -272,14 +298,16 @@ private JsonCallParameter parseRequest(final Object[] params) { private void validateParameters(final JsonCallParameter callParameters) { if (callParameters.getGasPrice() != null && (callParameters.getMaxFeePerGas().isPresent() - || callParameters.getMaxPriorityFeePerGas().isPresent())) { + || callParameters.getMaxPriorityFeePerGas().isPresent() + || callParameters.getMaxFeePerBlobGas().isPresent())) { throw new InvalidJsonRpcParameters( - "gasPrice cannot be used with maxFeePerGas or maxPriorityFeePerGas"); + "gasPrice cannot be used with maxFeePerGas or maxPriorityFeePerGas or maxFeePerBlobGas"); } if (callParameters.getGasLimit() > 0 && callParameters.getGasLimit() > txValidatorConf.maxTxGasLimit()) { - throw new InvalidJsonRpcParameters("gasLimit above maximum"); + throw new InvalidJsonRpcParameters( + "gasLimit above maximum of: " + txValidatorConf.maxTxGasLimit()); } } @@ -327,4 +355,18 @@ public record Response( @JsonProperty String gasLimit, @JsonProperty String baseFeePerGas, @JsonProperty String priorityFeePerGas) {} + + private record InvalidTransactionError(Optional maybeInvalidReason) + implements RpcMethodError { + + @Override + public int getCode() { + return -32000; + } + + @Override + public String getMessage() { + return maybeInvalidReason.orElse(""); + } + } } diff --git a/arithmetization/src/main/java/net/consensys/linea/rpc/tracegeneration/GenerateConflatedTracesV0.java b/arithmetization/src/main/java/net/consensys/linea/rpc/tracegeneration/GenerateConflatedTracesV0.java index 052851ea4b..874bed3041 100644 --- a/arithmetization/src/main/java/net/consensys/linea/rpc/tracegeneration/GenerateConflatedTracesV0.java +++ b/arithmetization/src/main/java/net/consensys/linea/rpc/tracegeneration/GenerateConflatedTracesV0.java @@ -22,6 +22,7 @@ import com.google.common.base.Stopwatch; import lombok.extern.slf4j.Slf4j; import net.consensys.linea.zktracer.ZkTracer; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; import org.hyperledger.besu.plugin.BesuContext; import org.hyperledger.besu.plugin.services.BesuConfiguration; import org.hyperledger.besu.plugin.services.TraceService; @@ -85,7 +86,7 @@ public TraceFile execute(final PluginRpcRequest request) { log.info("[TRACING] trace for {}-{} serialized to {} in {}", path, toBlock, fromBlock, sw); return new TraceFile(params.runtimeVersion(), path); } catch (Exception ex) { - throw new PluginRpcEndpointException(ex.getMessage()); + throw new PluginRpcEndpointException(RpcErrorType.PLUGIN_INTERNAL_ERROR, ex.getMessage()); } } diff --git a/gradle.properties b/gradle.properties index f5cc1daffd..80f6c4e31e 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,5 @@ releaseVersion=0.1.4-SNAPSHOT -besuVersion=24.2.0-SNAPSHOT +besuVersion=24.3-develop-5a0618f151 besuArtifactGroup=io.consensys.linea-besu distributionIdentifier=besu-sequencer-plugins distributionBaseUrl=https://artifacts.consensys.net/public/linea-besu/raw/names/linea-besu.tar.gz/versions/ diff --git a/native/Dockerfile-win-dockcross b/native/Dockerfile-win-dockcross index 4a3a393854..8e511f8f4d 100644 --- a/native/Dockerfile-win-dockcross +++ b/native/Dockerfile-win-dockcross @@ -1,9 +1,10 @@ -FROM dockcross/windows-shared-x64 +ARG IMAGE +FROM dockcross/$IMAGE ARG GO_VERSION=1.22.1 ARG GO_ARCHIVE_CHECKSUM=aab8e15785c997ae20f9c88422ee35d962c4562212bb0f879d052a35c8307c7f -ENV DEFAULT_DOCKCROSS_IMAGE native-windows-cross-compile +ENV DEFAULT_DOCKCROSS_IMAGE native-${IMAGE}-cross-compile RUN wget https://go.dev/dl/go$GO_VERSION.linux-amd64.tar.gz RUN echo "$GO_ARCHIVE_CHECKSUM go$GO_VERSION.linux-amd64.tar.gz" | sha256sum --check --status diff --git a/native/wsl.sh b/native/wsl.sh index a91bafefc9..64387d15aa 100644 --- a/native/wsl.sh +++ b/native/wsl.sh @@ -1,10 +1,17 @@ -#!/bin/bash +#!/bin/bash -x -docker build -f Dockerfile-win-dockcross -t native-windows-cross-compile . +docker build -f Dockerfile-win-dockcross --build-arg IMAGE=windows-shared-x64 -t native-windows-shared-x64-cross-compile . +docker build -f Dockerfile-win-dockcross --build-arg IMAGE=linux-x64 -t native-linux-x64-cross-compile . -docker run --rm native-windows-cross-compile > compress/build/native/native-windows-cross-compile +mkdir -p compress/build/native/ -compress/build/native/native-windows-cross-compile \ +docker run --rm native-windows-shared-x64-cross-compile > compress/build/native/native-windows-shared-x64-cross-compile +docker run --rm native-linux-x64-cross-compile > compress/build/native/native-linux-x64-cross-compile + +compress/build/native/native-windows-shared-x64-cross-compile --image native-windows-shared-x64-cross-compile \ bash -c "cd compress/compress-jni && CGO_ENABLED=1 GOOS=windows GOARCH=amd64 go build -buildmode=c-shared -o ../build/native/compress_jni.dll compress-jni.go" +compress/build/native/native-linux-x64-cross-compile --image native-linux-x64-cross-compile \ + bash -c "cd compress/compress-jni && + CGO_ENABLED=1 go build -buildmode=c-shared -o ../build/native/libcompress_jni.so compress-jni.go"