Check call arguments at the beggining (#215)

air/src/execution_step/air/call/resolved_call.rs

@@ -73,6 +73,13 @@ impl<'i> ResolvedCall<'i> {
         exec_ctx: &mut ExecutionCtx<'i>,
         trace_ctx: &mut TraceHandler,
     ) -> ExecutionResult<()> {
+        // it's necessary to check arguments before accessing state,
+        // because it would be undeterministic otherwise, for more details see
+        // https://github.com/fluencelabs/aquavm/issues/214
+        // also note that if there is a non-join error then the corresponding state
+        // won't be saved to data
+        self.check_args(exec_ctx)?;
+
         let state = self.prepare_current_executed_state(raw_call, exec_ctx, trace_ctx)?;
         if !state.should_execute() {
             state.maybe_set_prev_state(trace_ctx);
@@ -88,11 +95,14 @@ impl<'i> ResolvedCall<'i> {
 
         let request_params = match self.prepare_request_params(exec_ctx, tetraplet) {
             Ok(params) => params,
-            Err(e) => {
+            Err(e) if e.is_joinable() => {
                 // to keep states on join behaviour
                 state.maybe_set_prev_state(trace_ctx);
                 return Err(e);
             }
+            Err(e) => {
+                return Err(e);
+            }
         };
         let call_id = exec_ctx.next_call_request_id();
         exec_ctx.call_requests.insert(call_id, request_params);
@@ -112,7 +122,7 @@ impl<'i> ResolvedCall<'i> {
 
     fn prepare_request_params(
         &self,
-        exec_ctx: &ExecutionCtx<'i>,
+        exec_ctx: &ExecutionCtx<'_>,
         tetraplet: &SecurityTetraplet,
     ) -> ExecutionResult<CallRequestParams> {
         let ResolvedArguments {
@@ -159,8 +169,8 @@ impl<'i> ResolvedCall<'i> {
         use crate::execution_step::resolver::resolve_to_args;
 
         let function_args = self.function_arg_paths.iter();
-        let mut call_arguments = Vec::new();
-        let mut tetraplets = Vec::new();
+        let mut call_arguments = Vec::with_capacity(function_args.len());
+        let mut tetraplets = Vec::with_capacity(function_args.len());
         for instruction_value in function_args {
             let (arg, tetraplet) = resolve_to_args(instruction_value, exec_ctx)?;
             call_arguments.push(arg);
@@ -177,6 +187,22 @@ impl<'i> ResolvedCall<'i> {
 
         Ok(resolved_arguments)
     }
+
+    /// Lightweight version of resolve_args function that intended to check arguments of
+    /// a call instruction. It suppresses joinable errors.
+    fn check_args(&self, exec_ctx: &ExecutionCtx<'i>) -> ExecutionResult<()> {
+        // TODO: make this function more lightweight
+        use crate::execution_step::resolver::resolve_to_args;
+
+        self.function_arg_paths
+            .iter()
+            .map(|arg_path| match resolve_to_args(arg_path, exec_ctx) {
+                Ok(_) => Ok(()),
+                Err(e) if e.is_joinable() => Ok(()),
+                Err(e) => Err(e),
+            })
+            .collect::<_>()
+    }
 }
 
 /// Check output type name for being already in execution context.

air/src/execution_step/execution_context/streams_variables.rs

@@ -51,8 +51,7 @@ impl Streams {
     pub(crate) fn get(&self, name: &str, position: usize) -> Option<&RefCell<Stream>> {
         self.streams
             .get(name)
-            .map(|descriptors| find_closest(descriptors.iter(), position))
-            .flatten()
+            .and_then(|descriptors| find_closest(descriptors.iter(), position))
     }
 
     pub(crate) fn add_stream_value(
@@ -143,12 +142,7 @@ impl Streams {
     fn stream_generation_from_data(&self, name: &str, position: u32, iteration: usize) -> Option<u32> {
         self.data_restr_stream_generations
             .get(name)
-            .and_then(|scopes| {
-                scopes
-                    .get(&position)
-                    .map(|iterations| iterations.get(iteration))
-                    .flatten()
-            })
+            .and_then(|scopes| scopes.get(&position).and_then(|iterations| iterations.get(iteration)))
             .copied()
     }
 

air/tests/test_module/integration/lambda.rs

@@ -27,7 +27,6 @@ fn lambda_not_allowed_for_non_objects_and_arrays() {
     let mut set_variable_vm = create_avm(echo_call_service(), set_variable_peer_id);
 
     let local_peer_id = "local_peer_id";
-    let mut local_vm = create_avm(echo_call_service(), local_peer_id);
 
     let some_string = "some_string";
     let script = f!(r#"
@@ -37,8 +36,7 @@ fn lambda_not_allowed_for_non_objects_and_arrays() {
         )
         "#);
 
-    let result = checked_call_vm!(set_variable_vm, "asd", &script, "", "");
-    let result = call_vm!(local_vm, "asd", script, "", result.data);
+    let result = call_vm!(set_variable_vm, "asd", &script, "", "");
 
     let expected_error = CatchableError::LambdaApplierError(LambdaError::FieldAccessorNotMatchValue {
         value: json!(some_string),

air/tests/test_module/integration/last_error.rs

@@ -266,7 +266,6 @@ fn access_last_error_by_not_exists_field() {
     let mut fallible_vm = create_avm(fallible_call_service("fallible_call_service"), fallible_peer_id);
 
     let local_peer_id = "local_peer_id";
-    let mut local_vm = create_avm(echo_call_service(), local_peer_id);
 
     let non_exists_field_name = "non_exists_field";
     let script = f!(r#"
@@ -276,8 +275,7 @@ fn access_last_error_by_not_exists_field() {
         )
     "#);
 
-    let result = checked_call_vm!(fallible_vm, "asd", &script, "", "");
-    let result = call_vm!(local_vm, "asd", script, "", result.data);
+    let result = call_vm!(fallible_vm, "asd", &script, "", "");
 
     let expected_error = ExecutionError::Catchable(rc!(CatchableError::LambdaApplierError(
         LambdaError::ValueNotContainSuchField {

air/tests/test_module/issues/issue_214.rs

@@ -0,0 +1,71 @@
+/*
+ * 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_test_utils::prelude::*;
+
+use fstrings::f;
+use fstrings::format_args_f;
+
+#[test]
+// test for github.com/fluencelabs/aquavm/issues/214
+fn issue_214() {
+    let client_id = "client_peer_id";
+    let relay_id = "relay_peer_id";
+    let scalar = json!([]);
+    let error_handler = "error handler is called";
+    let variables_mapping = maplit::hashmap! {
+        "-relay-".to_string() => json!(relay_id),
+        "s".to_string() => scalar.clone(),
+        "error".to_string() => json!(error_handler), // this result should be returned by (2) call
+    };
+
+    let mut client = create_avm(
+        set_variables_call_service(variables_mapping, VariableOptionSource::FunctionName),
+        client_id,
+    );
+
+    let script = f!(r#"
+        (xor
+         (seq
+          (seq
+           (seq
+            (call %init_peer_id% ("getDataSrv" "-relay-") [] -relay-)
+            (call %init_peer_id% ("getDataSrv" "s") [] s)
+           )
+           (xor
+            (call -relay- ("op" "identity") [s.$.field!] res) ;; (1) should not produce data after calling on relay
+            (call %init_peer_id% ("errorHandlingSrv" "error") [%last_error% 1]) ;; (2) should be called
+           )
+          )
+          (xor
+           (call %init_peer_id% ("callbackSrv" "response") [res]) ;; join behaviour
+           (call %init_peer_id% ("errorHandlingSrv" "error") [%last_error% 2])
+          )
+         )
+         (call %init_peer_id% ("errorHandlingSrv" "error") [%last_error% 3])
+        )
+    "#);
+
+    let result = checked_call_vm!(client, client_id, &script, "", "");
+    let expected_trace = vec![
+        executed_state::scalar_string(relay_id),
+        executed_state::scalar(scalar),
+        executed_state::scalar_string(error_handler),
+    ];
+    let actual_trace = trace_from_result(&result);
+
+    assert_eq!(actual_trace, expected_trace);
+}

air/tests/test_module/issues/mod.rs

@@ -21,3 +21,4 @@ mod issue_178;
 mod issue_180;
 mod issue_206;
 mod issue_211;
+mod issue_214;