From 84fbbbf410f9a02dade27e6528fd27e3807f6b05 Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Fri, 10 Mar 2023 08:54:26 -0800 Subject: [PATCH] Couple minor fixes --- evm/src/cpu/kernel/asm/core/call.asm | 2 +- evm/src/cpu/kernel/asm/core/intrinsic_gas.asm | 9 +++ evm/src/cpu/kernel/asm/core/process_txn.asm | 44 +++++++++++--- evm/src/cpu/kernel/asm/mpt/hash/hash.asm | 4 +- .../cpu/kernel/constants/global_metadata.rs | 59 ++++++++++++------- evm/src/cpu/kernel/constants/txn_fields.rs | 19 +++--- evm/tests/basic_smart_contract.rs | 6 +- evm/tests/simple_transfer.rs | 5 +- 8 files changed, 102 insertions(+), 46 deletions(-) diff --git a/evm/src/cpu/kernel/asm/core/call.asm b/evm/src/cpu/kernel/asm/core/call.asm index 198a6cbb..cb66b7fa 100644 --- a/evm/src/cpu/kernel/asm/core/call.asm +++ b/evm/src/cpu/kernel/asm/core/call.asm @@ -144,7 +144,7 @@ global after_call_instruction: %macro set_new_ctx_gas_limit // stack: gas_limit, new_ctx %stack (gas_limit, new_ctx) - -> (new_ctx, @SEGMENT_CONTEXT_METADATA, @CTX_METADATA_CODE_SIZE, gas_limit, new_ctx) + -> (new_ctx, @SEGMENT_CONTEXT_METADATA, @CTX_METADATA_GAS_LIMIT, gas_limit, new_ctx) MSTORE_GENERAL // stack: new_ctx %endmacro diff --git a/evm/src/cpu/kernel/asm/core/intrinsic_gas.asm b/evm/src/cpu/kernel/asm/core/intrinsic_gas.asm index 5891807c..9e8a7f80 100644 --- a/evm/src/cpu/kernel/asm/core/intrinsic_gas.asm +++ b/evm/src/cpu/kernel/asm/core/intrinsic_gas.asm @@ -60,3 +60,12 @@ count_zeros_finish: SWAP1 JUMP + +// Convenience macro to call intrinsic_gas and return where we left off. +%macro intrinsic_gas + // stack: (empty) + PUSH %%after + %jump(intrinsic_gas) +%%after: + // stack: (empty) +%endmacro diff --git a/evm/src/cpu/kernel/asm/core/process_txn.asm b/evm/src/cpu/kernel/asm/core/process_txn.asm index 17770ba4..8a6c3afb 100644 --- a/evm/src/cpu/kernel/asm/core/process_txn.asm +++ b/evm/src/cpu/kernel/asm/core/process_txn.asm @@ -16,15 +16,18 @@ global process_normalized_txn: %min // stack: computed_fee, retdest %mstore_txn_field(@TXN_FIELD_COMPUTED_FEE_PER_GAS) - // stack: retdest - PUSH validate - %jump(intrinsic_gas) -global validate: - // stack: intrinsic_gas, retdest - POP // TODO: Assert gas_limit >= intrinsic_gas. + // Compute this transaction's intrinsic gas and store it. + %intrinsic_gas + %mstore_txn_field(@TXN_FIELD_INTRINSIC_GAS) // stack: retdest + + // Assert gas_limit >= intrinsic_gas. + %mload_txn_field(@TXN_FIELD_INTRINSIC_GAS) + %mload_txn_field(@TXN_FIELD_GAS_LIMIT) + %assert_ge + // TODO: Check that txn nonce matches account nonce. // TODO: Assert nonce is correct. // TODO: Assert sender has no code. @@ -110,7 +113,14 @@ global process_message_txn_insufficient_balance: PANIC // TODO global process_message_txn_return: - // TODO: Since there was no code to execute, do we still return leftover gas? + // Refund leftover gas. + // stack: retdest + %mload_txn_field(@TXN_FIELD_INTRINSIC_GAS) + %mload_txn_field(@TXN_FIELD_GAS_LIMIT) + SUB + // stack: leftover_gas, retdest + %refund_leftover_gas_cost + // stack: retdest JUMP global process_message_txn_code_loaded: @@ -124,7 +134,13 @@ global process_message_txn_code_loaded: %mload_txn_field(@TXN_FIELD_VALUE) %set_new_ctx_value %set_new_ctx_parent_ctx %set_new_ctx_parent_pc(process_message_txn_after_call) - %mload_txn_field(@TXN_FIELD_GAS_LIMIT) %set_new_ctx_gas_limit + // stack: new_ctx, retdest + + // The gas provided to the callee is gas_limit - intrinsic_gas. + %mload_txn_field(@TXN_FIELD_INTRINSIC_GAS) + %mload_txn_field(@TXN_FIELD_GAS_LIMIT) + SUB + %set_new_ctx_gas_limit // stack: new_ctx, retdest // TODO: Copy TXN_DATA to CALLDATA @@ -135,7 +151,17 @@ global process_message_txn_after_call: // stack: success, leftover_gas, new_ctx, retdest POP // TODO: Success will go into the receipt when we support that. // stack: leftover_gas, new_ctx, retdest - POP // TODO: Refund leftover gas. + %refund_leftover_gas_cost // stack: new_ctx, retdest POP JUMP + +%macro refund_leftover_gas_cost + // stack: leftover_gas + %mload_txn_field(@TXN_FIELD_COMPUTED_FEE_PER_GAS) + MUL + // stack: leftover_gas_cost + %mload_txn_field(@TXN_FIELD_ORIGIN) + // stack: origin, leftover_gas_cost + %add_eth +%endmacro diff --git a/evm/src/cpu/kernel/asm/mpt/hash/hash.asm b/evm/src/cpu/kernel/asm/mpt/hash/hash.asm index 9fe0edef..0c8beae7 100644 --- a/evm/src/cpu/kernel/asm/mpt/hash/hash.asm +++ b/evm/src/cpu/kernel/asm/mpt/hash/hash.asm @@ -135,9 +135,9 @@ encode_node_branch: // Get the next unused offset within the encoded child buffers. // Then immediately increment the next unused offset by 16, so any // recursive calls will use nonoverlapping offsets. - %mload_global_metadata(@TRIE_ENCODED_CHILD_SIZE) + %mload_global_metadata(@GLOBAL_METADATA_TRIE_ENCODED_CHILD_SIZE) DUP1 %add_const(16) - %mstore_global_metadata(@TRIE_ENCODED_CHILD_SIZE) + %mstore_global_metadata(@GLOBAL_METADATA_TRIE_ENCODED_CHILD_SIZE) // stack: base_offset, node_payload_ptr, encode_value, retdest // We will call encode_or_hash_node on each child. For the i'th child, we diff --git a/evm/src/cpu/kernel/constants/global_metadata.rs b/evm/src/cpu/kernel/constants/global_metadata.rs index f68a91f5..cd6c0d7e 100644 --- a/evm/src/cpu/kernel/constants/global_metadata.rs +++ b/evm/src/cpu/kernel/constants/global_metadata.rs @@ -30,10 +30,19 @@ pub(crate) enum GlobalMetadata { /// The sizes of the `TrieEncodedChild` and `TrieEncodedChildLen` buffers. In other words, the /// next available offset in these buffers. TrieEncodedChildSize = 14, + + // Block metadata. + BlockBeneficiary = 15, + BlockTimestamp = 16, + BlockNumber = 17, + BlockDifficulty = 18, + BlockGasLimit = 19, + BlockChainId = 20, + BlockBaseFee = 21, } impl GlobalMetadata { - pub(crate) const COUNT: usize = 13; + pub(crate) const COUNT: usize = 20; pub(crate) fn all() -> [Self; Self::COUNT] { [ @@ -50,33 +59,39 @@ impl GlobalMetadata { Self::TransactionTrieRootDigestAfter, Self::ReceiptTrieRootDigestAfter, Self::TrieEncodedChildSize, + Self::BlockBeneficiary, + Self::BlockTimestamp, + Self::BlockNumber, + Self::BlockDifficulty, + Self::BlockGasLimit, + Self::BlockChainId, + Self::BlockBaseFee, ] } /// The variable name that gets passed into kernel assembly code. pub(crate) fn var_name(&self) -> &'static str { match self { - GlobalMetadata::LargestContext => "GLOBAL_METADATA_LARGEST_CONTEXT", - GlobalMetadata::MemorySize => "GLOBAL_METADATA_MEMORY_SIZE", - GlobalMetadata::TrieDataSize => "GLOBAL_METADATA_TRIE_DATA_SIZE", - GlobalMetadata::StateTrieRoot => "GLOBAL_METADATA_STATE_TRIE_ROOT", - GlobalMetadata::TransactionTrieRoot => "GLOBAL_METADATA_TXN_TRIE_ROOT", - GlobalMetadata::ReceiptTrieRoot => "GLOBAL_METADATA_RECEIPT_TRIE_ROOT", - GlobalMetadata::StateTrieRootDigestBefore => "GLOBAL_METADATA_STATE_TRIE_DIGEST_BEFORE", - GlobalMetadata::TransactionTrieRootDigestBefore => { - "GLOBAL_METADATA_TXN_TRIE_DIGEST_BEFORE" - } - GlobalMetadata::ReceiptTrieRootDigestBefore => { - "GLOBAL_METADATA_RECEIPT_TRIE_DIGEST_BEFORE" - } - GlobalMetadata::StateTrieRootDigestAfter => "GLOBAL_METADATA_STATE_TRIE_DIGEST_AFTER", - GlobalMetadata::TransactionTrieRootDigestAfter => { - "GLOBAL_METADATA_TXN_TRIE_DIGEST_AFTER" - } - GlobalMetadata::ReceiptTrieRootDigestAfter => { - "GLOBAL_METADATA_RECEIPT_TRIE_DIGEST_AFTER" - } - GlobalMetadata::TrieEncodedChildSize => "TRIE_ENCODED_CHILD_SIZE", + Self::LargestContext => "GLOBAL_METADATA_LARGEST_CONTEXT", + Self::MemorySize => "GLOBAL_METADATA_MEMORY_SIZE", + Self::TrieDataSize => "GLOBAL_METADATA_TRIE_DATA_SIZE", + Self::StateTrieRoot => "GLOBAL_METADATA_STATE_TRIE_ROOT", + Self::TransactionTrieRoot => "GLOBAL_METADATA_TXN_TRIE_ROOT", + Self::ReceiptTrieRoot => "GLOBAL_METADATA_RECEIPT_TRIE_ROOT", + Self::StateTrieRootDigestBefore => "GLOBAL_METADATA_STATE_TRIE_DIGEST_BEFORE", + Self::TransactionTrieRootDigestBefore => "GLOBAL_METADATA_TXN_TRIE_DIGEST_BEFORE", + Self::ReceiptTrieRootDigestBefore => "GLOBAL_METADATA_RECEIPT_TRIE_DIGEST_BEFORE", + Self::StateTrieRootDigestAfter => "GLOBAL_METADATA_STATE_TRIE_DIGEST_AFTER", + Self::TransactionTrieRootDigestAfter => "GLOBAL_METADATA_TXN_TRIE_DIGEST_AFTER", + Self::ReceiptTrieRootDigestAfter => "GLOBAL_METADATA_RECEIPT_TRIE_DIGEST_AFTER", + Self::TrieEncodedChildSize => "GLOBAL_METADATA_TRIE_ENCODED_CHILD_SIZE", + Self::BlockBeneficiary => "GLOBAL_METADATA_BLOCK_BENEFICIARY", + Self::BlockTimestamp => "GLOBAL_METADATA_BLOCK_TIMESTAMP", + Self::BlockNumber => "GLOBAL_METADATA_BLOCK_NUMBER", + Self::BlockDifficulty => "GLOBAL_METADATA_BLOCK_DIFFICULTY", + Self::BlockGasLimit => "GLOBAL_METADATA_BLOCK_GAS_LIMIT", + Self::BlockChainId => "GLOBAL_METADATA_BLOCK_CHAIN_ID", + Self::BlockBaseFee => "GLOBAL_METADATA_BLOCK_BASE_FEE", } } } diff --git a/evm/src/cpu/kernel/constants/txn_fields.rs b/evm/src/cpu/kernel/constants/txn_fields.rs index cb434788..c2931f05 100644 --- a/evm/src/cpu/kernel/constants/txn_fields.rs +++ b/evm/src/cpu/kernel/constants/txn_fields.rs @@ -13,18 +13,19 @@ pub(crate) enum NormalizedTxnField { /// This is not technically a transaction field, as it depends on the block's base fee. ComputedFeePerGas = 5, GasLimit = 6, - To = 7, - Value = 8, + IntrinsicGas = 7, + To = 8, + Value = 9, /// The length of the data field. The data itself is stored in another segment. - DataLen = 9, - YParity = 10, - R = 11, - S = 12, - Origin = 13, + DataLen = 10, + YParity = 11, + R = 12, + S = 13, + Origin = 14, } impl NormalizedTxnField { - pub(crate) const COUNT: usize = 14; + pub(crate) const COUNT: usize = 15; pub(crate) fn all() -> [Self; Self::COUNT] { [ @@ -35,6 +36,7 @@ impl NormalizedTxnField { Self::MaxFeePerGas, Self::ComputedFeePerGas, Self::GasLimit, + Self::IntrinsicGas, Self::To, Self::Value, Self::DataLen, @@ -55,6 +57,7 @@ impl NormalizedTxnField { NormalizedTxnField::MaxFeePerGas => "TXN_FIELD_MAX_FEE_PER_GAS", NormalizedTxnField::ComputedFeePerGas => "TXN_FIELD_COMPUTED_FEE_PER_GAS", NormalizedTxnField::GasLimit => "TXN_FIELD_GAS_LIMIT", + NormalizedTxnField::IntrinsicGas => "TXN_FIELD_INTRINSIC_GAS", NormalizedTxnField::To => "TXN_FIELD_TO", NormalizedTxnField::Value => "TXN_FIELD_VALUE", NormalizedTxnField::DataLen => "TXN_FIELD_DATA_LEN", diff --git a/evm/tests/basic_smart_contract.rs b/evm/tests/basic_smart_contract.rs index e52b7a76..c127ae33 100644 --- a/evm/tests/basic_smart_contract.rs +++ b/evm/tests/basic_smart_contract.rs @@ -42,6 +42,7 @@ fn test_basic_smart_contract() -> anyhow::Result<()> { let add = get_opcode("ADD"); let stop = get_opcode("STOP"); let code = [push1, 3, push1, 4, add, stop]; + let code_gas = 3 + 3 + 3; let code_hash = keccak(code); let sender_account_before = AccountRlp { @@ -99,9 +100,10 @@ fn test_basic_smart_contract() -> anyhow::Result<()> { timing.filter(Duration::from_millis(100)).print(); let expected_state_trie_after = { + let txdata_gas = 2 * 16; + let gas_used = 21_000 + code_gas + txdata_gas; let sender_account_after = AccountRlp { - // TODO: Should be 21k; 1k gas should be refunded. - balance: sender_account_before.balance - value - 22_000 * 10, + balance: sender_account_before.balance - value - gas_used * 10, nonce: sender_account_before.nonce + 1, ..sender_account_before }; diff --git a/evm/tests/simple_transfer.rs b/evm/tests/simple_transfer.rs index 2d27abfe..a04e5af3 100644 --- a/evm/tests/simple_transfer.rs +++ b/evm/tests/simple_transfer.rs @@ -72,9 +72,10 @@ fn test_simple_transfer() -> anyhow::Result<()> { timing.filter(Duration::from_millis(100)).print(); let expected_state_trie_after = { + let txdata_gas = 2 * 16; + let gas_used = 21_000 + txdata_gas; let sender_account_after = AccountRlp { - // TODO: Should be 21k; 1k gas should be refunded. - balance: sender_account_before.balance - value - 22_000 * 10, + balance: sender_account_before.balance - value - gas_used * 10, nonce: sender_account_before.nonce + 1, ..sender_account_before };