From 2fc2220edc7cc3623ca506bb6efa13d74a2585a5 Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Wed, 12 Apr 2023 11:01:01 +0200 Subject: [PATCH 1/5] EVM/Kernel: remove the notion of ValidTransaction It implies that we now fetch sender's nonce and balance when applying transaction. If there are two transactions with the same sender in the block, the balance is fetch before the application, thus only the balance modification from the last transaction is taken into account on the sender's side. --- src/kernel_evm/kernel/src/block.rs | 109 ++++++++--------------------- 1 file changed, 28 insertions(+), 81 deletions(-) diff --git a/src/kernel_evm/kernel/src/block.rs b/src/kernel_evm/kernel/src/block.rs index 2113ac3e4d37..5b4c41bb7c7d 100644 --- a/src/kernel_evm/kernel/src/block.rs +++ b/src/kernel_evm/kernel/src/block.rs @@ -3,13 +3,12 @@ // SPDX-License-Identifier: MIT use crate::blueprint::Queue; -use crate::error::{Error, TransferError}; +use crate::error::Error; use crate::helpers::address_to_hash; use crate::inbox::Transaction; use crate::storage; use tezos_ethereum::address::EthereumAddress; use tezos_ethereum::signatures::EthereumTransactionCommon; -use tezos_smart_rollup_debug::debug_msg; use tezos_smart_rollup_host::path::OwnedPath; use tezos_smart_rollup_host::runtime::Runtime; @@ -47,15 +46,6 @@ pub struct L2Block { pub uncles: Vec, } -pub struct ValidTransaction { - pub sender_address: EthereumAddress, - pub sender_path: OwnedPath, - pub sender_nonce: U256, - pub sender_balance: Wei, - pub tx_hash: TransactionHash, - pub transaction: EthereumTransactionCommon, -} - impl L2Block { const DUMMY_QUANTITY: U256 = U256::zero(); const DUMMY_HASH: &str = "0000000000000000000000000000000000000000"; @@ -116,40 +106,6 @@ fn get_tx_receiver( Ok((path, tx.to)) } -// A transaction is valid if the signature is valid, its nonce is valid and it -// can pay for the gas -fn validate_transaction( - host: &mut Host, - transaction: Transaction, -) -> Result { - let tx = transaction.tx; - let (sender_path, sender_address) = get_tx_sender(&tx)?; - let sender_balance = - storage::read_account_balance(host, &sender_path).unwrap_or_else(|_| Wei::zero()); - let sender_nonce = - storage::read_account_nonce(host, &sender_path).unwrap_or(U256::zero()); - let nonce: U256 = tx.nonce; - // For now, we consider there's no gas to pay - let gas = Wei::zero(); - - if !tx.clone().verify_signature() { - Err(Error::Transfer(TransferError::InvalidSignature)) - } else if sender_nonce != nonce { - Err(Error::Transfer(TransferError::InvalidNonce)) - } else if sender_balance < gas { - Err(Error::Transfer(TransferError::NotEnoughBalance)) - } else { - Ok(ValidTransaction { - sender_address, - sender_path, - sender_nonce, - sender_balance, - tx_hash: transaction.tx_hash, - transaction: tx, - }) - } -} - // Update an account with the given balance and nonce (if one is given), and // initialize it if it doesn't already appear in the storage. fn update_account( @@ -173,9 +129,10 @@ fn update_account( fn make_receipt( block: &L2Block, - tx: &ValidTransaction, + tx: &Transaction, status: TransactionStatus, index: u32, + sender: EthereumAddress, to: EthereumAddress, ) -> TransactionReceipt { TransactionReceipt { @@ -183,7 +140,7 @@ fn make_receipt( index, block_hash: block.hash, block_number: block.number, - from: tx.sender_address, + from: sender, to: Some(to), cumulative_gas_used: U256::zero(), effective_gas_price: U256::zero(), @@ -198,15 +155,19 @@ fn make_receipt( fn apply_transaction( host: &mut Host, block: &L2Block, - transaction: &ValidTransaction, + transaction: &Transaction, index: u32, ) -> Result { - let tx = &transaction.transaction; - let value: U256 = tx.value; - let gas = Wei::zero(); + let (sender_path, sender_address) = get_tx_sender(&transaction.tx)?; + let sender_balance = + storage::read_account_balance(host, &sender_path).unwrap_or_else(|_| Wei::zero()); + let sender_nonce = + storage::read_account_nonce(host, &sender_path).unwrap_or(U256::zero()); + let value: U256 = transaction.tx.value; + let (dst_path, dst_address) = get_tx_receiver(&transaction.tx)?; // First pay for the gas - let src_balance_without_gas = transaction.sender_balance - gas; + let src_balance_without_gas = sender_balance - transaction.tx.gas_price; // The gas is paid even if there's not enough balance for the total // transaction let (src_balance, status) = if src_balance_without_gas < value { @@ -216,32 +177,24 @@ fn apply_transaction( }; update_account( host, - &transaction.sender_path, + &sender_path, src_balance, - Some(transaction.sender_nonce + U256::one()), + Some(sender_nonce + U256::one()), )?; - let (dst_path, dst_address) = get_tx_receiver(tx)?; if status == TransactionStatus::Success { let dst_balance = storage::read_account_balance(host, &dst_path) .unwrap_or_else(|_| Wei::zero()); update_account(host, &dst_path, dst_balance + value, None)?; }; - Ok(make_receipt(block, transaction, status, index, dst_address)) -} - -fn validate( - host: &mut Host, - transactions: Vec, -) -> Result, Error> { - let mut validated_transactions = Vec::new(); - for transaction in transactions { - match validate_transaction(host, transaction) { - Ok(valid_transaction) => validated_transactions.push(valid_transaction), - Err(e) => return Err(e), - } - } - Ok(validated_transactions) + Ok(make_receipt( + block, + transaction, + status, + index, + sender_address, + dst_address, + )) } // This function is only available in nightly, hence the need for redefinition @@ -259,7 +212,7 @@ fn try_collect(vec: Vec>) -> Result, E> { fn apply_transactions( host: &mut Host, block: &L2Block, - transactions: &[ValidTransaction], + transactions: &[Transaction], ) -> Result, Error> { try_collect( transactions @@ -276,16 +229,10 @@ pub fn produce(host: &mut Host, queue: Queue) -> Result<(), Error let next_level = current_level + 1; let transaction_hashes = proposal.transactions.iter().map(|tx| tx.tx_hash).collect(); - - match validate(host, proposal.transactions) { - Ok(transactions) => { - let valid_block = L2Block::new(next_level, transaction_hashes); - storage::store_current_block(host, &valid_block)?; - let receipts = apply_transactions(host, &valid_block, &transactions)?; - storage::store_transaction_receipts(host, &receipts)?; - } - Err(e) => debug_msg!(host, "Blueprint is invalid: {:?}\n", e), - } + let valid_block = L2Block::new(next_level, transaction_hashes); + storage::store_current_block(host, &valid_block)?; + let receipts = apply_transactions(host, &valid_block, &proposal.transactions)?; + storage::store_transaction_receipts(host, &receipts)?; } Ok(()) } -- GitLab From d1e9ed96c62f36815fb02d00fd7ca73ff9299d32 Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Thu, 20 Apr 2023 10:36:57 +0200 Subject: [PATCH 2/5] EVM/Kernel: omit the gas payment when applying a transaction --- src/kernel_evm/kernel/src/block.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/kernel_evm/kernel/src/block.rs b/src/kernel_evm/kernel/src/block.rs index 5b4c41bb7c7d..95e3e9510719 100644 --- a/src/kernel_evm/kernel/src/block.rs +++ b/src/kernel_evm/kernel/src/block.rs @@ -152,6 +152,8 @@ fn make_receipt( } // invariant: the transaction is valid +// NB: when applying a transaction, we omit the payment of gas as it is not +// properly implemented in the current state of the kernel. fn apply_transaction( host: &mut Host, block: &L2Block, @@ -166,14 +168,10 @@ fn apply_transaction( let value: U256 = transaction.tx.value; let (dst_path, dst_address) = get_tx_receiver(&transaction.tx)?; - // First pay for the gas - let src_balance_without_gas = sender_balance - transaction.tx.gas_price; - // The gas is paid even if there's not enough balance for the total - // transaction - let (src_balance, status) = if src_balance_without_gas < value { - (src_balance_without_gas, TransactionStatus::Failure) + let (src_balance, status) = if sender_balance < value { + (sender_balance, TransactionStatus::Failure) } else { - (src_balance_without_gas - value, TransactionStatus::Success) + (sender_balance - value, TransactionStatus::Success) }; update_account( host, -- GitLab From f3989a3a563e89a1a0c295cb028d3fc77e7f904d Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Wed, 12 Apr 2023 11:13:06 +0200 Subject: [PATCH 3/5] EVM/Kernel: make a failing receipt in case of invalid transaction --- src/kernel_evm/kernel/src/block.rs | 50 +++++++++++++++++------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/src/kernel_evm/kernel/src/block.rs b/src/kernel_evm/kernel/src/block.rs index 95e3e9510719..1b25e3820c32 100644 --- a/src/kernel_evm/kernel/src/block.rs +++ b/src/kernel_evm/kernel/src/block.rs @@ -168,31 +168,39 @@ fn apply_transaction( let value: U256 = transaction.tx.value; let (dst_path, dst_address) = get_tx_receiver(&transaction.tx)?; - let (src_balance, status) = if sender_balance < value { - (sender_balance, TransactionStatus::Failure) + if sender_balance < value + || sender_nonce != transaction.tx.nonce + || !transaction.tx.clone().verify_signature() + { + Ok(make_receipt( + block, + transaction, + TransactionStatus::Failure, + index, + sender_address, + dst_address, + )) } else { - (sender_balance - value, TransactionStatus::Success) - }; - update_account( - host, - &sender_path, - src_balance, - Some(sender_nonce + U256::one()), - )?; - - if status == TransactionStatus::Success { + update_account( + host, + &sender_path, + sender_balance - value, + Some(sender_nonce + U256::one()), + )?; + let dst_balance = storage::read_account_balance(host, &dst_path) .unwrap_or_else(|_| Wei::zero()); update_account(host, &dst_path, dst_balance + value, None)?; - }; - Ok(make_receipt( - block, - transaction, - status, - index, - sender_address, - dst_address, - )) + + Ok(make_receipt( + block, + transaction, + TransactionStatus::Success, + index, + sender_address, + dst_address, + )) + } } // This function is only available in nightly, hence the need for redefinition -- GitLab From 7953ceda17340986c3aa0b60cab7d1742e6518f2 Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Thu, 13 Apr 2023 16:43:22 +0200 Subject: [PATCH 4/5] EVM/Kernel: read transaction receipt's status --- src/kernel_evm/ethereum/src/transaction.rs | 31 ++++++++++++++++++++++ src/kernel_evm/kernel/src/error.rs | 3 ++- src/kernel_evm/kernel/src/genesis.rs | 2 +- src/kernel_evm/kernel/src/storage.rs | 26 +++++++++++++++--- 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/src/kernel_evm/ethereum/src/transaction.rs b/src/kernel_evm/ethereum/src/transaction.rs index 0fe698661a1d..e44eac39203b 100644 --- a/src/kernel_evm/ethereum/src/transaction.rs +++ b/src/kernel_evm/ethereum/src/transaction.rs @@ -22,6 +22,37 @@ pub enum TransactionStatus { Failure, } +pub enum TransactionStatusDecodingError { + InvalidEncoding, + InvalidVectorLength, +} + +impl TryFrom<&u8> for TransactionStatus { + type Error = TransactionStatusDecodingError; + + fn try_from(v: &u8) -> Result { + if *v == 0 { + Ok(Self::Failure) + } else if *v == 1 { + Ok(Self::Success) + } else { + Err(TransactionStatusDecodingError::InvalidEncoding) + } + } +} + +impl TryFrom<&Vec> for TransactionStatus { + type Error = TransactionStatusDecodingError; + + fn try_from(v: &Vec) -> Result { + if v.len() == 1 { + TransactionStatus::try_from(&v[0]) + } else { + Err(TransactionStatusDecodingError::InvalidVectorLength) + } + } +} + /// Transaction receipt, see https://ethereum.org/en/developers/docs/apis/json-rpc/#eth_gettransactionreceipt pub struct TransactionReceipt { /// Hash of the transaction. diff --git a/src/kernel_evm/kernel/src/error.rs b/src/kernel_evm/kernel/src/error.rs index 5df0110c2083..8595d17649e6 100644 --- a/src/kernel_evm/kernel/src/error.rs +++ b/src/kernel_evm/kernel/src/error.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: MIT use core::str::Utf8Error; -use tezos_smart_rollup_host::path::PathError; +use tezos_smart_rollup_host::path::{OwnedPath, PathError}; use tezos_smart_rollup_host::runtime::RuntimeError; #[derive(Debug)] @@ -18,6 +18,7 @@ pub enum StorageError { Path(PathError), Runtime(RuntimeError), InvalidLoadValue { expected: usize, actual: usize }, + InvalidEncoding { path: OwnedPath, value: Vec }, } #[derive(Debug)] diff --git a/src/kernel_evm/kernel/src/genesis.rs b/src/kernel_evm/kernel/src/genesis.rs index 70f2aee6ba68..64e2bf42cb79 100644 --- a/src/kernel_evm/kernel/src/genesis.rs +++ b/src/kernel_evm/kernel/src/genesis.rs @@ -132,7 +132,7 @@ fn store_genesis_receipts( status: TransactionStatus::Success, }; - let receipt_path = receipt_path(&receipt)?; + let receipt_path = receipt_path(&receipt.hash)?; storage::store_transaction_receipt(&receipt_path, host, &receipt)?; } diff --git a/src/kernel_evm/kernel/src/storage.rs b/src/kernel_evm/kernel/src/storage.rs index bdf841290b66..2a699169f596 100644 --- a/src/kernel_evm/kernel/src/storage.rs +++ b/src/kernel_evm/kernel/src/storage.rs @@ -14,7 +14,7 @@ use crate::error::{Error, StorageError}; use tezos_ethereum::account::*; use tezos_ethereum::eth_gen::{BlockHash, Hash, L2Level}; use tezos_ethereum::transaction::{ - TransactionHash, TransactionReceipt, TRANSACTION_HASH_SIZE, + TransactionHash, TransactionReceipt, TransactionStatus, TRANSACTION_HASH_SIZE, }; use tezos_ethereum::wei::Wei; @@ -46,6 +46,7 @@ const TRANSACTION_RECEIPT_TYPE: RefPath = RefPath::assert_from(b"/type"); const TRANSACTION_RECEIPT_STATUS: RefPath = RefPath::assert_from(b"/status"); const HASH_MAX_SIZE: usize = 32; +const TRANSACTION_RECEIPT_STATUS_SIZE: usize = 1; // We can read/store at most [128] transaction hashes per block. // TRANSACTION_HASH_SIZE * 128 = 4096. @@ -121,8 +122,8 @@ pub fn block_path(number: L2Level) -> Result { let number_path = OwnedPath::try_from(raw_number_path)?; concat(&EVM_BLOCKS, &number_path).map_err(Error::from) } -pub fn receipt_path(receipt: &TransactionReceipt) -> Result { - let hash = hex::encode(receipt.hash); +pub fn receipt_path(receipt_hash: &TransactionHash) -> Result { + let hash = hex::encode(receipt_hash); let raw_receipt_path: Vec = format!("/{}", &hash).into(); let receipt_path = OwnedPath::try_from(raw_receipt_path)?; concat(&TRANSACTIONS_RECEIPTS, &receipt_path).map_err(Error::from) @@ -355,12 +356,29 @@ pub fn store_transaction_receipts( receipts: &[TransactionReceipt], ) -> Result<(), Error> { for receipt in receipts { - let receipt_path = receipt_path(receipt)?; + let receipt_path = receipt_path(&receipt.hash)?; store_transaction_receipt(&receipt_path, host, receipt)?; } Ok(()) } +pub fn read_transaction_receipt_status( + host: &mut Host, + tx_hash: &TransactionHash, +) -> Result { + let receipt_path = receipt_path(tx_hash)?; + let status_path = concat(&receipt_path, &TRANSACTION_RECEIPT_STATUS)?; + let raw_status = host + .store_read(&status_path, 0, TRANSACTION_RECEIPT_STATUS_SIZE) + .map_err(Error::from)?; + TransactionStatus::try_from(&raw_status).map_err(|_| { + Error::Storage(StorageError::InvalidEncoding { + path: status_path, + value: raw_status, + }) + }) +} + const CHUNKED_TRANSACTIONS: RefPath = RefPath::assert_from(b"/chunked_transactions"); const CHUNKED_TRANSACTION_NUM_CHUNKS: RefPath = RefPath::assert_from(b"/num_chunks"); -- GitLab From d7d6c5c6a168f1d2f6accfc842fe19f48a75bc78 Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Wed, 12 Apr 2023 12:50:41 +0200 Subject: [PATCH 5/5] EVM/Kernel: test of invalid transactions producing failing receipts' status --- src/kernel_evm/kernel/src/block.rs | 77 ++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/src/kernel_evm/kernel/src/block.rs b/src/kernel_evm/kernel/src/block.rs index 1b25e3820c32..1c17262103bc 100644 --- a/src/kernel_evm/kernel/src/block.rs +++ b/src/kernel_evm/kernel/src/block.rs @@ -242,3 +242,80 @@ pub fn produce(host: &mut Host, queue: Queue) -> Result<(), Error } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::blueprint::Blueprint; + use crate::genesis; + use crate::inbox::Transaction; + use crate::storage::read_transaction_receipt_status; + use primitive_types::H256; + use tezos_ethereum::address::EthereumAddress; + use tezos_ethereum::transaction::TRANSACTION_HASH_SIZE; + use tezos_smart_rollup_mock::MockHost; + + fn string_to_h256_unsafe(s: &str) -> H256 { + let mut v: [u8; 32] = [0; 32]; + hex::decode_to_slice(s, &mut v).expect("Could not parse to 256 hex value."); + H256::from(v) + } + + fn dummy_eth_transaction() -> EthereumTransactionCommon { + let nonce = U256::from(0); + let gas_price = U256::from(40000000000u64); + let gas_limit = U256::from(21000); + let to = + EthereumAddress::from("423163e58aabec5daa3dd1130b759d24bef0f6ea".to_string()); + let value = U256::from(5000000000000000u64); + let data: Vec = hex::decode("deace8f5000000000000000000000000000000000000000000000000000000000000a4b100000000000000000000000041bca408a6b4029b42883aeb2c25087cab76cb58000000000000000000000000000000000000000000000000002386f26fc10000000000000000000000000000000000000000000000000000002357a49c7d75f600000000000000000000000000000000000000000000000000000000640b5549000000000000000000000000710bda329b2a6224e4b44833de30f38e7f81d5640000000000000000000000000000000000000000000000000000000000000000").unwrap(); + let v = U256::from(37); + let r = string_to_h256_unsafe( + "25dd6c973368c45ddfc17f5148e3f468a2e3f2c51920cbe9556a64942b0ab2eb", + ); + let s = string_to_h256_unsafe( + "31da07ce40c24b0a01f46fb2abc028b5ccd70dbd1cb330725323edc49a2a9558", + ); + EthereumTransactionCommon { + chain_id: U256::one(), + nonce, + gas_price, + gas_limit, + to, + value, + data, + v, + r, + s, + } + } + + #[test] + // Test if the invalid transactions are producing receipts with invalid status + fn test_invalid_transactions_receipt_status() { + let mut host = MockHost::default(); + let _ = genesis::init_block(&mut host); + + let tx_hash = [0; TRANSACTION_HASH_SIZE]; + + let invalid_tx = Transaction { + tx_hash, + tx: dummy_eth_transaction(), + }; + + let transactions: Vec = vec![invalid_tx]; + let queue = Queue { + proposals: vec![Blueprint { transactions }], + }; + + produce(&mut host, queue).expect("The block production failed."); + + match read_transaction_receipt_status(&mut host, &tx_hash) { + Ok(TransactionStatus::Failure) => (), + Ok(TransactionStatus::Success) => { + panic!("The receipt should have a failing status.") + } + Err(_) => panic!("Reading the receipt failed."), + } + } +} -- GitLab