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

Rationale for skipping BlockchainReferenceTest_612/suicideStorageCheck #1678

Open
OlivierBBB opened this issue Jan 6, 2025 · 1 comment
Open
Labels
bug Something isn't working debugging documentation Improvements or additions to documentation

Comments

@OlivierBBB
Copy link
Collaborator

OlivierBBB commented Jan 6, 2025

Main point

The reference tests, specifically BlockchainReferenceTest_612.java and suicideStorageCheck, contain a scenario which cannot happen on Linea, which is unsupported by both constraints and tracer. The issue manifests as a RomChunk that is un-locatable.

Details

The RLP_TXN module requires the CFI for (nontrivial) deployments and stores it in traceValue.codeFragmentIndex in RlpTxn.java. The computation goes as follows:

    traceValue.codeFragmentIndex =
        chunk.tx().getTo().isEmpty() && chunk.requireEvmExecution()
            ? this.romLex.getCodeFragmentIndexByMetadata(
                ContractMetadata.make(
                    Address.contractAddress(chunk.tx().getSender(), chunk.tx().getNonce()),
                    1,
                    true))
            : 0;

Note. That 1 is also hardcoded in the constraints. This could be changed, one could instead simply impose something à la

account/DEPLOYMENT_NUMBER_NEW = 1 + account/DEPLOYMENT_NUMBER

and drop the explicit value assignments. In this case the RlpTxnOperation ought to record the initial deployment number before the deployment transaction.

Note. This would serve no practical purpose.

How to break it

To trigger the failing test run BlockchainReferenceTest_612.java, specifically suicideStorageCheck:

  private static final String[] TEST_CONFIG_FILE_DIR_PATH = new String[] {
          // "BlockchainTests/ValidBlocks/bcStateTests/selfdestructBalance.json",
          // "BlockchainTests/ValidBlocks/bcStateTests/simpleSuicide.json",
          // "BlockchainTests/ValidBlocks/bcStateTests/suicideCoinbase.json",
          // "BlockchainTests/ValidBlocks/bcStateTests/suicideCoinbaseState.json",
          "BlockchainTests/ValidBlocks/bcStateTests/suicideStorageCheck.json"
  };

What breaks

In suicideStorageCheck there are 2 transactions:

  • the first one, a MESSAGE_CALL transaction, provokes a SELFDESTRUCT at address contract_address = 0xec0e71ad0a90ffe1909d27dac207f7680abba42d; the deploymentNumber thus shoots from 0 to 1;
  • the second one, a CONTRACT_CREATION transaction, deploys, lo and behold, at that same address, deployment_address = 0xec0e71ad0a90ffe1909d27dac207f7680abba42d; the deployment number thus shoots from 1 to 2;
  • being a deployment transaction, the RlpTxn looks for a RomChunk with deploymentNumber 1; the correct deploymentNumber, however, ought to be 2;

This cannot happen irl. But this raises the following issue. During the tracing one attempts to find a CFI based on getCodeFragmentIndexByMetadata with deployment number 1, though given the scenario the deployment number should be 2.

@OlivierBBB OlivierBBB added bug Something isn't working debugging documentation Improvements or additions to documentation labels Jan 6, 2025
@OlivierBBB OlivierBBB changed the title Multi transaction tests with CREATE's, CALL's and SELFDESTRUCT's Rationale for skipping BlockchainReferenceTest_612/suicideStorageCheck Jan 6, 2025
OlivierBBB added a commit that referenced this issue Jan 6, 2025
@letypequividelespoubelles
Copy link
Collaborator

agree to skip this ref test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working debugging documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants