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

clean: some cleaning #1850

Merged
merged 29 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
47decaa
clean: rm unused columns
letypequividelespoubelles Feb 19, 2025
5b1a657
clean: some cleaning
letypequividelespoubelles Feb 25, 2025
a6c1045
clean
letypequividelespoubelles Feb 25, 2025
af03dbb
more cleanng
letypequividelespoubelles Feb 25, 2025
686ee19
feat: added tests for Hub.unlatchStack() wrt CALL/CREATE and exceptions
OlivierBBB Feb 25, 2025
24adb0a
spotless
OlivierBBB Feb 25, 2025
c44461f
clean finish ?
letypequividelespoubelles Feb 25, 2025
163faf1
update go-corset
letypequividelespoubelles Feb 25, 2025
278d392
mxp cleaning
letypequividelespoubelles Feb 25, 2025
36e3b3e
Merge branch 'arith-dev' into 1849-some-cleaning
letypequividelespoubelles Feb 25, 2025
2a77de0
ras
letypequividelespoubelles Feb 25, 2025
15f633a
Merge branch '1834-rm-dep-number-status-infinity' into 1849-some-clea…
letypequividelespoubelles Feb 25, 2025
2154ba1
make it compile
letypequividelespoubelles Feb 25, 2025
85d7f00
make it compile again
letypequividelespoubelles Feb 25, 2025
1e5590c
update constrants
letypequividelespoubelles Feb 25, 2025
ca4b258
rm dead code + constraints
letypequividelespoubelles Feb 25, 2025
b94e065
more cleaning
letypequividelespoubelles Feb 25, 2025
eb283f1
update constraints
letypequividelespoubelles Feb 25, 2025
044dca8
clean: get rid of signals
letypequividelespoubelles Feb 25, 2025
d26aaae
ras
letypequividelespoubelles Feb 25, 2025
5b3f8a5
adjusted asserts for OobRdcTest
lorenzogentile404 Feb 26, 2025
711041b
spotless
lorenzogentile404 Feb 26, 2025
2a4c4ba
feat: factorise exceptions logic
letypequividelespoubelles Feb 26, 2025
f86693a
Merge branch '1849-some-cleaning' of github.com:Consensys/linea-trace…
letypequividelespoubelles Feb 26, 2025
929a7ae
Merge branch 'arith-dev' into 1849-some-cleaning
letypequividelespoubelles Feb 26, 2025
9c692dc
fixed null case in OobRdcTest
lorenzogentile404 Feb 26, 2025
b88a945
Merge branch '1849-some-cleaning' of github.com:Consensys/linea-trace…
lorenzogentile404 Feb 26, 2025
e92b3c1
fix typo
letypequividelespoubelles Feb 26, 2025
0eb2c3d
Merge branch '1849-some-cleaning' of github.com:Consensys/linea-trace…
letypequividelespoubelles Feb 26, 2025
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
2 changes: 1 addition & 1 deletion .github/actions/setup-go-corset/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ runs:

