From 911dfedd07c7e4e9e211d2307148651651de8c7e Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Thu, 23 Mar 2023 16:26:42 -0700 Subject: [PATCH 1/2] Rework CREATE, CREATE2 syscalls The old code was outdated in various ways, e.g. it was trying to share code with contract-creation txns, so I would ignore the diff and just look at the new code. Still a bunch of TODOs left here, like actually saving code that's returned. --- evm/src/cpu/kernel/aggregator.rs | 1 + evm/src/cpu/kernel/asm/core/create.asm | 172 +++++++++--------- .../cpu/kernel/asm/core/create_addresses.asm | 6 +- .../asm/core/create_contract_account.asm | 51 ++++++ evm/src/cpu/kernel/asm/core/gas.asm | 42 +++++ evm/src/cpu/kernel/asm/core/process_txn.asm | 25 +-- evm/src/cpu/kernel/asm/core/terminate.asm | 18 -- .../asm/mpt/insert/insert_trie_specific.asm | 1 + 8 files changed, 199 insertions(+), 117 deletions(-) create mode 100644 evm/src/cpu/kernel/asm/core/create_contract_account.asm diff --git a/evm/src/cpu/kernel/aggregator.rs b/evm/src/cpu/kernel/aggregator.rs index 6f45013d..251271ff 100644 --- a/evm/src/cpu/kernel/aggregator.rs +++ b/evm/src/cpu/kernel/aggregator.rs @@ -24,6 +24,7 @@ pub(crate) fn combined_kernel() -> Kernel { include_str!("asm/core/call.asm"), include_str!("asm/core/create.asm"), include_str!("asm/core/create_addresses.asm"), + include_str!("asm/core/create_contract_account.asm"), include_str!("asm/core/gas.asm"), include_str!("asm/core/intrinsic_gas.asm"), include_str!("asm/core/invalid.asm"), diff --git a/evm/src/cpu/kernel/asm/core/create.asm b/evm/src/cpu/kernel/asm/core/create.asm index 3157a262..11e89aa4 100644 --- a/evm/src/cpu/kernel/asm/core/create.asm +++ b/evm/src/cpu/kernel/asm/core/create.asm @@ -1,42 +1,27 @@ -// TODO: This file needs to be cleaned up. -// `create` is no longer being used for contract-creation txns, -// so it can be inlined. Also need to set metadata on new ctx. - -// The CREATE syscall. +// The CREATE syscall. Address will be +// address = KEC(RLP(sender, nonce))[12:] // // Pre stack: kexit_info, value, code_offset, code_len // Post stack: address global sys_create: + // stack: kexit_info, value, code_offset, code_len // TODO: Charge gas. %stack (kexit_info, value, code_offset, code_len) - -> (value, 0, @SEGMENT_MAIN_MEMORY, code_offset, code_len) + -> (value, code_offset, code_len, kexit_info) + PUSH sys_create_got_address + // stack: sys_create_got_address, value, code_offset, code_len, kexit_info %address - // stack: sender, value, CODE_ADDR: 3, code_len, sys_create_finish, kexit_info - %jump(create) -sys_create_finish: - // stack: address, kexit_info - SWAP1 - EXIT_KERNEL - -// Create a new contract account with the traditional address scheme, i.e. -// address = KEC(RLP(sender, nonce))[12:] -// This can be used both for the CREATE instruction and for contract-creation -// transactions. -// -// Pre stack: sender, endowment, CODE_ADDR: 3, code_len, retdest -// Post stack: address -// Note: CODE_ADDR refers to a (context, segment, offset) tuple. -global create: - // stack: sender, endowment, CODE_ADDR, code_len, retdest + // stack: sender, sys_create_got_address, value, code_offset, code_len, kexit_info DUP1 %nonce - - // stack: nonce, sender, endowment, CODE_ADDR, code_len, retdest - // Call get_create_address and have it return to create_inner. - %stack (nonce, sender) - -> (sender, nonce, create_inner, sender) + // stack: nonce, sender, sys_create_got_address, value, code_offset, code_len, kexit_info + SWAP1 + // stack: sender, nonce, sys_create_got_address, value, code_offset, code_len, kexit_info %jump(get_create_address) +sys_create_got_address: + // stack: address, value, code_offset, code_len, kexit_info + %jump(create_common) -// CREATE2; see EIP-1014. Address will be +// The CREATE2 syscall; see EIP-1014. Address will be // address = KEC(0xff || sender || salt || code_hash)[12:] // // Pre stack: kexit_info, value, code_offset, code_len, salt @@ -45,75 +30,100 @@ global sys_create2: // stack: kexit_info, value, code_offset, code_len, salt // TODO: Charge gas. SWAP4 - %stack (salt) -> (salt, sys_create2_got_address) - // stack: salt, sys_create2_got_address, value, code_offset, code_len, kexit_info + %stack (salt) -> (salt, create_common) + // stack: salt, create_common, value, code_offset, code_len, kexit_info + + // Hash the code. DUP5 // code_len DUP5 // code_offset PUSH @SEGMENT_MAIN_MEMORY GET_CONTEXT KECCAK_GENERAL - // stack: hash, salt, sys_create2_got_address, value, code_offset, code_len, kexit_info + // stack: hash, salt, create_common, value, code_offset, code_len, kexit_info + %address - // stack: sender, hash, salt, sys_create2_got_address, value, code_offset, code_len, kexit_info + // stack: sender, hash, salt, create_common, value, code_offset, code_len, kexit_info %jump(get_create2_address) -sys_create2_got_address: - // stack: address, value, code_offset, code_len, kexit_info - %address - %stack (sender, address, value, code_offset, code_len, kexit_info) - -> (address, sender, value, 0, @SEGMENT_MAIN_MEMORY, code_offset, code_len, - sys_create2_finish, kexit_info) - %jump(create_inner) -sys_create2_finish: - // stack: address, kexit_info - SWAP1 - EXIT_KERNEL -// Pre stack: address, sender, endowment, CODE_ADDR, code_len, retdest +// Pre stack: address, value, code_offset, code_len, kexit_info // Post stack: address -// Note: CODE_ADDR refers to a (context, segment, offset) tuple. -global create_inner: - // stack: address, sender, endowment, CODE_ADDR, code_len, retdest +global create_common: + // stack: address, value, code_offset, code_len, kexit_info DUP1 %insert_accessed_addresses_no_return - %stack (address, sender, endowment) - -> (sender, address, endowment, sender, address) - - %transfer_eth - // stack: transfer_eth_status, sender, address, CODE_ADDR, code_len, retdest - %jumpi(fault_exception) - // stack: sender, address, CODE_ADDR, code_len, retdest + // Increment the sender's nonce. + %address %increment_nonce - // stack: address, CODE_ADDR, code_len, retdest + // stack: address, value, code_offset, code_len, kexit_info + + // Deduct value from the caller. + DUP2 + %address + // stack: sender, value, address, value, code_offset, code_len, kexit_info + %deduct_eth + // stack: deduct_eth_status, address, value, code_offset, code_len, kexit_info + %jumpi(fault_exception) + // stack: address, value, code_offset, code_len, kexit_info + + // Create the new contract account in the state trie. + DUP1 DUP3 + // stack: value, address, address, value, code_offset, code_len, kexit_info + %create_contract_account + // stack: status, address, value, code_offset, code_len, kexit_info + %jumpi(fault_exception) + // stack: address, value, code_offset, code_len, kexit_info %create_context - // stack: new_ctx, address, CODE_ADDR, code_len, retdest - %stack (new_ctx, address, src_ctx, src_segment, src_offset, code_len) - -> (new_ctx, @SEGMENT_CODE, 0, - src_ctx, src_segment, src_offset, - code_len, run_constructor, - new_ctx, address) + // stack: new_ctx, address, value, code_offset, code_len, kexit_info + GET_CONTEXT + // stack: src_ctx, new_ctx, address, value, code_offset, code_len, kexit_info + + // Copy the code from txdata to the new context's code segment. + %stack (src_ctx, new_ctx, address, value, code_offset, code_len) + -> (new_ctx, @SEGMENT_CODE, 0, // DST + src_ctx, @SEGMENT_MAIN_MEMORY, code_offset, // SRC + code_len, + run_constructor, + new_ctx, value, address) %jump(memcpy) run_constructor: - // stack: new_ctx, address, retdest - // At this point, the initialization code has been loaded. - // Save our return address in memory, so we'll be in `after_constructor` - // after the new context returns. - // Note: We can't use %mstore_context_metadata because we're writing to - // memory owned by the new context, not the current one. - %stack (new_ctx) -> (new_ctx, @SEGMENT_CONTEXT_METADATA, - @CTX_METADATA_PARENT_PC, after_constructor, new_ctx) - MSTORE_GENERAL - // stack: new_ctx, address, retdest + // stack: new_ctx, value, address, kexit_info + %set_new_ctx_value + // stack: new_ctx, address, kexit_info - // Now, switch to the new context and go to usermode with PC=0. - SET_CONTEXT - // stack: (empty, since we're in the new context) - PUSH 0 - EXIT_KERNEL + // Each line in the block below does not change the stack. + DUP2 %set_new_ctx_addr + %address %set_new_ctx_caller + %set_new_ctx_parent_ctx + %set_new_ctx_parent_pc(after_constructor) + // stack: new_ctx, address, kexit_info + + // All but 1/64 of the sender's remaining gas goes to the constructor. + SWAP2 + // stack: kexit_info, address, new_ctx + %drain_all_but_one_64th_gas + %stack (kexit_info, drained_gas, address, new_ctx) -> (drained_gas, new_ctx, address, kexit_info) + %set_new_ctx_gas_limit + // stack: new_ctx, address, kexit_info + + %enter_new_ctx + // (Old context) stack: new_ctx, address, kexit_info after_constructor: - // stack: address, retdest - // TODO: If code was returned, store it in the account. - SWAP1 - JUMP + // stack: success, leftover_gas, new_ctx, address, kexit_info + SWAP2 + // stack: new_ctx, leftover_gas, success, address, kexit_info + POP // TODO: Ignoring new_ctx for now, but we will need it to store code that was returned, if any. + // stack: leftover_gas, success, address, kexit_info + %shl_const(192) + // stack: leftover_gas << 192, success, address, kexit_info + SWAP2 + // stack: address, success, leftover_gas << 192, kexit_info + MUL + // stack: address_if_success, leftover_gas << 192, kexit_info + SWAP2 + // stack: kexit_info, leftover_gas << 192, address_if_success + ADD + // stack: kexit_info, address_if_success + EXIT_KERNEL diff --git a/evm/src/cpu/kernel/asm/core/create_addresses.asm b/evm/src/cpu/kernel/asm/core/create_addresses.asm index d72d7e67..70f57b6f 100644 --- a/evm/src/cpu/kernel/asm/core/create_addresses.asm +++ b/evm/src/cpu/kernel/asm/core/create_addresses.asm @@ -19,7 +19,8 @@ global get_create_address: PUSH 0 // context // stack: RLP_ADDR: 3, rlp_len, retdest KECCAK_GENERAL - %mod_const(0x10000000000000000000000000000000000000000) // 2^160 + // stack: hash, retdest + %u256_to_addr // stack: address, retdest %observe_new_address SWAP1 @@ -54,8 +55,9 @@ get_create2_address_finish: POP %stack (retdest) -> (0, @SEGMENT_KERNEL_GENERAL, 0, 85, retdest) // context, segment, offset, len KECCAK_GENERAL + // stack: hash, retdest + %u256_to_addr // stack: address, retdest - %mod_const(0x10000000000000000000000000000000000000000) // 2^160 %observe_new_address SWAP1 JUMP diff --git a/evm/src/cpu/kernel/asm/core/create_contract_account.asm b/evm/src/cpu/kernel/asm/core/create_contract_account.asm new file mode 100644 index 00000000..5ce12779 --- /dev/null +++ b/evm/src/cpu/kernel/asm/core/create_contract_account.asm @@ -0,0 +1,51 @@ +// Create a smart contract account with the given address and the given endowment value. +// Pre stack: value, address +// Post stack: status +%macro create_contract_account + // stack: value, address + DUP2 %mpt_read_state_trie + // stack: existing_account_ptr, value, address + // If the account doesn't exist, there's need to check its balance or nonce, + // so we can skip ahead, setting existing_balance = existing_account_ptr = 0. + DUP1 ISZERO %jumpi(%%do_insert) + + // stack: existing_account_ptr, value, address + DUP1 %mload_trie_data // nonce = account[0] + // stack: nonce, existing_account_ptr, value, address + %jumpi(%%error_nonzero_nonce) + // stack: existing_account_ptr, value, address + %increment %mload_trie_data // balance = account[1] + +%%do_insert: + // stack: existing_balance, value, address + ADD + // stack: new_acct_value, address + // Write the new account's data to MPT data, and get a pointer to it. + %get_trie_data_size + // stack: account_ptr, new_acct_value, address + PUSH 1 %append_to_trie_data // nonce = 1 + // stack: account_ptr, new_acct_value, address + SWAP1 %append_to_trie_data // balance = new_acct_value + // stack: account_ptr, address + PUSH 0 %append_to_trie_data // storage_root = nil + // stack: account_ptr, address + PUSH @EMPTY_STRING_HASH %append_to_trie_data // code_hash = keccak('') + // stack: account_ptr, address + SWAP1 + // stack: address, account_ptr + %addr_to_state_key + // stack: state_key, account_ptr + %mpt_insert_state_trie + // stack: (empty) + PUSH 0 // success + %jump(%%end) + +// If the nonce is nonzero, that means a contract has already been deployed to this address. +// (This should be impossible with contract creation transactions or CREATE, but possible with CREATE2.) +// So we return 1 to indicate an error. +%%error_nonzero_nonce: + %stack (existing_account_ptr, address, value) -> (1) + +%%end: + // stack: status +%endmacro diff --git a/evm/src/cpu/kernel/asm/core/gas.asm b/evm/src/cpu/kernel/asm/core/gas.asm index 78cc065c..ad2966a5 100644 --- a/evm/src/cpu/kernel/asm/core/gas.asm +++ b/evm/src/cpu/kernel/asm/core/gas.asm @@ -56,3 +56,45 @@ global sys_gasprice: // stack: gas_price, kexit_info SWAP1 EXIT_KERNEL + +// Checks how much gas is remaining in this context, given the current kexit_info. +%macro leftover_gas + // stack: kexit_info + %shr_const(192) + // stack: gas_used + %mload_context_metadata(@CTX_METADATA_GAS_LIMIT) + // stack: gas_limit, gas_used + SWAP1 + // stack: gas_used, gas_limit + DUP2 DUP2 LT + // stack: gas_used < gas_limit, gas_used, gas_limit + SWAP2 + // stack: gas_limit, gas_used, gas_used < gas_limit + SUB + // stack: gas_limit - gas_used, gas_used < gas_limit + MUL + // stack: leftover_gas = (gas_limit - gas_used) * (gas_used < gas_limit) +%endmacro + +// Given the current kexit_info, drains all but one 64th of its remaining gas. +// Returns how much gas was drained. +%macro drain_all_but_one_64th_gas + // stack: kexit_info + DUP1 %leftover_gas + // stack: leftover_gas, kexit_info + %all_but_one_64th + // stack: all_but_one_64th, kexit_info + %stack (all_but_one_64th, kexit_info) -> (all_but_one_64th, kexit_info, all_but_one_64th) + %charge_gas + // stack: kexit_info, drained_gas +%endmacro + +// This is L(n), the "all but one 64th" function in the yellowpaper, i.e. +// L(n) = n - floor(n / 64) +%macro all_but_one_64th + // stack: n + DUP1 %div_const(64) + // stack: floor(n / 64), n + SWAP1 SUB + // stack: n - floor(n / 64) +%endmacro diff --git a/evm/src/cpu/kernel/asm/core/process_txn.asm b/evm/src/cpu/kernel/asm/core/process_txn.asm index 1c30d88a..b044e9ed 100644 --- a/evm/src/cpu/kernel/asm/core/process_txn.asm +++ b/evm/src/cpu/kernel/asm/core/process_txn.asm @@ -68,22 +68,15 @@ global process_contract_creation_txn: %jumpi(panic) // stack: address, retdest - // Write the new account's data to MPT data, and get a pointer to it. - %get_trie_data_size - // stack: account_ptr, address, retdest - PUSH 1 %append_to_trie_data // nonce = 1 - // stack: account_ptr, address, retdest - DUP2 %balance %mload_txn_field(@TXN_FIELD_VALUE) ADD %append_to_trie_data // balance = old_balance + txn_value - // stack: account_ptr, address, retdest - PUSH 0 %append_to_trie_data // storage_root = nil - // stack: account_ptr, address, retdest - PUSH @EMPTY_STRING_HASH %append_to_trie_data // code_hash = keccak('') - // stack: account_ptr, address, retdest - DUP2 - // stack: address, account_ptr, address, retdest - %addr_to_state_key - // stack: state_key, account_ptr, address, retdest - %mpt_insert_state_trie + // Create the new contract account in the state trie. + DUP1 + %mload_txn_field(@TXN_FIELD_VALUE) + // stack: value, address, address, retdest + %create_contract_account + // stack: status, address, retdest + // It should be impossible to create address collisions with a contract creation txn, + // since the address was derived from nonce, unlike with CREATE2. + %jumpi(panic) // stack: address, retdest %create_context diff --git a/evm/src/cpu/kernel/asm/core/terminate.asm b/evm/src/cpu/kernel/asm/core/terminate.asm index c124db34..bd86470e 100644 --- a/evm/src/cpu/kernel/asm/core/terminate.asm +++ b/evm/src/cpu/kernel/asm/core/terminate.asm @@ -96,21 +96,3 @@ global terminate_common: // stack: parent_pc, success, leftover_gas JUMP - -%macro leftover_gas - // stack: kexit_info - %shr_const(192) - // stack: gas_used - %mload_context_metadata(@CTX_METADATA_GAS_LIMIT) - // stack: gas_limit, gas_used - SWAP1 - // stack: gas_used, gas_limit - DUP2 DUP2 LT - // stack: gas_used < gas_limit, gas_used, gas_limit - SWAP2 - // stack: gas_limit, gas_used, gas_used < gas_limit - SUB - // stack: gas_limit - gas_used, gas_used < gas_limit - MUL - // stack: leftover_gas = (gas_limit - gas_used) * (gas_used < gas_limit) -%endmacro diff --git a/evm/src/cpu/kernel/asm/mpt/insert/insert_trie_specific.asm b/evm/src/cpu/kernel/asm/mpt/insert/insert_trie_specific.asm index 60df845e..37d7fda1 100644 --- a/evm/src/cpu/kernel/asm/mpt/insert/insert_trie_specific.asm +++ b/evm/src/cpu/kernel/asm/mpt/insert/insert_trie_specific.asm @@ -3,6 +3,7 @@ // Mutate the state trie, inserting the given key-value pair. // Pre stack: key, value_ptr, retdest // Post stack: (empty) +// TODO: Have this take an address and do %mpt_insert_state_trie? To match mpt_read_state_trie. global mpt_insert_state_trie: // stack: key, value_ptr, retdest %stack (key, value_ptr) From 39fdad8c86e406d534a67ddf7fac3908eecf3584 Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Wed, 29 Mar 2023 11:00:41 -0700 Subject: [PATCH 2/2] Feedback --- evm/src/cpu/kernel/asm/core/create.asm | 4 +--- evm/src/cpu/kernel/asm/core/create_contract_account.asm | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/evm/src/cpu/kernel/asm/core/create.asm b/evm/src/cpu/kernel/asm/core/create.asm index 11e89aa4..00f6c5d0 100644 --- a/evm/src/cpu/kernel/asm/core/create.asm +++ b/evm/src/cpu/kernel/asm/core/create.asm @@ -7,9 +7,7 @@ global sys_create: // stack: kexit_info, value, code_offset, code_len // TODO: Charge gas. %stack (kexit_info, value, code_offset, code_len) - -> (value, code_offset, code_len, kexit_info) - PUSH sys_create_got_address - // stack: sys_create_got_address, value, code_offset, code_len, kexit_info + -> (sys_create_got_address, value, code_offset, code_len, kexit_info) %address // stack: sender, sys_create_got_address, value, code_offset, code_len, kexit_info DUP1 %nonce diff --git a/evm/src/cpu/kernel/asm/core/create_contract_account.asm b/evm/src/cpu/kernel/asm/core/create_contract_account.asm index 5ce12779..00a19272 100644 --- a/evm/src/cpu/kernel/asm/core/create_contract_account.asm +++ b/evm/src/cpu/kernel/asm/core/create_contract_account.asm @@ -5,7 +5,7 @@ // stack: value, address DUP2 %mpt_read_state_trie // stack: existing_account_ptr, value, address - // If the account doesn't exist, there's need to check its balance or nonce, + // If the account doesn't exist, there's no need to check its balance or nonce, // so we can skip ahead, setting existing_balance = existing_account_ptr = 0. DUP1 ISZERO %jumpi(%%do_insert)