search

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 22 forks source link

`ToyExecutionEnvironmentV2` may need to use `EMPTY_VALIDATOR` instead of `DEFAULT_VALIDATOR` #1475

Open lorenzogentile404 opened 3 days ago

lorenzogentile404 commented 3 days ago

In order to deal with the issue https://github.com/Consensys/linea-tracer/issues/1460, we had to change the following in ToyExecutionEnvironmentV2:

  @Builder.Default
  private final TransactionProcessingResultValidator transactionProcessingResultValidator =
      TransactionProcessingResultValidator.EMPTY_VALIDATOR;
  // This was previously DEFAULT_VALIDATOR, however some tests we write are supposed to generate failing transactions
  // Thus we cannot use the DEFAULT_VALIDATOR since it asserts that the transaction is successful

The reason is that DEFAULT_VALIDATOR asserts that the transaction is successful, indeed:

@FunctionalInterface
public interface TransactionProcessingResultValidator {

  void accept(Transaction transaction, TransactionProcessingResult transactionProcessingResult);

  TransactionProcessingResultValidator DEFAULT_VALIDATOR =
      (t, r) -> {
        assertTrue(
            r.isSuccessful(),
            () ->
                "Transaction: %s not successful. %s"
                    .formatted(t.getHash().toString(), r.toString()));
      };

  TransactionProcessingResultValidator EMPTY_VALIDATOR = (t, r) -> {};
}

However, in some tests we intentionally create failing transactions. @gauravahuja does it make sense to you too?

OlivierBBB commented 3 days ago

The main point being: we want tests with failing transactions, we don't expect our tests to produce successful transactions. Thus using the DEFAULT_VALIDATOR seems out of place given that it asserts the transaction is successful.