From 35ce996eb1399126cbfb31125fb97e274839968c Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Wed, 8 May 2024 13:49:20 +0200 Subject: [PATCH 01/14] Use the json format from the go code for ValidationInputs. This allows us to dump ValidationInput messages from a runing nitro node, or even a system test, and use them to drive the benchmarking code. --- arbitrator/Cargo.lock | 168 ++++++++++++++++++++++++- arbitrator/bench/Cargo.toml | 5 +- arbitrator/bench/src/bin.rs | 10 +- arbitrator/bench/src/parse_input.rs | 187 ++++++++-------------------- arbitrator/bench/src/prepare.rs | 18 ++- 5 files changed, 234 insertions(+), 154 deletions(-) diff --git a/arbitrator/Cargo.lock b/arbitrator/Cargo.lock index 7c05d1fcf4..312042cfb0 100644 --- a/arbitrator/Cargo.lock +++ b/arbitrator/Cargo.lock @@ -55,6 +55,21 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0942ffc6dcaadf03badf6e6a2d0228460359d5e34b57ccdc720b7382dfbd5ec5" +[[package]] +name = "android-tzdata" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e999941b234f3131b00bc13c22d06e8c5ff726d1b6318ac7eb276997bbb4fef0" + +[[package]] +name = "android_system_properties" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +dependencies = [ + "libc", +] + [[package]] name = "anes" version = "0.1.6" @@ -181,6 +196,12 @@ dependencies = [ "rustc-demangle", ] +[[package]] +name = "base64" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" + [[package]] name = "bench" version = "0.1.0" @@ -191,6 +212,9 @@ dependencies = [ "gperftools", "hex", "prover", + "serde", + "serde_json", + "serde_with 3.8.1", ] [[package]] @@ -375,6 +399,19 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "chrono" +version = "0.4.38" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a21f936df1771bf62b77f047b726c4625ff2e8aa607c01ec06e5a05bd8463401" +dependencies = [ + "android-tzdata", + "iana-time-zone", + "num-traits", + "serde", + "windows-targets 0.52.5", +] + [[package]] name = "ciborium" version = "0.2.2" @@ -469,6 +506,12 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6245d59a3e82a7fc217c5828a6692dbc6dfb63a0c8c90495621f7b9d79704a0e" +[[package]] +name = "core-foundation-sys" +version = "0.8.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06ea2b9bc92be3c2baa9334a323ebca2d6f074ff852cd1d7b11064035cd3868f" + [[package]] name = "corosensei" version = "0.1.4" @@ -704,6 +747,7 @@ dependencies = [ "ident_case", "proc-macro2", "quote", + "strsim 0.10.0", "syn 2.0.52", ] @@ -742,6 +786,16 @@ dependencies = [ "parking_lot_core", ] +[[package]] +name = "deranged" +version = "0.3.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b42b6fa04a440b495c8b04d0e71b707c585f83cb9cb28cf8cd0d976c315e31b4" +dependencies = [ + "powerfmt", + "serde", +] + [[package]] name = "derivative" version = "2.2.0" @@ -1050,6 +1104,32 @@ name = "hex" version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +dependencies = [ + "serde", +] + +[[package]] +name = "iana-time-zone" +version = "0.1.60" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7ffbb5a1b541ea2561f8c41c087286cc091e21e556a4f09a8f6cbf17b69b141" +dependencies = [ + "android_system_properties", + "core-foundation-sys", + "iana-time-zone-haiku", + "js-sys", + "wasm-bindgen", + "windows-core", +] + +[[package]] +name = "iana-time-zone-haiku" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f" +dependencies = [ + "cc", +] [[package]] name = "ident_case" @@ -1071,6 +1151,7 @@ checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" dependencies = [ "autocfg", "hashbrown 0.12.3", + "serde", ] [[package]] @@ -1081,6 +1162,7 @@ checksum = "7b0b929d511467233429c45a44ac1dcaa21ba0f5ba11e4879e6ed28ddb4f9df4" dependencies = [ "equivalent", "hashbrown 0.14.3", + "serde", ] [[package]] @@ -1376,6 +1458,12 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-conv" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51d515d32fb182ee37cda2ccdcb92950d6a3c2893aa280e540671c2cd0f3b1d9" + [[package]] name = "num-derive" version = "0.4.2" @@ -1558,6 +1646,12 @@ dependencies = [ "plotters-backend", ] +[[package]] +name = "powerfmt" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" + [[package]] name = "ppv-lite86" version = "0.2.17" @@ -1637,7 +1731,7 @@ dependencies = [ "rustc-demangle", "serde", "serde_json", - "serde_with", + "serde_with 1.14.0", "sha2 0.9.9", "sha3 0.9.1", "smallvec", @@ -1966,7 +2060,25 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "678b5a069e50bf00ecd22d0cd8ddf7c236f68581b03db652061ed5eb13a312ff" dependencies = [ "serde", - "serde_with_macros", + "serde_with_macros 1.5.2", +] + +[[package]] +name = "serde_with" +version = "3.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ad483d2ab0149d5a5ebcd9972a3852711e0153d863bf5a5d0391d28883c4a20" +dependencies = [ + "base64", + "chrono", + "hex", + "indexmap 1.9.3", + "indexmap 2.2.5", + "serde", + "serde_derive", + "serde_json", + "serde_with_macros 3.8.1", + "time", ] [[package]] @@ -1981,6 +2093,18 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "serde_with_macros" +version = "3.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "65569b702f41443e8bc8bbb1c5779bd0450bbe723b56198980e80ec45780bce2" +dependencies = [ + "darling 0.20.8", + "proc-macro2", + "quote", + "syn 2.0.52", +] + [[package]] name = "sha2" version = "0.9.9" @@ -2219,6 +2343,37 @@ dependencies = [ "num_cpus", ] +[[package]] +name = "time" +version = "0.3.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5dfd88e563464686c916c7e46e623e520ddc6d79fa6641390f2e3fa86e83e885" +dependencies = [ + "deranged", + "itoa", + "num-conv", + "powerfmt", + "serde", + "time-core", + "time-macros", +] + +[[package]] +name = "time-core" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" + +[[package]] +name = "time-macros" +version = "0.2.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f252a68540fde3a3877aeea552b832b40ab9a69e318efd078774a01ddee1ccf" +dependencies = [ + "num-conv", + "time-core", +] + [[package]] name = "tiny-keccak" version = "2.0.2" @@ -2682,6 +2837,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-core" +version = "0.52.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9" +dependencies = [ + "windows-targets 0.52.5", +] + [[package]] name = "windows-sys" version = "0.33.0" diff --git a/arbitrator/bench/Cargo.toml b/arbitrator/bench/Cargo.toml index 396988ed83..3ab5b99b08 100644 --- a/arbitrator/bench/Cargo.toml +++ b/arbitrator/bench/Cargo.toml @@ -12,12 +12,15 @@ name = "benchbin" path = "src/bin.rs" [dependencies] -hex = "0.4.3" +hex = { version = "0.4.3", features = ["serde"] } eyre = "0.6.5" prover = { path = "../prover" } arbutil = { path = "../arbutil" } clap = { version = "4.4.8", features = ["derive"] } gperftools = { version = "0.2.0", optional = true } +serde = { version = "1.0.130", features = ["derive", "rc"] } +serde_json = "1.0.67" +serde_with = { version = "3.8.1", features = ["base64"] } [features] counters = [] diff --git a/arbitrator/bench/src/bin.rs b/arbitrator/bench/src/bin.rs index 23930652c2..5fc7bae72f 100644 --- a/arbitrator/bench/src/bin.rs +++ b/arbitrator/bench/src/bin.rs @@ -33,7 +33,7 @@ struct Args { fn main() -> eyre::Result<()> { let args = Args::parse(); - let step_sizes = [1, 1 << 10, 1 << 15, 1 << 20, 1 << 24]; + let step_sizes = [1, 1 << 10, 1 << 15, 1 << 20, 1 << 24, 1 << 26, 1 << 28]; if args.always_merkleize { println!("Running benchmark with always merkleize feature on"); } else { @@ -82,14 +82,16 @@ fn main() -> eyre::Result<()> { bail!("Machine too far => position {}", machine.get_steps()) } MachineStatus::Running => {} - MachineStatus::Finished => return Ok(()), + MachineStatus::Finished => { + break; + } } let start = std::time::Instant::now(); let _ = machine.hash(); let hash_end_time = start.elapsed(); hash_times.push(hash_end_time); num_iters += 1; - if num_iters == 100 { + if num_iters == 200 { break; } } @@ -102,7 +104,7 @@ fn main() -> eyre::Result<()> { let total_end_time = total.elapsed(); println!( - "avg hash time {:>12?}, avg step time {:>12?}, step size {:>8}, num_iters {}, total time {:>12?}", + "avg hash time {:>11?}, avg step time {:>12?}, step size {:>9}, num_iters {:>3}, total time {:>12?}", average(&hash_times), average(&step_times), step_size, diff --git a/arbitrator/bench/src/parse_input.rs b/arbitrator/bench/src/parse_input.rs index 32f1c15ed3..f04150ea22 100644 --- a/arbitrator/bench/src/parse_input.rs +++ b/arbitrator/bench/src/parse_input.rs @@ -1,163 +1,76 @@ -use std::io::{self, BufRead}; +use arbutil::Bytes32; +use serde::{Deserialize, Serialize}; +use serde_json; +use serde_with::base64::Base64; +use serde_with::As; +use serde_with::DisplayFromStr; +use std::{ + collections::HashMap, + io::{self, BufRead}, +}; + +mod prefixed_hex { + use serde::{self, Deserialize, Deserializer, Serializer}; + + pub fn serialize(bytes: &Vec, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&format!("0x{}", hex::encode(bytes))) + } -#[derive(Debug, Clone)] -pub struct Preimage { - pub type_: u32, - pub hash: Vec, - pub data: Vec, + pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> + where + D: Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + if s.starts_with("0x") { + hex::decode(&s[2..]).map_err(serde::de::Error::custom) + } else { + Err(serde::de::Error::custom("missing 0x prefix")) + } + } } -#[derive(Debug, Clone)] -pub struct Item { - pub preimages: Vec, -} +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct PreimageMap(HashMap>); -#[derive(Debug)] +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(rename_all = "PascalCase")] pub struct BatchInfo { pub number: u64, - pub data: Vec, + #[serde(with = "As::")] + pub data_b64: Vec, } -#[derive(Debug)] +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "PascalCase")] pub struct StartState { + #[serde(with = "prefixed_hex")] pub block_hash: Vec, + #[serde(with = "prefixed_hex")] pub send_root: Vec, pub batch: u64, pub pos_in_batch: u64, } -#[derive(Debug)] +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "PascalCase")] pub struct FileData { pub id: u64, pub has_delayed_msg: bool, pub delayed_msg_nr: u64, - pub items: Vec, - pub batch_info: BatchInfo, - pub delayed_msg: Vec, + #[serde(with = "As::>>")] + pub preimages_b64: HashMap>>, + pub batch_info: Vec, + #[serde(with = "As::")] + pub delayed_msg_b64: Vec, pub start_state: StartState, } impl FileData { pub fn from_reader(mut reader: R) -> io::Result { - let mut items = Vec::new(); - let mut batch_info = BatchInfo { - number: 0, - data: Vec::new(), - }; - let mut id = 0; - let mut has_delayed_msg = false; - let mut delayed_msg_nr = 0; - let mut delayed_msg = Vec::new(); - let mut start_state = StartState { - block_hash: Vec::new(), - send_root: Vec::new(), - batch: 0, - pos_in_batch: 0, - }; - - let mut line = String::new(); - while reader.read_line(&mut line)? > 0 { - if line.starts_with("Id:") { - id = line.split(':').nth(1).unwrap().trim().parse().unwrap(); - } else if line.starts_with("HasDelayedMsg:") { - has_delayed_msg = line.split(':').nth(1).unwrap().trim().parse().unwrap(); - } else if line.starts_with("DelayedMsgNr:") { - delayed_msg_nr = line.split(':').nth(1).unwrap().trim().parse().unwrap(); - } else if line.starts_with("Preimages:") { - items.push(Item::from_reader(&mut reader, &mut line)?); - } else if line.starts_with("BatchInfo:") { - let parts: Vec<_> = line.split(',').collect(); - batch_info.number = parts[0].split(':').nth(2).unwrap().trim().parse().unwrap(); - batch_info.data = hex::decode(parts[1].split(':').nth(1).unwrap().trim()).unwrap(); - } else if line.starts_with("DelayedMsg:") { - delayed_msg = hex::decode(line.split(':').nth(1).unwrap().trim()).unwrap(); - } else if line.starts_with("StartState:") { - let parts: Vec<_> = line.split(',').collect(); - - // Parsing block_hash - let block_hash_str = parts[0].split("BlockHash:").nth(1).unwrap().trim(); - start_state.block_hash = - hex::decode(block_hash_str.strip_prefix("0x").unwrap()).unwrap(); - - // Parsing send_root - let send_root_str = parts[1].split(':').nth(1).unwrap().trim(); - start_state.send_root = - hex::decode(send_root_str.strip_prefix("0x").unwrap()).unwrap(); - - // Parsing batch - start_state.batch = parts[2] - .split(':') - .nth(1) - .unwrap() - .trim() - .parse::() - .unwrap(); - - // Parsing pos_in_batch - start_state.pos_in_batch = parts[3] - .split(':') - .nth(1) - .unwrap() - .trim() - .parse::() - .unwrap(); - } - line.clear(); - } - - Ok(FileData { - id, - has_delayed_msg, - delayed_msg_nr, - items, - batch_info, - delayed_msg, - start_state, - }) - } -} - -impl Item { - pub fn from_reader(reader: &mut R, line: &mut String) -> io::Result { - let mut preimages = Vec::new(); - - loop { - if line.is_empty() - || line.starts_with("BatchInfo:") - || line.starts_with("DelayedMsg:") - || line.starts_with("StartState:") - { - break; - } - if line.starts_with("Preimages:") { - line.clear(); - while reader.read_line(line)? > 0 && line.starts_with('\t') { - let parts: Vec<_> = line.trim().split(',').collect(); - let type_ = parts[0].split(':').nth(1).unwrap().trim().parse().unwrap(); - let hash = hex::decode( - parts[1] - .split(':') - .nth(1) - .unwrap() - .trim() - .strip_prefix("0x") - .unwrap(), - ) - .unwrap(); - let data = hex::decode(parts[2].split(':').nth(1).unwrap().trim()).unwrap(); - preimages.push(Preimage { type_, hash, data }); - line.clear(); - } - continue; // To skip line.clear() at the end of the loop for this case - } - - line.clear(); - if reader.read_line(line)? == 0 { - // If EOF is reached - break; - } - } - - Ok(Item { preimages }) + let data = serde_json::from_reader(&mut reader)?; + return Ok(data); } } diff --git a/arbitrator/bench/src/prepare.rs b/arbitrator/bench/src/prepare.rs index daf0f48f8f..3cc884db6d 100644 --- a/arbitrator/bench/src/prepare.rs +++ b/arbitrator/bench/src/prepare.rs @@ -18,15 +18,11 @@ pub fn prepare_machine( let reader = BufReader::new(file); let data = FileData::from_reader(reader)?; - let item = data.items.first().unwrap().clone(); - let preimages = item.preimages; - let preimages = preimages + let preimages = data + .preimages_b64 .into_iter() - .map(|preimage| { - let hash: [u8; 32] = preimage.hash.try_into().unwrap(); - let hash: Bytes32 = hash.into(); - (hash, preimage.data) - }) + .map(|preimage| preimage.1.into_iter().map(|(k, v)| (k, v))) + .flatten() .collect::>>(); let preimage_resolver = move |_: u64, _: PreimageType, hash: Bytes32| -> Option { preimages @@ -59,11 +55,13 @@ pub fn prepare_machine( // println!("Adding sequencer inbox message"); let identifier = argument_data_to_inbox(0).unwrap(); - mach.add_inbox_msg(identifier, data.batch_info.number, data.batch_info.data); + for batch_info in data.batch_info.iter() { + mach.add_inbox_msg(identifier, batch_info.number, batch_info.data_b64.clone()); + } // println!("Adding delayed inbox message"); let identifier = argument_data_to_inbox(1).unwrap(); - mach.add_inbox_msg(identifier, data.delayed_msg_nr, data.delayed_msg); + mach.add_inbox_msg(identifier, data.delayed_msg_nr, data.delayed_msg_b64); Ok(mach) } From 9453b4024108b88a6b680fee27707ba48fe8f5ca Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Fri, 10 May 2024 16:41:52 +0200 Subject: [PATCH 02/14] Add more counters. We need to be able to see how frequently the memory is being hashed and from where. The problem is that with always_merkleize off, there are thousands of fewer calls to `Merkle::root()` and this is to help us figure out where the calls are coming from. Spoiler alert: It's from the flush_module macro. --- arbitrator/bench/src/bin.rs | 16 ++++++++++++--- arbitrator/prover/src/lib.rs | 2 +- arbitrator/prover/src/machine.rs | 34 ++++++++++++++++++++++++++++++++ arbitrator/prover/src/memory.rs | 22 +++++++++++++++++++++ arbitrator/prover/src/merkle.rs | 23 +++++++++++++++++++-- 5 files changed, 91 insertions(+), 6 deletions(-) diff --git a/arbitrator/bench/src/bin.rs b/arbitrator/bench/src/bin.rs index 5fc7bae72f..185f7e2642 100644 --- a/arbitrator/bench/src/bin.rs +++ b/arbitrator/bench/src/bin.rs @@ -13,7 +13,7 @@ use gperftools::heap_profiler::HEAP_PROFILER; use prover::machine::MachineStatus; #[cfg(feature = "counters")] -use prover::merkle; +use prover::{machine, memory, merkle}; #[derive(Parser, Debug)] #[command(author, version, about, long_about = None)] @@ -34,6 +34,8 @@ struct Args { fn main() -> eyre::Result<()> { let args = Args::parse(); let step_sizes = [1, 1 << 10, 1 << 15, 1 << 20, 1 << 24, 1 << 26, 1 << 28]; + // let step_sizes = [1, 1 << 10, 1 << 15, 1 << 20, 1 << 24]; + if args.always_merkleize { println!("Running benchmark with always merkleize feature on"); } else { @@ -65,7 +67,11 @@ fn main() -> eyre::Result<()> { .unwrap(); #[cfg(feature = "counters")] - merkle::reset_counters(); + { + machine::reset_counters(); + memory::reset_counters(); + merkle::reset_counters(); + } let total = std::time::Instant::now(); loop { let start = std::time::Instant::now(); @@ -112,7 +118,11 @@ fn main() -> eyre::Result<()> { total_end_time, ); #[cfg(feature = "counters")] - merkle::print_counters(); + { + machine::print_counters(); + memory::print_counters(); + merkle::print_counters(); + } } Ok(()) } diff --git a/arbitrator/prover/src/lib.rs b/arbitrator/prover/src/lib.rs index 4895f0cba7..1e3ecf4cd6 100644 --- a/arbitrator/prover/src/lib.rs +++ b/arbitrator/prover/src/lib.rs @@ -9,7 +9,7 @@ mod host; mod kzg; pub mod machine; /// cbindgen:ignore -mod memory; +pub mod memory; pub mod merkle; mod print; pub mod programs; diff --git a/arbitrator/prover/src/machine.rs b/arbitrator/prover/src/machine.rs index 6b0e1df3e1..fbbacd9fb6 100644 --- a/arbitrator/prover/src/machine.rs +++ b/arbitrator/prover/src/machine.rs @@ -32,6 +32,10 @@ use serde::{Deserialize, Serialize}; use serde_with::serde_as; use sha3::Keccak256; use smallvec::SmallVec; + +#[cfg(feature = "counters")] +use std::sync::atomic::{AtomicUsize, Ordering}; + use std::{ borrow::Cow, convert::{TryFrom, TryInto}, @@ -44,12 +48,37 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; + use wasmer_types::FunctionIndex; use wasmparser::{DataKind, ElementItems, ElementKind, Operator, RefType, TableType}; #[cfg(feature = "rayon")] use rayon::prelude::*; +#[cfg(feature = "counters")] +static GET_MODULES_MERKLE_COUNTER: AtomicUsize = AtomicUsize::new(0); + +#[cfg(feature = "counters")] +static FLUSH_MODULE_COUNTER: AtomicUsize = AtomicUsize::new(0); + +#[cfg(feature = "counters")] +pub fn print_counters() { + println!( + "GET_MODULES_MERKLE_COUNTER: {}", + GET_MODULES_MERKLE_COUNTER.load(Ordering::Relaxed) + ); + println!( + "FLUSH_MODULE_COUNTER: {}", + FLUSH_MODULE_COUNTER.load(Ordering::Relaxed) + ); +} + +#[cfg(feature = "counters")] +pub fn reset_counters() { + GET_MODULES_MERKLE_COUNTER.store(0, Ordering::Relaxed); + FLUSH_MODULE_COUNTER.store(0, Ordering::Relaxed); +} + fn hash_call_indirect_data(table: u32, ty: &FunctionType) -> Bytes32 { let mut h = Keccak256::new(); h.update("Call indirect:"); @@ -1918,6 +1947,8 @@ impl Machine { } macro_rules! flush_module { () => { + #[cfg(feature = "counters")] + FLUSH_MODULE_COUNTER.fetch_add(1, Ordering::Relaxed); if let Some(merkle) = self.modules_merkle.as_mut() { merkle.set(self.pc.module(), module.hash()); } @@ -2696,6 +2727,9 @@ impl Machine { } fn get_modules_merkle(&self) -> Cow { + #[cfg(feature = "counters")] + GET_MODULES_MERKLE_COUNTER.fetch_add(1, Ordering::Relaxed); + if let Some(merkle) = &self.modules_merkle { Cow::Borrowed(merkle) } else { diff --git a/arbitrator/prover/src/memory.rs b/arbitrator/prover/src/memory.rs index bba8e4124f..0c49a1b54b 100644 --- a/arbitrator/prover/src/memory.rs +++ b/arbitrator/prover/src/memory.rs @@ -11,11 +11,31 @@ use eyre::{bail, ErrReport, Result}; use serde::{Deserialize, Serialize}; use sha3::Keccak256; use std::{borrow::Cow, convert::TryFrom}; + +#[cfg(feature = "counters")] +use std::sync::atomic::{AtomicUsize, Ordering}; + use wasmer_types::Pages; #[cfg(feature = "rayon")] use rayon::prelude::*; +#[cfg(feature = "counters")] +static MEM_HASH_COUNTER: AtomicUsize = AtomicUsize::new(0); + +#[cfg(feature = "counters")] +pub fn reset_counters() { + MEM_HASH_COUNTER.store(0, Ordering::Relaxed); +} + +#[cfg(feature = "counters")] +pub fn print_counters() { + println!( + "Memory hash count: {}", + MEM_HASH_COUNTER.load(Ordering::Relaxed) + ); +} + pub struct MemoryType { pub min: Pages, pub max: Option, @@ -138,6 +158,8 @@ impl Memory { } pub fn hash(&self) -> Bytes32 { + #[cfg(feature = "counters")] + MEM_HASH_COUNTER.fetch_add(1, Ordering::Relaxed); let mut h = Keccak256::new(); h.update("Memory:"); h.update((self.buffer.len() as u64).to_be_bytes()); diff --git a/arbitrator/prover/src/merkle.rs b/arbitrator/prover/src/merkle.rs index 5a3cbadbc7..82c03e5d66 100644 --- a/arbitrator/prover/src/merkle.rs +++ b/arbitrator/prover/src/merkle.rs @@ -87,6 +87,21 @@ lazy_static! { map }; } +#[cfg(feature = "counters")] +lazy_static! { + static ref RESIZE_COUNTERS: HashMap<&'static MerkleType, AtomicUsize> = { + let mut map = HashMap::new(); + map.insert(&MerkleType::Empty, AtomicUsize::new(0)); + map.insert(&MerkleType::Value, AtomicUsize::new(0)); + map.insert(&MerkleType::Function, AtomicUsize::new(0)); + map.insert(&MerkleType::Instruction, AtomicUsize::new(0)); + map.insert(&MerkleType::Memory, AtomicUsize::new(0)); + map.insert(&MerkleType::Table, AtomicUsize::new(0)); + map.insert(&MerkleType::TableElement, AtomicUsize::new(0)); + map.insert(&MerkleType::Module, AtomicUsize::new(0)); + map + }; +} #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, Serialize, Deserialize, Sequence)] pub enum MerkleType { @@ -113,11 +128,12 @@ pub fn print_counters() { continue; } println!( - "{} New: {}, Root: {}, Set: {}", + "{} New: {}, Root: {}, Set: {} Resize: {}", ty.get_prefix(), NEW_COUNTERS[&ty].load(Ordering::Relaxed), ROOT_COUNTERS[&ty].load(Ordering::Relaxed), - SET_COUNTERS[&ty].load(Ordering::Relaxed) + SET_COUNTERS[&ty].load(Ordering::Relaxed), + RESIZE_COUNTERS[&ty].load(Ordering::Relaxed), ); } } @@ -131,6 +147,7 @@ pub fn reset_counters() { NEW_COUNTERS[&ty].store(0, Ordering::Relaxed); ROOT_COUNTERS[&ty].store(0, Ordering::Relaxed); SET_COUNTERS[&ty].store(0, Ordering::Relaxed); + RESIZE_COUNTERS[&ty].store(0, Ordering::Relaxed); } } @@ -382,6 +399,8 @@ impl Merkle { /// /// The extra space is filled with empty hashes. pub fn resize(&self, new_len: usize) -> Result { + #[cfg(feature = "counters")] + RESIZE_COUNTERS[&self.ty].fetch_add(1, Ordering::Relaxed); if new_len > self.capacity() { return Err( "Cannot resize to a length greater than the capacity of the tree.".to_owned(), From e8c750adc5fa5ecc39a8efbeb1a3eea60382ccf5 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Fri, 10 May 2024 17:02:09 +0200 Subject: [PATCH 03/14] Remove flush_module macro. The high-level benefit of this change is a 16.5% reduction in block processing time from ~17.6s to ~14.7s. The only job of that macro was to recalculate the hash of the module and update the module merkle tree's corresponding leaf with the new hash. Instead, the waits until the hash for the module is about to be calculated and calls `Merkle::set()` for each module in the modules vector. This eliminates thousands of calls to the possibly expensive `Merkle::root()` call on the `Memory` of each module. This pattern of delaying when to actually update the leaves of merkle trees until they will be used has a lot of potential for additional performance boosts. --- arbitrator/prover/src/machine.rs | 34 +++----------------------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/arbitrator/prover/src/machine.rs b/arbitrator/prover/src/machine.rs index fbbacd9fb6..5c4471a9fd 100644 --- a/arbitrator/prover/src/machine.rs +++ b/arbitrator/prover/src/machine.rs @@ -58,25 +58,17 @@ use rayon::prelude::*; #[cfg(feature = "counters")] static GET_MODULES_MERKLE_COUNTER: AtomicUsize = AtomicUsize::new(0); -#[cfg(feature = "counters")] -static FLUSH_MODULE_COUNTER: AtomicUsize = AtomicUsize::new(0); - #[cfg(feature = "counters")] pub fn print_counters() { println!( "GET_MODULES_MERKLE_COUNTER: {}", GET_MODULES_MERKLE_COUNTER.load(Ordering::Relaxed) ); - println!( - "FLUSH_MODULE_COUNTER: {}", - FLUSH_MODULE_COUNTER.load(Ordering::Relaxed) - ); } #[cfg(feature = "counters")] pub fn reset_counters() { GET_MODULES_MERKLE_COUNTER.store(0, Ordering::Relaxed); - FLUSH_MODULE_COUNTER.store(0, Ordering::Relaxed); } fn hash_call_indirect_data(table: u32, ty: &FunctionType) -> Bytes32 { @@ -1945,22 +1937,11 @@ impl Machine { func = &module.funcs[self.pc.func()]; }; } - macro_rules! flush_module { - () => { - #[cfg(feature = "counters")] - FLUSH_MODULE_COUNTER.fetch_add(1, Ordering::Relaxed); - if let Some(merkle) = self.modules_merkle.as_mut() { - merkle.set(self.pc.module(), module.hash()); - } - }; - } macro_rules! error { () => { error!("") }; ($format:expr $(, $message:expr)*) => {{ - flush_module!(); - if self.debug_info { println!("\n{} {}", "error on line".grey(), line!().pink()); println!($format, $($message.pink()),*); @@ -1979,7 +1960,6 @@ impl Machine { continue; } self.status = MachineStatus::Errored; - module = &mut self.modules[self.pc.module()]; break; }}; } @@ -1990,7 +1970,6 @@ impl Machine { println!("\n{}", "Machine out of steps".red()); self.status = MachineStatus::Errored; self.print_backtrace(true); - module = &mut self.modules[self.pc.module()]; break; } @@ -2050,9 +2029,6 @@ impl Machine { Value::RefNull => error!(), Value::InternalRef(pc) => { let changing_module = pc.module != self.pc.module; - if changing_module { - flush_module!(); - } self.pc = pc; if changing_module { module = &mut self.modules[self.pc.module()]; @@ -2072,7 +2048,6 @@ impl Machine { func = &module.funcs[self.pc.func()]; } Opcode::CrossModuleCall => { - flush_module!(); value_stack.push(Value::InternalRef(self.pc)); value_stack.push(self.pc.module.into()); value_stack.push(module.internals_offset.into()); @@ -2083,7 +2058,6 @@ impl Machine { reset_refs!(); } Opcode::CrossModuleForward => { - flush_module!(); let frame = frame_stack.last().unwrap(); value_stack.push(Value::InternalRef(self.pc)); value_stack.push(frame.caller_module.into()); @@ -2095,7 +2069,6 @@ impl Machine { reset_refs!(); } Opcode::CrossModuleInternalCall => { - flush_module!(); let call_internal = inst.argument_data as u32; let call_module = value_stack.pop().unwrap().assume_u32(); value_stack.push(Value::InternalRef(self.pc)); @@ -2118,7 +2091,6 @@ impl Machine { .ok() .and_then(|o| current_frame.caller_module_internals.checked_add(o)) .expect("Internal call function index overflow"); - flush_module!(); self.pc.module = current_frame.caller_module; self.pc.func = func_idx; self.pc.inst = 0; @@ -2550,7 +2522,6 @@ impl Machine { let dots = (modules.len() > 16).then_some("...").unwrap_or_default(); bail!("no program for {hash} in {{{}{dots}}}", keys.join(", ")) }; - flush_module!(); // put the new module's offset on the stack let index = self.modules.len() as u32; @@ -2563,7 +2534,6 @@ impl Machine { reset_refs!(); } Opcode::UnlinkModule => { - flush_module!(); self.modules.pop(); if let Some(cached) = &mut self.modules_merkle { cached.pop_leaf(); @@ -2603,7 +2573,6 @@ impl Machine { } } } - flush_module!(); if self.is_halted() && !self.stdio_output.is_empty() { // If we halted, print out any trailing output that didn't have a newline. Self::say(String::from_utf8_lossy(&self.stdio_output)); @@ -2731,6 +2700,9 @@ impl Machine { GET_MODULES_MERKLE_COUNTER.fetch_add(1, Ordering::Relaxed); if let Some(merkle) = &self.modules_merkle { + for (i, module) in self.modules.iter().enumerate() { + merkle.set(i, module.hash()); + } Cow::Borrowed(merkle) } else { Cow::Owned(Merkle::new( From 6ba0a693f58289c5acf9ff9c4bcaf604c7cb2d93 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Mon, 13 May 2024 14:27:03 +0200 Subject: [PATCH 04/14] Speed up block validation by another 27%. This change just marks memory as dirty when storing new values rather than actually calculating the new hash for the data in memory and then calculates the hashes for dirty indices just before creating the merkle tree. On one test block, this ended up saving 23 million calls to keccack. --- arbitrator/bench/src/bin.rs | 1 - arbitrator/prover/src/memory.rs | 50 ++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/arbitrator/bench/src/bin.rs b/arbitrator/bench/src/bin.rs index 185f7e2642..3b527fb8dd 100644 --- a/arbitrator/bench/src/bin.rs +++ b/arbitrator/bench/src/bin.rs @@ -34,7 +34,6 @@ struct Args { fn main() -> eyre::Result<()> { let args = Args::parse(); let step_sizes = [1, 1 << 10, 1 << 15, 1 << 20, 1 << 24, 1 << 26, 1 << 28]; - // let step_sizes = [1, 1 << 10, 1 << 15, 1 << 20, 1 << 24]; if args.always_merkleize { println!("Running benchmark with always merkleize feature on"); diff --git a/arbitrator/prover/src/memory.rs b/arbitrator/prover/src/memory.rs index 0c49a1b54b..78b5ba7259 100644 --- a/arbitrator/prover/src/memory.rs +++ b/arbitrator/prover/src/memory.rs @@ -10,7 +10,13 @@ use digest::Digest; use eyre::{bail, ErrReport, Result}; use serde::{Deserialize, Serialize}; use sha3::Keccak256; -use std::{borrow::Cow, convert::TryFrom}; +use std::{ + borrow::Cow, + collections::HashSet, + convert::TryFrom, + ops::Deref, + sync::{Arc, Mutex}, +}; #[cfg(feature = "counters")] use std::sync::atomic::{AtomicUsize, Ordering}; @@ -64,12 +70,14 @@ impl TryFrom<&wasmparser::MemoryType> for MemoryType { } } -#[derive(PartialEq, Eq, Clone, Debug, Default, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct Memory { buffer: Vec, #[serde(skip)] pub merkle: Option, pub max_size: u64, + #[serde(skip)] + dirty_indices: Arc>>, } fn hash_leaf(bytes: [u8; Memory::LEAF_SIZE]) -> Bytes32 { @@ -98,6 +106,16 @@ fn div_round_up(num: usize, denom: usize) -> usize { res } +impl PartialEq for Memory { + fn eq(&self, other: &Memory) -> bool { + self.buffer == other.buffer + && self.merkle == other.merkle + && self.max_size == other.max_size + && self.dirty_indices.lock().unwrap().deref() + == other.dirty_indices.lock().unwrap().deref() + } +} + impl Memory { pub const LEAF_SIZE: usize = 32; /// Only used when initializing a memory to determine its size @@ -111,6 +129,7 @@ impl Memory { buffer: vec![0u8; size], merkle: None, max_size, + dirty_indices: Arc::new(Mutex::new(HashSet::new())), } } @@ -120,6 +139,11 @@ impl Memory { pub fn merkelize(&self) -> Cow<'_, Merkle> { if let Some(m) = &self.merkle { + for idx in self.dirty_indices.lock().unwrap().iter() { + let leaf_idx = idx / Self::LEAF_SIZE; + m.set(leaf_idx, hash_leaf(self.get_leaf_data(leaf_idx))); + } + self.dirty_indices.lock().unwrap().clear(); return Cow::Borrowed(m); } // Round the size up to 8 byte long leaves, then round up to the next power of two number of leaves @@ -143,6 +167,7 @@ impl Memory { if size < leaves { m.resize(leaves).expect("Couldn't resize merkle tree"); } + self.dirty_indices.lock().unwrap().clear(); Cow::Owned(m) } @@ -251,16 +276,8 @@ impl Memory { let end_idx = end_idx as usize; let buf = value.to_le_bytes(); self.buffer[idx..end_idx].copy_from_slice(&buf[..bytes.into()]); - - if let Some(merkle) = self.merkle.take() { - let start_leaf = idx / Self::LEAF_SIZE; - merkle.set(start_leaf, hash_leaf(self.get_leaf_data(start_leaf))); - let end_leaf = (end_idx - 1) / Self::LEAF_SIZE; - if end_leaf != start_leaf { - merkle.set(end_leaf, hash_leaf(self.get_leaf_data(end_leaf))); - } - self.merkle = Some(merkle); - } + self.dirty_indices.lock().unwrap().insert(idx); + self.dirty_indices.lock().unwrap().insert(end_idx - 1); true } @@ -279,14 +296,7 @@ impl Memory { let idx = idx as usize; let end_idx = end_idx as usize; self.buffer[idx..end_idx].copy_from_slice(value); - - if let Some(merkle) = self.merkle.take() { - let start_leaf = idx / Self::LEAF_SIZE; - merkle.set(start_leaf, hash_leaf(self.get_leaf_data(start_leaf))); - // No need for second merkle - assert!(value.len() <= Self::LEAF_SIZE); - self.merkle = Some(merkle); - } + self.dirty_indices.lock().unwrap().insert(idx); true } From d247ebdca167fe6208325197f13c47de3575b88e Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Mon, 13 May 2024 15:23:22 +0200 Subject: [PATCH 05/14] Enable always_merkleize everywhere. This is to prove that the code is still fast enough to pass the test-go suite of tests if the `always_merkleize` feature is enabled. --- arbitrator/prover/src/lib.rs | 4 ++-- arbitrator/prover/src/machine.rs | 2 +- arbitrator/stylus/src/test/mod.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arbitrator/prover/src/lib.rs b/arbitrator/prover/src/lib.rs index 1e3ecf4cd6..b9dc032f40 100644 --- a/arbitrator/prover/src/lib.rs +++ b/arbitrator/prover/src/lib.rs @@ -101,7 +101,7 @@ unsafe fn arbitrator_load_machine_impl( &libraries, binary_path, true, - false, + true, false, debug_chain, debug_chain, @@ -117,7 +117,7 @@ unsafe fn arbitrator_load_machine_impl( pub unsafe extern "C" fn arbitrator_load_wavm_binary(binary_path: *const c_char) -> *mut Machine { let binary_path = cstr_to_string(binary_path); let binary_path = Path::new(&binary_path); - match Machine::new_from_wavm(binary_path, false) { + match Machine::new_from_wavm(binary_path, true) { Ok(mach) => Box::into_raw(Box::new(mach)), Err(err) => { eprintln!("Error loading binary: {err}"); diff --git a/arbitrator/prover/src/machine.rs b/arbitrator/prover/src/machine.rs index 5c4471a9fd..5aa7954653 100644 --- a/arbitrator/prover/src/machine.rs +++ b/arbitrator/prover/src/machine.rs @@ -1276,7 +1276,7 @@ impl Machine { &[soft_float, wasi_stub, user_test], bin, false, - false, + true, false, compile.debug.debug_funcs, true, diff --git a/arbitrator/stylus/src/test/mod.rs b/arbitrator/stylus/src/test/mod.rs index d7f3248d31..2f8819df86 100644 --- a/arbitrator/stylus/src/test/mod.rs +++ b/arbitrator/stylus/src/test/mod.rs @@ -150,7 +150,7 @@ fn new_test_machine(path: &str, compile: &CompileConfig) -> Result { &[lib], bin, false, - false, + true, true, compile.debug.debug_funcs, true, From 701327647c1a17b21e6c59ae297e5b9227a79b90 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Mon, 13 May 2024 16:24:03 +0200 Subject: [PATCH 06/14] Make always_merkleize the only strategy. This is now fast enough to just use it as the only implementation and simplify the code a bit. --- arbitrator/bench/src/bin.rs | 16 ++----------- arbitrator/bench/src/prepare.rs | 8 ++----- arbitrator/prover/src/lib.rs | 3 +-- arbitrator/prover/src/machine.rs | 37 +++++++++---------------------- arbitrator/prover/src/main.rs | 3 --- arbitrator/stylus/src/test/mod.rs | 1 - 6 files changed, 16 insertions(+), 52 deletions(-) diff --git a/arbitrator/bench/src/bin.rs b/arbitrator/bench/src/bin.rs index 3b527fb8dd..5c71448991 100644 --- a/arbitrator/bench/src/bin.rs +++ b/arbitrator/bench/src/bin.rs @@ -25,27 +25,15 @@ struct Args { /// Path to a machine.wavm.br #[arg(short, long)] machine_path: PathBuf, - - /// Should the memory tree always Merkleize - #[arg(short, long)] - always_merkleize: bool, } fn main() -> eyre::Result<()> { let args = Args::parse(); let step_sizes = [1, 1 << 10, 1 << 15, 1 << 20, 1 << 24, 1 << 26, 1 << 28]; - if args.always_merkleize { - println!("Running benchmark with always merkleize feature on"); - } else { - println!("Running benchmark with always merkleize feature off"); - } + println!("Running benchmark with always merkleize feature on"); for step_size in step_sizes { - let mut machine = prepare_machine( - args.preimages_path.clone(), - args.machine_path.clone(), - args.always_merkleize, - )?; + let mut machine = prepare_machine(args.preimages_path.clone(), args.machine_path.clone())?; let _ = machine.hash(); let mut hash_times = vec![]; let mut step_times = vec![]; diff --git a/arbitrator/bench/src/prepare.rs b/arbitrator/bench/src/prepare.rs index 3cc884db6d..aebc7ae895 100644 --- a/arbitrator/bench/src/prepare.rs +++ b/arbitrator/bench/src/prepare.rs @@ -9,11 +9,7 @@ use std::sync::Arc; use crate::parse_input::*; -pub fn prepare_machine( - preimages: PathBuf, - machines: PathBuf, - always_merkleize: bool, -) -> eyre::Result { +pub fn prepare_machine(preimages: PathBuf, machines: PathBuf) -> eyre::Result { let file = File::open(preimages)?; let reader = BufReader::new(file); @@ -33,7 +29,7 @@ pub fn prepare_machine( let binary_path = Path::new(&machines); // println!("Creating machine from binary_path"); - let mut mach = Machine::new_from_wavm(binary_path, always_merkleize)?; + let mut mach = Machine::new_from_wavm(binary_path)?; let block_hash: [u8; 32] = data.start_state.block_hash.try_into().unwrap(); let block_hash: Bytes32 = block_hash.into(); diff --git a/arbitrator/prover/src/lib.rs b/arbitrator/prover/src/lib.rs index b9dc032f40..0f537478eb 100644 --- a/arbitrator/prover/src/lib.rs +++ b/arbitrator/prover/src/lib.rs @@ -102,7 +102,6 @@ unsafe fn arbitrator_load_machine_impl( binary_path, true, true, - false, debug_chain, debug_chain, Default::default(), @@ -117,7 +116,7 @@ unsafe fn arbitrator_load_machine_impl( pub unsafe extern "C" fn arbitrator_load_wavm_binary(binary_path: *const c_char) -> *mut Machine { let binary_path = cstr_to_string(binary_path); let binary_path = Path::new(&binary_path); - match Machine::new_from_wavm(binary_path, true) { + match Machine::new_from_wavm(binary_path) { Ok(mach) => Box::into_raw(Box::new(mach)), Err(err) => { eprintln!("Error loading binary: {err}"); diff --git a/arbitrator/prover/src/machine.rs b/arbitrator/prover/src/machine.rs index 5aa7954653..c7550e642a 100644 --- a/arbitrator/prover/src/machine.rs +++ b/arbitrator/prover/src/machine.rs @@ -1221,7 +1221,6 @@ impl Machine { library_paths: &[PathBuf], binary_path: &Path, language_support: bool, - always_merkleize: bool, allow_hostapi_from_main: bool, debug_funcs: bool, debug_info: bool, @@ -1246,7 +1245,6 @@ impl Machine { &libraries, bin, language_support, - always_merkleize, allow_hostapi_from_main, debug_funcs, debug_info, @@ -1277,7 +1275,6 @@ impl Machine { bin, false, true, - false, compile.debug.debug_funcs, true, GlobalState::default(), @@ -1324,7 +1321,6 @@ impl Machine { libraries: &[WasmBinary<'_>], bin: WasmBinary<'_>, runtime_support: bool, - always_merkleize: bool, allow_hostapi_from_main: bool, debug_funcs: bool, debug_info: bool, @@ -1527,18 +1523,12 @@ impl Machine { let tables_hashes: Result<_, _> = module.tables.iter().map(Table::hash).collect(); module.tables_merkle = Merkle::new(MerkleType::Table, tables_hashes?); - - if always_merkleize { - module.memory.cache_merkle_tree(); - } - } - let mut modules_merkle = None; - if always_merkleize { - modules_merkle = Some(Merkle::new( - MerkleType::Module, - modules.iter().map(Module::hash).collect(), - )); + module.memory.cache_merkle_tree(); } + let modules_merkle = Some(Merkle::new( + MerkleType::Module, + modules.iter().map(Module::hash).collect(), + )); // find the first inbox index that's out of bounds let first_too_far = inbox_contents @@ -1572,7 +1562,7 @@ impl Machine { Ok(mach) } - pub fn new_from_wavm(wavm_binary: &Path, always_merkleize: bool) -> Result { + pub fn new_from_wavm(wavm_binary: &Path) -> Result { let mut modules: Vec = { let compressed = std::fs::read(wavm_binary)?; let Ok(modules) = brotli::decompress(&compressed, Dictionary::Empty) else { @@ -1598,17 +1588,12 @@ impl Machine { MerkleType::Function, module.funcs.iter().map(Function::hash).collect(), )); - if always_merkleize { - module.memory.cache_merkle_tree(); - } - } - let mut modules_merkle: Option = None; - if always_merkleize { - modules_merkle = Some(Merkle::new( - MerkleType::Module, - modules.iter().map(Module::hash).collect(), - )); + module.memory.cache_merkle_tree(); } + let modules_merkle = Some(Merkle::new( + MerkleType::Module, + modules.iter().map(Module::hash).collect(), + )); let mut mach = Machine { status: MachineStatus::Running, thread_state: ThreadState::Main, diff --git a/arbitrator/prover/src/main.rs b/arbitrator/prover/src/main.rs index 697d178fc7..01001b0794 100644 --- a/arbitrator/prover/src/main.rs +++ b/arbitrator/prover/src/main.rs @@ -35,8 +35,6 @@ struct Opts { #[structopt(long)] inbox_add_stub_headers: bool, #[structopt(long)] - always_merkleize: bool, - #[structopt(long)] debug_funcs: bool, #[structopt(long)] /// print modules to the console @@ -192,7 +190,6 @@ fn main() -> Result<()> { &opts.libraries, &opts.binary, true, - opts.always_merkleize, opts.allow_hostapi, opts.debug_funcs, true, diff --git a/arbitrator/stylus/src/test/mod.rs b/arbitrator/stylus/src/test/mod.rs index 2f8819df86..236e5639e6 100644 --- a/arbitrator/stylus/src/test/mod.rs +++ b/arbitrator/stylus/src/test/mod.rs @@ -151,7 +151,6 @@ fn new_test_machine(path: &str, compile: &CompileConfig) -> Result { bin, false, true, - true, compile.debug.debug_funcs, true, GlobalState::default(), From ccc1ee02b3e3bbc010790e4a2a156c9dbb0fed4c Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Fri, 17 May 2024 18:54:38 +0200 Subject: [PATCH 07/14] Regenerate the Cargo.lock --- arbitrator/Cargo.lock | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/arbitrator/Cargo.lock b/arbitrator/Cargo.lock index 5b82f10e8e..af9a89b149 100644 --- a/arbitrator/Cargo.lock +++ b/arbitrator/Cargo.lock @@ -55,6 +55,21 @@ version = "0.2.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5c6cb57a04249c6480766f7f7cef5467412af1490f8d1e243141daddada3264f" +[[package]] +name = "android-tzdata" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e999941b234f3131b00bc13c22d06e8c5ff726d1b6318ac7eb276997bbb4fef0" + +[[package]] +name = "android_system_properties" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +dependencies = [ + "libc", +] + [[package]] name = "anes" version = "0.1.6" @@ -396,7 +411,7 @@ dependencies = [ "iana-time-zone", "num-traits", "serde", - "windows-targets 0.52.5", + "windows-targets", ] [[package]] @@ -734,6 +749,7 @@ dependencies = [ "ident_case", "proc-macro2", "quote", + "strsim 0.11.1", "syn 2.0.64", ] @@ -1148,6 +1164,7 @@ checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26" dependencies = [ "equivalent", "hashbrown 0.14.5", + "serde", ] [[package]] @@ -1644,6 +1661,12 @@ dependencies = [ "plotters-backend", ] +[[package]] +name = "powerfmt" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" + [[package]] name = "ppv-lite86" version = "0.2.17" @@ -2065,7 +2088,7 @@ dependencies = [ "chrono", "hex", "indexmap 1.9.3", - "indexmap 2.2.5", + "indexmap 2.2.6", "serde", "serde_derive", "serde_json", @@ -2091,10 +2114,10 @@ version = "3.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "65569b702f41443e8bc8bbb1c5779bd0450bbe723b56198980e80ec45780bce2" dependencies = [ - "darling 0.20.8", + "darling 0.20.9", "proc-macro2", "quote", - "syn 2.0.52", + "syn 2.0.64", ] [[package]] @@ -2835,7 +2858,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9" dependencies = [ - "windows-targets 0.52.5", + "windows-targets", ] [[package]] From d2398d85d7fbee5897de260ed76308ab48c3f898 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Tue, 21 May 2024 13:33:32 +0200 Subject: [PATCH 08/14] Remove --always-merkleize The prover no longer runs in any other mode. --- Makefile | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 53b89c8d72..223fc08593 100644 --- a/Makefile +++ b/Makefile @@ -427,10 +427,10 @@ $(stylus_test_erc20_wasm): $(stylus_test_erc20_src) @touch -c $@ # cargo might decide to not rebuild the binary contracts/test/prover/proofs/float%.json: $(arbitrator_cases)/float%.wasm $(prover_bin) $(output_latest)/soft-float.wasm - $(prover_bin) $< -l $(output_latest)/soft-float.wasm -o $@ -b --allow-hostapi --require-success --always-merkleize + $(prover_bin) $< -l $(output_latest)/soft-float.wasm -o $@ -b --allow-hostapi --require-success contracts/test/prover/proofs/no-stack-pollution.json: $(arbitrator_cases)/no-stack-pollution.wasm $(prover_bin) - $(prover_bin) $< -o $@ --allow-hostapi --require-success --always-merkleize + $(prover_bin) $< -o $@ --allow-hostapi --require-success target/testdata/preimages.bin: mkdir -p `dirname $@` @@ -454,19 +454,19 @@ contracts/test/prover/proofs/global-state.json: echo "[]" > $@ contracts/test/prover/proofs/forward-test.json: $(arbitrator_cases)/forward-test.wasm $(arbitrator_tests_forward_deps) $(prover_bin) - $(prover_bin) $< -o $@ --allow-hostapi --always-merkleize $(patsubst %,-l %, $(arbitrator_tests_forward_deps)) + $(prover_bin) $< -o $@ --allow-hostapi $(patsubst %,-l %, $(arbitrator_tests_forward_deps)) contracts/test/prover/proofs/link.json: $(arbitrator_cases)/link.wasm $(arbitrator_tests_link_deps) $(prover_bin) - $(prover_bin) $< -o $@ --allow-hostapi --always-merkleize --stylus-modules $(arbitrator_tests_link_deps) --require-success + $(prover_bin) $< -o $@ --allow-hostapi --stylus-modules $(arbitrator_tests_link_deps) --require-success contracts/test/prover/proofs/dynamic.json: $(patsubst %,$(arbitrator_cases)/%.wasm, dynamic user) $(prover_bin) - $(prover_bin) $< -o $@ --allow-hostapi --always-merkleize --stylus-modules $(arbitrator_cases)/user.wasm --require-success + $(prover_bin) $< -o $@ --allow-hostapi --stylus-modules $(arbitrator_cases)/user.wasm --require-success contracts/test/prover/proofs/bulk-memory.json: $(patsubst %,$(arbitrator_cases)/%.wasm, bulk-memory) $(prover_bin) - $(prover_bin) $< -o $@ --allow-hostapi --always-merkleize --stylus-modules $(arbitrator_cases)/user.wasm -b + $(prover_bin) $< -o $@ --allow-hostapi --stylus-modules $(arbitrator_cases)/user.wasm -b contracts/test/prover/proofs/%.json: $(arbitrator_cases)/%.wasm $(prover_bin) - $(prover_bin) $< -o $@ --allow-hostapi --always-merkleize + $(prover_bin) $< -o $@ --allow-hostapi # strategic rules to minimize dependency building From 9539e9a2fbb9b8fe62e7912072315fba691466e2 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Wed, 22 May 2024 11:20:44 +0200 Subject: [PATCH 09/14] Make clone create completely independent layers structs. The default behavior of an Arc is to allow sharing between different threads and different instances. So, the derived behavior for cloning was to put the same references on the clone as on the orignal. But, we actually want separate clones of our merkle trees to be independent. That is, changing the data on the clone, should have no affect on the original (and vice versa.) The test added in this commit failed before the change and passes afer. --- arbitrator/prover/src/merkle.rs | 35 ++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/arbitrator/prover/src/merkle.rs b/arbitrator/prover/src/merkle.rs index ab7b458fda..56cbf56d99 100644 --- a/arbitrator/prover/src/merkle.rs +++ b/arbitrator/prover/src/merkle.rs @@ -32,7 +32,6 @@ use sha3::Keccak256; use std::{ collections::HashSet, convert::{TryFrom, TryInto}, - sync::Arc, }; #[cfg(feature = "rayon")] @@ -189,11 +188,11 @@ struct Layers { /// and passing a minimum depth. /// /// This structure does not contain the data itself, only the hashes. -#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[derive(Debug, Default, Serialize, Deserialize)] pub struct Merkle { ty: MerkleType, - #[serde(with = "arc_mutex_sedre")] - layers: Arc>, + #[serde(with = "mutex_sedre")] + layers: Mutex, min_depth: usize, } @@ -239,6 +238,12 @@ fn new_layer(ty: MerkleType, layer: &Vec, empty_hash: &'static Bytes32) new_layer } +impl Clone for Merkle { + fn clone(&self) -> Self { + Merkle::new_advanced(self.ty, self.layers.lock().data[0].clone(), self.min_depth) + } +} + impl Merkle { /// Creates a new Merkle tree with the given type and leaf hashes. /// The tree is built up to the minimum depth necessary to hold all the @@ -270,10 +275,10 @@ impl Merkle { layers.push(new_layer); layer_i += 1; } - let layers = Arc::new(Mutex::new(Layers { + let layers = Mutex::new(Layers { data: layers, dirt: dirty_indices, - })); + }); Merkle { ty, layers, @@ -435,11 +440,10 @@ impl PartialEq for Merkle { impl Eq for Merkle {} -pub mod arc_mutex_sedre { +pub mod mutex_sedre { use parking_lot::Mutex; - use std::sync::Arc; - pub fn serialize(data: &Arc>, serializer: S) -> Result + pub fn serialize(data: &Mutex, serializer: S) -> Result where S: serde::Serializer, T: serde::Serialize, @@ -447,12 +451,12 @@ pub mod arc_mutex_sedre { data.lock().serialize(serializer) } - pub fn deserialize<'de, D, T>(deserializer: D) -> Result>, D::Error> + pub fn deserialize<'de, D, T>(deserializer: D) -> Result, D::Error> where D: serde::Deserializer<'de>, T: serde::Deserialize<'de>, { - Ok(Arc::new(Mutex::new(T::deserialize(deserializer)?))) + Ok(Mutex::new(T::deserialize(deserializer)?)) } } @@ -566,6 +570,15 @@ fn emit_memory_zerohashes() { } } +#[test] +fn clone_is_separate() { + let merkle = Merkle::new_advanced(MerkleType::Value, vec![Bytes32::from([1; 32])], 4); + let m2 = merkle.clone(); + m2.resize(4).expect("resize failed"); + m2.set(3, Bytes32::from([2; 32])); + assert_ne!(merkle, m2); +} + #[test] fn serialization_roundtrip() { let merkle = Merkle::new_advanced(MerkleType::Value, vec![Bytes32::from([1; 32])], 4); From b075b20e3b99805fa3b12218ec8b341f9f203871 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Mon, 27 May 2024 16:14:48 +0200 Subject: [PATCH 10/14] Make clippy happy. --- arbitrator/bench/src/parse_input.rs | 6 +++--- arbitrator/bench/src/prepare.rs | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/arbitrator/bench/src/parse_input.rs b/arbitrator/bench/src/parse_input.rs index f04150ea22..decc67372a 100644 --- a/arbitrator/bench/src/parse_input.rs +++ b/arbitrator/bench/src/parse_input.rs @@ -24,8 +24,8 @@ mod prefixed_hex { D: Deserializer<'de>, { let s = String::deserialize(deserializer)?; - if s.starts_with("0x") { - hex::decode(&s[2..]).map_err(serde::de::Error::custom) + if let Some(s) = s.strip_prefix("0x") { + hex::decode(s).map_err(serde::de::Error::custom) } else { Err(serde::de::Error::custom("missing 0x prefix")) } @@ -71,6 +71,6 @@ pub struct FileData { impl FileData { pub fn from_reader(mut reader: R) -> io::Result { let data = serde_json::from_reader(&mut reader)?; - return Ok(data); + Ok(data) } } diff --git a/arbitrator/bench/src/prepare.rs b/arbitrator/bench/src/prepare.rs index aebc7ae895..668f5e1120 100644 --- a/arbitrator/bench/src/prepare.rs +++ b/arbitrator/bench/src/prepare.rs @@ -17,8 +17,7 @@ pub fn prepare_machine(preimages: PathBuf, machines: PathBuf) -> eyre::Result>>(); let preimage_resolver = move |_: u64, _: PreimageType, hash: Bytes32| -> Option { preimages From 8d074df86d199012b7ed7c1a5121b23730aea503 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Wed, 5 Jun 2024 16:49:40 +0200 Subject: [PATCH 11/14] Fix Clone for the Memory type. This was causing problems across with the CGo bindings. --- arbitrator/prover/src/memory.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/arbitrator/prover/src/memory.rs b/arbitrator/prover/src/memory.rs index 78b5ba7259..d5d0a0855d 100644 --- a/arbitrator/prover/src/memory.rs +++ b/arbitrator/prover/src/memory.rs @@ -10,13 +10,7 @@ use digest::Digest; use eyre::{bail, ErrReport, Result}; use serde::{Deserialize, Serialize}; use sha3::Keccak256; -use std::{ - borrow::Cow, - collections::HashSet, - convert::TryFrom, - ops::Deref, - sync::{Arc, Mutex}, -}; +use std::{borrow::Cow, collections::HashSet, convert::TryFrom, ops::Deref, sync::Mutex}; #[cfg(feature = "counters")] use std::sync::atomic::{AtomicUsize, Ordering}; @@ -70,14 +64,14 @@ impl TryFrom<&wasmparser::MemoryType> for MemoryType { } } -#[derive(Clone, Debug, Default, Serialize, Deserialize)] +#[derive(Debug, Default, Serialize, Deserialize)] pub struct Memory { buffer: Vec, #[serde(skip)] pub merkle: Option, pub max_size: u64, #[serde(skip)] - dirty_indices: Arc>>, + dirty_indices: Mutex>, } fn hash_leaf(bytes: [u8; Memory::LEAF_SIZE]) -> Bytes32 { @@ -116,6 +110,17 @@ impl PartialEq for Memory { } } +impl Clone for Memory { + fn clone(&self) -> Self { + Memory { + buffer: self.buffer.clone(), + merkle: self.merkle.clone(), + max_size: self.max_size, + dirty_indices: Mutex::new(self.dirty_indices.lock().unwrap().clone()), + } + } +} + impl Memory { pub const LEAF_SIZE: usize = 32; /// Only used when initializing a memory to determine its size @@ -129,7 +134,7 @@ impl Memory { buffer: vec![0u8; size], merkle: None, max_size, - dirty_indices: Arc::new(Mutex::new(HashSet::new())), + dirty_indices: Mutex::new(HashSet::new()), } } From 1974d33cc467b7c4568746bb4376c446c72fa2a2 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Thu, 6 Jun 2024 11:53:43 +0200 Subject: [PATCH 12/14] Remove race condition in merkelize. --- arbitrator/prover/src/memory.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arbitrator/prover/src/memory.rs b/arbitrator/prover/src/memory.rs index d5d0a0855d..441d25bd18 100644 --- a/arbitrator/prover/src/memory.rs +++ b/arbitrator/prover/src/memory.rs @@ -144,11 +144,12 @@ impl Memory { pub fn merkelize(&self) -> Cow<'_, Merkle> { if let Some(m) = &self.merkle { - for idx in self.dirty_indices.lock().unwrap().iter() { + let mut dirt = self.dirty_indices.lock().unwrap(); + for idx in dirt.iter() { let leaf_idx = idx / Self::LEAF_SIZE; m.set(leaf_idx, hash_leaf(self.get_leaf_data(leaf_idx))); } - self.dirty_indices.lock().unwrap().clear(); + dirt.clear(); return Cow::Borrowed(m); } // Round the size up to 8 byte long leaves, then round up to the next power of two number of leaves From fc5dd2adc5e80393bbcacfc28db43f02a78adb56 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Tue, 11 Jun 2024 10:25:56 +0200 Subject: [PATCH 13/14] Update Cargo.lock --- arbitrator/Cargo.lock | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arbitrator/Cargo.lock b/arbitrator/Cargo.lock index 0e0c8f5e82..5175b6b609 100644 --- a/arbitrator/Cargo.lock +++ b/arbitrator/Cargo.lock @@ -749,6 +749,7 @@ dependencies = [ "ident_case", "proc-macro2", "quote", + "strsim 0.11.1", "syn 2.0.66", ] @@ -2116,7 +2117,7 @@ dependencies = [ "darling 0.20.9", "proc-macro2", "quote", - "syn 2.0.64", + "syn 2.0.66", ] [[package]] From fe8c159eaf4fbf2e80dc1b234739ebd49991829c Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Tue, 2 Jul 2024 23:16:26 +0200 Subject: [PATCH 14/14] Make clippy happy. --- arbitrator/prover/src/merkle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arbitrator/prover/src/merkle.rs b/arbitrator/prover/src/merkle.rs index 241828f1e9..c58d31b1b1 100644 --- a/arbitrator/prover/src/merkle.rs +++ b/arbitrator/prover/src/merkle.rs @@ -204,7 +204,7 @@ fn new_layer(ty: MerkleType, layer: &[Bytes32], empty_hash: &'static Bytes32) -> impl Clone for Merkle { fn clone(&self) -> Self { - let leaves = self.layers.lock().data.get(0).cloned().unwrap_or_default(); + let leaves = self.layers.lock().data.first().cloned().unwrap_or_default(); Merkle::new_advanced(self.ty, leaves, self.min_depth) } }