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.

Sign up for GitHub

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

fix(OOB_CALL): confusion between call and callee gas for BLAKE #1366

Merged
merged 2 commits into from
Oct 3, 2024
Merged

fix(OOB_CALL): confusion between call and callee gas for BLAKE #1366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84addca
fix(OOB_CALL): confusion between call and callee gas for BLAKE
letypequividelespoubelles Oct 3, 2024
67c5a32
Merge branch 'arith-dev' into 1362-failing-oobcall-during-a-blake2f-call
letypequividelespoubelles Oct 3, 2024
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
27 changes: 11 additions & 16 deletions ...ation/src/main/java/net/consensys/linea/zktracer/module/hub/fragment/imc/ImcFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

import net.consensys.linea.zktracer.module.hub.Hub;
import net.consensys.linea.zktracer.module.hub.Trace;
Expand All @@ -27,7 +26,6 @@
import net.consensys.linea.zktracer.module.hub.fragment.imc.mmu.MmuCall;
import net.consensys.linea.zktracer.module.hub.fragment.imc.oob.OobCall;
import net.consensys.linea.zktracer.types.TransactionProcessingMetadata;
import org.apache.tuweni.bytes.Bytes;

/**
* IMCFragments embed data required for Inter-Module Communication, i.e. data that are required to
Expand Down Expand Up @@ -68,9 +66,6 @@ public static ImcFragment forTxInit(final Hub hub) {
// isdeployment == false
// non empty calldata
final TransactionProcessingMetadata currentTx = hub.txStack().current();
final boolean isMessageCallTransaction = currentTx.getBesuTransaction().getTo().isPresent();

final Optional<Bytes> txData = currentTx.getBesuTransaction().getData();
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK

final boolean shouldCopyTxCallData = currentTx.copyTransactionCallData();

final ImcFragment miscFragment = ImcFragment.empty(hub);
Expand All @@ -84,8 +79,8 @@ public ImcFragment callOob(OobCall f) {
} else {
oobIsSet = true;
}
this.hub.oob().call(f);
this.moduleCalls.add(f);
hub.oob().call(f);
moduleCalls.add(f);
return this;
}

Expand All @@ -96,7 +91,7 @@ public ImcFragment callMmu(MmuCall f) {
mmuIsSet = true;
}
// Note: the triggering of the MMU is made by the creation of the MmuCall
this.moduleCalls.add(f);
moduleCalls.add(f);
return this;
}

Expand All @@ -106,8 +101,8 @@ public ImcFragment callExp(ExpCall f) {
} else {
expIsSet = true;
}
this.hub.exp().call(f);
this.moduleCalls.add(f);
hub.exp().call(f);
moduleCalls.add(f);
return this;
}

Expand All @@ -117,8 +112,8 @@ public ImcFragment callMxp(MxpCall f) {
} else {
mxpIsSet = true;
}
this.hub.mxp().call(f);
this.moduleCalls.add(f);
hub.mxp().call(f);
moduleCalls.add(f);
return this;
}

Expand All @@ -128,17 +123,17 @@ public ImcFragment callStp(StpCall f) {
} else {
stpIsSet = true;
}
this.hub.stp().call(f);
this.moduleCalls.add(f);
hub.stp().call(f);
moduleCalls.add(f);
return this;
}

@Override
public Trace trace(Trace trace) {
trace.peekAtMiscellaneous(true);

for (TraceSubFragment subFragment : this.moduleCalls) {
subFragment.trace(trace, this.hub.state.stamps());
for (TraceSubFragment subFragment : moduleCalls) {
subFragment.trace(trace, hub.state.stamps());
}

return trace;
Expand Down
3 changes: 1 addition & 2 deletions ...onsensys/linea/zktracer/module/hub/fragment/imc/oob/precompiles/Blake2fParamsOobCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
public class Blake2fParamsOobCall extends OobCall {

BigInteger calleeGas;
BigInteger callGas; // TODO: remove this
BigInteger blakeR;
BigInteger blakeF;

Expand Down Expand Up @@ -63,7 +62,7 @@ public Trace trace(Trace trace) {
return trace
.pMiscOobFlag(true)
.pMiscOobInst(oobInstructionValue())
.pMiscOobData1(bigIntegerToBytes(callGas))
.pMiscOobData1(bigIntegerToBytes(calleeGas))
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK

.pMiscOobData2(ZERO)
.pMiscOobData3(ZERO)
.pMiscOobData4(booleanToBytes(ramSuccess)) // Set after the constructor
Expand Down
10 changes: 4 additions & 6 deletions ...onsensys/linea/zktracer/module/hub/section/call/precompileSubsection/BlakeSubsection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,10 @@ public BlakeSubsection(Hub hub, CallSection callSection) {
final Bytes blakeR = callData.slice(0, 4);
final Bytes blakeF = callData.slice(212, 1);

{
final boolean wellFormedF = blakeF.get(0) == 0 || blakeF.get(0) == 1;
final long rounds = blakeR.toLong();
final boolean sufficientGas = calleeGas >= rounds;
blakeSuccess = wellFormedF && sufficientGas;
}
final boolean wellFormedF = blakeF.get(0) == 0 || blakeF.get(0) == 1;
final long rounds = blakeR.toLong();
final boolean sufficientGas = calleeGas >= rounds;
blakeSuccess = wellFormedF && sufficientGas;

if (!blakeSuccess) {
this.setScenario(PRC_FAILURE_KNOWN_TO_RAM);
Expand Down
22 changes: 11 additions & 11 deletions arithmetization/src/main/java/net/consensys/linea/zktracer/module/oob/OobOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import static net.consensys.linea.zktracer.module.oob.Trace.CT_MAX_SSTORE;
import static net.consensys.linea.zktracer.module.oob.Trace.CT_MAX_XCALL;
import static net.consensys.linea.zktracer.module.oob.Trace.G_QUADDIVISOR;
import static net.consensys.linea.zktracer.runtime.callstack.CallFrame.getOpCode;
import static net.consensys.linea.zktracer.types.AddressUtils.getDeploymentAddress;
import static net.consensys.linea.zktracer.types.Conversions.bigIntegerToBoolean;
import static net.consensys.linea.zktracer.types.Conversions.booleanToBigInteger;
Expand Down Expand Up @@ -297,7 +298,7 @@ private void populateColumnsForEvmInstruction(MessageFrame frame) {
}

public void populateColumnsForPrecompile(MessageFrame frame) {
final OpCode opCode = OpCode.of(frame.getCurrentOperation().getOpcode());
final OpCode opCode = getOpCode(frame);
final long argsOffset =
Words.clampedToLong(
opCode.callCanTransferValue()
Expand All @@ -314,7 +315,7 @@ public void populateColumnsForPrecompile(MessageFrame frame) {
calleeGas = ((ModexpPricingOobCall) oobCall).getCallGas();
}
if (oobCall instanceof Blake2fParamsOobCall) {
calleeGas = ((Blake2fParamsOobCall) oobCall).getCallGas();
calleeGas = ((Blake2fParamsOobCall) oobCall).getCalleeGas();
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK

}

final BigInteger cds = EWord.of(frame.getStackItem(cdsIndex)).toUnsignedBigInteger();
Expand Down Expand Up @@ -458,13 +459,12 @@ public void populateColumnsForPrecompile(MessageFrame frame) {
setBlake2FCds(prcBlake2FCdsCall);
}
case OOB_INST_BLAKE_PARAMS -> {
Bytes callData = frame.shadowReadMemory(argsOffset, 213);
final Bytes callData = frame.shadowReadMemory(argsOffset, 213);
final BigInteger blakeR = callData.slice(0, 4).toUnsignedBigInteger();

final BigInteger blakeF = BigInteger.valueOf(toUnsignedInt(callData.get(212)));

final Blake2fParamsOobCall prcBlake2FParamsOobCall = (Blake2fParamsOobCall) oobCall;
prcBlake2FParamsOobCall.setCallGas(calleeGas);
prcBlake2FParamsOobCall.setCalleeGas(calleeGas);
prcBlake2FParamsOobCall.setBlakeR(blakeR);
prcBlake2FParamsOobCall.setBlakeF(blakeF);

Expand Down Expand Up @@ -556,7 +556,7 @@ private boolean callToLT(
outgoingData2[k] = arg1Lo;
outgoingData3[k] = arg2Hi;
outgoingData4[k] = arg2Lo;
boolean r = wcp.callLT(arg1, arg2);
final boolean r = wcp.callLT(arg1, arg2);
outgoingResLo[k] = booleanToBigInteger(r);
return r;
}
Expand All @@ -577,7 +577,7 @@ private boolean callToGT(
outgoingData2[k] = arg1Lo;
outgoingData3[k] = arg2Hi;
outgoingData4[k] = arg2Lo;
boolean r = wcp.callGT(arg1, arg2);
final boolean r = wcp.callGT(arg1, arg2);
outgoingResLo[k] = booleanToBigInteger(r);
return r;
}
Expand All @@ -594,7 +594,7 @@ private boolean callToISZERO(final int k, final BigInteger arg1Hi, final BigInte
outgoingData2[k] = arg1Lo;
outgoingData3[k] = BigInteger.ZERO;
outgoingData4[k] = BigInteger.ZERO;
boolean r = wcp.callISZERO(arg1);
final boolean r = wcp.callISZERO(arg1);
outgoingResLo[k] = booleanToBigInteger(r);
return r;
}
Expand All @@ -615,7 +615,7 @@ private boolean callToEQ(
outgoingData2[k] = arg1Lo;
outgoingData3[k] = arg2Hi;
outgoingData4[k] = arg2Lo;
boolean r = wcp.callEQ(arg1, arg2);
final boolean r = wcp.callEQ(arg1, arg2);
outgoingResLo[k] = booleanToBigInteger(r);
return r;
}
Expand Down Expand Up @@ -1183,7 +1183,7 @@ private void setBlake2FParams(Blake2fParamsOobCall prcBlake2FParamsOobCall) {
!callToLT(
0,
BigInteger.ZERO,
prcBlake2FParamsOobCall.getCallGas(),
prcBlake2FParamsOobCall.getCalleeGas(),
BigInteger.ZERO,
prcBlake2FParamsOobCall.getBlakeR()); // = ramSuccess

Expand All @@ -1203,7 +1203,7 @@ private void setBlake2FParams(Blake2fParamsOobCall prcBlake2FParamsOobCall) {
// Set returnGas
final BigInteger returnGas =
ramSuccess
? (prcBlake2FParamsOobCall.getCallGas().subtract(prcBlake2FParamsOobCall.getBlakeR()))
? (prcBlake2FParamsOobCall.getCalleeGas().subtract(prcBlake2FParamsOobCall.getBlakeR()))
: BigInteger.ZERO;

prcBlake2FParamsOobCall.setReturnGas(returnGas);
Expand Down
Loading