Apply suggestions from code review

Signed-off-by: Fabio Di Fabio <[email protected]>
Consensys · Jun 20, 2024 · b8b6f85 · b8b6f85
1 parent 9724f43
commit b8b6f85
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 18 deletions.
diff --git a/acceptance-tests/src/test/java/linea/plugin/acc/test/rpc/linea/SetExtraDataTest.java b/acceptance-tests/src/test/java/linea/plugin/acc/test/rpc/linea/SetExtraDataTest.java
@@ -29,6 +29,7 @@
 import linea.plugin.acc.test.TestCommandLineOptionsBuilder;
 import net.consensys.linea.config.LineaProfitabilityCliOptions;
 import net.consensys.linea.config.LineaProfitabilityConfiguration;
+import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
 import org.bouncycastle.crypto.digests.KeccakDigest;
 import org.hyperledger.besu.datatypes.Wei;
@@ -94,6 +95,24 @@ public void setUnsupportedExtraDataReturnsError() {
             "Unsupported extra data field 0x0000000000000000000000000000000000000000000000000000000000000000");
   }
 
+  @Test
+  public void setTooLongExtraDataReturnsError() {
+    final var tooLongExtraData = Bytes.concatenate(Bytes.of(1), Bytes32.ZERO);
+
+    final var reqLinea = new FailingLineaSetExtraDataRequest(tooLongExtraData);
+    final var respLinea = reqLinea.execute(minerNode.nodeRequests());
+    assertThat(respLinea.getMessage()).isEqualTo("Expected 32 bytes but got 33");
+  }
+
+  @Test
+  public void setTooShortExtraDataReturnsError() {
+    final var tooShortExtraData = Bytes32.ZERO.slice(1);
+
+    final var reqLinea = new FailingLineaSetExtraDataRequest(tooShortExtraData);
+    final var respLinea = reqLinea.execute(minerNode.nodeRequests());
+    assertThat(respLinea.getMessage()).isEqualTo("Expected 32 bytes but got 31");
+  }
+
   @Test
   public void successfulSetExtraData() {
     final var extraData =
@@ -219,9 +238,9 @@ public Boolean execute(final NodeRequests nodeRequests) {
 
   static class FailingLineaSetExtraDataRequest
       implements Transaction<org.web3j.protocol.core.Response.Error> {
-    private final Bytes32 extraData;
+    private final Bytes extraData;
 
-    public FailingLineaSetExtraDataRequest(final Bytes32 extraData) {
+    public FailingLineaSetExtraDataRequest(final Bytes extraData) {
       this.extraData = extraData;
     }
 

diff --git a/arithmetization/src/main/java/net/consensys/linea/extradata/LineaExtraDataHandler.java b/arithmetization/src/main/java/net/consensys/linea/extradata/LineaExtraDataHandler.java
@@ -36,14 +36,15 @@ public LineaExtraDataHandler(
       final RpcEndpointService rpcEndpointService,
       final LineaProfitabilityConfiguration profitabilityConf) {
     this.rpcEndpointService = rpcEndpointService;
-    extraDataConsumers = new ExtraDataConsumer[] {new Version1Consumer(profitabilityConf)};
+    this.extraDataConsumers = new ExtraDataConsumer[] {new Version1Consumer(profitabilityConf)};
   }
 
   public void handle(final Bytes rawExtraData) throws LineaExtraDataException {
 
     if (!Bytes.EMPTY.equals(rawExtraData)) {
       for (final ExtraDataConsumer extraDataConsumer : extraDataConsumers) {
         if (extraDataConsumer.canConsume(rawExtraData)) {
+          // strip first byte since it is the version already used to select the actual consumer
           final var extraData = rawExtraData.slice(1);
           extraDataConsumer.accept(extraData);
           return;
@@ -112,7 +113,7 @@ void updateMinGasPrice(final Long minGasPriceKWei) {
       if (!resp.getType().equals(JsonRpcResponseType.SUCCESS)) {
         throw new LineaExtraDataException(
             LineaExtraDataException.ErrorType.FAILED_CALLING_SET_MIN_GAS_PRICE,
-            "miner_setMinGasPrice failed: " + resp);
+            "Internal setMinGasPrice method failed: " + resp);
       }
     }
   }

diff --git a/arithmetization/src/main/java/net/consensys/linea/extradata/LineaExtraDataPlugin.java b/arithmetization/src/main/java/net/consensys/linea/extradata/LineaExtraDataPlugin.java
@@ -82,7 +82,7 @@ public void start() {
       }
       besuEventsService.addBlockAddedListener(
           addedBlockContext -> {
-            final var importedBlockHeader = blockchainService.getChainHeadHeader();
+            final var importedBlockHeader = addedBlockContext.getBlockHeader();
             final var latestExtraData = importedBlockHeader.getExtraData();
 
             try {

diff --git a/arithmetization/src/main/java/net/consensys/linea/rpc/linea/LineaSetExtraData.java b/arithmetization/src/main/java/net/consensys/linea/rpc/linea/LineaSetExtraData.java
@@ -15,6 +15,9 @@
 
 package net.consensys.linea.rpc.linea;
 
+import static net.consensys.linea.extradata.LineaExtraDataException.ErrorType.FAILED_CALLING_SET_EXTRA_DATA;
+import static net.consensys.linea.extradata.LineaExtraDataException.ErrorType.INVALID_ARGUMENT;
+
 import java.util.concurrent.atomic.AtomicInteger;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
@@ -62,9 +65,9 @@ public Boolean execute(final PluginRpcRequest request) {
       LOG_SEQUENCE.incrementAndGet();
     }
 
-    final var extraData = parseRequest(request.getParams());
-
     try {
+      final var extraData = parseRequest(request.getParams());
+
       updatePricingConf(extraData);
 
       updateStandardExtraData(extraData);
@@ -80,8 +83,7 @@ private void updateStandardExtraData(final Bytes32 extraData) {
         rpcEndpointService.call("miner_setExtraData", new Object[] {extraData.toHexString()});
     if (!resp.getType().equals(JsonRpcResponseType.SUCCESS)) {
       throw new LineaExtraDataException(
-          LineaExtraDataException.ErrorType.FAILED_CALLING_SET_EXTRA_DATA,
-          "miner_setExtraData failed: " + resp);
+          FAILED_CALLING_SET_EXTRA_DATA, "Internal setExtraData method failed: " + resp);
     }
   }
 
@@ -94,15 +96,19 @@ private void updatePricingConf(final Bytes32 extraData) {
   }
 
   private Bytes32 parseRequest(final Object[] params) {
-    final var rawParam = parameterParser.required(params, 0, String.class);
-    final var extraData = Bytes32.wrap(Bytes.fromHexStringLenient(rawParam));
-    log.atDebug()
-        .setMessage("[{}] set extra data, raw=[{}] parsed=[{}]")
-        .addArgument(LOG_SEQUENCE::get)
-        .addArgument(rawParam)
-        .addArgument(extraData::toHexString)
-        .log();
-    return extraData;
+    try {
+      final var rawParam = parameterParser.required(params, 0, String.class);
+      final var extraData = Bytes32.wrap(Bytes.fromHexStringLenient(rawParam));
+      log.atDebug()
+          .setMessage("[{}] set extra data, raw=[{}] parsed=[{}]")
+          .addArgument(LOG_SEQUENCE::get)
+          .addArgument(rawParam)
+          .addArgument(extraData::toHexString)
+          .log();
+      return extraData;
+    } catch (Exception e) {
+      throw new LineaExtraDataException(INVALID_ARGUMENT, e.getMessage());
+    }
   }
 
   public record Response(