From 798d7177e8c9c160c1a36c954daa982a20e95306 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Fri, 4 Oct 2024 17:15:18 +0530 Subject: [PATCH 01/18] Add a test to keep server_api json.go in sync with parse_inputs.rs --- arbitrator/prover/src/lib.rs | 36 ++++++ arbitrator/prover/src/parse_input.rs | 27 +++- .../validationinputjson_rustfiledata_test.go | 120 ++++++++++++++++++ validator/server_arb/machine.go | 14 ++ 4 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 system_tests/validationinputjson_rustfiledata_test.go diff --git a/arbitrator/prover/src/lib.rs b/arbitrator/prover/src/lib.rs index 08473c2598..993ed1b365 100644 --- a/arbitrator/prover/src/lib.rs +++ b/arbitrator/prover/src/lib.rs @@ -33,9 +33,12 @@ use machine::{ PreimageResolver, }; use once_cell::sync::OnceCell; +use parse_input::FileData; use static_assertions::const_assert_eq; use std::{ ffi::CStr, + fs::File, + io::BufReader, num::NonZeroUsize, os::raw::{c_char, c_int}, path::Path, @@ -84,6 +87,39 @@ pub unsafe extern "C" fn arbitrator_load_machine( } } +#[no_mangle] +pub unsafe extern "C" fn arbitrator_deserialize_and_serialize_file_data( + read_path: *const c_char, + write_path: *const c_char, +) -> c_int { + let read_path = cstr_to_string(read_path); + let write_path = cstr_to_string(write_path); + + let file = File::open(read_path); + let reader = match file { + Ok(file) => BufReader::new(file), + Err(err) => { + eprintln!("Failed to open read_path of FileData: {}", err); + return 1; + } + }; + let data = match FileData::from_reader(reader) { + Ok(data) => data, + Err(err) => { + eprintln!("Failed to deserialize FileData: {}", err); + return 2; + } + }; + + match data.write_to_file(&write_path) { + Ok(()) => 0, + Err(err) => { + eprintln!("Failed to serialize FileData: {}", err); + 3 + } + } +} + unsafe fn arbitrator_load_machine_impl( binary_path: *const c_char, library_paths: *const *const c_char, diff --git a/arbitrator/prover/src/parse_input.rs b/arbitrator/prover/src/parse_input.rs index fa7adb4c41..edfaddf26f 100644 --- a/arbitrator/prover/src/parse_input.rs +++ b/arbitrator/prover/src/parse_input.rs @@ -1,12 +1,14 @@ use arbutil::Bytes32; use serde::Deserialize; +use serde::Serialize; use serde_json; use serde_with::base64::Base64; use serde_with::As; use serde_with::DisplayFromStr; use std::{ collections::HashMap, - io::{self, BufRead}, + fs::File, + io::{self, BufRead, BufWriter}, }; /// prefixed_hex deserializes hex strings which are prefixed with `0x` @@ -16,7 +18,7 @@ use std::{ /// It is an error to use this deserializer on a string that does not /// begin with `0x`. mod prefixed_hex { - use serde::{self, Deserialize, Deserializer}; + use serde::{self, Deserialize, Deserializer, Serializer}; pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> where @@ -29,6 +31,14 @@ mod prefixed_hex { Err(serde::de::Error::custom("missing 0x prefix")) } } + + pub fn serialize(bytes: &Vec, serializer: S) -> Result + where + S: Serializer, + { + let hex_string = format!("0x{}", hex::encode(bytes)); + serializer.serialize_str(&hex_string) + } } #[derive(Debug)] @@ -61,7 +71,7 @@ impl From> for UserWasm { } } -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] #[serde(rename_all = "PascalCase")] pub struct BatchInfo { pub number: u64, @@ -69,7 +79,7 @@ pub struct BatchInfo { pub data_b64: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "PascalCase")] pub struct StartState { #[serde(with = "prefixed_hex")] @@ -88,7 +98,7 @@ pub struct StartState { /// /// Note: It is important to change this file whenever the go JSON /// serialization changes. -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "PascalCase")] pub struct FileData { pub id: u64, @@ -109,4 +119,11 @@ impl FileData { let data = serde_json::from_reader(&mut reader)?; Ok(data) } + + pub fn write_to_file(&self, file_path: &str) -> io::Result<()> { + let file = File::create(file_path)?; + let writer = BufWriter::new(file); + serde_json::to_writer_pretty(writer, &self)?; + Ok(()) + } } diff --git a/system_tests/validationinputjson_rustfiledata_test.go b/system_tests/validationinputjson_rustfiledata_test.go new file mode 100644 index 0000000000..a27a68b12c --- /dev/null +++ b/system_tests/validationinputjson_rustfiledata_test.go @@ -0,0 +1,120 @@ +package arbtest + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "os" + "path/filepath" + "reflect" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/ethdb" + + "github.com/offchainlabs/nitro/arbutil" + "github.com/offchainlabs/nitro/validator" + "github.com/offchainlabs/nitro/validator/server_api" + "github.com/offchainlabs/nitro/validator/server_arb" +) + +func getWriteDataFromRustSide(t *testing.T, inputJSON *server_api.InputJSON) []byte { + t.Helper() + dir := t.TempDir() + + readData, err := inputJSON.Marshal() + Require(t, err) + readPath := filepath.Join(dir, fmt.Sprintf("block_inputs_%d_read.json", inputJSON.Id)) + Require(t, os.WriteFile(readPath, readData, 0600)) + + writePath := filepath.Join(dir, fmt.Sprintf("block_inputs_%d_write.json", inputJSON.Id)) + Require(t, server_arb.DeserializeAndSerializeFileData(readPath, writePath)) + writeData, err := os.ReadFile(writePath) + Require(t, err) + + return writeData +} + +func TestGoInputJSONRustFileDataRoundtripWithoutUserWasms(t *testing.T) { + preimages := make(map[arbutil.PreimageType]map[common.Hash][]byte) + preimages[arbutil.Keccak256PreimageType] = make(map[common.Hash][]byte) + preimages[arbutil.Keccak256PreimageType][common.MaxHash] = []byte{1} + + // Don't include DebugChain as it isn't used on rust side + sampleValidationInput := &validator.ValidationInput{ + Id: 1, + HasDelayedMsg: true, + DelayedMsgNr: 2, + Preimages: preimages, + BatchInfo: []validator.BatchInfo{{Number: 3}}, + DelayedMsg: []byte{4}, + StartState: validator.GoGlobalState{ + BlockHash: common.MaxHash, + SendRoot: common.MaxHash, + Batch: 5, + PosInBatch: 6, + }, + } + sampleValidationInputJSON := server_api.ValidationInputToJson(sampleValidationInput) + writeData := getWriteDataFromRustSide(t, sampleValidationInputJSON) + + var resWithoutUserWasms server_api.InputJSON + Require(t, json.Unmarshal(writeData, &resWithoutUserWasms)) + if !reflect.DeepEqual(*sampleValidationInputJSON, resWithoutUserWasms) { + t.Fatal("ValidationInputJSON without UserWasms, mismatch on rust and go side") + } + +} + +type inputJSONWithUserWasmsOnly struct { + UserWasms map[ethdb.WasmTarget]map[common.Hash][]byte +} + +// UnmarshalJSON is a custom function defined to encapsulate how UserWasms are handled on the rust side. +// When ValidationInputToJson is called on ValidationInput, it compresses the wasm data byte array and +// then encodes this to a base64 string, this when deserialized on the rust side through FileData- the +// compressed data is first uncompressed and also the module hash (Bytes32) is read without the 0x prefix, +// so we define a custom UnmarshalJSON to extract UserWasms map from the data written by rust side. +func (u *inputJSONWithUserWasmsOnly) UnmarshalJSON(data []byte) error { + type rawUserWasms struct { + UserWasms map[ethdb.WasmTarget]map[string]string + } + var rawUWs rawUserWasms + if err := json.Unmarshal(data, &rawUWs); err != nil { + return err + } + tmp := make(map[ethdb.WasmTarget]map[common.Hash][]byte) + for wasmTarget, innerMap := range rawUWs.UserWasms { + tmp[wasmTarget] = make(map[common.Hash][]byte) + for hashKey, value := range innerMap { + valBytes, err := base64.StdEncoding.DecodeString(value) + if err != nil { + return err + } + tmp[wasmTarget][common.HexToHash("0x"+hashKey)] = valBytes + } + } + u.UserWasms = tmp + return nil +} + +func TestGoInputJSONRustFileDataRoundtripWithUserWasms(t *testing.T) { + userWasms := make(map[ethdb.WasmTarget]map[common.Hash][]byte) + userWasms["arch1"] = make(map[common.Hash][]byte) + userWasms["arch1"][common.MaxHash] = []byte{2} + + // Don't include DebugChain as it isn't used on rust side + sampleValidationInput := &validator.ValidationInput{ + Id: 1, + UserWasms: userWasms, + BatchInfo: []validator.BatchInfo{{Number: 1}}, // This needs to be set for FileData to successfully deserialize, else it errors for invalid type null + } + sampleValidationInputJSON := server_api.ValidationInputToJson(sampleValidationInput) + writeData := getWriteDataFromRustSide(t, sampleValidationInputJSON) + + var resUserWasmsOnly inputJSONWithUserWasmsOnly + Require(t, json.Unmarshal(writeData, &resUserWasmsOnly)) + if !reflect.DeepEqual(userWasms, resUserWasmsOnly.UserWasms) { + t.Fatal("ValidationInputJSON with UserWasms only, mismatch on rust and go side") + } +} diff --git a/validator/server_arb/machine.go b/validator/server_arb/machine.go index 1e73e6b212..d11f015916 100644 --- a/validator/server_arb/machine.go +++ b/validator/server_arb/machine.go @@ -312,6 +312,20 @@ func (m *ArbitratorMachine) DeserializeAndReplaceState(path string) error { } } +func DeserializeAndSerializeFileData(readPath, writePath string) error { + cReadPath := C.CString(readPath) + cWritePath := C.CString(writePath) + status := C.arbitrator_deserialize_and_serialize_file_data(cReadPath, cWritePath) + C.free(unsafe.Pointer(cReadPath)) + C.free(unsafe.Pointer(cWritePath)) + + if status != 0 { + return fmt.Errorf("failed to call arbitrator_deserialize_and_serialize_file_data. Error code: %d", status) + } else { + return nil + } +} + func (m *ArbitratorMachine) AddSequencerInboxMessage(index uint64, data []byte) error { defer runtime.KeepAlive(m) m.mutex.Lock() From 115afdc4d5c9ebadb985a581e6e19a73b635a59b Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Wed, 9 Oct 2024 17:10:48 +0530 Subject: [PATCH 02/18] Jit prover should accept InputJSON format and execute a full block --- arbitrator/jit/src/machine.rs | 120 ++++++++++++++++++---------------- arbitrator/jit/src/main.rs | 5 ++ arbitrator/jit/src/prepare.rs | 73 +++++++++++++++++++++ system_tests/common_test.go | 6 +- system_tests/program_test.go | 8 ++- 5 files changed, 150 insertions(+), 62 deletions(-) create mode 100644 arbitrator/jit/src/prepare.rs diff --git a/arbitrator/jit/src/machine.rs b/arbitrator/jit/src/machine.rs index 02523f740a..0d74c74ef6 100644 --- a/arbitrator/jit/src/machine.rs +++ b/arbitrator/jit/src/machine.rs @@ -2,8 +2,8 @@ // For license information, see https://github.com/nitro/blob/master/LICENSE use crate::{ - arbcompress, caller_env::GoRuntimeState, program, socket, stylus_backend::CothreadHandler, - wasip1_stub, wavmio, Opts, + arbcompress, caller_env::GoRuntimeState, prepare::prepare_env, program, socket, + stylus_backend::CothreadHandler, wasip1_stub, wavmio, Opts, }; use arbutil::{Bytes32, Color, PreimageType}; use eyre::{bail, ErrReport, Result, WrapErr}; @@ -215,72 +215,76 @@ pub struct WasmEnv { impl WasmEnv { pub fn cli(opts: &Opts) -> Result { - let mut env = WasmEnv::default(); - env.process.forks = opts.forks; - env.process.debug = opts.debug; + if let Some(json_inputs) = opts.json_inputs.clone() { + prepare_env(json_inputs, opts.debug) + } else { + let mut env = WasmEnv::default(); + env.process.forks = opts.forks; + env.process.debug = opts.debug; - let mut inbox_position = opts.inbox_position; - let mut delayed_position = opts.delayed_inbox_position; + let mut inbox_position = opts.inbox_position; + let mut delayed_position = opts.delayed_inbox_position; - for path in &opts.inbox { - let mut msg = vec![]; - File::open(path)?.read_to_end(&mut msg)?; - env.sequencer_messages.insert(inbox_position, msg); - inbox_position += 1; - } - for path in &opts.delayed_inbox { - let mut msg = vec![]; - File::open(path)?.read_to_end(&mut msg)?; - env.delayed_messages.insert(delayed_position, msg); - delayed_position += 1; - } + for path in &opts.inbox { + let mut msg = vec![]; + File::open(path)?.read_to_end(&mut msg)?; + env.sequencer_messages.insert(inbox_position, msg); + inbox_position += 1; + } + for path in &opts.delayed_inbox { + let mut msg = vec![]; + File::open(path)?.read_to_end(&mut msg)?; + env.delayed_messages.insert(delayed_position, msg); + delayed_position += 1; + } - if let Some(path) = &opts.preimages { - let mut file = BufReader::new(File::open(path)?); - let mut preimages = Vec::new(); - let filename = path.to_string_lossy(); - loop { - let mut size_buf = [0u8; 8]; - match file.read_exact(&mut size_buf) { - Ok(()) => {} - Err(err) if err.kind() == ErrorKind::UnexpectedEof => break, - Err(err) => bail!("Failed to parse {filename}: {}", err), + if let Some(path) = &opts.preimages { + let mut file = BufReader::new(File::open(path)?); + let mut preimages = Vec::new(); + let filename = path.to_string_lossy(); + loop { + let mut size_buf = [0u8; 8]; + match file.read_exact(&mut size_buf) { + Ok(()) => {} + Err(err) if err.kind() == ErrorKind::UnexpectedEof => break, + Err(err) => bail!("Failed to parse {filename}: {}", err), + } + let size = u64::from_le_bytes(size_buf) as usize; + let mut buf = vec![0u8; size]; + file.read_exact(&mut buf)?; + preimages.push(buf); + } + let keccak_preimages = env.preimages.entry(PreimageType::Keccak256).or_default(); + for preimage in preimages { + let mut hasher = Keccak256::new(); + hasher.update(&preimage); + let hash = hasher.finalize().into(); + keccak_preimages.insert(hash, preimage); } - let size = u64::from_le_bytes(size_buf) as usize; - let mut buf = vec![0u8; size]; - file.read_exact(&mut buf)?; - preimages.push(buf); - } - let keccak_preimages = env.preimages.entry(PreimageType::Keccak256).or_default(); - for preimage in preimages { - let mut hasher = Keccak256::new(); - hasher.update(&preimage); - let hash = hasher.finalize().into(); - keccak_preimages.insert(hash, preimage); } - } - fn parse_hex(arg: &Option, name: &str) -> Result { - match arg { - Some(arg) => { - let mut arg = arg.as_str(); - if arg.starts_with("0x") { - arg = &arg[2..]; + fn parse_hex(arg: &Option, name: &str) -> Result { + match arg { + Some(arg) => { + let mut arg = arg.as_str(); + if arg.starts_with("0x") { + arg = &arg[2..]; + } + let mut bytes32 = [0u8; 32]; + hex::decode_to_slice(arg, &mut bytes32) + .wrap_err_with(|| format!("failed to parse {} contents", name))?; + Ok(bytes32.into()) } - let mut bytes32 = [0u8; 32]; - hex::decode_to_slice(arg, &mut bytes32) - .wrap_err_with(|| format!("failed to parse {} contents", name))?; - Ok(bytes32.into()) + None => Ok(Bytes32::default()), } - None => Ok(Bytes32::default()), } - } - let last_block_hash = parse_hex(&opts.last_block_hash, "--last-block-hash")?; - let last_send_root = parse_hex(&opts.last_send_root, "--last-send-root")?; - env.small_globals = [opts.inbox_position, opts.position_within_message]; - env.large_globals = [last_block_hash, last_send_root]; - Ok(env) + let last_block_hash = parse_hex(&opts.last_block_hash, "--last-block-hash")?; + let last_send_root = parse_hex(&opts.last_send_root, "--last-send-root")?; + env.small_globals = [opts.inbox_position, opts.position_within_message]; + env.large_globals = [last_block_hash, last_send_root]; + Ok(env) + } } pub fn send_results(&mut self, error: Option, memory_used: Pages) { diff --git a/arbitrator/jit/src/main.rs b/arbitrator/jit/src/main.rs index e432dc215c..6e44500215 100644 --- a/arbitrator/jit/src/main.rs +++ b/arbitrator/jit/src/main.rs @@ -10,6 +10,7 @@ use structopt::StructOpt; mod arbcompress; mod caller_env; mod machine; +mod prepare; mod program; mod socket; mod stylus_backend; @@ -46,6 +47,10 @@ pub struct Opts { debug: bool, #[structopt(long)] require_success: bool, + // JSON inputs supercede any of the command-line inputs which could + // be specified in the JSON file. + #[structopt(long)] + json_inputs: Option, } fn main() -> Result<()> { diff --git a/arbitrator/jit/src/prepare.rs b/arbitrator/jit/src/prepare.rs new file mode 100644 index 0000000000..7cf46143cc --- /dev/null +++ b/arbitrator/jit/src/prepare.rs @@ -0,0 +1,73 @@ +// Copyright 2022-2024, Offchain Labs, Inc. +// For license information, see https://github.com/nitro/blob/master/LICENSE + +use crate::WasmEnv; +use arbutil::{Bytes32, PreimageType}; +use eyre::Ok; +use prover::parse_input::FileData; +use std::env; +use std::fs::File; +use std::io::BufReader; +use std::path::PathBuf; + +// local_target matches rawdb.LocalTarget() on the go side. +// While generating json_inputs file, one should make sure user_wasms map +// has entry for the system's arch that jit validation is being run on +pub fn local_target() -> String { + if env::consts::OS == "linux" { + match env::consts::ARCH { + "arm64" => "arm64".to_string(), + "amd64" => "amd64".to_string(), + _ => "host".to_string(), + } + } else { + "host".to_string() + } +} + +pub fn prepare_env(json_inputs: PathBuf, debug: bool) -> eyre::Result { + let file = File::open(json_inputs)?; + let reader = BufReader::new(file); + + let data = FileData::from_reader(reader)?; + + let mut env = WasmEnv::default(); + env.process.forks = false; // Should be set to false when using json_inputs + env.process.debug = debug; + + let block_hash: [u8; 32] = data.start_state.block_hash.try_into().unwrap(); + let block_hash: Bytes32 = block_hash.into(); + let send_root: [u8; 32] = data.start_state.send_root.try_into().unwrap(); + let send_root: Bytes32 = send_root.into(); + let bytes32_vals: [Bytes32; 2] = [block_hash, send_root]; + let u64_vals: [u64; 2] = [data.start_state.batch, data.start_state.pos_in_batch]; + env.small_globals = u64_vals; + env.large_globals = bytes32_vals; + + for batch_info in data.batch_info.iter() { + env.sequencer_messages + .insert(batch_info.number, batch_info.data_b64.clone()); + } + + if data.delayed_msg_nr != 0 && data.delayed_msg_b64.len() != 0 { + env.delayed_messages + .insert(data.delayed_msg_nr, data.delayed_msg_b64.clone()); + } + + for (ty, inner_map) in data.preimages_b64 { + let preimage_ty = PreimageType::try_from(ty as u8)?; + let map = env.preimages.entry(preimage_ty).or_default(); + for (hash, preimage) in inner_map { + map.insert(hash, preimage); + } + } + + if let Some(user_wasms) = data.user_wasms.get(&local_target()) { + for (module_hash, module_asm) in user_wasms.iter() { + env.module_asms + .insert(*module_hash, module_asm.as_vec().into()); + } + } + + Ok(env) +} diff --git a/system_tests/common_test.go b/system_tests/common_test.go index 93c38b5eae..5667c4bcd7 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -1699,8 +1699,8 @@ func logParser[T any](t *testing.T, source string, name string) func(*types.Log) // recordBlock writes a json file with all of the data needed to validate a block. // -// This can be used as an input to the arbitrator prover to validate a block. -func recordBlock(t *testing.T, block uint64, builder *NodeBuilder) { +// This can be used as an input to the arbitrator's prover (target=rawdb.TargetWavm) and jit (target=rawdb.LocalTarget()) binaries to validate a block. +func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, target ethdb.WasmTarget) { t.Helper() ctx := builder.ctx inboxPos := arbutil.MessageIndex(block) @@ -1716,7 +1716,7 @@ func recordBlock(t *testing.T, block uint64, builder *NodeBuilder) { } validationInputsWriter, err := inputs.NewWriter(inputs.WithSlug(t.Name())) Require(t, err) - inputJson, err := builder.L2.ConsensusNode.StatelessBlockValidator.ValidationInputsAt(ctx, inboxPos, rawdb.TargetWavm) + inputJson, err := builder.L2.ConsensusNode.StatelessBlockValidator.ValidationInputsAt(ctx, inboxPos, target) if err != nil { Fatal(t, "failed to get validation inputs", block, err) } diff --git a/system_tests/program_test.go b/system_tests/program_test.go index cf8cd72559..4e7fc8dfd8 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -425,7 +425,7 @@ func storageTest(t *testing.T, jit bool) { // Captures a block_input_.json file for the block that included the // storage write transaction. - recordBlock(t, receipt.BlockNumber.Uint64(), builder) + recordBlock(t, receipt.BlockNumber.Uint64(), builder, rawdb.TargetWavm) } func TestProgramTransientStorage(t *testing.T) { @@ -492,6 +492,12 @@ func transientStorageTest(t *testing.T, jit bool) { } validateBlocks(t, 7, jit, builder) + + // Captures a block_input_.json file for the block that included the storage + // related transaction. Has userwasms for WasmTarget recognized by jit binary + receipt, err := EnsureTxSucceeded(ctx, l2client, tx) + Require(t, err) + recordBlock(t, receipt.BlockNumber.Uint64(), builder, rawdb.LocalTarget()) } func TestProgramMath(t *testing.T) { From 3a6e8681b630cacaace4c8b9af98bf2d2a4cb459 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Wed, 9 Oct 2024 19:40:51 +0530 Subject: [PATCH 03/18] fix clippy error --- arbitrator/jit/src/prepare.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arbitrator/jit/src/prepare.rs b/arbitrator/jit/src/prepare.rs index 7cf46143cc..6c1b0b84de 100644 --- a/arbitrator/jit/src/prepare.rs +++ b/arbitrator/jit/src/prepare.rs @@ -49,7 +49,7 @@ pub fn prepare_env(json_inputs: PathBuf, debug: bool) -> eyre::Result { .insert(batch_info.number, batch_info.data_b64.clone()); } - if data.delayed_msg_nr != 0 && data.delayed_msg_b64.len() != 0 { + if data.delayed_msg_nr != 0 && !data.delayed_msg_b64.is_empty() { env.delayed_messages .insert(data.delayed_msg_nr, data.delayed_msg_b64.clone()); } From be58669225e8ecf604182a2d65c1d115cf12686d Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 11:56:28 +0530 Subject: [PATCH 04/18] change implimentation and add CI step to execute arbitrator prover using block input json --- .github/workflows/ci.yml | 12 ++ arbitrator/prover/src/lib.rs | 36 ------ arbitrator/prover/src/parse_input.rs | 27 +--- system_tests/common_test.go | 12 +- system_tests/program_test.go | 5 +- .../validationinputjson_rustfiledata_test.go | 120 ------------------ validator/server_arb/machine.go | 14 -- 7 files changed, 26 insertions(+), 200 deletions(-) delete mode 100644 system_tests/validationinputjson_rustfiledata_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a944f08f40..0b5592dafc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -169,6 +169,18 @@ jobs: run: | echo "Running redis tests" >> full.log TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... + + - name: create block input json file + if: matrix.test-mode == 'defaults' + run: | + echo "BLOCK_INPUT_JSON_PATH=$(pwd)/target/block_input.json" >> "$GITHUB_ENV" + ${{ github.workspace }}/.github/workflows/gotestsum.sh --run TestProgramStorage$ --count 1 + + - name: run arbitrator prover on block input json + if: matrix.test-mode == 'defaults' + run: | + make build-prover-bin + target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs=$BLOCK_INPUT_JSON_PATH - name: run challenge tests if: matrix.test-mode == 'challenge' diff --git a/arbitrator/prover/src/lib.rs b/arbitrator/prover/src/lib.rs index 993ed1b365..08473c2598 100644 --- a/arbitrator/prover/src/lib.rs +++ b/arbitrator/prover/src/lib.rs @@ -33,12 +33,9 @@ use machine::{ PreimageResolver, }; use once_cell::sync::OnceCell; -use parse_input::FileData; use static_assertions::const_assert_eq; use std::{ ffi::CStr, - fs::File, - io::BufReader, num::NonZeroUsize, os::raw::{c_char, c_int}, path::Path, @@ -87,39 +84,6 @@ pub unsafe extern "C" fn arbitrator_load_machine( } } -#[no_mangle] -pub unsafe extern "C" fn arbitrator_deserialize_and_serialize_file_data( - read_path: *const c_char, - write_path: *const c_char, -) -> c_int { - let read_path = cstr_to_string(read_path); - let write_path = cstr_to_string(write_path); - - let file = File::open(read_path); - let reader = match file { - Ok(file) => BufReader::new(file), - Err(err) => { - eprintln!("Failed to open read_path of FileData: {}", err); - return 1; - } - }; - let data = match FileData::from_reader(reader) { - Ok(data) => data, - Err(err) => { - eprintln!("Failed to deserialize FileData: {}", err); - return 2; - } - }; - - match data.write_to_file(&write_path) { - Ok(()) => 0, - Err(err) => { - eprintln!("Failed to serialize FileData: {}", err); - 3 - } - } -} - unsafe fn arbitrator_load_machine_impl( binary_path: *const c_char, library_paths: *const *const c_char, diff --git a/arbitrator/prover/src/parse_input.rs b/arbitrator/prover/src/parse_input.rs index edfaddf26f..fa7adb4c41 100644 --- a/arbitrator/prover/src/parse_input.rs +++ b/arbitrator/prover/src/parse_input.rs @@ -1,14 +1,12 @@ use arbutil::Bytes32; use serde::Deserialize; -use serde::Serialize; use serde_json; use serde_with::base64::Base64; use serde_with::As; use serde_with::DisplayFromStr; use std::{ collections::HashMap, - fs::File, - io::{self, BufRead, BufWriter}, + io::{self, BufRead}, }; /// prefixed_hex deserializes hex strings which are prefixed with `0x` @@ -18,7 +16,7 @@ use std::{ /// It is an error to use this deserializer on a string that does not /// begin with `0x`. mod prefixed_hex { - use serde::{self, Deserialize, Deserializer, Serializer}; + use serde::{self, Deserialize, Deserializer}; pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> where @@ -31,14 +29,6 @@ mod prefixed_hex { Err(serde::de::Error::custom("missing 0x prefix")) } } - - pub fn serialize(bytes: &Vec, serializer: S) -> Result - where - S: Serializer, - { - let hex_string = format!("0x{}", hex::encode(bytes)); - serializer.serialize_str(&hex_string) - } } #[derive(Debug)] @@ -71,7 +61,7 @@ impl From> for UserWasm { } } -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "PascalCase")] pub struct BatchInfo { pub number: u64, @@ -79,7 +69,7 @@ pub struct BatchInfo { pub data_b64: Vec, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize)] #[serde(rename_all = "PascalCase")] pub struct StartState { #[serde(with = "prefixed_hex")] @@ -98,7 +88,7 @@ pub struct StartState { /// /// Note: It is important to change this file whenever the go JSON /// serialization changes. -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize)] #[serde(rename_all = "PascalCase")] pub struct FileData { pub id: u64, @@ -119,11 +109,4 @@ impl FileData { let data = serde_json::from_reader(&mut reader)?; Ok(data) } - - pub fn write_to_file(&self, file_path: &str) -> io::Result<()> { - let file = File::create(file_path)?; - let writer = BufWriter::new(file); - serde_json::to_writer_pretty(writer, &self)?; - Ok(()) - } } diff --git a/system_tests/common_test.go b/system_tests/common_test.go index dbb2b86f13..95e8883e6b 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -36,7 +36,6 @@ import ( "github.com/offchainlabs/nitro/util/headerreader" "github.com/offchainlabs/nitro/util/redisutil" "github.com/offchainlabs/nitro/util/signature" - "github.com/offchainlabs/nitro/validator/inputs" "github.com/offchainlabs/nitro/validator/server_api" "github.com/offchainlabs/nitro/validator/server_common" "github.com/offchainlabs/nitro/validator/valnode" @@ -1719,7 +1718,7 @@ func logParser[T any](t *testing.T, source string, name string) func(*types.Log) // recordBlock writes a json file with all of the data needed to validate a block. // // This can be used as an input to the arbitrator prover to validate a block. -func recordBlock(t *testing.T, block uint64, builder *NodeBuilder) { +func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, blockInputJSONPath string) { t.Helper() ctx := builder.ctx inboxPos := arbutil.MessageIndex(block) @@ -1733,15 +1732,14 @@ func recordBlock(t *testing.T, block uint64, builder *NodeBuilder) { break } } - validationInputsWriter, err := inputs.NewWriter(inputs.WithSlug(t.Name())) - Require(t, err) inputJson, err := builder.L2.ConsensusNode.StatelessBlockValidator.ValidationInputsAt(ctx, inboxPos, rawdb.TargetWavm) if err != nil { Fatal(t, "failed to get validation inputs", block, err) } - if err := validationInputsWriter.Write(&inputJson); err != nil { - Fatal(t, "failed to write validation inputs", block, err) - } + contents, err := json.Marshal(inputJson) + Require(t, err) + err = os.WriteFile(blockInputJSONPath, contents, 0600) + Require(t, err) } func populateMachineDir(t *testing.T, cr *github.ConsensusRelease) string { diff --git a/system_tests/program_test.go b/system_tests/program_test.go index 4755096b26..d2c3336af7 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -425,7 +425,10 @@ func storageTest(t *testing.T, jit bool) { // Captures a block_input_.json file for the block that included the // storage write transaction. - recordBlock(t, receipt.BlockNumber.Uint64(), builder) + blockInputJSONPath := os.Getenv("BLOCK_INPUT_JSON_PATH") + if blockInputJSONPath != "" { + recordBlock(t, receipt.BlockNumber.Uint64(), builder, blockInputJSONPath) + } } func TestProgramTransientStorage(t *testing.T) { diff --git a/system_tests/validationinputjson_rustfiledata_test.go b/system_tests/validationinputjson_rustfiledata_test.go deleted file mode 100644 index a27a68b12c..0000000000 --- a/system_tests/validationinputjson_rustfiledata_test.go +++ /dev/null @@ -1,120 +0,0 @@ -package arbtest - -import ( - "encoding/base64" - "encoding/json" - "fmt" - "os" - "path/filepath" - "reflect" - "testing" - - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/ethdb" - - "github.com/offchainlabs/nitro/arbutil" - "github.com/offchainlabs/nitro/validator" - "github.com/offchainlabs/nitro/validator/server_api" - "github.com/offchainlabs/nitro/validator/server_arb" -) - -func getWriteDataFromRustSide(t *testing.T, inputJSON *server_api.InputJSON) []byte { - t.Helper() - dir := t.TempDir() - - readData, err := inputJSON.Marshal() - Require(t, err) - readPath := filepath.Join(dir, fmt.Sprintf("block_inputs_%d_read.json", inputJSON.Id)) - Require(t, os.WriteFile(readPath, readData, 0600)) - - writePath := filepath.Join(dir, fmt.Sprintf("block_inputs_%d_write.json", inputJSON.Id)) - Require(t, server_arb.DeserializeAndSerializeFileData(readPath, writePath)) - writeData, err := os.ReadFile(writePath) - Require(t, err) - - return writeData -} - -func TestGoInputJSONRustFileDataRoundtripWithoutUserWasms(t *testing.T) { - preimages := make(map[arbutil.PreimageType]map[common.Hash][]byte) - preimages[arbutil.Keccak256PreimageType] = make(map[common.Hash][]byte) - preimages[arbutil.Keccak256PreimageType][common.MaxHash] = []byte{1} - - // Don't include DebugChain as it isn't used on rust side - sampleValidationInput := &validator.ValidationInput{ - Id: 1, - HasDelayedMsg: true, - DelayedMsgNr: 2, - Preimages: preimages, - BatchInfo: []validator.BatchInfo{{Number: 3}}, - DelayedMsg: []byte{4}, - StartState: validator.GoGlobalState{ - BlockHash: common.MaxHash, - SendRoot: common.MaxHash, - Batch: 5, - PosInBatch: 6, - }, - } - sampleValidationInputJSON := server_api.ValidationInputToJson(sampleValidationInput) - writeData := getWriteDataFromRustSide(t, sampleValidationInputJSON) - - var resWithoutUserWasms server_api.InputJSON - Require(t, json.Unmarshal(writeData, &resWithoutUserWasms)) - if !reflect.DeepEqual(*sampleValidationInputJSON, resWithoutUserWasms) { - t.Fatal("ValidationInputJSON without UserWasms, mismatch on rust and go side") - } - -} - -type inputJSONWithUserWasmsOnly struct { - UserWasms map[ethdb.WasmTarget]map[common.Hash][]byte -} - -// UnmarshalJSON is a custom function defined to encapsulate how UserWasms are handled on the rust side. -// When ValidationInputToJson is called on ValidationInput, it compresses the wasm data byte array and -// then encodes this to a base64 string, this when deserialized on the rust side through FileData- the -// compressed data is first uncompressed and also the module hash (Bytes32) is read without the 0x prefix, -// so we define a custom UnmarshalJSON to extract UserWasms map from the data written by rust side. -func (u *inputJSONWithUserWasmsOnly) UnmarshalJSON(data []byte) error { - type rawUserWasms struct { - UserWasms map[ethdb.WasmTarget]map[string]string - } - var rawUWs rawUserWasms - if err := json.Unmarshal(data, &rawUWs); err != nil { - return err - } - tmp := make(map[ethdb.WasmTarget]map[common.Hash][]byte) - for wasmTarget, innerMap := range rawUWs.UserWasms { - tmp[wasmTarget] = make(map[common.Hash][]byte) - for hashKey, value := range innerMap { - valBytes, err := base64.StdEncoding.DecodeString(value) - if err != nil { - return err - } - tmp[wasmTarget][common.HexToHash("0x"+hashKey)] = valBytes - } - } - u.UserWasms = tmp - return nil -} - -func TestGoInputJSONRustFileDataRoundtripWithUserWasms(t *testing.T) { - userWasms := make(map[ethdb.WasmTarget]map[common.Hash][]byte) - userWasms["arch1"] = make(map[common.Hash][]byte) - userWasms["arch1"][common.MaxHash] = []byte{2} - - // Don't include DebugChain as it isn't used on rust side - sampleValidationInput := &validator.ValidationInput{ - Id: 1, - UserWasms: userWasms, - BatchInfo: []validator.BatchInfo{{Number: 1}}, // This needs to be set for FileData to successfully deserialize, else it errors for invalid type null - } - sampleValidationInputJSON := server_api.ValidationInputToJson(sampleValidationInput) - writeData := getWriteDataFromRustSide(t, sampleValidationInputJSON) - - var resUserWasmsOnly inputJSONWithUserWasmsOnly - Require(t, json.Unmarshal(writeData, &resUserWasmsOnly)) - if !reflect.DeepEqual(userWasms, resUserWasmsOnly.UserWasms) { - t.Fatal("ValidationInputJSON with UserWasms only, mismatch on rust and go side") - } -} diff --git a/validator/server_arb/machine.go b/validator/server_arb/machine.go index d11f015916..1e73e6b212 100644 --- a/validator/server_arb/machine.go +++ b/validator/server_arb/machine.go @@ -312,20 +312,6 @@ func (m *ArbitratorMachine) DeserializeAndReplaceState(path string) error { } } -func DeserializeAndSerializeFileData(readPath, writePath string) error { - cReadPath := C.CString(readPath) - cWritePath := C.CString(writePath) - status := C.arbitrator_deserialize_and_serialize_file_data(cReadPath, cWritePath) - C.free(unsafe.Pointer(cReadPath)) - C.free(unsafe.Pointer(cWritePath)) - - if status != 0 { - return fmt.Errorf("failed to call arbitrator_deserialize_and_serialize_file_data. Error code: %d", status) - } else { - return nil - } -} - func (m *ArbitratorMachine) AddSequencerInboxMessage(index uint64, data []byte) error { defer runtime.KeepAlive(m) m.mutex.Lock() From f0d14fb2f681d4f10c4844bf83141cec1380d98d Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 12:33:35 +0530 Subject: [PATCH 05/18] fix ci step and comment other steps in test(defaults) to speedup verification. will be undone after debug fix env variable setting impl fix gotestsum invocation fix gotestsum invocation fix gotestsum invocation --- .github/workflows/ci.yml | 45 ++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0b5592dafc..bd69e8c905 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,21 +140,21 @@ jobs: echo "GOGC=80" >> "$GITHUB_ENV" echo "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}" >> "$GITHUB_ENV" - - name: run tests without race detection and path state scheme - if: matrix.test-mode == 'defaults' - env: - TEST_STATE_SCHEME: path - run: | - echo "Running tests with Path Scheme" >> full.log - ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover - - - name: run tests without race detection and hash state scheme - if: matrix.test-mode == 'defaults' - env: - TEST_STATE_SCHEME: hash - run: | - echo "Running tests with Hash Scheme" >> full.log - ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m + # - name: run tests without race detection and path state scheme + # if: matrix.test-mode == 'defaults' + # env: + # TEST_STATE_SCHEME: path + # run: | + # echo "Running tests with Path Scheme" >> full.log + # ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover + + # - name: run tests without race detection and hash state scheme + # if: matrix.test-mode == 'defaults' + # env: + # TEST_STATE_SCHEME: hash + # run: | + # echo "Running tests with Hash Scheme" >> full.log + # ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m - name: run tests with race detection and hash state scheme if: matrix.test-mode == 'race' @@ -164,23 +164,22 @@ jobs: echo "Running tests with Hash Scheme" >> full.log ${{ github.workspace }}/.github/workflows/gotestsum.sh --race --timeout 30m - - name: run redis tests - if: matrix.test-mode == 'defaults' - run: | - echo "Running redis tests" >> full.log - TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... + # - name: run redis tests + # if: matrix.test-mode == 'defaults' + # run: | + # echo "Running redis tests" >> full.log + # TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... - name: create block input json file if: matrix.test-mode == 'defaults' run: | - echo "BLOCK_INPUT_JSON_PATH=$(pwd)/target/block_input.json" >> "$GITHUB_ENV" - ${{ github.workspace }}/.github/workflows/gotestsum.sh --run TestProgramStorage$ --count 1 + BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format short-verbose -- -run TestProgramStorage$ --count 1 - name: run arbitrator prover on block input json if: matrix.test-mode == 'defaults' run: | make build-prover-bin - target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs=$BLOCK_INPUT_JSON_PATH + target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_input.json" - name: run challenge tests if: matrix.test-mode == 'challenge' From d50926ed69c02028ddce01bcb32732d0324dd7a7 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 13:44:18 +0530 Subject: [PATCH 06/18] fix gotestsum invocation --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd69e8c905..c194a4936b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,7 +173,7 @@ jobs: - name: create block input json file if: matrix.test-mode == 'defaults' run: | - BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format short-verbose -- -run TestProgramStorage$ --count 1 + BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 - name: run arbitrator prover on block input json if: matrix.test-mode == 'defaults' From a1cb06dfd95c4043234eef0c1eba0fd932af4459 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 13:55:47 +0530 Subject: [PATCH 07/18] uncomment CI steps --- .github/workflows/ci.yml | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c194a4936b..dcb3b9f422 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,21 +140,21 @@ jobs: echo "GOGC=80" >> "$GITHUB_ENV" echo "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}" >> "$GITHUB_ENV" - # - name: run tests without race detection and path state scheme - # if: matrix.test-mode == 'defaults' - # env: - # TEST_STATE_SCHEME: path - # run: | - # echo "Running tests with Path Scheme" >> full.log - # ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover - - # - name: run tests without race detection and hash state scheme - # if: matrix.test-mode == 'defaults' - # env: - # TEST_STATE_SCHEME: hash - # run: | - # echo "Running tests with Hash Scheme" >> full.log - # ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m + - name: run tests without race detection and path state scheme + if: matrix.test-mode == 'defaults' + env: + TEST_STATE_SCHEME: path + run: | + echo "Running tests with Path Scheme" >> full.log + ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover + + - name: run tests without race detection and hash state scheme + if: matrix.test-mode == 'defaults' + env: + TEST_STATE_SCHEME: hash + run: | + echo "Running tests with Hash Scheme" >> full.log + ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m - name: run tests with race detection and hash state scheme if: matrix.test-mode == 'race' @@ -164,11 +164,11 @@ jobs: echo "Running tests with Hash Scheme" >> full.log ${{ github.workspace }}/.github/workflows/gotestsum.sh --race --timeout 30m - # - name: run redis tests - # if: matrix.test-mode == 'defaults' - # run: | - # echo "Running redis tests" >> full.log - # TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... + - name: run redis tests + if: matrix.test-mode == 'defaults' + run: | + echo "Running redis tests" >> full.log + TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... - name: create block input json file if: matrix.test-mode == 'defaults' From 70c5b3f40c4851b682208cdb6a645a84eca25160 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 14:56:05 +0530 Subject: [PATCH 08/18] add ci step to run jit binary on the block input json file --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcb3b9f422..6722b38ed6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -180,6 +180,12 @@ jobs: run: | make build-prover-bin target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_input.json" + + - name: run jit prover on block input json + if: matrix.test-mode == 'defaults' + run: | + make build-jit + target/bin/jit --binary target/machines/latest/replay.wasm --cranelift --json-inputs="${{ github.workspace }}/target/block_input.json" - name: run challenge tests if: matrix.test-mode == 'challenge' From 62d76d08d653e846aff3b5275c207d9aa2223db3 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 16:02:06 +0530 Subject: [PATCH 09/18] check if theres a local target mismatch on go and rust side --- .github/workflows/ci.yml | 52 +++++++++++++++++------------------ arbitrator/jit/src/prepare.rs | 2 +- system_tests/program_test.go | 1 + 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6722b38ed6..d50fe5da59 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,21 +140,21 @@ jobs: echo "GOGC=80" >> "$GITHUB_ENV" echo "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}" >> "$GITHUB_ENV" - - name: run tests without race detection and path state scheme - if: matrix.test-mode == 'defaults' - env: - TEST_STATE_SCHEME: path - run: | - echo "Running tests with Path Scheme" >> full.log - ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover - - - name: run tests without race detection and hash state scheme - if: matrix.test-mode == 'defaults' - env: - TEST_STATE_SCHEME: hash - run: | - echo "Running tests with Hash Scheme" >> full.log - ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m + # - name: run tests without race detection and path state scheme + # if: matrix.test-mode == 'defaults' + # env: + # TEST_STATE_SCHEME: path + # run: | + # echo "Running tests with Path Scheme" >> full.log + # ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover + + # - name: run tests without race detection and hash state scheme + # if: matrix.test-mode == 'defaults' + # env: + # TEST_STATE_SCHEME: hash + # run: | + # echo "Running tests with Hash Scheme" >> full.log + # ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m - name: run tests with race detection and hash state scheme if: matrix.test-mode == 'race' @@ -164,22 +164,22 @@ jobs: echo "Running tests with Hash Scheme" >> full.log ${{ github.workspace }}/.github/workflows/gotestsum.sh --race --timeout 30m - - name: run redis tests - if: matrix.test-mode == 'defaults' - run: | - echo "Running redis tests" >> full.log - TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... + # - name: run redis tests + # if: matrix.test-mode == 'defaults' + # run: | + # echo "Running redis tests" >> full.log + # TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... - name: create block input json file if: matrix.test-mode == 'defaults' run: | - BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 + BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format standard-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 - - name: run arbitrator prover on block input json - if: matrix.test-mode == 'defaults' - run: | - make build-prover-bin - target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_input.json" + # - name: run arbitrator prover on block input json + # if: matrix.test-mode == 'defaults' + # run: | + # make build-prover-bin + # target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_input.json" - name: run jit prover on block input json if: matrix.test-mode == 'defaults' diff --git a/arbitrator/jit/src/prepare.rs b/arbitrator/jit/src/prepare.rs index 6c1b0b84de..b766936f3e 100644 --- a/arbitrator/jit/src/prepare.rs +++ b/arbitrator/jit/src/prepare.rs @@ -68,6 +68,6 @@ pub fn prepare_env(json_inputs: PathBuf, debug: bool) -> eyre::Result { .insert(*module_hash, module_asm.as_vec().into()); } } - + eprintln!("localTarget {}", local_target()); Ok(env) } diff --git a/system_tests/program_test.go b/system_tests/program_test.go index 92aeddaf53..1f195630a6 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -429,6 +429,7 @@ func storageTest(t *testing.T, jit bool) { if blockInputJSONPath != "" { recordBlock(t, receipt.BlockNumber.Uint64(), builder, []ethdb.WasmTarget{rawdb.TargetWavm, rawdb.LocalTarget()}, blockInputJSONPath) } + log.Info("printing debug info", "localTarget", rawdb.LocalTarget()) } func TestProgramTransientStorage(t *testing.T) { From 93fca3d126de1e64fefaf1a1ce1223b5b0fbf12c Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 16:17:02 +0530 Subject: [PATCH 10/18] add more debug info --- arbitrator/jit/src/prepare.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arbitrator/jit/src/prepare.rs b/arbitrator/jit/src/prepare.rs index b766936f3e..68cdcd3479 100644 --- a/arbitrator/jit/src/prepare.rs +++ b/arbitrator/jit/src/prepare.rs @@ -68,6 +68,13 @@ pub fn prepare_env(json_inputs: PathBuf, debug: bool) -> eyre::Result { .insert(*module_hash, module_asm.as_vec().into()); } } + + eprintln!( + "env Info. OS- {}, Arch- {}", + env::consts::OS, + env::consts::ARCH + ); eprintln!("localTarget {}", local_target()); + Ok(env) } From 5e355bc623e30b1ccc787bead4c6161ccf301639 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 16:33:10 +0530 Subject: [PATCH 11/18] check if jit proving succeeds in CI --- .github/workflows/ci.yml | 7 +++++-- arbitrator/jit/src/prepare.rs | 11 ++--------- system_tests/program_test.go | 1 - 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d50fe5da59..40215fa163 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,7 +173,7 @@ jobs: - name: create block input json file if: matrix.test-mode == 'defaults' run: | - BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format standard-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 + BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 # - name: run arbitrator prover on block input json # if: matrix.test-mode == 'defaults' @@ -185,7 +185,10 @@ jobs: if: matrix.test-mode == 'defaults' run: | make build-jit - target/bin/jit --binary target/machines/latest/replay.wasm --cranelift --json-inputs="${{ github.workspace }}/target/block_input.json" + if [ -n "$(target/bin/jit --binary target/machines/latest/replay.wasm --cranelift --json-inputs='${{ github.workspace }}/target/block_input.json')" ]; then + echo "Error: Command produced output." + exit 1 + fi - name: run challenge tests if: matrix.test-mode == 'challenge' diff --git a/arbitrator/jit/src/prepare.rs b/arbitrator/jit/src/prepare.rs index 68cdcd3479..e7a7ba0f4d 100644 --- a/arbitrator/jit/src/prepare.rs +++ b/arbitrator/jit/src/prepare.rs @@ -16,8 +16,8 @@ use std::path::PathBuf; pub fn local_target() -> String { if env::consts::OS == "linux" { match env::consts::ARCH { - "arm64" => "arm64".to_string(), - "amd64" => "amd64".to_string(), + "aarch64" => "arm64".to_string(), + "x86_64" => "amd64".to_string(), _ => "host".to_string(), } } else { @@ -69,12 +69,5 @@ pub fn prepare_env(json_inputs: PathBuf, debug: bool) -> eyre::Result { } } - eprintln!( - "env Info. OS- {}, Arch- {}", - env::consts::OS, - env::consts::ARCH - ); - eprintln!("localTarget {}", local_target()); - Ok(env) } diff --git a/system_tests/program_test.go b/system_tests/program_test.go index 1f195630a6..92aeddaf53 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -429,7 +429,6 @@ func storageTest(t *testing.T, jit bool) { if blockInputJSONPath != "" { recordBlock(t, receipt.BlockNumber.Uint64(), builder, []ethdb.WasmTarget{rawdb.TargetWavm, rawdb.LocalTarget()}, blockInputJSONPath) } - log.Info("printing debug info", "localTarget", rawdb.LocalTarget()) } func TestProgramTransientStorage(t *testing.T) { From 7259ae122d694f08b005d8efc0a461f8314d729e Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 16:51:02 +0530 Subject: [PATCH 12/18] remove debug statements and comments --- .github/workflows/ci.yml | 50 ++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 40215fa163..f0f44744f5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,21 +140,21 @@ jobs: echo "GOGC=80" >> "$GITHUB_ENV" echo "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}" >> "$GITHUB_ENV" - # - name: run tests without race detection and path state scheme - # if: matrix.test-mode == 'defaults' - # env: - # TEST_STATE_SCHEME: path - # run: | - # echo "Running tests with Path Scheme" >> full.log - # ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover - - # - name: run tests without race detection and hash state scheme - # if: matrix.test-mode == 'defaults' - # env: - # TEST_STATE_SCHEME: hash - # run: | - # echo "Running tests with Hash Scheme" >> full.log - # ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m + - name: run tests without race detection and path state scheme + if: matrix.test-mode == 'defaults' + env: + TEST_STATE_SCHEME: path + run: | + echo "Running tests with Path Scheme" >> full.log + ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover + + - name: run tests without race detection and hash state scheme + if: matrix.test-mode == 'defaults' + env: + TEST_STATE_SCHEME: hash + run: | + echo "Running tests with Hash Scheme" >> full.log + ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m - name: run tests with race detection and hash state scheme if: matrix.test-mode == 'race' @@ -164,22 +164,22 @@ jobs: echo "Running tests with Hash Scheme" >> full.log ${{ github.workspace }}/.github/workflows/gotestsum.sh --race --timeout 30m - # - name: run redis tests - # if: matrix.test-mode == 'defaults' - # run: | - # echo "Running redis tests" >> full.log - # TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... + - name: run redis tests + if: matrix.test-mode == 'defaults' + run: | + echo "Running redis tests" >> full.log + TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... - name: create block input json file if: matrix.test-mode == 'defaults' run: | BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 - # - name: run arbitrator prover on block input json - # if: matrix.test-mode == 'defaults' - # run: | - # make build-prover-bin - # target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_input.json" + - name: run arbitrator prover on block input json + if: matrix.test-mode == 'defaults' + run: | + make build-prover-bin + target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_input.json" - name: run jit prover on block input json if: matrix.test-mode == 'defaults' From bb5e8b7456a491901e68fa9e8e8213ec240648b9 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 10 Oct 2024 21:24:46 +0530 Subject: [PATCH 13/18] address PR comments --- arbnode/api.go | 2 +- staker/stateless_block_validator.go | 2 +- system_tests/common_test.go | 4 ++-- system_tests/program_test.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arbnode/api.go b/arbnode/api.go index 8afb78770b..2dabd41bff 100644 --- a/arbnode/api.go +++ b/arbnode/api.go @@ -59,5 +59,5 @@ func (a *BlockValidatorDebugAPI) ValidateMessageNumber( func (a *BlockValidatorDebugAPI) ValidationInputsAt(ctx context.Context, msgNum hexutil.Uint64, target ethdb.WasmTarget, ) (server_api.InputJSON, error) { - return a.val.ValidationInputsAt(ctx, arbutil.MessageIndex(msgNum), []ethdb.WasmTarget{target}) + return a.val.ValidationInputsAt(ctx, arbutil.MessageIndex(msgNum), target) } diff --git a/staker/stateless_block_validator.go b/staker/stateless_block_validator.go index 786cf3aa94..d9c9c5446b 100644 --- a/staker/stateless_block_validator.go +++ b/staker/stateless_block_validator.go @@ -511,7 +511,7 @@ func (v *StatelessBlockValidator) ValidateResult( return true, &entry.End, nil } -func (v *StatelessBlockValidator) ValidationInputsAt(ctx context.Context, pos arbutil.MessageIndex, targets []ethdb.WasmTarget) (server_api.InputJSON, error) { +func (v *StatelessBlockValidator) ValidationInputsAt(ctx context.Context, pos arbutil.MessageIndex, targets ...ethdb.WasmTarget) (server_api.InputJSON, error) { entry, err := v.CreateReadyValidationEntry(ctx, pos) if err != nil { return server_api.InputJSON{}, err diff --git a/system_tests/common_test.go b/system_tests/common_test.go index 287882655f..6bc3fe4af8 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -1718,7 +1718,7 @@ func logParser[T any](t *testing.T, source string, name string) func(*types.Log) // recordBlock writes a json file with all of the data needed to validate a block. // // This can be used as an input to the arbitrator prover to validate a block. -func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, targets []ethdb.WasmTarget, blockInputJSONPath string) { +func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, blockInputJSONPath string, targets ...ethdb.WasmTarget) { t.Helper() ctx := builder.ctx inboxPos := arbutil.MessageIndex(block) @@ -1732,7 +1732,7 @@ func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, targets []eth break } } - inputJson, err := builder.L2.ConsensusNode.StatelessBlockValidator.ValidationInputsAt(ctx, inboxPos, targets) + inputJson, err := builder.L2.ConsensusNode.StatelessBlockValidator.ValidationInputsAt(ctx, inboxPos, targets...) if err != nil { Fatal(t, "failed to get validation inputs", block, err) } diff --git a/system_tests/program_test.go b/system_tests/program_test.go index 92aeddaf53..9d5b173fc3 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -427,7 +427,7 @@ func storageTest(t *testing.T, jit bool) { // storage write transaction. Include wasm targets necessary for arbitrator prover and jit binaries blockInputJSONPath := os.Getenv("BLOCK_INPUT_JSON_PATH") if blockInputJSONPath != "" { - recordBlock(t, receipt.BlockNumber.Uint64(), builder, []ethdb.WasmTarget{rawdb.TargetWavm, rawdb.LocalTarget()}, blockInputJSONPath) + recordBlock(t, receipt.BlockNumber.Uint64(), builder, blockInputJSONPath, rawdb.TargetWavm, rawdb.LocalTarget()) } } From f83a10fd3d98b25d831c8a94df74003f26b0dd35 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Fri, 11 Oct 2024 19:22:27 +0530 Subject: [PATCH 14/18] use inputs.Writer to generate the json file --- .github/workflows/ci.yml | 6 ++--- system_tests/common_test.go | 16 +++++++++---- system_tests/program_test.go | 9 ++++--- validator/inputs/writer.go | 46 +++++++++++++++++++++++++----------- 4 files changed, 52 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f0f44744f5..c1ca8aa698 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,19 +173,19 @@ jobs: - name: create block input json file if: matrix.test-mode == 'defaults' run: | - BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 + gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 --validator.inputswriter.basedir=""${{ github.workspace }}/target" - name: run arbitrator prover on block input json if: matrix.test-mode == 'defaults' run: | make build-prover-bin - target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_input.json" + target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_inputs.json" - name: run jit prover on block input json if: matrix.test-mode == 'defaults' run: | make build-jit - if [ -n "$(target/bin/jit --binary target/machines/latest/replay.wasm --cranelift --json-inputs='${{ github.workspace }}/target/block_input.json')" ]; then + if [ -n "$(target/bin/jit --binary target/machines/latest/replay.wasm --cranelift --json-inputs='${{ github.workspace }}/target/block_inputs.json')" ]; then echo "Error: Command produced output." exit 1 fi diff --git a/system_tests/common_test.go b/system_tests/common_test.go index 6bc3fe4af8..189e83c965 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -36,6 +36,7 @@ import ( "github.com/offchainlabs/nitro/util/headerreader" "github.com/offchainlabs/nitro/util/redisutil" "github.com/offchainlabs/nitro/util/signature" + "github.com/offchainlabs/nitro/validator/inputs" "github.com/offchainlabs/nitro/validator/server_api" "github.com/offchainlabs/nitro/validator/server_common" "github.com/offchainlabs/nitro/validator/valnode" @@ -1718,7 +1719,7 @@ func logParser[T any](t *testing.T, source string, name string) func(*types.Log) // recordBlock writes a json file with all of the data needed to validate a block. // // This can be used as an input to the arbitrator prover to validate a block. -func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, blockInputJSONPath string, targets ...ethdb.WasmTarget) { +func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, baseDirectory string, targets ...ethdb.WasmTarget) { t.Helper() ctx := builder.ctx inboxPos := arbutil.MessageIndex(block) @@ -1732,14 +1733,19 @@ func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, blockInputJSO break } } + validationInputsWriter, err := inputs.NewWriter( + inputs.WithBaseDir(baseDirectory), + inputs.WithTimestampDirEnabled(false), + inputs.WithBlockIdInFileNameEnabled(false), + ) + Require(t, err) inputJson, err := builder.L2.ConsensusNode.StatelessBlockValidator.ValidationInputsAt(ctx, inboxPos, targets...) if err != nil { Fatal(t, "failed to get validation inputs", block, err) } - contents, err := json.Marshal(inputJson) - Require(t, err) - err = os.WriteFile(blockInputJSONPath, contents, 0600) - Require(t, err) + if err := validationInputsWriter.Write(&inputJson); err != nil { + Fatal(t, "failed to write validation inputs", block, err) + } } func populateMachineDir(t *testing.T, cr *github.ConsensusRelease) string { diff --git a/system_tests/program_test.go b/system_tests/program_test.go index 9d5b173fc3..d4c64d7436 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/binary" + "flag" "fmt" "math" "math/big" @@ -393,6 +394,8 @@ func errorTest(t *testing.T, jit bool) { validateBlocks(t, 7, jit, builder) } +var validatorInputsWriterBaseDir = flag.String("validator.inputswriter.basedir", "", "Base directory for validationInputsWriter") + func TestProgramStorage(t *testing.T) { t.Parallel() storageTest(t, true) @@ -425,9 +428,9 @@ func storageTest(t *testing.T, jit bool) { // Captures a block_input_.json file for the block that included the // storage write transaction. Include wasm targets necessary for arbitrator prover and jit binaries - blockInputJSONPath := os.Getenv("BLOCK_INPUT_JSON_PATH") - if blockInputJSONPath != "" { - recordBlock(t, receipt.BlockNumber.Uint64(), builder, blockInputJSONPath, rawdb.TargetWavm, rawdb.LocalTarget()) + flag.Parse() + if *validatorInputsWriterBaseDir != "" { + recordBlock(t, receipt.BlockNumber.Uint64(), builder, *validatorInputsWriterBaseDir, rawdb.TargetWavm, rawdb.LocalTarget()) } } diff --git a/validator/inputs/writer.go b/validator/inputs/writer.go index a45e584f52..fc7456b8bf 100644 --- a/validator/inputs/writer.go +++ b/validator/inputs/writer.go @@ -17,20 +17,25 @@ import ( // // The path can be nested under a slug directory so callers can provide a // recognizable name to differentiate various contexts in which the InputJSON -// is being written. If the Writer is configured by calling SetSlug, then the +// is being written. If the Writer is configured by calling WithSlug, then the // path will be like: // // $HOME/.arbuitrum/validation-inputs///block_inputs_.json // +// The inclusion of BlockId in the file's name is on by default, however that can be disabled +// by calling WithBlockIdInFileNameEnabled(false). In which case, the path will be like: +// +// $HOME/.arbuitrum/validation-inputs///block_inputs.json +// // The inclusion of a timestamp directory is on by default to avoid conflicts which // would result in files being overwritten. However, the Writer can be configured // to not use a timestamp directory. If the Writer is configured by calling -// SetUseTimestampDir(false), then the path will be like: +// WithTimestampDirEnabled(false), then the path will be like: // // $HOME/.arbuitrum/validation-inputs//block_inputs_.json // // Finally, to give complete control to the clients, the base directory can be -// set directly with SetBaseDir. In which case, the path will be like: +// set directly with WithBaseDir. In which case, the path will be like: // // /block_inputs_.json // or @@ -38,10 +43,11 @@ import ( // or // ///block_inputs_.json type Writer struct { - clock Clock - baseDir string - slug string - useTimestampDir bool + clock Clock + baseDir string + slug string + useTimestampDir bool + useBlockIdInFileName bool } // WriterOption is a function that configures a Writer. @@ -66,10 +72,11 @@ func NewWriter(options ...WriterOption) (*Writer, error) { } baseDir := filepath.Join(homeDir, ".arbitrum", "validation-inputs") w := &Writer{ - clock: realClock{}, - baseDir: baseDir, - slug: "", - useTimestampDir: true, + clock: realClock{}, + baseDir: baseDir, + slug: "", + useTimestampDir: true, + useBlockIdInFileName: true, } for _, o := range options { o(w) @@ -114,6 +121,13 @@ func WithTimestampDirEnabled(useTimestampDir bool) WriterOption { } } +// WithBlockIdInFileNameEnabled controls the inclusion of Block Id in the input json file's name +func WithBlockIdInFileNameEnabled(useBlockIdInFileName bool) WriterOption { + return func(w *Writer) { + w.useBlockIdInFileName = useBlockIdInFileName + } +} + // Write writes the given InputJSON to a file in JSON format. func (w *Writer) Write(json *server_api.InputJSON) error { dir := w.baseDir @@ -132,9 +146,13 @@ func (w *Writer) Write(json *server_api.InputJSON) error { if err != nil { return err } - if err = os.WriteFile( - filepath.Join(dir, fmt.Sprintf("block_inputs_%d.json", json.Id)), - contents, 0600); err != nil { + var fileName string + if w.useBlockIdInFileName { + fileName = filepath.Join(dir, fmt.Sprintf("block_inputs_%d.json", json.Id)) + } else { + fileName = filepath.Join(dir, "block_inputs.json") + } + if err = os.WriteFile(fileName, contents, 0600); err != nil { return err } return nil From 05be76e4686254d9070884a6f221b535ff75e2de Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Fri, 11 Oct 2024 19:31:39 +0530 Subject: [PATCH 15/18] use inputs.Writer to generate the json file --- .github/workflows/ci.yml | 4 ++-- system_tests/common_test.go | 16 +++++++++---- system_tests/program_test.go | 11 +++++---- validator/inputs/writer.go | 46 +++++++++++++++++++++++++----------- 4 files changed, 52 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcb3b9f422..671615233a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,13 +173,13 @@ jobs: - name: create block input json file if: matrix.test-mode == 'defaults' run: | - BLOCK_INPUT_JSON_PATH="${{ github.workspace }}/target/block_input.json" gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 + gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 --validator.inputswriter.basedir=""${{ github.workspace }}/target" - name: run arbitrator prover on block input json if: matrix.test-mode == 'defaults' run: | make build-prover-bin - target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_input.json" + target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_inputs.json" - name: run challenge tests if: matrix.test-mode == 'challenge' diff --git a/system_tests/common_test.go b/system_tests/common_test.go index 95e8883e6b..f4e2edad42 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -36,6 +36,7 @@ import ( "github.com/offchainlabs/nitro/util/headerreader" "github.com/offchainlabs/nitro/util/redisutil" "github.com/offchainlabs/nitro/util/signature" + "github.com/offchainlabs/nitro/validator/inputs" "github.com/offchainlabs/nitro/validator/server_api" "github.com/offchainlabs/nitro/validator/server_common" "github.com/offchainlabs/nitro/validator/valnode" @@ -1718,7 +1719,7 @@ func logParser[T any](t *testing.T, source string, name string) func(*types.Log) // recordBlock writes a json file with all of the data needed to validate a block. // // This can be used as an input to the arbitrator prover to validate a block. -func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, blockInputJSONPath string) { +func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, baseDirectory string) { t.Helper() ctx := builder.ctx inboxPos := arbutil.MessageIndex(block) @@ -1732,14 +1733,19 @@ func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, blockInputJSO break } } + validationInputsWriter, err := inputs.NewWriter( + inputs.WithBaseDir(baseDirectory), + inputs.WithTimestampDirEnabled(false), + inputs.WithBlockIdInFileNameEnabled(false), + ) + Require(t, err) inputJson, err := builder.L2.ConsensusNode.StatelessBlockValidator.ValidationInputsAt(ctx, inboxPos, rawdb.TargetWavm) if err != nil { Fatal(t, "failed to get validation inputs", block, err) } - contents, err := json.Marshal(inputJson) - Require(t, err) - err = os.WriteFile(blockInputJSONPath, contents, 0600) - Require(t, err) + if err := validationInputsWriter.Write(&inputJson); err != nil { + Fatal(t, "failed to write validation inputs", block, err) + } } func populateMachineDir(t *testing.T, cr *github.ConsensusRelease) string { diff --git a/system_tests/program_test.go b/system_tests/program_test.go index d2c3336af7..d007a40ddd 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/binary" + "flag" "fmt" "math" "math/big" @@ -393,6 +394,8 @@ func errorTest(t *testing.T, jit bool) { validateBlocks(t, 7, jit, builder) } +var validatorInputsWriterBaseDir = flag.String("validator.inputswriter.basedir", "", "Base directory for validationInputsWriter") + func TestProgramStorage(t *testing.T) { t.Parallel() storageTest(t, true) @@ -424,10 +427,10 @@ func storageTest(t *testing.T, jit bool) { validateBlocks(t, 2, jit, builder) // Captures a block_input_.json file for the block that included the - // storage write transaction. - blockInputJSONPath := os.Getenv("BLOCK_INPUT_JSON_PATH") - if blockInputJSONPath != "" { - recordBlock(t, receipt.BlockNumber.Uint64(), builder, blockInputJSONPath) + // storage write transaction. Include wasm targets necessary for arbitrator prover and jit binaries + flag.Parse() + if *validatorInputsWriterBaseDir != "" { + recordBlock(t, receipt.BlockNumber.Uint64(), builder, *validatorInputsWriterBaseDir) } } diff --git a/validator/inputs/writer.go b/validator/inputs/writer.go index a45e584f52..fc7456b8bf 100644 --- a/validator/inputs/writer.go +++ b/validator/inputs/writer.go @@ -17,20 +17,25 @@ import ( // // The path can be nested under a slug directory so callers can provide a // recognizable name to differentiate various contexts in which the InputJSON -// is being written. If the Writer is configured by calling SetSlug, then the +// is being written. If the Writer is configured by calling WithSlug, then the // path will be like: // // $HOME/.arbuitrum/validation-inputs///block_inputs_.json // +// The inclusion of BlockId in the file's name is on by default, however that can be disabled +// by calling WithBlockIdInFileNameEnabled(false). In which case, the path will be like: +// +// $HOME/.arbuitrum/validation-inputs///block_inputs.json +// // The inclusion of a timestamp directory is on by default to avoid conflicts which // would result in files being overwritten. However, the Writer can be configured // to not use a timestamp directory. If the Writer is configured by calling -// SetUseTimestampDir(false), then the path will be like: +// WithTimestampDirEnabled(false), then the path will be like: // // $HOME/.arbuitrum/validation-inputs//block_inputs_.json // // Finally, to give complete control to the clients, the base directory can be -// set directly with SetBaseDir. In which case, the path will be like: +// set directly with WithBaseDir. In which case, the path will be like: // // /block_inputs_.json // or @@ -38,10 +43,11 @@ import ( // or // ///block_inputs_.json type Writer struct { - clock Clock - baseDir string - slug string - useTimestampDir bool + clock Clock + baseDir string + slug string + useTimestampDir bool + useBlockIdInFileName bool } // WriterOption is a function that configures a Writer. @@ -66,10 +72,11 @@ func NewWriter(options ...WriterOption) (*Writer, error) { } baseDir := filepath.Join(homeDir, ".arbitrum", "validation-inputs") w := &Writer{ - clock: realClock{}, - baseDir: baseDir, - slug: "", - useTimestampDir: true, + clock: realClock{}, + baseDir: baseDir, + slug: "", + useTimestampDir: true, + useBlockIdInFileName: true, } for _, o := range options { o(w) @@ -114,6 +121,13 @@ func WithTimestampDirEnabled(useTimestampDir bool) WriterOption { } } +// WithBlockIdInFileNameEnabled controls the inclusion of Block Id in the input json file's name +func WithBlockIdInFileNameEnabled(useBlockIdInFileName bool) WriterOption { + return func(w *Writer) { + w.useBlockIdInFileName = useBlockIdInFileName + } +} + // Write writes the given InputJSON to a file in JSON format. func (w *Writer) Write(json *server_api.InputJSON) error { dir := w.baseDir @@ -132,9 +146,13 @@ func (w *Writer) Write(json *server_api.InputJSON) error { if err != nil { return err } - if err = os.WriteFile( - filepath.Join(dir, fmt.Sprintf("block_inputs_%d.json", json.Id)), - contents, 0600); err != nil { + var fileName string + if w.useBlockIdInFileName { + fileName = filepath.Join(dir, fmt.Sprintf("block_inputs_%d.json", json.Id)) + } else { + fileName = filepath.Join(dir, "block_inputs.json") + } + if err = os.WriteFile(fileName, contents, 0600); err != nil { return err } return nil From ceb785ef3237e85ce6a036855034d2be2682574a Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Fri, 11 Oct 2024 20:12:46 +0530 Subject: [PATCH 16/18] typo fix --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 671615233a..660f9f4dea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,7 +173,7 @@ jobs: - name: create block input json file if: matrix.test-mode == 'defaults' run: | - gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 --validator.inputswriter.basedir=""${{ github.workspace }}/target" + gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 --validator.inputswriter.basedir="${{ github.workspace }}/target" - name: run arbitrator prover on block input json if: matrix.test-mode == 'defaults' From eb2c5be47bca0f1089a61a1365884fbec17a3c02 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Tue, 15 Oct 2024 14:55:06 +0530 Subject: [PATCH 17/18] address PR comments --- .github/workflows/ci.yml | 4 ++-- system_tests/common_test.go | 28 ++++++++++++++++++++++------ system_tests/program_test.go | 8 +++----- validator/inputs/writer.go | 8 +++----- 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 660f9f4dea..b4966a9e95 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,13 +173,13 @@ jobs: - name: create block input json file if: matrix.test-mode == 'defaults' run: | - gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 --validator.inputswriter.basedir="${{ github.workspace }}/target" + gotestsum --format short-verbose -- -run TestProgramStorage$ ./system_tests/... --count 1 --recordBlockInputs.WithBaseDir="${{ github.workspace }}/target" --recordBlockInputs.WithTimestampDirEnabled=false --recordBlockInputs.WithBlockIdInFileNameEnabled=false - name: run arbitrator prover on block input json if: matrix.test-mode == 'defaults' run: | make build-prover-bin - target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/block_inputs.json" + target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs="${{ github.workspace }}/target/TestProgramStorage/block_inputs.json" - name: run challenge tests if: matrix.test-mode == 'challenge' diff --git a/system_tests/common_test.go b/system_tests/common_test.go index f4e2edad42..7700ec28e4 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -9,6 +9,7 @@ import ( "encoding/binary" "encoding/hex" "encoding/json" + "flag" "io" "math/big" "net" @@ -1716,10 +1717,18 @@ func logParser[T any](t *testing.T, source string, name string) func(*types.Log) } } +var ( + recordBlockInputsEnable = flag.Bool("recordBlockInputs.enable", true, "Whether to record block inputs as a json file") + recordBlockInputsWithSlug = flag.String("recordBlockInputs.WithSlug", "", "Slug directory for validationInputsWriter") + recordBlockInputsWithBaseDir = flag.String("recordBlockInputs.WithBaseDir", "", "Base directory for validationInputsWriter") + recordBlockInputsWithTimestampDirEnabled = flag.Bool("recordBlockInputs.WithTimestampDirEnabled", true, "Whether to add timestamp directory while recording block inputs") + recordBlockInputsWithBlockIdInFileNameEnabled = flag.Bool("recordBlockInputs.WithBlockIdInFileNameEnabled", true, "Whether to record block inputs using test specific block_id") +) + // recordBlock writes a json file with all of the data needed to validate a block. // // This can be used as an input to the arbitrator prover to validate a block. -func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, baseDirectory string) { +func recordBlock(t *testing.T, block uint64, builder *NodeBuilder) { t.Helper() ctx := builder.ctx inboxPos := arbutil.MessageIndex(block) @@ -1733,11 +1742,18 @@ func recordBlock(t *testing.T, block uint64, builder *NodeBuilder, baseDirectory break } } - validationInputsWriter, err := inputs.NewWriter( - inputs.WithBaseDir(baseDirectory), - inputs.WithTimestampDirEnabled(false), - inputs.WithBlockIdInFileNameEnabled(false), - ) + var options []inputs.WriterOption + options = append(options, inputs.WithTimestampDirEnabled(*recordBlockInputsWithTimestampDirEnabled)) + options = append(options, inputs.WithBlockIdInFileNameEnabled(*recordBlockInputsWithBlockIdInFileNameEnabled)) + if *recordBlockInputsWithBaseDir != "" { + options = append(options, inputs.WithBaseDir(*recordBlockInputsWithBaseDir)) + } + if *recordBlockInputsWithSlug != "" { + options = append(options, inputs.WithSlug(*recordBlockInputsWithSlug)) + } else { + options = append(options, inputs.WithSlug(t.Name())) + } + validationInputsWriter, err := inputs.NewWriter(options...) Require(t, err) inputJson, err := builder.L2.ConsensusNode.StatelessBlockValidator.ValidationInputsAt(ctx, inboxPos, rawdb.TargetWavm) if err != nil { diff --git a/system_tests/program_test.go b/system_tests/program_test.go index d007a40ddd..d6324df301 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -394,8 +394,6 @@ func errorTest(t *testing.T, jit bool) { validateBlocks(t, 7, jit, builder) } -var validatorInputsWriterBaseDir = flag.String("validator.inputswriter.basedir", "", "Base directory for validationInputsWriter") - func TestProgramStorage(t *testing.T) { t.Parallel() storageTest(t, true) @@ -426,11 +424,11 @@ func storageTest(t *testing.T, jit bool) { validateBlocks(t, 2, jit, builder) - // Captures a block_input_.json file for the block that included the + // Captures a block_inputs json file for the block that included the // storage write transaction. Include wasm targets necessary for arbitrator prover and jit binaries flag.Parse() - if *validatorInputsWriterBaseDir != "" { - recordBlock(t, receipt.BlockNumber.Uint64(), builder, *validatorInputsWriterBaseDir) + if *recordBlockInputsEnable { + recordBlock(t, receipt.BlockNumber.Uint64(), builder) } } diff --git a/validator/inputs/writer.go b/validator/inputs/writer.go index fc7456b8bf..1a476c52a3 100644 --- a/validator/inputs/writer.go +++ b/validator/inputs/writer.go @@ -146,13 +146,11 @@ func (w *Writer) Write(json *server_api.InputJSON) error { if err != nil { return err } - var fileName string + fileName := "block_inputs.json" if w.useBlockIdInFileName { - fileName = filepath.Join(dir, fmt.Sprintf("block_inputs_%d.json", json.Id)) - } else { - fileName = filepath.Join(dir, "block_inputs.json") + fileName = fmt.Sprintf("block_inputs_%d.json", json.Id) } - if err = os.WriteFile(fileName, contents, 0600); err != nil { + if err = os.WriteFile(filepath.Join(dir, fileName), contents, 0600); err != nil { return err } return nil From 2b1d7a26cd557333183f4358f7180ecb0d74aff5 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Tue, 15 Oct 2024 19:22:20 +0530 Subject: [PATCH 18/18] prioritize test flag over the test's decision to record a block --- system_tests/common_test.go | 4 ++++ system_tests/program_test.go | 6 +----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/system_tests/common_test.go b/system_tests/common_test.go index 7700ec28e4..274e368b74 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -1730,6 +1730,10 @@ var ( // This can be used as an input to the arbitrator prover to validate a block. func recordBlock(t *testing.T, block uint64, builder *NodeBuilder) { t.Helper() + flag.Parse() + if !*recordBlockInputsEnable { + return + } ctx := builder.ctx inboxPos := arbutil.MessageIndex(block) for { diff --git a/system_tests/program_test.go b/system_tests/program_test.go index d6324df301..9aac1eb589 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -7,7 +7,6 @@ import ( "bytes" "context" "encoding/binary" - "flag" "fmt" "math" "math/big" @@ -426,10 +425,7 @@ func storageTest(t *testing.T, jit bool) { // Captures a block_inputs json file for the block that included the // storage write transaction. Include wasm targets necessary for arbitrator prover and jit binaries - flag.Parse() - if *recordBlockInputsEnable { - recordBlock(t, receipt.BlockNumber.Uint64(), builder) - } + recordBlock(t, receipt.BlockNumber.Uint64(), builder) } func TestProgramTransientStorage(t *testing.T) {