fix: line count discrepancy (#468)

Signed-off-by: franklin.delehelle@consensys.net
Consensys · Dec 6, 2023 · 18236a9 · 18236a9
1 parent 8392a83
commit 18236a9
Show file tree

Hide file tree

Showing 11 changed files with 116 additions and 32 deletions.
diff --git a/arithmetization/src/main/java/net/consensys/linea/zktracer/ZkTracer.java b/arithmetization/src/main/java/net/consensys/linea/zktracer/ZkTracer.java
@@ -225,7 +225,7 @@ public Map<String, Integer> getModulesLineCount() {
                                         "Module "
                                             + m.moduleKey()
                                             + " not found in spillings.toml"))));
-    modulesLineCount.put("BLOCK_TX", hub.tx().number());
+    modulesLineCount.put("BLOCK_TX", hub.cumulatedTxCount());
     return modulesLineCount;
   }
 }
diff --git a/arithmetization/src/main/java/net/consensys/linea/zktracer/bytestheta/BaseBytes.java b/arithmetization/src/main/java/net/consensys/linea/zktracer/bytestheta/BaseBytes.java
@@ -15,6 +15,8 @@
 
 package net.consensys.linea.zktracer.bytestheta;
 
+import java.util.Objects;
+
 import net.consensys.linea.zktracer.types.Bytes16;
 import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.bytes.MutableBytes32;
@@ -50,6 +52,31 @@ protected BaseBytes(final Bytes32 arg) {
     bytes32 = arg.mutableCopy();
   }
 
+  /**
+   * The hashing must only be done on the numeric value wrapped by this class, so that sets of
+   * operations parameterized by these values hash correctly.
+   *
+   * @return this instance hash
+   */
+  @Override
+  public int hashCode() {
+    return Objects.hash(this.bytes32);
+  }
+
+  /**
+   * The equality must only be computed on the numeric value wrapped by this class, so that sets of
+   * operations parameterized by these values hash correctly.
+   *
+   * @return whether this == o
+   */
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) return true;
+    if (o == null || getClass() != o.getClass()) return false;
+    final BaseBytes that = (BaseBytes) o;
+    return Objects.equals(this.bytes32, that.bytes32);
+  }
+
   /**
    * Returns a new `Bytes16` object that is the high section (first 16 bytes) of the bytes32`
    * instance variable.

diff --git a/arithmetization/src/main/java/net/consensys/linea/zktracer/module/ext/ExtOperation.java b/arithmetization/src/main/java/net/consensys/linea/zktracer/module/ext/ExtOperation.java
@@ -67,10 +67,10 @@ public boolean equals(Object o) {
     if (this == o) return true;
     if (o == null || getClass() != o.getClass()) return false;
     final ExtOperation that = (ExtOperation) o;
-    return Objects.equals(opCode, that.opCode)
-        && Objects.equals(arg1, that.arg1)
-        && Objects.equals(arg2, that.arg2)
-        && Objects.equals(arg3, that.arg3);
+    return Objects.equals(this.opCode, that.opCode)
+        && Objects.equals(this.arg1, that.arg1)
+        && Objects.equals(this.arg2, that.arg2)
+        && Objects.equals(this.arg3, that.arg3);
   }
 
   public ExtOperation(OpCode opCode, Bytes32 arg1, Bytes32 arg2, Bytes32 arg3) {

diff --git a/arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/Hub.java b/arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/Hub.java
@@ -901,6 +901,10 @@ public int lineCount() {
     return this.state.lineCount();
   }
 
+  public int cumulatedTxCount() {
+    return this.state.txCount();
+  }
+
   void traceOperation(MessageFrame frame) {
     boolean updateReturnData =
         this.opCodeData().isHalt()

diff --git a/arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/State.java b/arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/State.java
@@ -61,6 +61,10 @@ Trace commit(Trace hubTrace) {
     return hubTrace;
   }
 
+  int txCount() {
+    return this.state.size();
+  }
+
   /**
    * @return the cumulated line numbers for all currently traced transactions
    */

diff --git a/...ation/src/test/java/net/consensys/linea/zktracer/module/ext/TestDuplicatedOperations.java b/...ation/src/test/java/net/consensys/linea/zktracer/module/ext/TestDuplicatedOperations.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package net.consensys.linea.zktracer.module.ext;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import net.consensys.linea.zktracer.opcode.OpCode;
+import net.consensys.linea.zktracer.testing.BytecodeCompiler;
+import net.consensys.linea.zktracer.testing.BytecodeRunner;
+import net.consensys.linea.zktracer.testing.EvmExtension;
+import org.apache.tuweni.bytes.Bytes;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+@ExtendWith(EvmExtension.class)
+public class TestDuplicatedOperations {
+  @Test
+  void testDuplicate() {
+    BytecodeRunner.of(
+            BytecodeCompiler.newProgram()
+                .push(
+                    Bytes.fromHexString(
+                        "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"))
+                .push(0)
+                .push(0)
+                .op(OpCode.MULMOD)
+                .push(
+                    Bytes.fromHexString(
+                        "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"))
+                .push(0)
+                .push(0)
+                .op(OpCode.MULMOD)
+                .compile())
+        .zkTracerValidator(
+            zkTracer -> {
+              assertThat(zkTracer.getModulesLineCount().get("EXT")).isEqualTo(9);
+            })
+        .run();
+  }
+}
diff --git a/...metization/src/test/java/net/consensys/linea/zktracer/module/hub/CallEmptyNoStopTest.java b/...metization/src/test/java/net/consensys/linea/zktracer/module/hub/CallEmptyNoStopTest.java
@@ -15,6 +15,8 @@
 
 package net.consensys.linea.zktracer.module.hub;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 import java.util.List;
 
 import net.consensys.linea.zktracer.opcode.OpCode;
