From 367546b82cd5f133b956857bf48d279512b157b2 Mon Sep 17 00:00:00 2001 From: Mike Voronov Date: Tue, 7 Feb 2023 21:07:02 +0300 Subject: [PATCH] feat(trace-handler): improve data deserialization version check (#451) --- Cargo.lock | 2 +- air/src/preparation_step/errors.rs | 59 +++++++- ...ted_version.rs => interpreter_versions.rs} | 11 +- air/src/preparation_step/mod.rs | 5 +- air/src/preparation_step/preparation.rs | 22 ++- .../features/data_merging/data_merge.rs | 3 +- .../features/misc/version_check.rs | 3 +- air/tests/test_module/main.rs | 1 + air/tests/test_module/negative_tests/mod.rs | 17 +++ .../negative_tests/preparation_step.rs | 76 ++++++++++ crates/air-lib/interpreter-data/CHANGELOG.md | 5 + crates/air-lib/interpreter-data/Cargo.toml | 2 +- .../interpreter-data/src/interpreter_data.rs | 135 +++++++++--------- crates/air-lib/test-utils/src/lib.rs | 7 - 14 files changed, 250 insertions(+), 98 deletions(-) rename air/src/preparation_step/{minimal_supported_version.rs => interpreter_versions.rs} (73%) create mode 100644 air/tests/test_module/negative_tests/mod.rs create mode 100644 air/tests/test_module/negative_tests/preparation_step.rs diff --git a/Cargo.lock b/Cargo.lock index 037dbc99..677ea82e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -111,7 +111,7 @@ dependencies = [ [[package]] name = "air-interpreter-data" -version = "0.6.0" +version = "0.6.1" dependencies = [ "air-interpreter-cid", "air-interpreter-interface", diff --git a/air/src/preparation_step/errors.rs b/air/src/preparation_step/errors.rs index 2a5c8b99..3eb0b953 100644 --- a/air/src/preparation_step/errors.rs +++ b/air/src/preparation_step/errors.rs @@ -16,6 +16,7 @@ use crate::ToErrorCode; use air_interpreter_data::data_version; +use air_interpreter_data::Versions; use serde_json::Error as SerdeJsonError; use strum::IntoEnumIterator; @@ -33,19 +34,42 @@ pub enum PreparationError { /// Errors occurred on executed trace deserialization. #[error( - "an error occurred while executed trace deserialization: {0:?}.\ - Probably it's a data of an old version that can't be converted to '{}'.\n\ - Data: {1:?}", + "an error occurred while data deserialization: {error:?}.\n\ + AquaVM version is {} and it expect data of {} version,\ + it's failed to get version of AquaVM produced this data.\n\ + data: {data:?}", + super::interpreter_version(), data_version() )] - DataDeFailed(SerdeJsonError, Vec), + DataDeFailed { data: Vec, error: SerdeJsonError }, + + /// Errors occurred on executed trace deserialization + /// when it was possible to recover versions. + #[error( + "an error occurred while data deserialization: {error:?}.\n\ + AquaVM's version is {} and it expects data of {} version.\n\ + Supplied data version is {}, it's produced by AquaVM of {} version.\n\ + Data: {data:?}", + super::interpreter_version(), + data_version(), + versions.data_version, + versions.interpreter_version, + )] + DataDeFailedWithVersions { + data: Vec, + error: SerdeJsonError, + versions: Versions, + }, /// Error occurred on call results deserialization. #[error( - "error occurred while deserialize call results: {0:?}.\n\ - Call results: {1:?}" + "error occurred while deserialize call results: {error:?}.\n\ + Call results: {call_results:?}" )] - CallResultsDeFailed(SerdeJsonError, Vec), + CallResultsDeFailed { + call_results: Vec, + error: SerdeJsonError, + }, /// Error occurred when a version of interpreter produced supplied data is less then minimal. #[error("supplied data was produced by `{actual_version}` version of interpreter, but minimum `{required_version}` version is required")] @@ -61,3 +85,24 @@ impl ToErrorCode for PreparationError { crate::generate_to_error_code!(self, PreparationError, PREPARATION_ERROR_START_ID) } } + +impl PreparationError { + pub fn data_de_failed(data: Vec, error: SerdeJsonError) -> Self { + Self::DataDeFailed { data, error } + } + + pub fn data_de_failed_with_versions(data: Vec, error: SerdeJsonError, versions: Versions) -> Self { + Self::DataDeFailedWithVersions { data, error, versions } + } + + pub fn call_results_de_failed(call_results: Vec, error: SerdeJsonError) -> Self { + Self::CallResultsDeFailed { call_results, error } + } + + pub fn unsupported_interpreter_version(actual_version: semver::Version, required_version: semver::Version) -> Self { + Self::UnsupportedInterpreterVersion { + actual_version, + required_version, + } + } +} diff --git a/air/src/preparation_step/minimal_supported_version.rs b/air/src/preparation_step/interpreter_versions.rs similarity index 73% rename from air/src/preparation_step/minimal_supported_version.rs rename to air/src/preparation_step/interpreter_versions.rs index ed4e500f..a6db6b27 100644 --- a/air/src/preparation_step/minimal_supported_version.rs +++ b/air/src/preparation_step/interpreter_versions.rs @@ -18,13 +18,22 @@ use once_cell::sync::Lazy; use std::str::FromStr; +// TODO: change to 0.35.0 in the next PR static MINIMAL_SUPPORTED_VERSION: Lazy = Lazy::new(|| semver::Version::from_str("0.31.2").expect("valid minimal supported version specified")); +static INTERPRETER_VERSION: Lazy = + Lazy::new(|| semver::Version::from_str(env!("CARGO_PKG_VERSION")).expect("invalid data format version specified")); + // This local is intended to check that set version is correct at the AquaVM start for graceful error message. thread_local!(static _MINIMAL_SUPPORTED_VERSION_CHECK: &'static semver::Version = Lazy::force(&MINIMAL_SUPPORTED_VERSION)); -/// Return minimal support version interpreter. +/// Returns a minimal support version by this interpreter. pub fn min_supported_version() -> &'static semver::Version { Lazy::force(&MINIMAL_SUPPORTED_VERSION) } + +/// Returns a current interpreter version. +pub fn interpreter_version() -> &'static semver::Version { + Lazy::force(&INTERPRETER_VERSION) +} diff --git a/air/src/preparation_step/mod.rs b/air/src/preparation_step/mod.rs index 19e1d027..9e2b4e92 100644 --- a/air/src/preparation_step/mod.rs +++ b/air/src/preparation_step/mod.rs @@ -15,7 +15,7 @@ */ mod errors; -mod minimal_supported_version; +mod interpreter_versions; mod preparation; pub use errors::PreparationError; @@ -23,4 +23,5 @@ pub use errors::PreparationError; pub(crate) use preparation::prepare; pub(crate) use preparation::PreparationDescriptor; -use minimal_supported_version::min_supported_version; +use interpreter_versions::interpreter_version; +use interpreter_versions::min_supported_version; diff --git a/air/src/preparation_step/preparation.rs b/air/src/preparation_step/preparation.rs index c9359c57..1bc3e905 100644 --- a/air/src/preparation_step/preparation.rs +++ b/air/src/preparation_step/preparation.rs @@ -75,10 +75,20 @@ pub(crate) fn prepare<'i>( } fn try_to_data(raw_data: &[u8]) -> PreparationResult { - use PreparationError::DataDeFailed; + // treat empty slice as an empty data, + // it allows abstracting from an internal format for an empty data + if raw_data.is_empty() { + return Ok(InterpreterData::new(super::min_supported_version().clone())); + } - InterpreterData::try_from_slice(raw_data, super::min_supported_version()) - .map_err(|err| DataDeFailed(err, raw_data.to_vec())) + InterpreterData::try_from_slice(raw_data).map_err(|de_error| to_date_de_error(raw_data.to_vec(), de_error)) +} + +fn to_date_de_error(raw_data: Vec, de_error: serde_json::Error) -> PreparationError { + match InterpreterData::try_get_versions(&raw_data) { + Ok(versions) => PreparationError::data_de_failed_with_versions(raw_data, de_error, versions), + Err(_) => PreparationError::data_de_failed(raw_data, de_error), + } } #[tracing::instrument(skip_all)] @@ -89,16 +99,16 @@ fn make_exec_ctx( run_parameters: RunParameters, ) -> PreparationResult> { let call_results = serde_json::from_slice(call_results) - .map_err(|e| PreparationError::CallResultsDeFailed(e, call_results.to_vec()))?; + .map_err(|e| PreparationError::call_results_de_failed(call_results.to_vec(), e))?; let ctx = ExecutionCtx::new(prev_ingredients, current_ingredients, call_results, run_parameters); Ok(ctx) } fn check_version_compatibility(data: &InterpreterData) -> PreparationResult<()> { - if &data.interpreter_version < super::min_supported_version() { + if &data.versions.interpreter_version < super::min_supported_version() { return Err(PreparationError::UnsupportedInterpreterVersion { - actual_version: data.interpreter_version.clone(), + actual_version: data.versions.interpreter_version.clone(), required_version: super::min_supported_version().clone(), }); } diff --git a/air/tests/test_module/features/data_merging/data_merge.rs b/air/tests/test_module/features/data_merging/data_merge.rs index 63e7a34f..dd0c7e0e 100644 --- a/air/tests/test_module/features/data_merging/data_merge.rs +++ b/air/tests/test_module/features/data_merging/data_merge.rs @@ -284,8 +284,7 @@ fn fold_merge() { local_vms_results[6].data.clone() ); - let data = InterpreterData::try_from_slice(&result_7.data, &semver::Version::new(1, 1, 1)) - .expect("data should be well-formed"); + let data = InterpreterData::try_from_slice(&result_7.data).expect("data should be well-formed"); let stream_1_generations = data .global_streams .get("$stream_1") diff --git a/air/tests/test_module/features/misc/version_check.rs b/air/tests/test_module/features/misc/version_check.rs index 907d0147..2edb38a9 100644 --- a/air/tests/test_module/features/misc/version_check.rs +++ b/air/tests/test_module/features/misc/version_check.rs @@ -21,11 +21,12 @@ use air_test_utils::prelude::*; #[test] fn minimal_version_check() { let mut vm = create_avm(echo_call_service(), ""); + let script = "(null)"; let actual_version = semver::Version::new(0, 31, 1); let current_data = InterpreterData::new(actual_version.clone()); let current_data = serde_json::to_vec(¤t_data).expect("default serializer shouldn't fail"); - let result = call_vm!(vm, <_>::default(), "", "", current_data); + let result = call_vm!(vm, <_>::default(), script, "", current_data); let expected_error = PreparationError::UnsupportedInterpreterVersion { actual_version, diff --git a/air/tests/test_module/main.rs b/air/tests/test_module/main.rs index 7dbba066..b495015a 100644 --- a/air/tests/test_module/main.rs +++ b/air/tests/test_module/main.rs @@ -21,3 +21,4 @@ mod features; mod instructions; mod integration; mod issues; +mod negative_tests; diff --git a/air/tests/test_module/negative_tests/mod.rs b/air/tests/test_module/negative_tests/mod.rs new file mode 100644 index 00000000..bc83a18c --- /dev/null +++ b/air/tests/test_module/negative_tests/mod.rs @@ -0,0 +1,17 @@ +/* + * Copyright 2023 Fluence Labs Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +mod preparation_step; diff --git a/air/tests/test_module/negative_tests/preparation_step.rs b/air/tests/test_module/negative_tests/preparation_step.rs new file mode 100644 index 00000000..b9000983 --- /dev/null +++ b/air/tests/test_module/negative_tests/preparation_step.rs @@ -0,0 +1,76 @@ +/* + * Copyright 2022 Fluence Labs Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use air::PreparationError; +use air_test_utils::prelude::*; + +use serde::Deserialize; +use serde::Serialize; + +#[test] +fn invalid_data_without_versions() { + #[derive(Serialize, Deserialize)] + struct InvalidDataStruct { + pub trace: Vec, + } + + let vm_peer_id = "some_peer_id"; + let mut vm = create_avm(unit_call_service(), vm_peer_id); + + let script = r#"(null)"#; + let invalid_data = InvalidDataStruct { trace: vec![1, 2, 3] }; + let invalid_data = serde_json::to_vec(&invalid_data).unwrap(); + + let result = call_vm!(vm, <_>::default(), script, "", invalid_data.clone()); + + let expected_serde_error = serde_json::from_slice::(&invalid_data).err().unwrap(); + let expected_error = PreparationError::DataDeFailed { + data: invalid_data, + error: expected_serde_error, + }; + assert!(check_error(&result, expected_error)); +} + +#[test] +fn invalid_data_with_versions() { + #[derive(Serialize, Deserialize)] + struct InvalidDataStruct { + pub trace: Vec, + #[serde(flatten)] + pub versions: Versions, + } + + let vm_peer_id = "some_peer_id"; + let mut vm = create_avm(unit_call_service(), vm_peer_id); + + let script = r#"(null)"#; + let versions = Versions::new(semver::Version::new(1, 1, 1)); + let invalid_data = InvalidDataStruct { + trace: vec![1, 2, 3], + versions: versions.clone(), + }; + let invalid_data = serde_json::to_vec(&invalid_data).unwrap(); + + let result = call_vm!(vm, <_>::default(), script, "", invalid_data.clone()); + + let expected_serde_error = serde_json::from_slice::(&invalid_data).err().unwrap(); + let expected_error = PreparationError::DataDeFailedWithVersions { + data: invalid_data, + error: expected_serde_error, + versions, + }; + assert!(check_error(&result, expected_error)); +} diff --git a/crates/air-lib/interpreter-data/CHANGELOG.md b/crates/air-lib/interpreter-data/CHANGELOG.md index 6e6ef639..e608a6d1 100644 --- a/crates/air-lib/interpreter-data/CHANGELOG.md +++ b/crates/air-lib/interpreter-data/CHANGELOG.md @@ -1,3 +1,8 @@ +## Version 0.6.1 +[PR](https://github.com/fluencelabs/aquavm/pull/451): +- move data and interpreter versions into a new structure +- add new API to get versions from data + ## Version 0.6.0 [PR 419](https://github.com/fluencelabs/aquavm/pull/419): - Rename data's `cid_store` field to `value_store`. diff --git a/crates/air-lib/interpreter-data/Cargo.toml b/crates/air-lib/interpreter-data/Cargo.toml index 7c4174f1..579c58be 100644 --- a/crates/air-lib/interpreter-data/Cargo.toml +++ b/crates/air-lib/interpreter-data/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "air-interpreter-data" description = "Data format of the AIR interpreter" -version = "0.6.0" +version = "0.6.1" authors = ["Fluence Labs"] edition = "2018" license = "Apache-2.0" diff --git a/crates/air-lib/interpreter-data/src/interpreter_data.rs b/crates/air-lib/interpreter-data/src/interpreter_data.rs index b15ecf6c..ad8e7969 100644 --- a/crates/air-lib/interpreter-data/src/interpreter_data.rs +++ b/crates/air-lib/interpreter-data/src/interpreter_data.rs @@ -33,6 +33,10 @@ use serde::Serialize; /// have the following format. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct InterpreterData { + /// Versions of data and an interpreter produced this data. + #[serde(flatten)] + pub versions: Versions, + /// Trace of AIR execution, which contains executed call, par, fold, and ap states. pub trace: ExecutionTrace, @@ -42,36 +46,41 @@ pub struct InterpreterData { #[serde(rename = "streams")] // for compatibility with versions <= 0.2.1 pub global_streams: GlobalStreamGens, - /// Version of this data format. - pub version: semver::Version, - - /// Last exposed to a peer call request id. All next call request ids will be bigger than this. - #[serde(default)] - #[serde(rename = "lcid")] - pub last_call_request_id: u32, - /// Contains maximum generation for each private stream. This info will be used while merging /// values in streams. #[serde(default)] #[serde(rename = "r_streams")] pub restricted_streams: RestrictedStreamGens, - /// Version of interpreter produced this data. - pub interpreter_version: semver::Version, + /// Last exposed to a peer call request id. All next call request ids will be bigger than this. + #[serde(default)] + #[serde(rename = "lcid")] + pub last_call_request_id: u32, /// CID-to-somethings mappings. pub cid_info: CidInfo, } +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Versions { + /// Version of this data format. + #[serde(rename = "version")] // for compatibility with versions <= 0.6.0 + pub data_version: semver::Version, + + /// Version of an interpreter produced this data. + pub interpreter_version: semver::Version, +} + impl InterpreterData { pub fn new(interpreter_version: semver::Version) -> Self { + let versions = Versions::new(interpreter_version); + Self { + versions, trace: ExecutionTrace::default(), global_streams: GlobalStreamGens::new(), - version: crate::data_version().clone(), last_call_request_id: 0, restricted_streams: RestrictedStreamGens::new(), - interpreter_version, cid_info: <_>::default(), } } @@ -85,34 +94,40 @@ impl InterpreterData { last_call_request_id: u32, interpreter_version: semver::Version, ) -> Self { + let versions = Versions::new(interpreter_version); + Self { + versions, trace, global_streams: streams, - version: crate::data_version().clone(), last_call_request_id, restricted_streams, - interpreter_version, cid_info, } } /// Tries to de InterpreterData from slice according to the data version. - pub fn try_from_slice( - slice: &[u8], - min_support_version: &semver::Version, - ) -> Result { - // treat empty slice as an empty interpreter data allows abstracting from - // the internal format for empty data. - if slice.is_empty() { - return Ok(Self::new(min_support_version.clone())); - } - + pub fn try_from_slice(slice: &[u8]) -> Result { measure!( serde_json::from_slice(slice), tracing::Level::INFO, "serde_json::from_slice" ) } + + /// Tries to de only versions part of interpreter data. + pub fn try_get_versions(slice: &[u8]) -> Result { + serde_json::from_slice(slice) + } +} + +impl Versions { + pub fn new(interpreter_version: semver::Version) -> Self { + Self { + data_version: crate::data_version().clone(), + interpreter_version, + } + } } #[derive(Debug, Default, Clone, Serialize, Deserialize)] @@ -134,62 +149,42 @@ mod tests { use serde::Serialize; #[test] - #[ignore] // TODO: fix tests - fn compatible_with_0_2_0_version() { - #[derive(Serialize, Deserialize)] - struct InterpreterData0_2_0 { + fn compatible_with_0_6_0_version() { + #[derive(Debug, Clone, Serialize, Deserialize)] + pub struct InterpreterData0_6_0 { pub trace: ExecutionTrace, - pub streams: GlobalStreamGens, - pub version: semver::Version, - } - - // test 0.2.0 to 0.2.2 conversion - let data_0_2_0 = InterpreterData0_2_0 { - trace: ExecutionTrace::default(), - streams: GlobalStreamGens::default(), - version: semver::Version::new(0, 2, 0), - }; - - let data_0_2_0_se = serde_json::to_vec(&data_0_2_0).unwrap(); - let data_0_2_1 = serde_json::from_slice::(&data_0_2_0_se); - assert!(data_0_2_1.is_ok()); - - // test 0.2.2 to 0.2.0 conversion - let data_0_2_2 = InterpreterData::new(semver::Version::new(1, 1, 1)); - let data_0_2_2_se = serde_json::to_vec(&data_0_2_2).unwrap(); - let data_0_2_0 = serde_json::from_slice::(&data_0_2_2_se); - assert!(data_0_2_0.is_ok()); - } - - #[test] - #[ignore] // TODO: fix tests - fn compatible_with_0_2_1_version() { - #[derive(Serialize, Deserialize)] - struct InterpreterData0_2_1 { - pub trace: ExecutionTrace, - pub streams: GlobalStreamGens, + #[serde(rename = "streams")] // for compatibility with versions <= 0.2.1 + pub global_streams: GlobalStreamGens, pub version: semver::Version, #[serde(default)] #[serde(rename = "lcid")] pub last_call_request_id: u32, + #[serde(default)] + #[serde(rename = "r_streams")] + pub restricted_streams: RestrictedStreamGens, + pub interpreter_version: semver::Version, + pub cid_info: CidInfo, } - // test 0.2.1 to 0.2.2 conversion - let data_0_2_1 = InterpreterData0_2_1 { + // test 0.6.0 to 0.6.1 conversion + let data_0_6_0 = InterpreterData0_6_0 { trace: ExecutionTrace::default(), - streams: GlobalStreamGens::default(), - version: semver::Version::new(0, 2, 1), - last_call_request_id: 1, + global_streams: GlobalStreamGens::default(), + version: semver::Version::new(0, 2, 0), + last_call_request_id: 0, + restricted_streams: RestrictedStreamGens::default(), + interpreter_version: semver::Version::new(0, 1, 1), + cid_info: CidInfo::default(), }; - let data_0_2_1_se = serde_json::to_vec(&data_0_2_1).unwrap(); - let data_0_2_2 = serde_json::from_slice::(&data_0_2_1_se); - assert!(data_0_2_2.is_ok()); + let data_0_6_0_se = serde_json::to_vec(&data_0_6_0).unwrap(); + let data_0_6_1 = serde_json::from_slice::(&data_0_6_0_se); + assert!(data_0_6_1.is_ok()); - // test 0.2.2 to 0.2.1 conversion - let data_0_2_2 = InterpreterData::new(semver::Version::new(1, 1, 1)); - let data_0_2_2_se = serde_json::to_vec(&data_0_2_2).unwrap(); - let data_0_2_0 = serde_json::from_slice::(&data_0_2_2_se); - assert!(data_0_2_0.is_ok()); + // test 0.6.1 to 0.6.0 conversion + let data_0_6_1 = InterpreterData::new(semver::Version::new(1, 1, 1)); + let data_0_6_1_se = serde_json::to_vec(&data_0_6_1).unwrap(); + let data_0_6_0 = serde_json::from_slice::(&data_0_6_1_se); + assert!(data_0_6_0.is_ok()); } } diff --git a/crates/air-lib/test-utils/src/lib.rs b/crates/air-lib/test-utils/src/lib.rs index 4e2bd81c..b1742d9f 100644 --- a/crates/air-lib/test-utils/src/lib.rs +++ b/crates/air-lib/test-utils/src/lib.rs @@ -166,12 +166,5 @@ pub fn is_interpreter_succeded(result: &RawAVMOutcome) -> bool { } pub fn check_error(result: &RawAVMOutcome, error: impl ToErrorCode + ToString) -> bool { - println!( - "{} == {} && {} == {}", - result.ret_code, - error.to_error_code(), - result.error_message, - error.to_string() - ); result.ret_code == error.to_error_code() && result.error_message == error.to_string() }