`HUB` debugging continued #1492

OlivierBBB · 2024-11-09T12:00:16Z

No description provided.

added a new "canonical" method for AccountSnapshot's

The WorldUpdater of the frame gets discarded in case of an exceptional halt. Besu does this BEFORE it traceContextExit. This fix does creates the TxFinalizationSection before the wiping of the WorldUpdater so as to extract the "unupdated" balances. Note. We will have to apply a similar fix to other operations that make snapshots at the boundaries of context changes: CALL's, CREATE's, RETURN's and SELFDESTRUCT's to name a few, and more generally those opcodes that are REVERT sensitive.

In the TraceSection() constructor we were using the Hub's raisesOogxOrIsUnexceptional method, which queries the currentTraceSection. However, and at that point, this trace section isn't the current one. And its 'exceptions' object is that of the previous opcode.

In the prewarming section we create account and storage fragments using the current hub.stamp(). However it is only later that we create the TraceSection. It is this step that raises the HUB_STAMP. We therefore pre-increment the HUB_STAMP when filling the DOM/SUB stamps.

There was a bug in that we may want to write the result of a KECCAK even if we don't set the hashInfoFlag. This occurs for unexceptional KEC's and for unexceptional, unaborted CREATE2's, both with nonzero size parameter.

GAS_NEXT now returns remainingGas - upfrontGasCost this works for most opCodes. CALL's and CREATE's have specialized methods to amend GAS_NEXT depending on the scenario.

Amending GAS_NEXT is more involved for precompiles.

…n the relevant sections

Elite dead beef

We talk about child context's and child frames a lot. Also child frames are created also for deployments where "caller" isn't the right term for the parent context.

This is required for both CALL's and CREATE's that actually "enter" a child context with non empty bytecode and nonempty initialization code respectively. We need to write both CCSR: CHILD_CONTEXT_SELF_REVERTS CCRS: CHILD_CONTEXT_REVERT_STAMP This information only becomes avaiable once the child frame's relevant fields were discovered.

This applies whenever we call a SMC with nonempty bytecode.

…ated data

this fragment is initialized only for the TraceSection's arising from exceptional opCodes. It is appended to its section at the moment of line counting.

OlivierBBB · 2024-11-16T04:55:52Z

arithmetization/src/main/java/net/consensys/linea/zktracer/module/gas/Gas.java

@@ -74,7 +74,7 @@ public void commit(List<MappedByteBuffer> buffers) {
  public void resolvePostExecution(
      Hub hub, MessageFrame frame, Operation.OperationResult operationResult) {
    gasParameters.gasActual(BigInteger.valueOf(commonValues.gasActual));
-    gasParameters.gasCost(BigInteger.valueOf(commonValues.gasCost()));
+    gasParameters.gasCost(BigInteger.valueOf(commonValues.gasCostToTrace()));


We don't always compute the gas cost (e.g. if an exception occurs that has greater priority, e.g. SUX/SOX/STATICX/MXPX/... but we always fill the GAS_COST column. We fill it with some value which we call gasCostToTrace.

OlivierBBB · 2024-11-16T04:57:59Z

arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/AccountSnapshot.java

+        hub.transients.conflation().deploymentInfo(),
+        isAddressWarm(hub.messageFrame(), address));
+  }
+


At times the trace method we are using provides us with a WorldView. When that is the case it is often more accurate to use this world rather than the maybe deprecated hub.messageFrame().getWorldUpdater() that we use in the other canonical method. This method is then preferred.

This issue arose e.g. in traceEndTransaction and was the reason for why some accounts appeared with invalid balances.

OlivierBBB · 2024-11-16T04:59:19Z

arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/Hub.java