- name: Install Go Corset
shell: bash
run: go install github.com/consensys/go-corset/cmd/go-corset@a899fbd
run: go install github.com/consensys/go-corset/cmd/go-corset@4e884ec
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ public class AccountSnapshot {
private int deploymentNumber;
private boolean deploymentStatus;

// TODO: is there a "canonical" way to take a snapshot fo an account
// where getWorldUpdater().get(address) return null ?

/**
* Canonical way of creating an account snapshot.
*
Expand Down Expand Up @@ -241,9 +238,9 @@ public AccountSnapshot setDeploymentNumber(DeploymentInfo deploymentInfo) {
return this.deploymentNumber(deploymentInfo.deploymentNumber(address));
}

public AccountSnapshot decrementDeploymentNumberByOne() {
public void decrementDeploymentNumberByOne() {
checkState(deploymentNumber > 0);
return this.deploymentNumber(deploymentNumber - 1);
this.deploymentNumber(deploymentNumber - 1);
}

public AccountSnapshot setDeploymentInfo(Hub hub) {
Expand All @@ -261,9 +258,4 @@ public AccountSnapshot deployByteCode(Bytecode code) {

return new AccountSnapshot(address, nonce, balance, true, code, deploymentNumber, false);
}

public AccountSnapshot copyDeploymentInfoFrom(AccountSnapshot snapshot) {
return this.deploymentNumber(snapshot.deploymentNumber)
.deploymentStatus(snapshot.deploymentStatus);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -647,8 +647,6 @@ public void traceContextEnter(MessageFrame frame) {

@Override
public void traceContextExit(MessageFrame frame) {
this.currentFrame().initializeFrame(frame); // TODO: is it needed ?

exitDeploymentFromDeploymentInfoPov(frame);

// We take a snapshot before exiting the transaction
Expand All @@ -672,7 +670,6 @@ public void traceContextExit(MessageFrame frame) {
}

defers.resolveUponContextExit(this, this.currentFrame());
// TODO: verify me please @Olivier
if (this.currentFrame().opCode() == REVERT || Exceptions.any(pch.exceptions())) {
defers.resolveUponRollback(this, frame, this.currentFrame());
}
Expand All @@ -685,7 +682,6 @@ public void traceContextExit(MessageFrame frame) {
public void traceContextReEnter(MessageFrame frame) {
// Note: the update of the currentId call frame is made during traceContextExit of the child
// frame
this.currentFrame().initializeFrame(frame); // TODO: is it needed ?
defers.resolveUponContextReEntry(this, this.currentFrame());
this.unlatchStack(frame, this.currentFrame().childSpanningSection());
}
Expand Down Expand Up @@ -914,9 +910,8 @@ public void unlatchStack(MessageFrame frame, TraceSection section) {

if (line.needsResult()) {
Bytes result = Bytes.EMPTY;
// Only pop from the stack if no exceptions have been encountered
// TODO: when we call this from contextReenter, pch.exceptions is not the one from the
// caller/creater ?
// Note: when we call this from contextReenter, pch.exceptions is the one from the last
// opcode of the caller/creater ?
if (Exceptions.none(pch.exceptions())) {
result = frame.getStackItem(0).copy();
}
Expand Down Expand Up @@ -949,10 +944,6 @@ void processStateExec(MessageFrame frame) {
}
}

// TODO: how do these implementations of remainingGas()
// and expectedGas() behave with respect to resuming
// execution after a CALL / CREATE ? One of them is
// necessarily false ...
public long remainingGas() {
return this.state().processingPhase() == TX_EXEC
? this.currentFrame().frame().getRemainingGas()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import net.consensys.linea.zktracer.module.hub.signals.AbortingConditions;
import net.consensys.linea.zktracer.module.hub.signals.Exceptions;
import net.consensys.linea.zktracer.module.hub.signals.TracedException;
import net.consensys.linea.zktracer.module.hub.state.State;
import net.consensys.linea.zktracer.opcode.InstructionFamily;
import net.consensys.linea.zktracer.opcode.OpCode;
import net.consensys.linea.zktracer.opcode.gas.MxpType;
Expand All @@ -60,8 +59,6 @@ public final class StackFragment implements TraceFragment {
@Getter private final OpCode opCode;
@Getter private final int rawOpCode;
@Setter private boolean jumpDestinationVettingRequired;
@Setter private boolean validJumpDestination;
private final State.HubTransactionState.Stamps stamps;
private final CommonFragmentValues commonFragmentValues;
private final EWord pushValue;

Expand Down Expand Up @@ -139,7 +136,6 @@ private StackFragment(
jumpDestinationVettingRequired = false;
}

this.stamps = hub.state().stamps();
this.commonFragmentValues = commonFragmentValues;
this.pushValue = opCode.isPush() ? EWord.of(getPushValue(hub)) : EWord.ZERO;
}
Expand Down Expand Up @@ -278,7 +274,7 @@ public Trace trace(Trace trace) {
.pStackStaticFlag(stack.getCurrentOpcodeData().stackSettings().forbiddenInStatic())
.pStackPushValueHi(pushValue.hi())
.pStackPushValueLo(pushValue.lo())
.pStackJumpDestinationVettingRequired(jumpDestinationVettingRequired) // TODO: confirm this
.pStackJumpDestinationVettingRequired(jumpDestinationVettingRequired)
// Exception flag
.pStackOpcx(tracedException == INVALID_OPCODE)
.pStackSux(tracedException == STACK_UNDERFLOW)
Expand All @@ -295,8 +291,7 @@ public Trace trace(Trace trace) {
.pStackHashInfoFlag(hashInfoFlag)
.pStackHashInfoKeccakHi(hashInfoKeccak.hi())
.pStackHashInfoKeccakLo(hashInfoKeccak.lo())
.pStackLogInfoFlag(this.traceLog()) // TODO: confirm this
;
.pStackLogInfoFlag(traceLog());
}

private void tracedExceptionSanityChecks(TracedException tracedException) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ public final class AccountFragment

@Getter private final AccountSnapshot oldState;
@Getter private final AccountSnapshot newState;
@Setter private int deploymentNumberInfinity = 0; // retconned on conflation end
@Setter private boolean existsInfinity = false; // retconned on conflation end
@Setter private boolean requiresRomlex;
private int codeFragmentIndex;
private final Optional<Bytes> addressToTrim;
Expand Down Expand Up @@ -157,8 +155,6 @@ public Trace trace(Trace trace) {
.pAccountDeploymentStatus(oldState.deploymentStatus())
.pAccountDeploymentNumberNew(newState.deploymentNumber())
.pAccountDeploymentStatusNew(newState.deploymentStatus())
.pAccountDeploymentNumberInfty(deploymentNumberInfinity)
.pAccountDeploymentStatusInfty(existsInfinity)
.pAccountTrmFlag(addressToTrim.isPresent())
.pAccountTrmRawAddressHi(addressToTrim.map(a -> EWord.of(a).hi()).orElse(Bytes.EMPTY))
.pAccountIsPrecompile(isPrecompile(oldState.address()));
Expand All @@ -184,8 +180,8 @@ public void resolveAtEndTransaction(

@Override
public void resolvePostConflation(Hub hub, WorldView world) {
deploymentNumberInfinity = hub.deploymentNumberOf(newState.address());
existsInfinity = world.get(newState.address()) != null;
// Note: we DO need the CFI, even if we don't have the romlex flag on, for CFI consistency
// argument.
try {
codeFragmentIndex =
hub.getCodeFragmentIndexByMetaData(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ public static RlpAddrSubFragment makeFragment(Hub hub, Address deploymentAddress
final MessageFrame frame = hub.currentFrame().frame();
final Bytes32 salt = Bytes32.leftPad(frame.getStackItem(3));
final Bytes initCode = OperationAncillaries.initCode(frame);
final Bytes32 hash =
Hash.keccak256(initCode); // TODO: could be done better, we compute the HASH two times
final Bytes32 hash = Hash.keccak256(initCode);
hub.rlpAddr().callRlpAddrCreate2(frame, salt, hash);
return new RlpAddrSubFragment((short) 2, deploymentAddress, salt, hash);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,14 @@
import static net.consensys.linea.zktracer.module.hub.HubProcessingPhase.*;
import static net.consensys.linea.zktracer.module.hub.HubProcessingPhase.TX_EXEC;

import java.util.function.Supplier;

import lombok.RequiredArgsConstructor;
import lombok.experimental.Accessors;
import net.consensys.linea.zktracer.ZkTracer;
import net.consensys.linea.zktracer.module.hub.Hub;
import net.consensys.linea.zktracer.module.hub.Trace;
import net.consensys.linea.zktracer.module.hub.fragment.TraceFragment;
import net.consensys.linea.zktracer.module.hub.signals.Exceptions;
import net.consensys.linea.zktracer.module.hub.signals.TracedException;
import net.consensys.linea.zktracer.runtime.callstack.CallFrame;
import net.consensys.linea.zktracer.types.EWord;
import net.consensys.linea.zktracer.types.TransactionProcessingMetadata;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt256;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.evm.EVM;
import org.hyperledger.besu.evm.EvmSpecVersion;
import org.hyperledger.besu.evm.internal.EvmConfiguration;
import org.hyperledger.besu.evm.operation.Operation;
import org.hyperledger.besu.evm.operation.OperationRegistry;
import org.hyperledger.besu.evm.operation.SelfDestructOperation;
import org.hyperledger.besu.evm.worldstate.WorldView;

@Accessors(fluent = true, chain = false)
@RequiredArgsConstructor
Expand Down Expand Up @@ -71,17 +54,10 @@ private boolean isUnexceptional() {
return Exceptions.none(commonFragmentValues.exceptions);
}

public boolean txReverts() {
return commonFragmentValues.txMetadata.statusCode();
}

public Trace trace(Trace trace) {
final CallFrame frame = commonFragmentValues.callFrame;
final TransactionProcessingMetadata tx = commonFragmentValues.txMetadata;
final boolean isExec = commonFragmentValues.hubProcessingPhase == TX_EXEC;
final boolean oogx =
commonFragmentValues.tracedException() == TracedException.OUT_OF_GAS_EXCEPTION;
final boolean nonOogException = Exceptions.any(commonFragmentValues.exceptions) && !oogx;
return trace
.absoluteTransactionNumber(tx.getAbsoluteTransactionNumber())
.relativeBlockNumber(tx.getRelativeBlockNumber())
Expand Down Expand Up @@ -117,111 +93,11 @@ public Trace trace(Trace trace) {
.gasCost(Bytes.ofUnsignedLong(commonFragmentValues.gasCostToTrace()))
.gasNext(
Bytes.ofUnsignedLong(isExec && isUnexceptional() ? commonFragmentValues.gasNext : 0))
.refundCounter(
(commonFragmentValues.hubProcessingPhase == TX_EXEC)
? commonFragmentValues.gasRefund
: 0)
.refundCounterNew(
(commonFragmentValues.hubProcessingPhase == TX_EXEC)
? commonFragmentValues.gasRefundNew
: 0)
.refundCounter(isExec ? commonFragmentValues.gasRefund : 0)
.refundCounterNew(isExec ? commonFragmentValues.gasRefundNew : 0)
.twoLineInstruction(commonFragmentValues.TLI)
.counterTli(twoLineInstructionCounter)
.nonStackRows((short) commonFragmentValues.numberOfNonStackRows)
.counterNsr((short) nonStackRowsCounter);
}

static long computeGasCost(Hub hub, WorldView world) {

switch (hub.opCodeData().instructionFamily()) {
case ADD, MOD, SHF, BIN, WCP, EXT, BATCH, MACHINE_STATE, PUSH_POP, DUP, SWAP, INVALID -> {
if (Exceptions.outOfGasException(hub.pch().exceptions())
|| Exceptions.none(hub.pch().exceptions())) {
return hub.opCode().getData().stackSettings().staticGas().cost();
}
return 0;
}
case STORAGE -> {
switch (hub.opCode()) {
case SSTORE -> {
return gasCostSstore(hub, world);
}
case SLOAD -> {
return gasCostSload(hub, world);
}
default -> throw new RuntimeException(
"Gas cost not covered for " + hub.opCode().toString());
}
}
case HALT -> {
switch (hub.opCode()) {
case STOP -> {
return 0;
}
case RETURN, REVERT -> {
Bytes offset = hub.messageFrame().getStackItem(0);
Bytes size = hub.messageFrame().getStackItem(0);
return Exceptions.memoryExpansionException(hub.pch().exceptions())
? 0
: ZkTracer.gasCalculator.memoryExpansionGasCost(
hub.messageFrame(), offset.toLong(), size.toLong());
}
case SELFDESTRUCT -> {
SelfDestructOperation op = new SelfDestructOperation(ZkTracer.gasCalculator);
Operation.OperationResult operationResult =
op.execute(
hub.messageFrame(),
new EVM(
new OperationRegistry(),
ZkTracer.gasCalculator,
EvmConfiguration.DEFAULT,
EvmSpecVersion.LONDON));
long gasCost = operationResult.getGasCost();
Address recipient = Address.extract((Bytes32) hub.messageFrame().getStackItem(0));
Wei inheritance = world.get(hub.messageFrame().getRecipientAddress()).getBalance();
return ZkTracer.gasCalculator.selfDestructOperationGasCost(
world.get(recipient), inheritance);
}
}
return 0;
}
default -> {
throw new RuntimeException("Gas cost not covered for " + hub.opCode().toString());
}
}
}

static long gasCostSstore(Hub hub, WorldView world) {

final Address address = hub.currentFrame().accountAddress();
final Bytes32 storageKey = EWord.of(hub.messageFrame().getStackItem(0));

final UInt256 storageKeyUint256 = UInt256.fromBytes(hub.messageFrame().getStackItem(0));
final UInt256 valueNextUint256 = UInt256.fromBytes(hub.messageFrame().getStackItem(1));

final Supplier<UInt256> valueCurrentSupplier =
() -> world.get(address).getStorageValue(storageKeyUint256);
final Supplier<UInt256> valueOriginalSupplier =
() -> world.get(address).getOriginalStorageValue(storageKeyUint256);

final long storageCost =
ZkTracer.gasCalculator.calculateStorageCost(
valueNextUint256, valueCurrentSupplier, valueOriginalSupplier);
final boolean storageSlotWarmth =
hub.currentFrame().frame().getWarmedUpStorage().contains(address, storageKey);

return storageCost + (storageSlotWarmth ? 0L : ZkTracer.gasCalculator.getColdSloadCost());
}

static long gasCostSload(Hub hub, WorldView world) {
final Address address = hub.currentFrame().accountAddress();
final Bytes32 storageKey = EWord.of(hub.messageFrame().getStackItem(0));
final boolean storageSlotWarmth =
hub.currentFrame().frame().getWarmedUpStorage().contains(address, storageKey);

return ZkTracer.gasCalculator.getSloadOperationGasCost()
+ (storageSlotWarmth
? ZkTracer.gasCalculator.getWarmStorageReadCost()
: ZkTracer.gasCalculator.getColdSloadCost());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,8 @@ public CommonFragmentValues(Hub hub) {
this.height = callFrame.stack().getHeight();
this.heightNew = callFrame.stack().getHeightNew();

// TODO: partial solution, will not work in general
this.gasExpected = isExec ? computeGasExpected() : 0;
this.gasActual = isExec ? computeGasRemaining() : 0;
this.gasExpected = computeGasExpected();
this.gasActual = computeGasRemaining();
this.gasCost = isExec ? computeGasCost() : 0;
this.gasNext = isExec ? computeGasNext(exceptions) : 0;
this.gasCostExcluduingDeploymentCost = isExec ? computeGasCostExcludingDeploymentCost() : 0;
Expand Down Expand Up @@ -210,11 +209,10 @@ public long computeGasRemaining() {
}

public long computeGasExpected() {
if (hub.state().processingPhase() != TX_EXEC) return 0;

final CallFrame currentFrame = hub.currentFrame();

if (hub.state().processingPhase() != TX_EXEC) return 0;

if (currentFrame.executionPaused()) {
currentFrame.unpauseCurrentFrame();
return currentFrame.lastValidGasNext();
Expand Down Expand Up @@ -260,19 +258,19 @@ public long computeGasNext(short exceptions) {
return switch (hub.opCodeData().instructionFamily()) {
case KEC, COPY, STACK_RAM, STORAGE, LOG, HALT -> gasAfterDeductingCost;
case CREATE -> gasAfterDeductingCost;
// TODO: this is only part of the story because of
// Note: this is only part of the story because of
// 1. nonempty init code CREATE's where gas is paid out of pocket
// This is done in the CREATE section
case CALL -> gasAfterDeductingCost;
// TODO: this is only part of the story because of
// Note: this is only part of the story because of
// 1. aborts with value transfers (immediately reapStipend)
// 2. EOA calls with value transfer (immediately reapStipend)
// 3. SMC calls: gas paid out of pocket
// 4. PRC calls: gas paid out of pocket + special PRC cost + returned gas
// This is done in the CALL section

default -> // ADD, MUL, MOD, EXT, WCP, BIN, SHF, CONTEXT, ACCOUNT, TRANSACTION, BATCH, JUMP,
// MACHINE_STATE, PUSH_POP, DUP, SWAP, INVALID
// TODO: this may not work for EXP, EXTCODEHASH, EXTCODESIZE, BALANCE as they require extra
// care for pricing because of ⒈ warmth and ⒉ log computations
gasAfterDeductingCost;
};
}
Expand All @@ -298,8 +296,6 @@ public long gasCostToTrace() {
return 0;
}

// TODO @Olivier: special care for CALL's and CREATE's

return gasCost;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public long effectiveChildContextGasAllowance() {
public Trace trace(Trace trace) {
return trace
.pMiscStpFlag(true)
.pMiscStpInstruction(opCode.byteValue() & 0xff)
.pMiscStpInstruction(opCode.unsignedByteValue())
.pMiscStpGasHi(gas.hi())
.pMiscStpGasLo(gas.lo())
.pMiscStpValueHi(value.hi())
Expand Down
Loading
Loading