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

Implement EIP-158 and EIP-2200 which allow to pass 7 extra spec tests #18

Merged
merged 6 commits into from
Feb 7, 2024
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
9 changes: 8 additions & 1 deletion src/blockchain/blockchain.zig
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ pub const Blockchain = struct {
var gas_available = block.header.gas_limit;
for (block.transactions) |tx| {
const txn_info = try checkTransaction(allocator, tx, block.header.base_fee_per_gas, gas_available, txn_signer);

const env: Environment = .{
.origin = txn_info.sender_address,
.block_hashes = chain.last_256_blocks_hashes,
Expand All @@ -171,6 +172,7 @@ pub const Blockchain = struct {
.chain_id = chain.chain_id,
};

try state.startTx();
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is a new method in statedb which does some tx-scope clearing for:

  • Account access lists
  • Storage slot accessed list.
  • and other tasks that are relevant whenever we start a new tx.

const exec_tx_result = try processTransaction(allocator, env, tx);
gas_available -= exec_tx_result.gas_used;

Expand Down Expand Up @@ -288,7 +290,12 @@ pub const Blockchain = struct {

// TODO: self destruct processing
// for address in output.accounts_to_delete:
// destroy_account(env.state, address)
// destroy_account(env.state, address)

for (env.state.touched_addresses.items) |address| {
if (env.state.isEmpty(address))
env.state.destroyAccount(address);
}
Comment on lines +295 to +298
Copy link
Owner Author

Choose a reason for hiding this comment

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

Now statedb tracks all touched addresses, so it can destroy them as explained in EIP-158.


return .{ .gas_used = total_gas_used };
}
Expand Down
90 changes: 83 additions & 7 deletions src/blockchain/vm.zig
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,80 @@ const EVMOneHost = struct {
evmclog.debug("setStorage addr=0x{} key={} value={}", .{ fmtSliceHexLower(&address), fmtSliceHexLower(&key.*.bytes), fmtSliceHexLower(&value.*.bytes) });

const vm: *VM = @as(*VM, @alignCast(@ptrCast(ctx.?)));

const k = std.mem.readIntSlice(u256, &key.*.bytes, std.builtin.Endian.Big);
const storage_status: evmc.enum_evmc_storage_status = blk: {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is the most important gap and change of this PR.
See the link mentioned in L163 and EIP-2200.

What we do now, is that stated has a reference to the state of the world when the tx started. This allows to do this logic of comparing the original vs current vs new value to charge the appropriate gas.

const original_value = vm.env.state.getOriginalStorage(address, k);
const current_value = vm.env.state.getStorage(address, k);
const new_value = value.*.bytes;
const zero = std.mem.zeroes([32]u8);

// See: https://evmc.ethereum.org/group__EVMC.html#gae012fd6b8e5c23806b507c2d3e9fb1aa

// EIP-220: 2.
if (std.mem.eql(u8, &current_value, &new_value)) {
break :blk evmc.EVMC_STORAGE_ASSIGNED;
}
// EIP-220: 3.

// EIP-220: 3.1
if (std.mem.eql(u8, &original_value, &current_value)) {
// EIP-220: 3.1.1
if (std.mem.eql(u8, &original_value, &zero)) {
// 0->0->Z
break :blk evmc.EVMC_STORAGE_ADDED;
}
if (std.mem.eql(u8, &new_value, &zero)) {
// X->X->0
break :blk evmc.EVMC_STORAGE_DELETED;
}
// X->X->Z
break :blk evmc.EVMC_STORAGE_MODIFIED;
}

// EIP-220: 3.2
// X != Y

// EIP-220: 3.2.1
if (!std.mem.eql(u8, &original_value, &zero)) {
// EIP-220: 3.2.1.1
if (std.mem.eql(u8, &current_value, &zero)) {
// X->0->Z
break :blk evmc.EVMC_STORAGE_DELETED_ADDED;
}
// EIP-220: 3.2.1.2
if (std.mem.eql(u8, &new_value, &zero)) {
// X->Y->0
break :blk evmc.EVMC_STORAGE_MODIFIED_DELETED;
}
}

// EIP-220: 3.2.2
if (std.mem.eql(u8, &original_value, &new_value)) {
if (std.mem.eql(u8, &current_value, &zero)) {
// X->0->X
break :blk evmc.EVMC_STORAGE_DELETED_RESTORED;
}
// EIP-220: 3.2.2.1
if (std.mem.eql(u8, &original_value, &zero)) {
// 0->Y->0
break :blk evmc.EVMC_STORAGE_ADDED_DELETED;
}
// X->Y->X
break :blk evmc.EVMC_STORAGE_MODIFIED_RESTORED;
}

break :blk evmc.EVMC_STORAGE_ASSIGNED;
};

vm.env.state.setStorage(address, k, value.*.bytes) catch |err| switch (err) {
// From EVMC docs: "The VM MUST make sure that the account exists. This requirement is only a formality
// because VM implementations only modify storage of the account of the current execution context".
error.AccountDoesNotExist => @panic("set storage in non-existent account"),
error.OutOfMemory => @panic("OOO"),
};

return evmc.EVMC_STORAGE_ADDED; // TODO(jsign): fix https://evmc.ethereum.org/group__EVMC.html#gae012fd6b8e5c23806b507c2d3e9fb1aa
return storage_status;
}

fn get_balance(ctx: ?*evmc.struct_evmc_host_context, addr: [*c]const evmc.evmc_address) callconv(.C) evmc.evmc_uint256be {
Expand Down Expand Up @@ -301,6 +366,8 @@ const EVMOneHost = struct {
error.OutOfMemory => @panic("OOO"),
};

const recipient_addr = fromEVMCAddress(msg.*.recipient);

// Send value.
const value = std.mem.readInt(u256, &msg.*.value.bytes, std.builtin.Endian.Big);
if (value > 0) {
Expand All @@ -321,8 +388,8 @@ const EVMOneHost = struct {
vm.env.state.setBalance(sender, sender_balance - value) catch |err| switch (err) {
error.OutOfMemory => @panic("OOO"),
};
const receipient_balance = vm.env.state.getAccount(fromEVMCAddress(msg.*.recipient)).balance;
vm.env.state.setBalance(sender, receipient_balance + value) catch |err| switch (err) {
const recipient_balance = vm.env.state.getAccount(recipient_addr).balance;
Comment on lines -324 to +391
Copy link
Owner Author

Choose a reason for hiding this comment

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

There was a typo here and a small refactoring.

vm.env.state.setBalance(sender, recipient_balance + value) catch |err| switch (err) {
error.OutOfMemory => @panic("OOO"),
};
}
Expand All @@ -339,12 +406,21 @@ const EVMOneHost = struct {
code.len,
);

// If the *CALL failed, we restore the previous statedb.
if (result.status_code != evmc.EVMC_SUCCESS)
vm.env.state.* = prev_statedb
else // otherwise, we free the backup and indireclty commit to the changes that happened.
if (result.status_code == evmc.EVMC_SUCCESS) {
// Free the backup and indireclty commit to the changes that happened.
prev_statedb.deinit();

// EIP-158.
if (vm.env.state.isEmpty(recipient_addr))
vm.env.state.addTouchedAddress(recipient_addr) catch |err| switch (err) {
error.OutOfMemory => @panic("OOO"),
};
} else {
// If the *CALL failed, we restore the previous statedb.
vm.env.state.* = prev_statedb;
}

evmclog.debug("call() depth={d} ended", .{msg.*.depth});
Copy link
Owner Author

Choose a reason for hiding this comment

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

Added this log since it's useful to see also when we finished in a call scope.

return result;
}
};
Expand Down
5 changes: 3 additions & 2 deletions src/engine_api/execution_payload.zig
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ pub fn newPayloadV2Handler(params: *ExecutionPayload, allocator: std.mem.Allocat
// vm.run_block(params.to_block(), params.transactions);

// But so far, just print the content of the payload
std.log.info("newPayloadV2Handler: {any}", .{params});
// std.log.info("newPayloadV2Handler: {any}", .{params});

var block = params.toBlock();
std.debug.print("block number={}\n", .{block.header.block_number});
_ = block;
// std.debug.print("block number={}\n", .{block.header.block_number});
Comment on lines -71 to +75
Copy link
Owner Author

Choose a reason for hiding this comment

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

For now, I commented this since it added some extra noise in the logs.

params.deinit(allocator);
}
12 changes: 4 additions & 8 deletions src/exec-spec-tests/execspectests.zig
Original file line number Diff line number Diff line change
Expand Up @@ -215,20 +215,16 @@ pub const AccountStateHex = struct {

const AccountStorageHex = std.json.ArrayHashMap(HexString);

var test_allocator = std.testing.allocator;
test "execution-spec-tests" {
var ft = try Fixture.fromBytes(test_allocator, @embedFile("fixtures/exec-spec-fixture.json"));
var allocator = std.testing.allocator;

var ft = try Fixture.fromBytes(allocator, @embedFile("fixtures/exec-spec-fixture.json"));
defer ft.deinit();

var it = ft.tests.value.map.iterator();
var count: usize = 0;
while (it.next()) |entry| {
try std.testing.expect(try entry.value_ptr.run(test_allocator));
try std.testing.expect(try entry.value_ptr.run(allocator));
count += 1;

// TODO: Only run the first test for now. Then we can enable all and continue with the integration.
if (count == 1) {
break;
}
}
}
52 changes: 48 additions & 4 deletions src/state/statedb.zig
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,22 @@ const AddressKeySet = common.AddressKeySet;
const AccountData = state.AccountData;
const AccountState = state.AccountState;
const Bytes32 = types.Bytes32;
const ArrayList = std.ArrayList;
const log = std.log.scoped(.statedb);

pub const StateDB = struct {
const AccountDB = std.AutoHashMap(Address, AccountState);

allocator: Allocator,
// original_db contains the state of the world when the transaction starts.
// It assists in charging the right amount of gas for SSTORE.
original_db: ?AccountDB = null,
Comment on lines +20 to +22
Copy link
Owner Author

Choose a reason for hiding this comment

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

Useful for EIP-2200 impl.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ideally we would use some sort of journal, but let's not overcomplicate things at this point.

// db contains the state of the world while the current transaction is executing.
// (i.e: current call scope)
db: AccountDB,

// Tx-scoped lists.
touched_addresses: ArrayList(Address),
accessed_accounts: AddressSet,
accessed_storage_keys: AddressKeySet,
Comment on lines +27 to 30
Copy link
Owner Author

Choose a reason for hiding this comment

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

These are the tx-scoped fields for access lists and EIP-158 EIPs.


Expand All @@ -31,14 +40,38 @@ pub const StateDB = struct {
.db = db,
.accessed_accounts = AddressSet.init(allocator),
.accessed_storage_keys = AddressKeySet.init(allocator),
.touched_addresses = ArrayList(Address).init(allocator),
};
}

pub fn deinit(self: *StateDB) void {
self.db.deinit();
self.accessed_accounts.deinit();
self.accessed_storage_keys.deinit();
if (self.original_db) |*original_db| {
original_db.deinit();
}
}

pub fn startTx(self: *StateDB) !void {
if (self.original_db) |*original_db| {
original_db.deinit();
}
self.original_db = try self.db.clone();
self.accessed_accounts.clearRetainingCapacity();
self.accessed_storage_keys.clearRetainingCapacity();
}

pub fn isEmpty(self: StateDB, addr: Address) bool {
const account = self.getAccountOpt(addr) orelse return false;
return account.nonce == 0 and account.code.len == 0 and account.balance == 0;
}

pub fn getAccountOpt(self: *StateDB, addr: Address) ?AccountData {
pub fn addTouchedAddress(self: *StateDB, addr: Address) !void {
try self.touched_addresses.append(addr);
}

pub fn getAccountOpt(self: StateDB, addr: Address) ?AccountData {
const account_data = self.db.get(addr) orelse return null;
return .{
.nonce = account_data.nonce,
Expand All @@ -60,13 +93,22 @@ pub const StateDB = struct {
return account.storage.get(key) orelse std.mem.zeroes(Bytes32);
}

pub fn getOriginalStorage(self: *StateDB, addr: Address, key: u256) Bytes32 {
const account = self.original_db.?.get(addr) orelse return std.mem.zeroes(Bytes32);
return account.storage.get(key) orelse std.mem.zeroes(Bytes32);
}

pub fn getAllStorage(self: *StateDB, addr: Address) ?std.AutoHashMap(u256, Bytes32) {
const account = self.db.get(addr) orelse return null;
return account.storage;
}

pub fn setStorage(self: *StateDB, addr: Address, key: u256, value: Bytes32) !void {
var account = self.db.getPtr(addr) orelse return error.AccountDoesNotExist;
if (std.mem.eql(u8, &value, &std.mem.zeroes(Bytes32))) {
_ = account.storage.remove(key);
return;
}
Comment on lines +108 to +111
Copy link
Owner Author

Choose a reason for hiding this comment

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

This was a bug, since writing the zero value should mean we remove the key/value pair from the state. (i.e: empty, not explicitly zero). Note that this is required for now in the "MPT world" for the current spec tests to pass, this will evolve with VKT.

try account.storage.put(key, value);
}

Expand Down Expand Up @@ -114,9 +156,11 @@ pub const StateDB = struct {
// A much smarter way is doing some "diff" style snapshotting or similar.
return StateDB{
.allocator = self.allocator,
.db = try self.db.cloneWithAllocator(self.allocator),
.accessed_accounts = try self.accessed_accounts.cloneWithAllocator(self.allocator),
.accessed_storage_keys = try self.accessed_storage_keys.cloneWithAllocator(self.allocator),
.db = try self.db.clone(),
.original_db = try self.original_db.?.clone(),
.accessed_accounts = try self.accessed_accounts.clone(),
.accessed_storage_keys = try self.accessed_storage_keys.clone(),
.touched_addresses = try self.touched_addresses.clone(),
};
}
};
Loading