From a294c7e2ed619a264116b495fe76c713a47c2acc Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Wed, 17 May 2023 15:53:16 +0200 Subject: [PATCH] Some fixes to contract creation (#1044) * Fixes * Fixes * Remove panics in create * Fix charge gas for code deposit * Handle error in constructor --- evm/src/cpu/kernel/asm/core/create.asm | 31 ++++---- .../asm/core/create_contract_account.asm | 21 +++--- evm/src/cpu/kernel/asm/core/process_txn.asm | 70 ++++++++++++++----- 3 files changed, 76 insertions(+), 46 deletions(-) diff --git a/evm/src/cpu/kernel/asm/core/create.asm b/evm/src/cpu/kernel/asm/core/create.asm index 0dabef7d..b5079f3c 100644 --- a/evm/src/cpu/kernel/asm/core/create.asm +++ b/evm/src/cpu/kernel/asm/core/create.asm @@ -79,22 +79,14 @@ global create_common: %checkpoint - // 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 + DUP1 + // stack: 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 + DUP2 DUP2 %address %transfer_eth %jumpi(fault_exception) DUP2 DUP2 %address %journal_add_balance_transfer // Add journal entry for the balance transfer. %create_context @@ -150,15 +142,16 @@ after_constructor: PUSH 0 %mload_current(@SEGMENT_RETURNDATA) %eq_const(0xEF) %jumpi(fault_exception) // Charge gas for the code size. - SWAP3 - // stack: kexit_info, success, address, leftover_gas + // stack: leftover_gas, success, address, kexit_info %returndatasize // Size of the code. - // stack: code_size, kexit_info, success, address, leftover_gas - DUP1 %gt_const(@MAX_CODE_SIZE) - %jumpi(fault_exception) - // stack: code_size, kexit_info, success, address, leftover_gas - %mul_const(@GAS_CODEDEPOSIT) %charge_gas - SWAP3 + // stack: code_size, leftover_gas, success, address, kexit_info + DUP1 %gt_const(@MAX_CODE_SIZE) %jumpi(fault_exception) + // stack: code_size, leftover_gas, success, address, kexit_info + %mul_const(@GAS_CODEDEPOSIT) + // stack: code_size_cost, leftover_gas, success, address, kexit_info + DUP2 DUP2 GT %jumpi(fault_exception) + SWAP1 SUB + // stack: leftover_gas, success, address, kexit_info // Store the code hash of the new contract. GET_CONTEXT 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 339ea4a6..efaaff9b 100644 --- a/evm/src/cpu/kernel/asm/core/create_contract_account.asm +++ b/evm/src/cpu/kernel/asm/core/create_contract_account.asm @@ -2,25 +2,24 @@ // Pre stack: value, address // Post stack: status %macro create_contract_account - // stack: value, address - DUP2 %journal_add_account_created - DUP2 %insert_touched_addresses - DUP2 %mpt_read_state_trie - // stack: existing_account_ptr, value, address + // stack: address + DUP1 %insert_touched_addresses + DUP1 %mpt_read_state_trie + // stack: existing_account_ptr, address // 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) - // stack: existing_account_ptr, value, address + // stack: existing_account_ptr, address DUP1 %mload_trie_data // nonce = account[0] - // stack: nonce, existing_account_ptr, value, address + // stack: nonce, existing_account_ptr, address %jumpi(%%error_nonzero_nonce) - // stack: existing_account_ptr, value, address + // stack: existing_account_ptr, address %increment %mload_trie_data // balance = account[1] %%do_insert: - // stack: existing_balance, value, address - ADD + // stack: existing_balance, address + DUP2 %journal_add_account_created // stack: new_acct_value, address // Write the new account's data to MPT data, and get a pointer to it. %get_trie_data_size @@ -47,7 +46,7 @@ // (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) + %stack (existing_account_ptr, address) -> (1) %%end: // stack: status diff --git a/evm/src/cpu/kernel/asm/core/process_txn.asm b/evm/src/cpu/kernel/asm/core/process_txn.asm index a935a783..6e02c928 100644 --- a/evm/src/cpu/kernel/asm/core/process_txn.asm +++ b/evm/src/cpu/kernel/asm/core/process_txn.asm @@ -88,32 +88,37 @@ global process_contract_creation_txn: // stack: origin, retdest DUP1 %nonce // stack: origin_nonce, origin, retdest + %decrement // Need the non-incremented nonce SWAP1 // stack: origin, origin_nonce, retdest %get_create_address // stack: address, retdest - // Deduct value from caller. - %mload_txn_field(@TXN_FIELD_VALUE) - %mload_txn_field(@TXN_FIELD_ORIGIN) - %deduct_eth - // stack: deduct_eth_status, address, retdest - %jumpi(panic) - // stack: address, retdest + %checkpoint // Create the new contract account in the state trie. DUP1 - %mload_txn_field(@TXN_FIELD_VALUE) - // stack: value, address, address, retdest + // stack: 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) + %jumpi(create_contract_account_fault) +global gtra: + + // stack: address, retdest + // Transfer value to new contract + DUP1 %mload_txn_field(@TXN_FIELD_VALUE) + SWAP1 + %mload_txn_field(@TXN_FIELD_ORIGIN) + DUP3 DUP3 DUP3 + %transfer_eth %jumpi(panic) + %journal_add_balance_transfer // stack: address, retdest %create_context // stack: new_ctx, address, retdest +global gtr: // Copy the code from txdata to the new context's code segment. PUSH process_contract_creation_txn_after_code_loaded @@ -123,10 +128,10 @@ global process_contract_creation_txn: PUSH 0 // SRC.context PUSH 0 // DST.offset PUSH @SEGMENT_CODE // DST.segment - DUP7 // DST.context = new_ctx + DUP8 // DST.context = new_ctx %jump(memcpy) -process_contract_creation_txn_after_code_loaded: +global process_contract_creation_txn_after_code_loaded: // stack: new_ctx, address, retdest // Each line in the block below does not change the stack. @@ -143,19 +148,21 @@ process_contract_creation_txn_after_code_loaded: global process_contract_creation_txn_after_constructor: // stack: success, leftover_gas, new_ctx, address, retdest - POP // TODO: Success will go into the receipt when we support that. + DUP1 POP // TODO: Success will go into the receipt when we support that. + ISZERO %jumpi(contract_creation_fault_3) // EIP-3541: Reject new contract code starting with the 0xEF byte - PUSH 0 %mload_current(@SEGMENT_RETURNDATA) %eq_const(0xEF) %assert_zero // TODO: need to revert changes here. + PUSH 0 %mload_current(@SEGMENT_RETURNDATA) %eq_const(0xEF) %jumpi(contract_creation_fault_3) // stack: leftover_gas, new_ctx, address, retdest %returndatasize // Size of the code. // stack: code_size, leftover_gas, new_ctx, address, retdest - DUP1 %gt_const(@MAX_CODE_SIZE) %jumpi(panic) // TODO: need to revert changes here. +global hrt: + DUP1 %gt_const(@MAX_CODE_SIZE) %jumpi(contract_creation_fault_4) // stack: code_size, leftover_gas, new_ctx, address, retdest %mul_const(@GAS_CODEDEPOSIT) SWAP1 // stack: leftover_gas, codedeposit_cost, new_ctx, address, retdest - DUP2 DUP2 LT %jumpi(panic) // TODO: need to revert changes here. + DUP2 DUP2 LT %jumpi(contract_creation_fault_4) // stack: leftover_gas, codedeposit_cost, new_ctx, address, retdest SUB @@ -351,3 +358,34 @@ process_message_txn_fail: SUB // stack: gas_limit - intrinsic_gas %endmacro + +create_contract_account_fault: + %revert_checkpoint + // stack: address, retdest + POP + PUSH 0 // leftover gas + %pay_coinbase_and_refund_sender + %delete_all_touched_addresses + %delete_all_selfdestructed_addresses + JUMP + +contract_creation_fault_3: + %revert_checkpoint + // stack: success, leftover_gas, new_ctx, address, retdest + %pop3 + PUSH 0 // leftover gas + %pay_coinbase_and_refund_sender + %delete_all_touched_addresses + %delete_all_selfdestructed_addresses + JUMP + +contract_creation_fault_4: + %revert_checkpoint + %revert_checkpoint + // stack: code_size, leftover_gas, new_ctx, address, retdest + %pop4 + PUSH 0 // leftover gas + %pay_coinbase_and_refund_sender + %delete_all_touched_addresses + %delete_all_selfdestructed_addresses + JUMP