feat(trace-handler): improve data deserialization version check (#451)

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",

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:?}.\
+        "an error occurred while data deserialization: {error:?}.\n\
-    Probably it's a data of an old version that can't be converted to '{}'.\n\
+        AquaVM version is {} and it expect data of {} version,\
-    Data: {1:?}",
+        it's failed to get version of AquaVM produced this data.\n\
+        data: {data:?}",
+        super::interpreter_version(),
         data_version()
     )]
-    DataDeFailed(SerdeJsonError, Vec<u8>),
+    DataDeFailed { data: Vec<u8>, 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<u8>,
+        error: SerdeJsonError,
+        versions: Versions,
+    },
 
     /// Error occurred on call results deserialization.
     #[error(
-        "error occurred while deserialize call results: {0:?}.\n\
+        "error occurred while deserialize call results: {error:?}.\n\
-    Call results: {1:?}"
+    Call results: {call_results:?}"
     )]
-    CallResultsDeFailed(SerdeJsonError, Vec<u8>),
+    CallResultsDeFailed {
+        call_results: Vec<u8>,
+        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<u8>, error: SerdeJsonError) -> Self {
+        Self::DataDeFailed { data, error }
+    }
+
+    pub fn data_de_failed_with_versions(data: Vec<u8>, error: SerdeJsonError, versions: Versions) -> Self {
+        Self::DataDeFailedWithVersions { data, error, versions }
+    }
+
+    pub fn call_results_de_failed(call_results: Vec<u8>, 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,
+        }
+    }
+}

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<semver::Version> =
     Lazy::new(|| semver::Version::from_str("0.31.2").expect("valid minimal supported version specified"));
 
+static INTERPRETER_VERSION: Lazy<semver::Version> =
+    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)
+}

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;

air/src/preparation_step/preparation.rs

@@ -75,10 +75,20 @@ pub(crate) fn prepare<'i>(
 }
 
 fn try_to_data(raw_data: &[u8]) -> PreparationResult<InterpreterData> {
-    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())
+    InterpreterData::try_from_slice(raw_data).map_err(|de_error| to_date_de_error(raw_data.to_vec(), de_error))
-        .map_err(|err| DataDeFailed(err, raw_data.to_vec()))
+}
+
+fn to_date_de_error(raw_data: Vec<u8>, 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<ExecutionCtx<'static>> {
     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(),
         });
     }

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))
+    let data = InterpreterData::try_from_slice(&result_7.data).expect("data should be well-formed");
-        .expect("data should be well-formed");
     let stream_1_generations = data
         .global_streams
         .get("$stream_1")

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(&current_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,

air/tests/test_module/main.rs

@@ -21,3 +21,4 @@ mod features;
 mod instructions;
 mod integration;
 mod issues;
+mod negative_tests;

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;

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<u8>,
+    }
+
+    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::<InterpreterData>(&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<u8>,
+        #[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::<InterpreterData>(&invalid_data).err().unwrap();
+    let expected_error = PreparationError::DataDeFailedWithVersions {
+        data: invalid_data,
+        error: expected_serde_error,
+        versions,
+    };
+    assert!(check_error(&result, expected_error));
+}

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`.

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"

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.
+    /// Last exposed to a peer call request id. All next call request ids will be bigger than this.
-    pub interpreter_version: semver::Version,
+    #[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(
+    pub fn try_from_slice(slice: &[u8]) -> Result<Self, serde_json::Error> {
-        slice: &[u8],
-        min_support_version: &semver::Version,
-    ) -> Result<Self, serde_json::Error> {
-        // 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()));
-        }
-
         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<Versions, serde_json::Error> {
+        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_6_0_version() {
-    fn compatible_with_0_2_0_version() {
+        #[derive(Debug, Clone, Serialize, Deserialize)]
-        #[derive(Serialize, Deserialize)]
+        pub struct InterpreterData0_6_0 {
-        struct InterpreterData0_2_0 {
             pub trace: ExecutionTrace,
-            pub streams: GlobalStreamGens,
+            #[serde(rename = "streams")] // for compatibility with versions <= 0.2.1
-            pub version: semver::Version,
+            pub global_streams: GlobalStreamGens,
-        }
-
-        // 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::<InterpreterData>(&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::<InterpreterData0_2_0>(&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,
             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
+        // test 0.6.0 to 0.6.1 conversion
-        let data_0_2_1 = InterpreterData0_2_1 {
+        let data_0_6_0 = InterpreterData0_6_0 {
             trace: ExecutionTrace::default(),
-            streams: GlobalStreamGens::default(),
+            global_streams: GlobalStreamGens::default(),
-            version: semver::Version::new(0, 2, 1),
+            version: semver::Version::new(0, 2, 0),
-            last_call_request_id: 1,
+            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_6_0_se = serde_json::to_vec(&data_0_6_0).unwrap();
-        let data_0_2_2 = serde_json::from_slice::<InterpreterData>(&data_0_2_1_se);
+        let data_0_6_1 = serde_json::from_slice::<InterpreterData>(&data_0_6_0_se);
-        assert!(data_0_2_2.is_ok());
+        assert!(data_0_6_1.is_ok());
 
-        // test 0.2.2 to 0.2.1 conversion
+        // test 0.6.1 to 0.6.0 conversion
-        let data_0_2_2 = InterpreterData::new(semver::Version::new(1, 1, 1));
+        let data_0_6_1 = 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_6_1_se = serde_json::to_vec(&data_0_6_1).unwrap();
-        let data_0_2_0 = serde_json::from_slice::<InterpreterData0_2_1>(&data_0_2_2_se);
+        let data_0_6_0 = serde_json::from_slice::<InterpreterData0_6_0>(&data_0_6_1_se);
-        assert!(data_0_2_0.is_ok());
+        assert!(data_0_6_0.is_ok());
     }
 }

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()
 }