Consensys / linea-tracer

Part of the Linea stack responsible for extracting data from the execution of an EVM client in order to construct large matrices called execution traces.
https://linea.build
Other
35 stars 27 forks source link

better categorization of failing root causes #1420

Open FlorianHuc opened 1 month ago

FlorianHuc commented 1 month ago

In case corset throw an exception because balance is too big, instead of throwing a generic exception like:

balanceXorByteCodeAddressLoXorExpData2XorHashInfoKeccakLoXorStorageKeyHiXorFromAddressLo has invalid width (256bits)

We should send a specific exception like 'BalanceTooBigException" so that we can ignore tests throwing such exceptions.

We need to implement the same for other known limitations.

DavePearce commented 1 month ago

@FlorianHuc Ok, let's have a think about this. The key challenge is that corset itself knows nothing about balances, etc. Currently, for the problematic column we have this:

public Trace pAccountBalance(final Bytes b) {
    if (filled.get(134)) {
      throw new IllegalStateException("hub.account/BALANCE already set");
    } else {
      filled.set(134);
    }

    // Trim array to size
    Bytes bs = b.trimLeadingZeros();
    // Sanity check against expected width
    if (bs.bitLength() > 128) {
      throw new IllegalArgumentException(
          "balanceXorByteCodeAddressLoXorExpData2XorHashInfoKeccakLoXorStorageKeyHiXorFromAddressLo has invalid width ("
              + bs.bitLength()
              + "bits)");
    }

Clearly, at a minimum, the second exception could use the column name hub.account/BALANCE instead of the cryptic register name balanceXorByteCode.... That's easy to fix.

Other options:

Then, we could filter on the type of exception and the specific column as well. Is that enough?

FlorianHuc commented 1 month ago

would it make sense to check this in:

public class AccountSnapshot {
  private Address address;
  private long nonce;
  private Wei balance; 

?

Maybe we could imagine a mechanism allowing the 'inputs' to the arithmetization to be validated but still centralized so that it's easy to find the hypothesys that are made?

DavePearce commented 1 month ago

would it make sense to check this in:

Hmmm, I'm not really following you here. The Trace.java file where the exceptions are arising is automatically generated from the constraints themselves.

Maybe we could imagine a mechanism allowing the 'inputs' to the arithmetization to be validated but still centralized so that it's easy to find the hypothesys that are made?

To be honest, a sensible option would be to provide a constructor for AccountBalance (i.e. dropping the @AllArgsConstructor annotation) and then throwing a specific exception within that for the balance.