@@ -634,7 +634,7 @@ public void traceContextEnter(MessageFrame frame) {

      final long callDataContextNumber = callStack.currentCallFrame().contextNumber();

-      currentFrame().rememberGasNextBeforePausing();
+      currentFrame().rememberGasNextBeforePausing(this);


We need to get the gasNext from the hub, likely from the commonValuesFragment of the currentTraceSection;

OlivierBBB · 2024-11-16T04:59:44Z

arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/Hub.java

      }
    }

    defers.resolveUponContextExit(this, this.currentFrame());
    // TODO: verify me please @Olivier
    if (this.currentFrame().opCode() == REVERT || Exceptions.any(pch.exceptions())) {
-      defers.resolvePostRollback(this, frame, this.currentFrame());
+      defers.resolveUponRollback(this, frame, this.currentFrame());


Renaming to be consistent with the other methods.

OlivierBBB · 2024-11-16T05:03:02Z

arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/Hub.java

+      if (state.getProcessingPhase() != TX_SKIP
+          && frame.getState() == MessageFrame.State.COMPLETED_SUCCESS) {
+        this.state.setProcessingPhase(TX_FINL);
+        new TxFinalizationSection(this, frame.getWorldUpdater(), false);


There are now 2 places that can trigger the creation of a TxFinalizationSection:

traceContextExit

tracePostExecution

the reason being: the root context behaves differently in Besu. But looking at Besu I don't remember why that is. And maybe we want to change that and only trace it during traceContextExit

you mean we don't traceContextExit the root context in case of exception ?

OlivierBBB · 2024-11-16T05:35:22Z

...ion/src/main/java/net/consensys/linea/zktracer/module/hub/section/TxFinalizationSection.java

    recipientSnapshotAfterTxFinalization.turnOnWarmth(); // purely constraints based

    final Address coinbaseAddress = coinbaseSnapshotBeforeTxFinalization.address();
-    coinbaseSnapshotAfterFinalization = AccountSnapshot.canonical(hub, coinbaseAddress);
+    coinbaseSnapshotAfterFinalization = AccountSnapshot.canonical(hub, world, coinbaseAddress);


Using the correct world to make snapshots. This world is provided to us in traceContextExit (in the unexceptional case.)

OlivierBBB · 2024-11-16T05:37:57Z

.../src/main/java/net/consensys/linea/zktracer/module/hub/section/TxPreWarmingMacroSection.java

@@ -80,7 +80,13 @@ public TxPreWarmingMacroSection(WorldView world, Hub hub) {

                  final DomSubStampsSubFragment domSubStampsSubFragment =
                      new DomSubStampsSubFragment(
-                          DomSubStampsSubFragment.DomSubType.STANDARD, hub.stamp(), 0, 0, 0, 0, 0);
+                          DomSubStampsSubFragment.DomSubType.STANDARD,
+                          hub.stamp() + 1,


We have to pre-emptively raise the hub.stamp() as this only happens later.

yes true, we raise h only when creating the section ! :) nice

OlivierBBB · 2024-11-16T05:38:52Z

...hmetization/src/main/java/net/consensys/linea/zktracer/module/hub/section/TxSkipSection.java

                DomSubStampsSubFragment.standardDomSubStamps(hub.stamp(), 0));

    // recipient account fragment
    final AccountFragment recipientAccountFragment =
        hub.factories()
            .accountFragment()
-            .make(
+            .makeWithTrm(


This is required by the arithmetization, specifically: in the permuted domain the first appearance of an account should be with some trimming.

there is a issue for this, did you link it ?

edit: I did

OlivierBBB · 2024-11-16T05:42:02Z

...tization/src/main/java/net/consensys/linea/zktracer/module/hub/section/call/CallSection.java

-          postOpcodeCallerSnapshot.decrementBalanceBy(value);
-          preOpcodeCalleeSnapshot.decrementBalanceBy(value);
+          preOpcodeCalleeSnapshot = postOpcodeCallerSnapshot;
+          postOpcodeCalleeSnapshot = preOpcodeCallerSnapshot;


OlivierBBB · 2024-11-16T05:44:37Z

...tization/src/main/java/net/consensys/linea/zktracer/module/hub/section/call/CallSection.java

        scenarioFragment.setScenario(CALL_SMC_FAILURE_WONT_REVERT);

        if (selfCallWithNonzeroValueTransfer) {
          childContextExitCallerSnapshot.decrementBalanceBy(value);
          reEntryCalleeSnapshot.decrementBalanceBy(value);
        }

+        int childId = hub.currentFrame().childFramesId().getLast();
+        CallFrame childFrame = hub.callStack().getById(childId);
+        int childContextRevertStamp = childFrame.revertStamp();


Note. This revertStamp is that of a "self-induced" revert. Not the actual revert stamp of the child context. The child call / deployment may be successful (in which case we don't enter here) and be reverted later. Here we only care about children that are failures.

Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>

We (or I) don't really understand when Besu's MessageFrame is EXCEPTIONAL_HALT, in particular as it relates to - invalidCodePrefixException - maxCodeSizeException for RETURN instructions in a deployment context

OlivierBBB · 2024-11-16T06:48:14Z

arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/Hub.java

@@ -753,7 +753,7 @@ public void tracePostExecution(MessageFrame frame, Operation.OperationResult ope
    }

    if (frame.getDepth() == 0
-        && (frame.getState() == MessageFrame.State.EXCEPTIONAL_HALT || opCode() == REVERT)) {
+        && (isExceptional() || opCode() == REVERT)) {


It's unclear exactly when Besu's MessageFrame enters the EXCEPTIONAL_HALT state. This is particularly true wrt deployment related exceptions (invalidCodePrefixException and maxCodeSizeException) for whatever reason.

letypequividelespoubelles

LGTM, some minor comments

letypequividelespoubelles · 2024-11-16T06:40:05Z

arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/AccountSnapshot.java

+  public AccountSnapshot setDeploymentInfo(Hub hub) {
+    return this.setDeploymentInfo(hub.transients.conflation().deploymentInfo());
+  }
+
  public AccountSnapshot setDeploymentInfo(DeploymentInfo deploymentInfo) {


Why not having a @Setter ?

letypequividelespoubelles · 2024-11-16T06:43:18Z

arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/Hub.java

+      if (state.getProcessingPhase() != TX_SKIP
+          && frame.getState() == MessageFrame.State.COMPLETED_SUCCESS) {
+        this.state.setProcessingPhase(TX_FINL);
+        new TxFinalizationSection(this, frame.getWorldUpdater(), false);


you mean we don't traceContextExit the root context in case of exception ?

letypequividelespoubelles · 2024-11-16T06:45:05Z

arithmetization/src/main/java/net/consensys/linea/zktracer/module/hub/Hub.java

@@ -750,6 +751,11 @@ public void tracePostExecution(MessageFrame frame, Operation.OperationResult ope
    if (!this.currentFrame().opCode().isCall() && !this.currentFrame().opCode().isCreate()) {
      this.unlatchStack(frame, currentSection);
    }
+
+    if (frame.getDepth() == 0 && (isExceptional() || opCode() == REVERT)) {


That's strange that we don't traceContextExit in case of exception in the root context ...

I probably phrased that wrong. We certainly don't traceContextReEnter as there is nothing to re-enter. TBH I'm not sure it makes sense to have two entry points for TxFinalizationSection.

I'd prefer to have only one entry point, yes ...

letypequividelespoubelles · 2024-11-16T06:48:48Z

...metization/src/main/java/net/consensys/linea/zktracer/module/hub/fragment/StackFragment.java

-          case CREATE2 -> Exceptions.none(exceptions) && aborts.none() && gp.messageSize() > 0;
-          default -> false;
-        };
+        Exceptions.none(exceptions)


not sure it's an optim, we'll add two more operation (Exceptions.none and gp.messageSize > 0 ) for every stack row ... and it doesn't make the code much easier to read

I exchanged positions so that these checks are done after the switch. I believe that Java is smart enough to ignore conditions in a chain of &&'s if a previous condition was false.

letypequividelespoubelles · 2024-11-16T06:51:28Z

.../main/java/net/consensys/linea/zktracer/module/hub/fragment/common/CommonFragmentValues.java

+        //  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
+


can't we get the gasNext from besu (ie from the next section, in seal() method ?

letypequividelespoubelles · 2024-11-16T06:58:45Z

.../src/main/java/net/consensys/linea/zktracer/module/hub/section/TxPreWarmingMacroSection.java

@@ -80,7 +80,13 @@ public TxPreWarmingMacroSection(WorldView world, Hub hub) {

                  final DomSubStampsSubFragment domSubStampsSubFragment =
                      new DomSubStampsSubFragment(
-                          DomSubStampsSubFragment.DomSubType.STANDARD, hub.stamp(), 0, 0, 0, 0, 0);
+                          DomSubStampsSubFragment.DomSubType.STANDARD,
+                          hub.stamp() + 1,


yes true, we raise h only when creating the section ! :) nice

letypequividelespoubelles · 2024-11-16T06:59:09Z

...hmetization/src/main/java/net/consensys/linea/zktracer/module/hub/section/TxSkipSection.java

                DomSubStampsSubFragment.standardDomSubStamps(hub.stamp(), 0));

    // recipient account fragment
    final AccountFragment recipientAccountFragment =
        hub.factories()
            .accountFragment()
-            .make(
+            .makeWithTrm(


there is a issue for this, did you link it ?

edit: I did

letypequividelespoubelles · 2024-11-16T07:03:17Z

testing/src/main/java/net/consensys/linea/testing/BytecodeRunner.java

@@ -107,7 +107,7 @@ public void run(Wei senderBalance, Long gasLimit, List<ToyAccount> additionalAcc
        ToyTransaction.builder()
            .sender(senderAccount)
            .to(receiverAccount)
-            .value(Wei.ONE)
+            .value(Wei.of(69))


Mostly to disambiguate with other 1's in the trace 🙂.

letypequividelespoubelles · 2024-11-16T07:03:59Z

testing/src/main/java/net/consensys/linea/testing/BytecodeRunner.java

@@ -41,7 +41,7 @@
 */
 @Accessors(fluent = true)
 public final class BytecodeRunner {
-  public static final long DEFAULT_GAS_LIMIT = 25_000_000L;
+  public static final long DEFAULT_GAS_LIMIT = 61_000_000L;


could use the constant from GlabalConsta 'LINEA_BLOCK_GAS_LIMIT`or whatever

OlivierBBB · 2024-11-16T07:21:29Z

#1492 (comment)

No sure. I don't think this would work at the boundaries of execution contexts for instance.

OlivierBBB added 7 commits November 8, 2024 15:23

feat: simple test for call data manipulations inside of a CALL

c2f814f

fix: CallDataTests fix

353aed9

feat: simplest possible value transfer tests (w/ or w/o REVERT)

8254192

ras

26249ef

fix: remove redundant "parentTrace" field

56bfc83

fix: correct AccountSnapshots for TX_FINL phase

306a9f9

added a new "canonical" method for AccountSnapshot's

OlivierBBB linked an issue Nov 9, 2024 that may be closed by this pull request

HUB debugging continued #1490

Closed

OlivierBBB added 22 commits November 11, 2024 17:41

fix: correct gasCost in GAS + moved method to CommonFragmentValues

2cb5baa

ras: renaming test

cda3310

ras: extracted failing KECCAK tests

f293ba5

ras: slight simplification for stack/HASH_INFO_FLAG

7a92ea5

fix: triggerHashInfo -> writeHashInfoResult

365ad65

There was a bug in that we may want to write the result of a KECCAK even if we don't set the hashInfoFlag. This occurs for unexceptional KEC's and for unexceptional, unaborted CREATE2's, both with nonzero size parameter.

ras

ceeb2f0

ras

65134e2

fix: unlatching the stack for aborted CALL's and CREATE's

994941a

feat: straightforward CALL tests

290f05a

feat: some more simple CALL tests

08a0ad5

fix: CommonValueFragemnt's GAS_NEXT

f816fe8

GAS_NEXT now returns remainingGas - upfrontGasCost this works for most opCodes. CALL's and CREATE's have specialized methods to amend GAS_NEXT depending on the scenario.

fix: amending GAS_NEXT for CALL's in some cases (no PRC yet)

d41dfdc

Amending GAS_NEXT is more involved for precompiles.

fix: unlatch stack for aborted CREATE's

c806dad

fix: stack unlatching for aborted CALL's and CREATE's done directly i…

c36f4a6

…n the relevant sections

ras: changed the default value of a yet to be unlatched stack item

665c1a8

Elite dead beef

ras: renaming mostly

0f17ac4

fix: trim createe address in TX_SKIP section

afd8830

fix: corrected "lastValidGasNext" value

2f181b0

fix: correcting GAS_NEXT for CREATE's with 63/64-ths stuff

af4c7e5

fix: give frame to CallSection construtor

c2e0705

OlivierBBB added 17 commits November 15, 2024 14:15

fix: ACC instruction family now does what the spec recommends

ba52c2e

ras: new way to set deployment info data in AccountSnapshot

aa075e5

fixes to CALL's

787bec6

ras: rename CallFrame's callerId -> parentId

b3fb4dd

We talk about child context's and child frames a lot. Also child frames are created also for deployments where "caller" isn't the right term for the parent context.

ras: clean up

90570f8

fix: CallSection MISC fragment schedules itself for ContextReEntry

18dfa8b

This applies whenever we call a SMC with nonempty bytecode.

ras: readability for CreateSection

4243d04

fix: CreateSection MISC fragment schedules itself for ContextReEntry

684e699

fix: finalContextFragment was receiving incongruous "return data" rel…

fa9eb5b

…ated data

fix: CallSection now uses the correct child context revert data

98f3c35

fix: TraceSection includes an exceptionalContextFragment

2314760

this fragment is initialized only for the TraceSection's arising from exceptional opCodes. It is appended to its section at the moment of line counting.

ras

d47b0a1

fix: we grag CCSR and CCRS from the CallFrame directly

283e195

ras

c7f4178

Merge branch 'arith-dev' into 1490-hub-debugging-continued

0764fe7

spotless

de5ea47

OlivierBBB commented Nov 16, 2024

View reviewed changes

OlivierBBB requested a review from letypequividelespoubelles November 16, 2024 05:50

OlivierBBB and others added 4 commits November 16, 2024 12:54

ras

2c90d8c

spotless

a8a06dc

Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>

fix: rely on the HUB's assessment of exceptions

bbfc78c

We (or I) don't really understand when Besu's MessageFrame is EXCEPTIONAL_HALT, in particular as it relates to - invalidCodePrefixException - maxCodeSizeException for RETURN instructions in a deployment context

spotless

8d829a2

OlivierBBB commented Nov 16, 2024

View reviewed changes

letypequividelespoubelles approved these changes Nov 16, 2024

View reviewed changes

letypequividelespoubelles linked an issue Nov 16, 2024 that may be closed by this pull request

Calling TRM on deployment address during unexceptional, unaborted CREATE instructions #1447

Closed

fix: fewer checks

15728b6

OlivierBBB merged commit 44af7e6 into arith-dev Nov 16, 2024
3 checks passed

OlivierBBB deleted the 1490-hub-debugging-continued branch November 16, 2024 07:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`HUB` debugging continued #1492

`HUB` debugging continued #1492

OlivierBBB commented Nov 9, 2024

OlivierBBB Nov 16, 2024

OlivierBBB Nov 16, 2024

OlivierBBB Nov 16, 2024

OlivierBBB Nov 16, 2024

OlivierBBB Nov 16, 2024

letypequividelespoubelles Nov 16, 2024

OlivierBBB Nov 16, 2024

OlivierBBB Nov 16, 2024

letypequividelespoubelles Nov 16, 2024

OlivierBBB Nov 16, 2024

letypequividelespoubelles Nov 16, 2024 •

edited

Loading

OlivierBBB Nov 16, 2024

OlivierBBB Nov 16, 2024

OlivierBBB Nov 16, 2024

letypequividelespoubelles left a comment

letypequividelespoubelles Nov 16, 2024

letypequividelespoubelles Nov 16, 2024

letypequividelespoubelles Nov 16, 2024

OlivierBBB Nov 16, 2024

letypequividelespoubelles Nov 16, 2024

letypequividelespoubelles Nov 16, 2024

OlivierBBB Nov 16, 2024

letypequividelespoubelles Nov 16, 2024

letypequividelespoubelles Nov 16, 2024

letypequividelespoubelles Nov 16, 2024 •

edited

Loading

letypequividelespoubelles Nov 16, 2024

OlivierBBB Nov 16, 2024

letypequividelespoubelles Nov 16, 2024

OlivierBBB commented Nov 16, 2024

HUB debugging continued #1492

HUB debugging continued #1492

Conversation

OlivierBBB commented Nov 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

letypequividelespoubelles Nov 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

letypequividelespoubelles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

letypequividelespoubelles Nov 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlivierBBB commented Nov 16, 2024

`HUB` debugging continued #1492

`HUB` debugging continued #1492

letypequividelespoubelles Nov 16, 2024 •

edited

Loading

letypequividelespoubelles Nov 16, 2024 •

edited

Loading