Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: BlockCapturer for Created Accounts #1291

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,36 @@

import org.apache.tuweni.units.bigints.UInt256;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.evm.account.Account;
import org.hyperledger.besu.evm.worldstate.WorldView;

public record StorageSnapshot(String address, String key, String value) {
public static Optional<StorageSnapshot> from(
Address address, UInt256 key, final WorldView world) {
return Optional.ofNullable(world.get(address))
.map(
account ->
new StorageSnapshot(
address.toHexString(),
key.toHexString(),
account.getStorageValue(key).toHexString()));
// Lookup account that was touched
Account account = world.get(address);
// Check account *really* exists from a storage perspective. This is important to distinguish
// for accounts which are created *during* the conflation. Such accounts may have technically
// existed before the conflation (e.g. they had a non-zero balance) but could still have been
// "created" during the conflation. In such case, this snapshot would be simply assigning 0x0
// to the given storage locations. However, we don't want to create a storage snapshot in such
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does taking a storage snapshot insert a storage value ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, at least according to the current EYP, storage does not matter when it comes to checking the possibility of doing a CREATE2

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, at least according to the current EYP

Yeah, but at the end of the day, we are not measuring against the EYP. We are comparing against execution on MAINNET using BESU. What I'm looking at is what BESU does. If that deviates from the EYP ... well, we still have to follow BESU.

However, I agree that it does seem wierd and I can follow up with BESU to see why they do it that way (or even it could be a bug).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does taking a storage snapshot insert a storage value ?

Yes, in the replay environment it does. See ReplayExecutionEnvironment.initWorld(). One option might be to drop StorageSnapshots which assign 0x0 at the beginning of the conflation. It has the same effect. Maybe you prefer this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But does taking a storage snapshot change the world state ? If so that is a massive problem.

Copy link
Collaborator

@OlivierBBB OlivierBBB Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case we do want to define storage snapshots even if the target account has empty storage or doesn't even exist yet. Simply because it's possible in Ethereum and we will want to test that, see #1205, but it should be side-effect free

// cases, as this then leads to a CREATE[2] failure when executing the conflation.
if (accountExists(account)) {
// Accounts exists, so create snapshot.
return Optional.of(
new StorageSnapshot(
address.toHexString(),
key.toHexString(),
account.getStorageValue(key).toHexString()));
} else {
return Optional.empty();
}
}

private static boolean accountExists(final Account account) {
Copy link
Collaborator

@OlivierBBB OlivierBBB Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIU the EYP defines for an account $a$

  • existence as existence in the state, i.e. $\sigma[a] \neq \varnothing$
  • EMPTY as having nontrivial nonce, balance or code
  • DEAD as either not existing or being empty
  • prohibited from being deployed at as existing and either having nontrivial nonce, code or storage
image image

along with EIP-7610

So maybe a more accurate name would be accountMayNotBeDeployedAt / deploymentIsProhibitedAt or so.

// The account exists if it has sent a transaction
// or already has its code initialized.
return account != null
&& (account.getNonce() != 0 || !account.getCode().isEmpty() || !account.isStorageEmpty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,4 +188,9 @@ void mainnet1339346ContextRevertTwice() {
void legacyTxWithoutChainID() {
replay(LINEA_SEPOLIA, "254251.sepolia.json.gz");
}

@Test
void incorrectCreationCapture() {
replay(LINEA_MAINNET, "4323985.json.gz");
}
}
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

package net.consensys.linea.testing;

import java.io.File;
import java.io.IOException;
import java.io.Reader;
import java.math.BigInteger;
import java.nio.file.Path;
Expand All @@ -24,6 +26,7 @@
import com.google.gson.Gson;
import lombok.Builder;
import lombok.extern.slf4j.Slf4j;
import net.consensys.linea.blockcapture.BlockCapturer;
import net.consensys.linea.blockcapture.snapshots.AccountSnapshot;
import net.consensys.linea.blockcapture.snapshots.BlockSnapshot;
import net.consensys.linea.blockcapture.snapshots.ConflationSnapshot;
Expand All @@ -35,6 +38,7 @@
import net.consensys.linea.zktracer.ZkTracer;
import net.consensys.linea.zktracer.module.constants.GlobalConstants;
import net.consensys.linea.zktracer.module.hub.Hub;
import org.apache.commons.io.FileUtils;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.units.bigints.UInt256;
import org.hyperledger.besu.consensus.clique.CliqueHelpers;
Expand Down Expand Up @@ -65,6 +69,12 @@ public class ReplayExecutionEnvironment {
/** Used for checking resulting trace files. */
private static final CorsetValidator CORSET_VALIDATOR = new CorsetValidator();

/**
* Determines whether to enable block capturing for conflations executed by this environment. This
* is used for primarily for debugging the block capturer.
*/
private static final boolean debugBlockCapturer = false;

/**
* Determines whether transaction results should be checked against expected results embedded in
* replay files. This gives an additional level of assurance that the tests properly reflect
Expand All @@ -75,14 +85,14 @@ public class ReplayExecutionEnvironment {
*/
private final boolean txResultChecking;

private final ZkTracer tracer = new ZkTracer();
private final ZkTracer zkTracer = new ZkTracer();

public void checkTracer(String inputFilePath) {
// Generate the output file path based on the input file path
Path inputPath = Paths.get(inputFilePath);
String outputFileName = inputPath.getFileName().toString().replace(".json.gz", ".lt");
Path outputPath = inputPath.getParent().resolve(outputFileName);
this.tracer.writeToFile(outputPath);
this.zkTracer.writeToFile(outputPath);
log.info("trace written to `{}`", outputPath);
// validation is disabled by default for replayBulk
// assertThat(CORSET_VALIDATOR.validate(outputPath).isValid()).isTrue();
Expand All @@ -104,7 +114,7 @@ public void replay(BigInteger chainId, final Reader replayFile) {
return;
}
this.executeFrom(chainId, conflation);
ExecutionEnvironment.checkTracer(tracer, CORSET_VALIDATOR, Optional.of(log));
ExecutionEnvironment.checkTracer(zkTracer, CORSET_VALIDATOR, Optional.of(log));
}

public void replay(BigInteger chainId, final Reader replayFile, String inputFilePath) {
Expand All @@ -122,7 +132,7 @@ public void replay(BigInteger chainId, final Reader replayFile, String inputFile

public void replay(BigInteger chainId, ConflationSnapshot conflation) {
this.executeFrom(chainId, conflation);
ExecutionEnvironment.checkTracer(tracer, CORSET_VALIDATOR, Optional.of(log));
ExecutionEnvironment.checkTracer(zkTracer, CORSET_VALIDATOR, Optional.of(log));
}

/**
Expand All @@ -133,11 +143,33 @@ public void replay(BigInteger chainId, ConflationSnapshot conflation) {
* @param conflation the conflation to replay
*/
private void executeFrom(final BigInteger chainId, final ConflationSnapshot conflation) {
ConflationAwareOperationTracer tracer = this.zkTracer;
BlockCapturer capturer = null;
// Configure block capturer (if applicable)
if (debugBlockCapturer) {
// Initialise world state from conflation
MutableWorldState world = initWorld(conflation);
capturer = new BlockCapturer();
capturer.setWorld(world.updater());
// Sequence zktracer and capturer
tracer = ConflationAwareOperationTracer.sequence(tracer, capturer);
}
// Execute the conflation
executeFrom(chainId, conflation, tracer, this.txResultChecking);
//
if (debugBlockCapturer) {
writeCaptureToFile(conflation, capturer);
}
}

private static void executeFrom(
final BigInteger chainId,
final ConflationSnapshot conflation,
final ConflationAwareOperationTracer tracer,
final boolean txResultChecking) {
BlockHashOperation.BlockHashLookup blockHashLookup = conflation.toBlockHashLookup();
ReferenceTestWorldState world =
ReferenceTestWorldState.create(new HashMap<>(), EvmConfiguration.DEFAULT);
// Initialise world state from conflation
initWorld(world.updater(), conflation);
MutableWorldState world = initWorld(conflation);
// Construct the transaction processor
final MainnetTransactionProcessor transactionProcessor =
ExecutionEnvironment.getProtocolSpec(chainId).getTransactionProcessor();
Expand All @@ -163,7 +195,7 @@ private void executeFrom(final BigInteger chainId, final ConflationSnapshot conf
header,
tx,
CliqueHelpers.getProposerOfBlock(header),
buildOperationTracer(tx, txs.getOutcome()),
buildOperationTracer(tx, txs.getOutcome(), tracer, txResultChecking),
blockHashLookup,
false,
Wei.ZERO);
Expand All @@ -176,36 +208,38 @@ private void executeFrom(final BigInteger chainId, final ConflationSnapshot conf
}

public Hub getHub() {
return tracer.getHub();
return zkTracer.getHub();
}

/**
* Initialise a world updater given a conflation. Observe this can be applied to any WorldUpdater,
* such as SimpleWorld.
* Initialise a fresh world state from a conflation.
*
* @param world The world to be initialised.
* @param conflation The conflation from which to initialise.
*/
private static void initWorld(WorldUpdater world, final ConflationSnapshot conflation) {
private static MutableWorldState initWorld(final ConflationSnapshot conflation) {
ReferenceTestWorldState world =
ReferenceTestWorldState.create(new HashMap<>(), EvmConfiguration.DEFAULT);
WorldUpdater updater = world.updater();
for (AccountSnapshot account : conflation.accounts()) {
// Construct contract address
Address addr = Address.fromHexString(account.address());
// Create account
MutableAccount acc =
world.createAccount(
updater.createAccount(
Words.toAddress(addr), account.nonce(), Wei.fromHexString(account.balance()));
// Update code
acc.setCode(Bytes.fromHexString(account.code()));
}
// Initialise storage
for (StorageSnapshot s : conflation.storage()) {
world
updater
.getAccount(Words.toAddress(Bytes.fromHexString(s.address())))
.setStorageValue(UInt256.fromHexString(s.key()), UInt256.fromHexString(s.value()));
}
//
world.commit();
// Commit changes
updater.commit();
// Done
return world;
}

/**
Expand All @@ -216,7 +250,11 @@ private static void initWorld(WorldUpdater world, final ConflationSnapshot confl
* @param txs TransactionResultSnapshot which contains the expected result of this transaction.
* @return An implementation of OperationTracer which packages up the appropriate behavour.
*/
private OperationTracer buildOperationTracer(Transaction tx, TransactionResultSnapshot txs) {
private static OperationTracer buildOperationTracer(
Transaction tx,
TransactionResultSnapshot txs,
ConflationAwareOperationTracer tracer,
boolean txResultChecking) {
if (txs == null) {
String hash = tx.getHash().toHexString();
log.info("tx `{}` outcome not checked (missing)", hash);
Expand All @@ -229,4 +267,30 @@ private OperationTracer buildOperationTracer(Transaction tx, TransactionResultSn
return ConflationAwareOperationTracer.sequence(txs.check(), tracer);
}
}

// Write the captured replay for a given conflation snapshot to a file. This is used to debug the
// BlockCapturer by
// making sure, for example, that captured replays still execute correctly.
private static void writeCaptureToFile(ConflationSnapshot conflation, BlockCapturer capturer) {
// Extract capture name
String json = capturer.toJson();
// Determine suitable filename
long startBlock = Long.MAX_VALUE;
long endBlock = Long.MIN_VALUE;
//
for (BlockSnapshot blk : conflation.blocks()) {
startBlock = Math.min(startBlock, blk.header().number());
endBlock = Math.max(endBlock, blk.header().number());
}
//
String filename = String.format("capture-%d-%d.json", startBlock, endBlock);
try {
File file = new File(filename);
log.info("Writing capture to " + file.getCanonicalPath());
FileUtils.writeStringToFile(file, json);
} catch (IOException e) {
// Problem writing capture
throw new RuntimeException(e);
}
}
}
Loading