From dde280ee35e4a48b7957a20ecb90212aa62c7110 Mon Sep 17 00:00:00 2001 From: Hai | Kreate Date: Mon, 22 Apr 2024 14:54:58 +0700 Subject: [PATCH] feat: lazy update beneficiary balance to avoid "implicit" dependency among consecutive transactions --- Cargo.lock | 87 ++++++++++---------- Cargo.toml | 2 +- README.md | 2 - src/block_stm.rs | 52 ++++++++---- src/lib.rs | 20 +++-- src/main.rs | 7 +- src/mv_memory.rs | 57 ++++++------- src/scheduler.rs | 2 +- src/vm.rs | 209 +++++++++++++++++++++++++++++++++++++---------- 9 files changed, 295 insertions(+), 143 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d118d0d1..d53dbde0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -184,9 +184,9 @@ checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" [[package]] name = "aurora-engine-modexp" -version = "1.0.0" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bfacad86e9e138fca0670949eb8ed4ffdf73a55bded8887efe0863cd1a3a6f70" +checksum = "0aef7712851e524f35fbbb74fa6599c5cd8692056a1c36f9ca0d2001b670e7e5" dependencies = [ "hex", "num", @@ -1065,8 +1065,7 @@ checksum = "adad44e29e4c806119491a7f06f03de4d1af22c3a680dd47f1e6e179439d1f56" [[package]] name = "revm" version = "8.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72a454c1c650b2b2e23f0c461af09e6c31e1d15e1cbebe905a701c46b8a50afc" +source = "git+https://github.com/risechain/revm?rev=3c046a6aa734d914dc37bcade8ac5932a9599a23#3c046a6aa734d914dc37bcade8ac5932a9599a23" dependencies = [ "auto_impl", "cfg-if", @@ -1080,8 +1079,7 @@ dependencies = [ [[package]] name = "revm-interpreter" version = "4.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d322f2730cd300e99d271a1704a2dfb8973d832428f5aa282aaa40e2473b5eec" +source = "git+https://github.com/risechain/revm?rev=3c046a6aa734d914dc37bcade8ac5932a9599a23#3c046a6aa734d914dc37bcade8ac5932a9599a23" dependencies = [ "revm-primitives", "serde", @@ -1090,8 +1088,7 @@ dependencies = [ [[package]] name = "revm-precompile" version = "6.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "931f692f3f4fc72ec39d5d270f8e9d208c4a6008de7590ee96cf948e3b6d3f8d" +source = "git+https://github.com/risechain/revm?rev=3c046a6aa734d914dc37bcade8ac5932a9599a23#3c046a6aa734d914dc37bcade8ac5932a9599a23" dependencies = [ "aurora-engine-modexp", "c-kzg", @@ -1107,8 +1104,7 @@ dependencies = [ [[package]] name = "revm-primitives" version = "3.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cbbc9640790cebcb731289afb7a7d96d16ad94afeb64b5d0b66443bd151e79d6" +source = "git+https://github.com/risechain/revm?rev=3c046a6aa734d914dc37bcade8ac5932a9599a23#3c046a6aa734d914dc37bcade8ac5932a9599a23" dependencies = [ "alloy-primitives", "auto_impl", @@ -1210,9 +1206,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.32" +version = "0.38.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "65e04861e65f21776e67888bfbea442b3642beaa0138fdb1dd7a84a52dffdb89" +checksum = "70dc5ec042f7a43c4a73241207cecc9873a06d45debb38b329f8541d85c2730f" dependencies = [ "bitflags 2.5.0", "errno", @@ -1261,9 +1257,9 @@ dependencies = [ [[package]] name = "secp256k1" -version = "0.28.2" +version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d24b59d129cdadea20aea4fb2352fa053712e5d713eee47d700cd4b2bc002f10" +checksum = "0e0cc0f1cf93f4969faf3ea1c7d8a9faed25918d96affa959720823dfe86d4f3" dependencies = [ "rand", "secp256k1-sys", @@ -1271,9 +1267,9 @@ dependencies = [ [[package]] name = "secp256k1-sys" -version = "0.9.2" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5d1746aae42c19d583c3c1a8c646bfad910498e2051c551a7f2e3c0c9fbb7eb" +checksum = "1433bd67156263443f14d603720b082dd3121779323fce20cba2aa07b874bc1b" dependencies = [ "cc", ] @@ -1324,9 +1320,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.115" +version = "1.0.116" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "12dc5c46daa8e9fdf4f5e71b6cf9a53f2487da0e86e55808e2d35539666497dd" +checksum = "3e17db7126d17feb94eb3fad46bf1a96b034e8aacbc2e775fe81505f8b0b2813" dependencies = [ "indexmap", "itoa", @@ -1576,7 +1572,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.4", + "windows-targets 0.52.5", ] [[package]] @@ -1596,17 +1592,18 @@ dependencies = [ [[package]] name = "windows-targets" -version = "0.52.4" +version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7dd37b7e5ab9018759f893a1952c9420d060016fc19a472b4bb20d1bdd694d1b" +checksum = "6f0713a46559409d202e70e28227288446bf7841d3211583a4b53e3f6d96e7eb" dependencies = [ - "windows_aarch64_gnullvm 0.52.4", - "windows_aarch64_msvc 0.52.4", - "windows_i686_gnu 0.52.4", - "windows_i686_msvc 0.52.4", - "windows_x86_64_gnu 0.52.4", - "windows_x86_64_gnullvm 0.52.4", - "windows_x86_64_msvc 0.52.4", + "windows_aarch64_gnullvm 0.52.5", + "windows_aarch64_msvc 0.52.5", + "windows_i686_gnu 0.52.5", + "windows_i686_gnullvm", + "windows_i686_msvc 0.52.5", + "windows_x86_64_gnu 0.52.5", + "windows_x86_64_gnullvm 0.52.5", + "windows_x86_64_msvc 0.52.5", ] [[package]] @@ -1617,9 +1614,9 @@ checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" [[package]] name = "windows_aarch64_gnullvm" -version = "0.52.4" +version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcf46cf4c365c6f2d1cc93ce535f2c8b244591df96ceee75d8e83deb70a9cac9" +checksum = "7088eed71e8b8dda258ecc8bac5fb1153c5cffaf2578fc8ff5d61e23578d3263" [[package]] name = "windows_aarch64_msvc" @@ -1629,9 +1626,9 @@ checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" [[package]] name = "windows_aarch64_msvc" -version = "0.52.4" +version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da9f259dd3bcf6990b55bffd094c4f7235817ba4ceebde8e6d11cd0c5633b675" +checksum = "9985fd1504e250c615ca5f281c3f7a6da76213ebd5ccc9561496568a2752afb6" [[package]] name = "windows_i686_gnu" @@ -1641,9 +1638,15 @@ checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" [[package]] name = "windows_i686_gnu" -version = "0.52.4" +version = "0.52.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "88ba073cf16d5372720ec942a8ccbf61626074c6d4dd2e745299726ce8b89670" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b474d8268f99e0995f25b9f095bc7434632601028cf86590aea5c8a5cb7801d3" +checksum = "87f4261229030a858f36b459e748ae97545d6f1ec60e5e0d6a3d32e0dc232ee9" [[package]] name = "windows_i686_msvc" @@ -1653,9 +1656,9 @@ checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" [[package]] name = "windows_i686_msvc" -version = "0.52.4" +version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1515e9a29e5bed743cb4415a9ecf5dfca648ce85ee42e15873c3cd8610ff8e02" +checksum = "db3c2bf3d13d5b658be73463284eaf12830ac9a26a90c717b7f771dfe97487bf" [[package]] name = "windows_x86_64_gnu" @@ -1665,9 +1668,9 @@ checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" [[package]] name = "windows_x86_64_gnu" -version = "0.52.4" +version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5eee091590e89cc02ad514ffe3ead9eb6b660aedca2183455434b93546371a03" +checksum = "4e4246f76bdeff09eb48875a0fd3e2af6aada79d409d33011886d3e1581517d9" [[package]] name = "windows_x86_64_gnullvm" @@ -1677,9 +1680,9 @@ checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" [[package]] name = "windows_x86_64_gnullvm" -version = "0.52.4" +version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77ca79f2451b49fa9e2af39f0747fe999fcda4f5e241b2898624dca97a1f2177" +checksum = "852298e482cd67c356ddd9570386e2862b5673c85bd5f88df9ab6802b334c596" [[package]] name = "windows_x86_64_msvc" @@ -1689,9 +1692,9 @@ checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" [[package]] name = "windows_x86_64_msvc" -version = "0.52.4" +version = "0.52.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32b752e52a2da0ddfbdbcc6fceadfeede4c939ed16d13e648833a61dfb611ed8" +checksum = "bec47e5bfd1bff0eeaf6d8b485cc1074891a197ab4225d504cb7a1ab88b02bf0" [[package]] name = "winnow" diff --git a/Cargo.toml b/Cargo.toml index 952ba9e0..92dcae77 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" [dependencies] dashmap = "5.5.3" -revm = "8.0.0" +revm = { git = "https://github.com/risechain/revm", rev = "3c046a6aa734d914dc37bcade8ac5932a9599a23" } [lints] rust.missing_debug_implementations = "warn" diff --git a/README.md b/README.md index 87729405..90ab6875 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,6 @@ Finally, while Aptos and Polygon embed their Block-STM implementation directly i ### V1 TODO -- Properly handle the block's beneficiary account, which makes all transactions interdependent when paying gas. We should distinguish beneficiary reads from execution reads (like `address` and `balance` opcodes) so we can defer or atomically update the beneficiary balance. We may pass in a custom `PostExecutionHandler::reward_beneficiary` to solve this. -- Properly check for changed account infos in `revm`'s `ResultAndState` before adding them to the write set. - Complete a robust version that passes all the relevant [ethereum/tests](https://github.com/ethereum/tests). - Design more tests for larger blocks with complex state transitions and dependencies (ERC-20 and Uniswap transactions, etc.); contribute upstream if appropriate. - Provide deep benchmarks, including a [Reth](https://github.com/paradigmxyz/reth) integration for syncing & building blocks. diff --git a/src/block_stm.rs b/src/block_stm.rs index 76e734e3..676f389b 100644 --- a/src/block_stm.rs +++ b/src/block_stm.rs @@ -10,7 +10,7 @@ use crate::{ mv_memory::MvMemory, scheduler::Scheduler, vm::{Vm, VmExecutionResult}, - ExecutionTask, Storage, Task, TxVersion, ValidationTask, + ExecutionTask, MemoryLocation, MemoryValue, Storage, Task, TxVersion, ValidationTask, }; /// An interface to execute Block-STM. @@ -30,8 +30,16 @@ impl BlockSTM { let block_size = txs.len(); let scheduler = Scheduler::new(block_size); let mv_memory = Arc::new(MvMemory::new(block_size)); - let vm = Vm::new(storage.clone(), block_env, txs.clone(), mv_memory.clone()); - let execution_results = Mutex::new(vec![None; txs.len()]); + let vm = Vm::new( + storage.clone(), + block_env.clone(), + txs.clone(), + mv_memory.clone(), + ); + // TODO: Better error handling + let mut beneficiary_account_info = storage.basic(block_env.coinbase).unwrap(); + // TODO: Should we move this to `Vm`? + let execution_results = (0..block_size).map(|_| Mutex::new(None)).collect(); // TODO: Better thread handling thread::scope(|scope| { for _ in 0..concurrency_level.into() { @@ -76,17 +84,33 @@ impl BlockSTM { } }); - println!("MV Memory snapshot length: {}", mv_memory.snapshot().len()); - - // TODO: Better error handling - let execution_results = execution_results.lock().unwrap(); + // We lazily evaluate the final beneficiary account's balance at the end of each transaction + // to avoid "implicit" dependency among consecutive transactions that read & write there. execution_results .iter() - .cloned() - // TODO: Better error handling - // Scheduler shouldn't claim `done` when - // there is still a `None`` result. - .map(|r| r.unwrap()) + // TODO: Better error handling. Scheduler shouldn't claim `done` when there is still + // a `None`` result. + .map(|m| m.lock().unwrap().clone().unwrap()) + .enumerate() + .map(|(tx_idx, mut result_and_state)| { + // TODO: Support pre-EIP-3651 when the beneficiary account may not be loaded and be + // present in the state changes map. + let beneficiary_account = + result_and_state.state.get_mut(&block_env.coinbase).unwrap(); + beneficiary_account.mark_touch(); + // TODO: Better error handling & make this much faster + match mv_memory.read_absolute(&MemoryLocation::Basic(block_env.coinbase), tx_idx) { + MemoryValue::Basic(account) => { + beneficiary_account_info = account; + } + MemoryValue::LazyBeneficiaryBalance(addition) => { + beneficiary_account_info.balance += addition; + } + _ => unreachable!(), + } + beneficiary_account.info = beneficiary_account_info.clone(); + result_and_state + }) .collect() } } @@ -102,7 +126,7 @@ fn try_execute( mv_memory: &Arc, vm: &Vm, scheduler: &Scheduler, - execution_results: &Mutex>>, + execution_results: &Vec>>, tx_version: &TxVersion, ) -> Option { match vm.execute(tx_version.tx_idx) { @@ -119,7 +143,7 @@ fn try_execute( read_set, write_set, } => { - execution_results.lock().unwrap()[tx_version.tx_idx] = Some(result_and_state); + *execution_results[tx_version.tx_idx].lock().unwrap() = Some(result_and_state); let wrote_new_location = mv_memory.record(tx_version, read_set, write_set); scheduler.finish_execution(tx_version, wrote_new_location) } diff --git a/src/lib.rs b/src/lib.rs index 46fa1167..c370d09e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,9 +4,10 @@ use revm::primitives::{AccountInfo, Address, U256}; -// For simplicity, we first stop at the address & storage level. We -// can still break an address into smaller memory locations to -// minimize re-executions on "broad" state conflicts? +// TODO: More granularity here, for instance, to separate an account's +// balance, nonce, etc. instead of marking conflict at the whole account. +// That way we may also generalize beneficiary balance's lazy update +// behaviour into `MemoryValue` for more use cases. // TODO: It would be nice if we could tie the different cases of // memory locations & values at the type level, to prevent lots of // matches & potentially dangerous mismatch mistakes. @@ -17,9 +18,18 @@ enum MemoryLocation { Storage((Address, U256)), } -#[derive(Clone)] +#[derive(Debug, Clone)] enum MemoryValue { Basic(AccountInfo), + // We lazily update the beneficiary balance to avoid continuous + // dependencies as all transactions read and write to it. We + // either evaluate all these beneficiary account states at the + // end of BlockSTM, or when there is an explicit read. + // Important: The value of this lazy (update) balance is the gas + // it receives in the transaction, to be added to the absolute + // balance at the end of the previous transaction. + // We can probably generalize this to `AtomicBalanceAddition`. + LazyBeneficiaryBalance(U256), Storage(U256), } @@ -47,7 +57,7 @@ struct TxVersion { // The origin of a memory read. It could be from the live multi-version // data structure or from storage (chain state before block execution). -#[derive(PartialEq)] +#[derive(Debug, PartialEq)] enum ReadOrigin { // The previous transaction version that wrote the value. MvMemory(TxVersion), diff --git a/src/main.rs b/src/main.rs index 010eb146..2f662e8d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,7 +6,7 @@ use revm::primitives::{ alloy_primitives::U160, env::TxEnv, AccountInfo, Address, BlockEnv, ResultAndState, TransactTo, U256, }; -use revm::{Evm, InMemoryDB}; +use revm::{DatabaseCommit, Evm, InMemoryDB}; use std::num::NonZeroUsize; use std::sync::Arc; use std::thread; @@ -23,7 +23,7 @@ fn execute_sequential(mut db: InMemoryDB, txs: Arc>) -> Vec`! -use dashmap::DashMap; +// ~x2 performance gain over a naive `RwLock`! +use dashmap::{mapref::entry::Entry, DashMap}; use crate::{ MemoryLocation, MemoryValue, ReadOrigin, ReadSet, TxIdx, TxIncarnation, TxVersion, WriteSet, }; -#[derive(Clone)] +#[derive(Debug, Clone)] enum MemoryEntry { Data(TxIncarnation, MemoryValue), // When an incarnation is aborted due to a validation failure, the @@ -58,24 +58,6 @@ impl MvMemory { } } - // Return the value written by the highest transaction for every location - // that was written to by some transaction. - // - // TODO: More properly type this, especially depending on who needs it. - // TODO: We can easily parallize this. - pub(crate) fn snapshot(&self) -> HashMap { - let mut ret = HashMap::new(); - for entry in self.data.iter() { - let location = entry.key(); - // TODO: Replace MAX with no. transactions? - // TODO: Inline the relevant part from `self.read`? - if let ReadMemoryResult::Ok { value, .. } = self.read(location, std::usize::MAX) { - ret.insert(location.clone(), value); - } - } - ret - } - // Apply a new pair of read & write sets to the multi-version data structure. // Return whether a write occurred to a memory location not written to by // the previous incarnation of the same transaction. This determines whether @@ -90,17 +72,19 @@ impl MvMemory { *self.last_read_set[tx_version.tx_idx].lock().unwrap() = read_set; for (location, value) in write_set.iter() { - // TODO: Cleaner insert/upsert here let entry = MemoryEntry::Data(tx_version.tx_incarnation, value.clone()); - match self.data.get_mut(location) { - Some(location_map) => { - location_map.insert(tx_version.tx_idx, entry); + // We must not use `get_mut` here, else there's a race condition where + // two threads get `None` first, then the latter's `insert` overwrote + // the former's. + match self.data.entry(location.clone()) { + Entry::Occupied(location_map) => { + location_map.get().insert(tx_version.tx_idx, entry); } - _ => { + Entry::Vacant(vacant) => { // TODO: Fine-tune the number of shards let location_map = DashMap::with_shard_amount(2); location_map.insert(tx_version.tx_idx, entry); - self.data.insert(location.clone(), location_map); + vacant.insert(location_map); } } } @@ -160,8 +144,9 @@ impl MvMemory { } ReadMemoryResult::Ok { version, .. } => match prior_origin { // TODO: Verify this logic as it's not explicitly described - // in the paper - ReadOrigin::Storage => return false, + // in the paper. We're setting `true` here to at least accept + // lazily loading beneficiary account all the way to storage. + ReadOrigin::Storage => return true, ReadOrigin::MvMemory(v) => { if *v != version { return false; @@ -227,4 +212,16 @@ impl MvMemory { }, } } + + // Mainly for post-processing like fully evaluating benficiary accounts + // TODO: Better error handling + pub(crate) fn read_absolute(&self, location: &MemoryLocation, tx_idx: TxIdx) -> MemoryValue { + match self.data.get(location).unwrap().get(&tx_idx).as_deref() { + Some(MemoryEntry::Data(_, value)) => value.clone(), + edge => { + println!("{tx_idx}: {:?}", edge); + todo!() + } + } + } } diff --git a/src/scheduler.rs b/src/scheduler.rs index e38d3b7d..90faf204 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -181,7 +181,7 @@ impl Scheduler { // gets re-executed before the dependency can be added. // TODO: Better error handling, including asserting that both indices are in range. pub(crate) fn add_dependency(&self, tx_idx: TxIdx, blocking_tx_idx: TxIdx) -> bool { - // NOTE: This is an important lock to prevent a race condition where the blocking + // This is an important lock to prevent a race condition where the blocking // transaction completes re-execution before this dependecy can be added. let blocking_transaction_status = self.transactions_status[blocking_tx_idx].lock().unwrap(); if let TxIncarnationStatus::Executed(_) = *blocking_transaction_status { diff --git a/src/vm.rs b/src/vm.rs index 903b9bb2..da588048 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -1,10 +1,11 @@ -use std::sync::Arc; +use std::{cell::RefCell, sync::Arc}; use revm::{ primitives::{ - AccountInfo, Address, BlockEnv, Bytecode, EVMError, ResultAndState, TxEnv, B256, U256, + AccountInfo, Address, BlockEnv, Bytecode, CancunSpec, EVMError, ResultAndState, TxEnv, + B256, U256, }, - Database, Evm, + Database, Evm, Handler, }; use crate::{ @@ -27,6 +28,7 @@ pub(crate) enum VmExecutionResult { // transaction with revm. It provides values from the multi-version data // structure & storage, and tracks the read set of the current execution. struct VmDb { + block_env: BlockEnv, tx_idx: TxIdx, mv_memory: Arc, storage: Arc, @@ -34,8 +36,14 @@ struct VmDb { } impl VmDb { - fn new(tx_idx: TxIdx, mv_memory: Arc, storage: Arc) -> Self { + fn new( + block_env: BlockEnv, + tx_idx: TxIdx, + mv_memory: Arc, + storage: Arc, + ) -> Self { Self { + block_env, tx_idx, mv_memory, storage, @@ -43,27 +51,84 @@ impl VmDb { } } - fn read(&mut self, location: MemoryLocation) -> Result { - // TODO: Better error handling - match self.mv_memory.read(&location, self.tx_idx) { + // TODO: Distinguish beneficiary balance reads to call their dedicated + // read function. Should we branch here or just init MvMemory with + // the benediciary address? + fn read( + &mut self, + location: &MemoryLocation, + tx_idx: TxIdx, + update_read_set: bool, + ) -> Result { + // Dedicated handling for the beneficiary account + // TODO: Refactor & find a better place for this specific logic + if let MemoryLocation::Basic(address) = *location { + if address == self.block_env.coinbase { + if tx_idx == 0 { + if update_read_set { + self.read_set.push((location.clone(), ReadOrigin::Storage)); + } + return self.storage.basic(address).map(MemoryValue::Basic); + } + match self.mv_memory.read(location, tx_idx) { + ReadMemoryResult::Ok { version, value } => { + if update_read_set { + self.read_set + .push((location.clone(), ReadOrigin::MvMemory(version))); + } + match value { + MemoryValue::Basic(account) => return Ok(MemoryValue::Basic(account)), + MemoryValue::LazyBeneficiaryBalance(addition) => { + // TODO: Better error handling + // TODO: Handle stack overflow when recursing at a very high tx idx + match self.read(location, tx_idx - 1, false) { + Ok(MemoryValue::Basic(mut beneficiary_account)) => { + // TODO: Write this new absolute value to MvMemory + // to avoid future recalculations. + beneficiary_account.balance += addition; + return Ok(MemoryValue::Basic(beneficiary_account)); + } + _ => unreachable!(), + } + } + _ => unreachable!(), + } + } + _ => { + // Wait for the previous transaction to complete processing + // the beneficiary account. + // TODO: Ideally, we can detect and abort if the wait is + // expected to be long. Sleep a bit here if needed? + return self.read(location, tx_idx, update_read_set); + } + } + } + } + + // Main handling for BlockSTM + match self.mv_memory.read(location, tx_idx) { ReadMemoryResult::ReadError { blocking_tx_idx } => { Err(ReadError::BlockingIndex(blocking_tx_idx)) } ReadMemoryResult::NotFound => { - self.read_set.push((location.clone(), ReadOrigin::Storage)); + if update_read_set { + self.read_set.push((location.clone(), ReadOrigin::Storage)); + } match location { MemoryLocation::Basic(address) => { - self.storage.basic(address).map(MemoryValue::Basic) + self.storage.basic(*address).map(MemoryValue::Basic) } MemoryLocation::Storage((address, index)) => self .storage - .storage(address, index) + .storage(*address, *index) .map(MemoryValue::Storage), } } ReadMemoryResult::Ok { version, value } => { - self.read_set - .push((location, ReadOrigin::MvMemory(version))); + if update_read_set { + self.read_set + .push((location.clone(), ReadOrigin::MvMemory(version))); + } Ok(value) } } @@ -73,11 +138,25 @@ impl VmDb { impl Database for VmDb { type Error = ReadError; - fn basic(&mut self, address: Address) -> Result, Self::Error> { - match self.read(MemoryLocation::Basic(address)) { - Ok(MemoryValue::Basic(value)) => Ok(Some(value)), - Ok(MemoryValue::Storage(_)) => Err(ReadError::InvalidMemoryLocationType), + // TODO: More granularity here to ensure we only record dependencies for, + // for instance, only an account's balance instead of the whole account + // info. That way we may also generalize beneficiary balance's lazy update + // behaviour into `MemoryValue` for more use cases. + fn basic( + &mut self, + address: Address, + // TODO: Better way for REVM to notifiy explicit reads + is_preload: bool, + ) -> Result, Self::Error> { + // We preload a mock beneficiary account, to only lazy evaluate it on + // explicit reads and once BlockSTM is completed. + if address == self.block_env.coinbase && is_preload { + return Ok(Some(AccountInfo::default())); + } + match self.read(&MemoryLocation::Basic(address), self.tx_idx, !is_preload) { Err(err) => Err(err), + Ok(MemoryValue::Basic(value)) => Ok(Some(value)), + _ => Err(ReadError::InvalidMemoryLocationType), } } @@ -85,11 +164,21 @@ impl Database for VmDb { self.storage.code_by_hash(code_hash) } - fn storage(&mut self, address: Address, index: U256) -> Result { - match self.read(MemoryLocation::Storage((address, index))) { - Ok(MemoryValue::Basic(_)) => Err(ReadError::InvalidMemoryLocationType), - Ok(MemoryValue::Storage(value)) => Ok(value), + fn storage( + &mut self, + address: Address, + index: U256, + // TODO: Better way for REVM to notifiy explicit reads + is_preload: bool, + ) -> Result { + match self.read( + &MemoryLocation::Storage((address, index)), + self.tx_idx, + !is_preload, + ) { Err(err) => Err(err), + Ok(MemoryValue::Storage(value)) => Ok(value), + _ => Err(ReadError::InvalidMemoryLocationType), } } @@ -139,11 +228,33 @@ impl Vm { // value are added to the write set, possibly replacing a pair with a prior value // (if it is not the first time the transaction wrote to this location during the // execution). + #[allow(clippy::arc_with_non_send_sync)] // TODO: Fix at REVM? pub(crate) fn execute(&self, tx_idx: TxIdx) -> VmExecutionResult { - let mut db = VmDb::new(tx_idx, self.mv_memory.clone(), self.storage.clone()); + let mut db = VmDb::new( + self.block_env.clone(), + tx_idx, + self.mv_memory.clone(), + self.storage.clone(), + ); + // The amount this transaction needs to pay to the beneficiary account for + // atomic update. + let gas_payment = RefCell::new(U256::ZERO); + // TODO: Support OP & receive Spec as a BlockSTM input. + let mut handler = Handler::mainnet::(); + // TODO: Bring to `self` instead of constructing every call? + handler.post_execution.reward_beneficiary = Arc::new(|context, gas| { + *gas_payment.borrow_mut() = context + .evm + .env + .effective_gas_price() + .saturating_sub(context.evm.env.block.basefee) + * U256::from(gas.spent() - gas.refunded() as u64); + Ok(()) + }); let mut evm = Evm::builder() .with_db(&mut db) + .with_handler(handler) .with_block_env(self.block_env.clone()) .with_tx_env(self.txs.get(tx_idx).unwrap().clone()) .build(); @@ -152,33 +263,28 @@ impl Vm { drop(evm); // to reclaim the DB match evm_result { - Ok(result_and_state) => VmExecutionResult::Ok { - result_and_state: result_and_state.clone(), - // TODO: Confirm that this is correct. For instance, - // that REVM doesn't use some internal values/caches and hence - // miss some `VmDb` calls that populate the read set. - read_set: db.read_set, - write_set: result_and_state + Ok(result_and_state) => { + let mut explicitly_wrote_to_coinbase = false; + let mut write_set: Vec<(MemoryLocation, MemoryValue)> = result_and_state .state .iter() .flat_map(|(address, account)| { - // TODO: More granularity here to ensure we only notify - // new value writes and properly skip old-value locations. - // TODO: Confirm that we're not missing anything, like bytecode. - let mut writes = vec![]; - // TODO: Properly handle beneficiary account then remove this - // check. Currently, every transaction reads and writes to the - // beneficiary account of the block making all blocks sequential - // by definition. We should distinguish beneficiary reads from - // sender & balance opcode reads so we can defer or atomically - // update the beneficiary account's balance. We probably want to - // pass in a custom `PostExecutionHandler::reward_beneficiary`. - if address != &self.block_env.coinbase { - // TODO: Checking if the account's basic info is changed - // before registering it as a new write. + let mut writes = Vec::new(); + // TODO: Verify that depending on this touched status is correct! + if account.is_touched() { + // TODO: More granularity here to ensure we only notify new + // memory writes, for instance, only an account's balance instead + // of the whole account. That way we may also generalize beneficiary + // balance's lazy update behaviour into `MemoryValue` for more use cases. + // TODO: Confirm that we're not missing anything, like bytecode. + let mut account_info = account.info.clone(); + if address == &self.block_env.coinbase { + account_info.balance += *gas_payment.borrow(); + explicitly_wrote_to_coinbase = true; + } writes.push(( MemoryLocation::Basic(*address), - MemoryValue::Basic(account.info.clone()), + MemoryValue::Basic(account_info), )); for (slot, value) in account.changed_storage_slots() { writes.push(( @@ -189,8 +295,21 @@ impl Vm { } writes }) - .collect(), - }, + .collect(); + + if !explicitly_wrote_to_coinbase { + write_set.push(( + MemoryLocation::Basic(self.block_env.coinbase), + MemoryValue::LazyBeneficiaryBalance(*gas_payment.borrow()), + )); + } + + VmExecutionResult::Ok { + result_and_state: result_and_state.clone(), + read_set: db.read_set, + write_set, + } + } Err(EVMError::Database(ReadError::BlockingIndex(blocking_tx_idx))) => { VmExecutionResult::ReadError { blocking_tx_idx } }