@@ -85,7 +87,8 @@ void test() {
         .zkTracerValidator(
             zkTracer -> {
               // Ensure we don't have any superfluous STOP
-              assert zkTracer.getHub().state().currentTxTrace().getTrace().size() == 11;
+              assertThat(zkTracer.getHub().state().currentTxTrace().getTrace().size())
+                  .isEqualTo(10);
             })
         .build()
         .run();

diff --git a/...linea/zktracer/module/hub/TwoPlusTwo.java → ...a/zktracer/module/hub/TestTwoPlusTwo.java b/...linea/zktracer/module/hub/TwoPlusTwo.java → ...a/zktracer/module/hub/TestTwoPlusTwo.java
@@ -23,7 +23,7 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 
 @ExtendWith(EvmExtension.class)
-public class TwoPlusTwo {
+public class TestTwoPlusTwo {
   @Test
   void testAdd() {
     BytecodeRunner.of(BytecodeCompiler.newProgram().push(32).push(27).op(OpCode.ADD).compile())

diff --git a/arithmetization/src/test/java/net/consensys/linea/zktracer/testing/BytecodeRunner.java b/arithmetization/src/test/java/net/consensys/linea/zktracer/testing/BytecodeRunner.java
@@ -16,8 +16,12 @@
 package net.consensys.linea.zktracer.testing;
 
 import java.util.List;
+import java.util.function.Consumer;
 
 import com.google.common.base.Preconditions;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import net.consensys.linea.zktracer.ZkTracer;
 import org.apache.tuweni.bytes.Bytes;
 import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.crypto.SECP256K1;
@@ -29,14 +33,24 @@
 /**
  * A BytecodeRunner takes bytecode, then run it in a single transaction in a single block, and
  * ensures that it executed correctly.
- *
- * @param byteCode the byte code to test
  */
-public record BytecodeRunner(Bytes byteCode) {
+@Accessors(fluent = true)
+public final class BytecodeRunner {
+  private final Bytes byteCode;
+
+  /**
+   * @param byteCode the byte code to test
+   */
+  public BytecodeRunner(Bytes byteCode) {
+    this.byteCode = byteCode;
+  }
+
   public static BytecodeRunner of(Bytes byteCode) {
     return new BytecodeRunner(byteCode);
   }
 
+  @Setter private Consumer<ZkTracer> zkTracerValidator = zkTracer -> {};
+
   public void run() {
     Preconditions.checkArgument(byteCode != null, "byteCode cannot be empty");
 
@@ -68,6 +82,7 @@ public void run() {
     ToyExecutionEnvironment.builder()
         .testValidator(x -> {})
         .toyWorld(toyWorld)
+        .zkTracerValidator(zkTracerValidator)
         .transaction(tx)
         .build()
         .run();

diff --git a/...etization/src/test/java/net/consensys/linea/zktracer/testing/ToyExecutionEnvironment.java b/...etization/src/test/java/net/consensys/linea/zktracer/testing/ToyExecutionEnvironment.java
@@ -122,8 +122,6 @@ private void execute() {
     tracer.traceStartBlock(header, mockBlockBody);
 
     for (Transaction tx : mockBlockBody.getTransactions()) {
-      tracer.traceStartTransaction(toyWorld.updater(), tx);
-
       final TransactionProcessingResult result =
           transactionProcessor.processTransaction(
               null,
@@ -138,17 +136,6 @@ private void execute() {
               false,
               Wei.ZERO);
 
-      long transactionGasUsed = tx.getGasLimit() - result.getGasRemaining();
-
-      tracer.traceEndTransaction(
-          toyWorld.updater(),
-          tx,
-          result.isSuccessful(),
-          result.getOutput(),
-          result.getLogs(),
-          transactionGasUsed,
-          0);
-
       this.testValidator.accept(result);
       this.zkTracerValidator.accept(tracer);
     }

diff --git a/reference-tests/src/test/java/net/consensys/linea/CorsetBlockProcessor.java b/reference-tests/src/test/java/net/consensys/linea/CorsetBlockProcessor.java
@@ -124,7 +124,6 @@ public BlockProcessingResult processBlock(
 
       zkTracer.traceStartConflation(1);
       zkTracer.traceStartBlock(blockHeader, blockBody);
-      zkTracer.traceStartTransaction(worldState, transaction);
       final TransactionProcessingResult result =
           transactionProcessor.processTransaction(
               blockchain,
@@ -138,14 +137,6 @@ public BlockProcessingResult processBlock(
               TransactionValidationParams.processingBlock(),
               privateMetadataUpdater,
               blobGasPrice);
-      zkTracer.traceEndTransaction(
-          worldState,
-          transaction,
-          result.isSuccessful(),
-          result.getOutput(),
-          result.getLogs(),
-          transaction.getGasLimit() - result.getGasRemaining(),
-          0);
 
       if (result.isInvalid()) {
         String errorMessage =