Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make the trace deterministic #1346

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

package net.consensys.linea.zktracer.container.module;

import java.util.Comparator;
import java.util.List;

import net.consensys.linea.zktracer.container.ModuleOperation;
import net.consensys.linea.zktracer.container.stacked.ModuleOperationStackedSet;
import org.hyperledger.besu.evm.worldstate.WorldView;
Expand Down Expand Up @@ -46,4 +49,8 @@ default int lineCount() {
default void traceEndConflation(final WorldView state) {
operations().finishConflation();
}

default List<E> sortOperations(Comparator<E> comparator) {
return operations().sortOperations(comparator);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@

package net.consensys.linea.zktracer.container.stacked;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.List;
import java.util.Set;

import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -135,4 +138,10 @@ public void finishConflation() {
operationsInTransaction().clear();
lineCounter.enter(); // this is not mandatory but it is more consistent
}

public List<E> sortOperations(Comparator<E> comparator) {
final List<E> sortedOperations = new ArrayList<>(getAll());
sortedOperations.sort(comparator);
return sortedOperations;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public List<ColumnHeader> columnsHeaders() {
public void commit(List<MappedByteBuffer> buffers) {
final Trace trace = new Trace(buffers);
int stamp = 0;
for (AddOperation op : operations.getAll()) {
for (AddOperation op : sortOperations(new AddOperationComparator())) {
op.trace(++stamp, trace);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import static net.consensys.linea.zktracer.module.constants.GlobalConstants.LLARGE;

import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.experimental.Accessors;
import net.consensys.linea.zktracer.bytestheta.BaseBytes;
import net.consensys.linea.zktracer.container.ModuleOperation;
import net.consensys.linea.zktracer.opcode.OpCode;
Expand All @@ -27,13 +29,14 @@
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt256;

@Accessors(fluent = true)
@EqualsAndHashCode(onlyExplicitlyIncluded = true, callSuper = false)
public final class AddOperation extends ModuleOperation {
private static final UInt256 TWO_TO_THE_128 = UInt256.ONE.shiftLeft(128);

@EqualsAndHashCode.Include private final OpCode opCode;
@EqualsAndHashCode.Include private final Bytes32 arg1;
@EqualsAndHashCode.Include private final Bytes32 arg2;
@EqualsAndHashCode.Include @Getter private final OpCode opCode;
@EqualsAndHashCode.Include @Getter private final Bytes32 arg1;
@EqualsAndHashCode.Include @Getter private final Bytes32 arg2;
private BaseBytes res;
private int ctMax;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems odd that you've implemented all the Comparators separately from the class itself. You can have e.g. done class AddOperation extends ModuleOperation implements Comparable<AddOperation> then added a compareTo() method as required for the interface. Just saves lines I guess --- not a big deal though.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the reason for doing it this way is to avoid implemented a Comparator for all Operation classes? I think you still could do it though by modifying the signature of OperationSetModule.sortOperations(). I'd have to check it worked though --- so not worth bothering!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, one reason for having AddOperation implement Comparable<AddOperation> is that you can then modify ModuleOperationStackedSet to require that its elements must be sortable like so:

public class ModuleOperationStackedSet<E extends ModuleOperation & Comparable<E>>
    extends StackedSet<E> {

That just helps reduce the chance of a problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some inconsistency, in that e.g. BlockHash does things differently. But perhaps it has to?

made more consistent. For BlockHash we have to sort before commit time, as we have to trigger WCP.

Does ExpOperation need the annotation @EqualsAndHashCode(onlyExplicitlyIncluded = true, callSuper = false)

added

* Copyright ConsenSys Inc.
*
* 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.add;

import java.util.Comparator;

public class AddOperationComparator implements Comparator<AddOperation> {
public int compare(AddOperation op1, AddOperation op2) {
// First sort by OpCode
final int opCodeComp = op1.opCode().compareTo(op2.opCode());
if (opCodeComp != 0) {
return opCodeComp;
}
// Second sort by Arg1
final int arg1Comp = op1.arg1().compareTo(op2.arg1());
if (arg1Comp != 0) {
return arg1Comp;
}
// Third, sort by Arg2
return op1.arg2().compareTo(op2.arg2());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void commit(List<MappedByteBuffer> buffers) {
final Trace trace = new Trace(buffers);

int stamp = 0;
for (BinOperation op : operations.getAll()) {
for (BinOperation op : operations.sortOperations(new BinOperationComparator())) {
op.traceBinOperation(++stamp, trace);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright ConsenSys Inc.
*
* 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.bin;

import java.util.Comparator;

public class BinOperationComparator implements Comparator<BinOperation> {
public int compare(BinOperation op1, BinOperation op2) {
// First sort by OpCode
final int opCodeComp = op1.opCode().compareTo(op2.opCode());
if (opCodeComp != 0) {
return opCodeComp;
}
// Second sort by Arg1
final int arg1Comp = op1.arg1().getBytes32().compareTo(op2.arg1().getBytes32());
if (arg1Comp != 0) {
return arg1Comp;
}
// Third, sort by Arg2
return op1.arg2().getBytes32().compareTo(op2.arg2().getBytes32());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public List<ColumnHeader> columnsHeaders() {
@Override
public void commit(List<MappedByteBuffer> buffers) {
final Trace trace = new Trace(buffers);
for (EucOperation eucOperation : operations.getAll()) {
for (EucOperation eucOperation : operations.sortOperations(new EucOperationComparator())) {
eucOperation.trace(trace);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright ConsenSys Inc.
*
* 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.euc;

import java.util.Comparator;

public class EucOperationComparator implements Comparator<EucOperation> {
public int compare(EucOperation op1, EucOperation op2) {
// Second sort by Dividend
final int arg1Comp = op1.dividend().compareTo(op2.dividend());
if (arg1Comp != 0) {
return arg1Comp;
} else {
// Second, sort by Divisor
return op1.divisor().compareTo(op2.divisor());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void commit(List<MappedByteBuffer> buffers) {
final Trace trace = new Trace(buffers);

int stamp = 0;
for (ExpOperation expOp : operations.getAll()) {
for (ExpOperation expOp : operations.sortOperations(new ExpOperationComparator())) {
expOp.traceComputation(++stamp, trace);
expOp.traceMacro(stamp, trace);
expOp.tracePreprocessing(stamp, trace);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.experimental.Accessors;
import net.consensys.linea.zktracer.container.ModuleOperation;
import net.consensys.linea.zktracer.module.constants.GlobalConstants;
import net.consensys.linea.zktracer.module.hub.Hub;
Expand All @@ -50,7 +51,9 @@

@Getter
public class ExpOperation extends ModuleOperation {
letypequividelespoubelles marked this conversation as resolved.
Show resolved Hide resolved
@EqualsAndHashCode.Include ExpCall expCall;
@EqualsAndHashCode.Include
@Accessors(fluent = true)
ExpCall expCall;

protected short pComputationPltJmp = 0;
protected Bytes pComputationRawAcc; // (last row) paired with RawByte
Expand Down Expand Up @@ -90,8 +93,8 @@ public ExpOperation(ExpCall expCall, Wcp wcp, Hub hub) {
long dynCost = (long) GlobalConstants.GAS_CONST_G_EXP_BYTE * exponent.byteLength();

// Fill expCall
explogExpCall.setExponent(exponent);
explogExpCall.setDynCost(dynCost);
explogExpCall.exponent(exponent);
explogExpCall.dynCost(dynCost);

// Execute preprocessing
preComputeForExplog(explogExpCall);
Expand All @@ -100,7 +103,7 @@ public ExpOperation(ExpCall expCall, Wcp wcp, Hub hub) {
ModexpLogExpCall modexplogExpCall = (ModexpLogExpCall) expCall;

// Extract inputs
ModexpMetadata modexpMetadata = modexplogExpCall.getModexpMetadata();
final ModexpMetadata modexpMetadata = modexplogExpCall.getModexpMetadata();
final int bbsInt = modexpMetadata.bbs().toUnsignedBigInteger().intValueExact();
final int ebsInt = modexpMetadata.ebs().toUnsignedBigInteger().intValueExact();
checkArgument(modexpMetadata.callData().size() - 96 - bbsInt >= 0);
Expand All @@ -123,28 +126,28 @@ public ExpOperation(ExpCall expCall, Wcp wcp, Hub hub) {

public void preComputeForExplog(ExplogExpCall explogExpCall) {
pMacroExpInst = EXP_INST_EXPLOG;
pMacroData1 = explogExpCall.getExponent().hi();
pMacroData2 = explogExpCall.getExponent().lo();
pMacroData5 = Bytes.ofUnsignedLong(explogExpCall.getDynCost());
pMacroData1 = explogExpCall.exponent().hi();
pMacroData2 = explogExpCall.exponent().lo();
pMacroData5 = Bytes.ofUnsignedLong(explogExpCall.dynCost());
initArrays(CT_MAX_PRPRC_EXP_LOG + 1);

// Preprocessing
// First row
pPreprocessingWcpFlag[0] = true;
pPreprocessingWcpArg1Hi[0] = Bytes.EMPTY;
pPreprocessingWcpArg1Lo[0] = explogExpCall.getExponent().hi();
pPreprocessingWcpArg1Lo[0] = explogExpCall.exponent().hi();
pPreprocessingWcpArg2Hi[0] = Bytes.EMPTY;
pPreprocessingWcpArg2Lo[0] = Bytes.EMPTY;
pPreprocessingWcpInst[0] = UnsignedByte.of(EVM_INST_ISZERO);
final boolean expnHiIsZero = wcp.callISZERO(explogExpCall.getExponent().hi());
final boolean expnHiIsZero = wcp.callISZERO(explogExpCall.exponent().hi());
;
pPreprocessingWcpRes[0] = expnHiIsZero;

// Linking constraints and fill rawAcc
pComputationPltJmp = 16;
pComputationRawAcc = explogExpCall.getExponent().hi();
pComputationRawAcc = explogExpCall.exponent().hi();
if (expnHiIsZero) {
pComputationRawAcc = explogExpCall.getExponent().lo();
pComputationRawAcc = explogExpCall.exponent().lo();
}

// Fill trimAcc
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright ConsenSys Inc.
*
* 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.exp;

import static net.consensys.linea.zktracer.module.constants.GlobalConstants.EXP_INST_EXPLOG;
import static net.consensys.linea.zktracer.module.constants.GlobalConstants.EXP_INST_MODEXPLOG;

import java.util.Comparator;

import net.consensys.linea.zktracer.module.hub.fragment.imc.exp.ExplogExpCall;
import net.consensys.linea.zktracer.module.hub.fragment.imc.exp.ModexpLogExpCall;

public class ExpOperationComparator implements Comparator<ExpOperation> {
@Override
public int compare(ExpOperation op1, ExpOperation op2) {
final int instructionComp =
Integer.compare(op1.expCall().expInstruction(), op2.expCall().expInstruction());
if (instructionComp != 0) {
return instructionComp;
}

if (op1.expCall.expInstruction() == EXP_INST_EXPLOG) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem ideal. I would have thought it makes more sense to add compareTo to the ExpCall interface and, hence, its implementations (ExplogExpCall and ModexpLogExpCall). Otherwise, this is a bit fragile is someone adds a new implementation of ExpCall ... they might forget to update this bit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do agree.

final ExplogExpCall o1 = (ExplogExpCall) op1.expCall();
final ExplogExpCall o2 = (ExplogExpCall) op2.expCall();

final int dynCostComp = Long.compare(o1.dynCost(), o2.dynCost());
if (dynCostComp != 0) {
return dynCostComp;
}
return o1.exponent().compareTo(o2.exponent());
}

if (op1.expCall.expInstruction() == EXP_INST_MODEXPLOG) {
final ModexpLogExpCall o1 = (ModexpLogExpCall) op1.expCall();
final ModexpLogExpCall o2 = (ModexpLogExpCall) op2.expCall();

final int cdsCutoffComp = Integer.compare(o1.getCdsCutoff(), o2.getCdsCutoff());
if (cdsCutoffComp != 0) {
return cdsCutoffComp;
}
final int ebsCutoffComp = Integer.compare(o1.getEbsCutoff(), o2.getEbsCutoff());
if (ebsCutoffComp != 0) {
return ebsCutoffComp;
}
final int leadLogComp = o1.getLeadLog().compareTo(o2.getLeadLog());
if (leadLogComp != 0) {
return leadLogComp;
}

return o1.getRawLeadingWord().compareTo(o2.getRawLeadingWord());
}

throw new IllegalStateException("Unknown instruction type");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void commit(List<MappedByteBuffer> buffers) {
final Trace trace = new Trace(buffers);

int stamp = 0;
for (ExtOperation operation : operations.getAll()) {
for (ExtOperation operation : operations.sortOperations(new ExtOperationComparator())) {
operation.trace(trace, ++stamp);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import static net.consensys.linea.zktracer.module.constants.GlobalConstants.MMEDIUM;

import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.experimental.Accessors;
import net.consensys.linea.zktracer.bytestheta.BaseBytes;
import net.consensys.linea.zktracer.bytestheta.BaseTheta;
import net.consensys.linea.zktracer.bytestheta.BytesArray;
Expand All @@ -30,13 +32,14 @@
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt256;

@Accessors(fluent = true)
@EqualsAndHashCode(onlyExplicitlyIncluded = true, callSuper = false)
public class ExtOperation extends ModuleOperation {

@EqualsAndHashCode.Include private final OpCode opCode;
@EqualsAndHashCode.Include private final BaseBytes arg1;
@EqualsAndHashCode.Include private final BaseBytes arg2;
@EqualsAndHashCode.Include private final BaseBytes arg3;
@EqualsAndHashCode.Include @Getter private final OpCode opCode;
@EqualsAndHashCode.Include @Getter private final BaseBytes arg1;
@EqualsAndHashCode.Include @Getter private final BaseBytes arg2;
@EqualsAndHashCode.Include @Getter private final BaseBytes arg3;
private final boolean isOneLineInstruction;

private BaseTheta result;
Expand Down
Loading
Loading