-
Notifications
You must be signed in to change notification settings - Fork 2
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
Changes from all commits
1486191
5c546e1
f9d24bd
c79f9b2
59c8407
7e44b23
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -171,6 +172,7 @@ pub const Blockchain = struct { | |
.chain_id = chain.chain_id, | ||
}; | ||
|
||
try state.startTx(); | ||
const exec_tx_result = try processTransaction(allocator, env, tx); | ||
gas_available -= exec_tx_result.gas_used; | ||
|
||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 }; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the most important gap and change of this PR. 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, ¤t_value, &new_value)) { | ||
break :blk evmc.EVMC_STORAGE_ASSIGNED; | ||
} | ||
// EIP-220: 3. | ||
|
||
// EIP-220: 3.1 | ||
if (std.mem.eql(u8, &original_value, ¤t_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, ¤t_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, ¤t_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 { | ||
|
@@ -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) { | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"), | ||
}; | ||
} | ||
|
@@ -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}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Useful for EIP-2200 impl. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
||
|
@@ -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, | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
|
||
|
@@ -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(), | ||
}; | ||
} | ||
}; |
There was a problem hiding this comment.
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: