From 1925ccd8b6efdc2fa8e578ed1ef852402b323c37 Mon Sep 17 00:00:00 2001 From: Mathieu Dutour Sikiric Date: Mon, 3 Feb 2025 15:06:31 +0100 Subject: [PATCH] Several corrections to `linera-execution` code. (#3231) ## Motivation The work on the integration of the EVM into the `linera-execution` led to code reading and so to identification of some ways to improve the code. ## Proposal The following was done: * The functions `user` and `user_without_abi` are only used for testing purposes and so marked as such. * The functions `find_dependencies` and `describe_applications_with_dependencies` are always used with an empty last argument in the code. The code is thus simplified. * Many `clone()` operations in `system.rs` are not needed even though clippy did not detect this. * The use of `Sender` / `oneshot::Sender` is inconcistent in the code. ## Test Plan The CI should detect it. ## Release Plan Follow the normal release plan. ## Links None. --- linera-execution/src/applications.rs | 18 ++------ linera-execution/src/execution.rs | 3 +- linera-execution/src/execution_state_actor.rs | 8 ++-- linera-execution/src/lib.rs | 2 + linera-execution/src/system.rs | 15 +++---- .../src/unit_tests/applications_tests.rs | 41 +++---------------- 6 files changed, 21 insertions(+), 66 deletions(-) diff --git a/linera-execution/src/applications.rs b/linera-execution/src/applications.rs index 2d12a0f2a3c..f6da145ef05 100644 --- a/linera-execution/src/applications.rs +++ b/linera-execution/src/applications.rs @@ -1,7 +1,7 @@ // Copyright (c) Zefchain Labs, Inc. // SPDX-License-Identifier: Apache-2.0 -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use linera_base::{data_types::UserApplicationDescription, identifiers::UserApplicationId}; use linera_views::{ @@ -104,7 +104,6 @@ where pub async fn find_dependencies( &self, mut stack: Vec, - registered_apps: &HashMap, ) -> Result, SystemExecutionError> { // What we return at the end. let mut result = Vec::new(); @@ -129,11 +128,7 @@ where seen.insert(id); // 2. Schedule all the (yet unseen) dependencies, then this entry for a second visit. stack.push(id); - let app = if let Some(app) = registered_apps.get(&id) { - app.clone() - } else { - self.describe_application(id).await? - }; + let app = self.describe_application(id).await?; for child in app.required_application_ids.iter().rev() { if !seen.contains(child) { stack.push(*child); @@ -147,16 +142,11 @@ where pub async fn describe_applications_with_dependencies( &self, ids: Vec, - extra_registered_apps: &HashMap, ) -> Result, SystemExecutionError> { - let ids_with_deps = self.find_dependencies(ids, extra_registered_apps).await?; + let ids_with_deps = self.find_dependencies(ids).await?; let mut result = Vec::new(); for id in ids_with_deps { - let description = if let Some(description) = extra_registered_apps.get(&id) { - description.clone() - } else { - self.describe_application(id).await? - }; + let description = self.describe_application(id).await?; result.push(description); } Ok(result) diff --git a/linera-execution/src/execution.rs b/linera-execution/src/execution.rs index 5796ad34e96..3dfefa8f7d7 100644 --- a/linera-execution/src/execution.rs +++ b/linera-execution/src/execution.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use std::{ - collections::{BTreeMap, BTreeSet, HashMap}, + collections::{BTreeMap, BTreeSet}, mem, vec, }; @@ -289,7 +289,6 @@ where .registry .describe_applications_with_dependencies( applications_to_describe.into_iter().collect(), - &HashMap::new(), ) .await?; diff --git a/linera-execution/src/execution_state_actor.rs b/linera-execution/src/execution_state_actor.rs index 8432d3a4654..f744bcaab28 100644 --- a/linera-execution/src/execution_state_actor.rs +++ b/linera-execution/src/execution_state_actor.rs @@ -499,14 +499,14 @@ pub enum ExecutionRequest { CloseChain { application_id: UserApplicationId, #[debug(skip)] - callback: oneshot::Sender>, + callback: Sender>, }, ChangeApplicationPermissions { application_id: UserApplicationId, application_permissions: ApplicationPermissions, #[debug(skip)] - callback: oneshot::Sender>, + callback: Sender>, }, CreateApplication { @@ -515,7 +515,7 @@ pub enum ExecutionRequest { parameters: Vec, required_application_ids: Vec, #[debug(skip)] - callback: oneshot::Sender>, + callback: Sender>, }, FetchUrl { @@ -530,7 +530,7 @@ pub enum ExecutionRequest { #[debug(with = hex_debug)] payload: Vec, #[debug(skip)] - callback: oneshot::Sender>, + callback: Sender>, }, ReadBlobContent { diff --git a/linera-execution/src/lib.rs b/linera-execution/src/lib.rs index afe45455c5d..f1101b5ec7c 100644 --- a/linera-execution/src/lib.rs +++ b/linera-execution/src/lib.rs @@ -1153,6 +1153,7 @@ impl Operation { } /// Creates a new user application operation following the `application_id`'s [`Abi`]. + #[cfg(with_testing)] pub fn user( application_id: UserApplicationId, operation: &A::Operation, @@ -1162,6 +1163,7 @@ impl Operation { /// Creates a new user application operation assuming that the `operation` is valid for the /// `application_id`. + #[cfg(with_testing)] pub fn user_without_abi( application_id: UserApplicationId<()>, operation: &impl Serialize, diff --git a/linera-execution/src/system.rs b/linera-execution/src/system.rs index 6f5f346b50b..75dcaddfd28 100644 --- a/linera-execution/src/system.rs +++ b/linera-execution/src/system.rs @@ -657,7 +657,7 @@ where self.record_bytecode_blobs(blobs_to_register, txn_tracker) .await?; outcome.messages.push(message); - new_application = Some((app_id, instantiation_argument.clone())); + new_application = Some((app_id, instantiation_argument)); } RequestApplication { chain_id, @@ -861,7 +861,7 @@ where SystemExecutionError::InvalidCommitteeCreation ); if epoch == chain_next_epoch { - self.committees.get_mut().insert(epoch, committee.clone()); + self.committees.get_mut().insert(epoch, committee); self.epoch.set(Some(epoch)); } } @@ -872,18 +872,13 @@ where for application in applications { self.check_and_record_bytecode_blobs(&application.bytecode_id, txn_tracker) .await?; - self.registry - .register_application(application.clone()) - .await?; + self.registry.register_application(application).await?; } } RequestApplication(application_id) => { let applications = self .registry - .describe_applications_with_dependencies( - vec![application_id], - &Default::default(), - ) + .describe_applications_with_dependencies(vec![application_id]) .await?; let message = RawOutgoingMessage { destination: Destination::Recipient(context.message_id.chain_id), @@ -1051,7 +1046,7 @@ where } } self.registry - .register_new_application(id, parameters.clone(), required_application_ids.clone()) + .register_new_application(id, parameters, required_application_ids) .await?; // Send a message to ourself to increment the message ID. let message = RawOutgoingMessage { diff --git a/linera-execution/src/unit_tests/applications_tests.rs b/linera-execution/src/unit_tests/applications_tests.rs index 6803e23a26b..3420c101df6 100644 --- a/linera-execution/src/unit_tests/applications_tests.rs +++ b/linera-execution/src/unit_tests/applications_tests.rs @@ -57,41 +57,16 @@ fn registry(graph: impl IntoIterator)>) -> ApplicationRegi async fn test_topological_sort() { let mut view = ApplicationRegistryView::new().await; view.import(registry([(1, vec![2, 3])])).unwrap(); - assert!(view - .find_dependencies(vec![app_id(1)], &Default::default()) - .await - .is_err()); + assert!(view.find_dependencies(vec![app_id(1)]).await.is_err()); view.import(registry([(3, vec![2]), (2, vec![]), (0, vec![1])])) .unwrap(); - let app_ids = view - .find_dependencies(vec![app_id(1)], &Default::default()) - .await - .unwrap(); + let app_ids = view.find_dependencies(vec![app_id(1)]).await.unwrap(); assert_eq!(app_ids, Vec::from_iter([2, 3, 1].into_iter().map(app_id))); - let app_ids = view - .find_dependencies(vec![app_id(0)], &Default::default()) - .await - .unwrap(); + let app_ids = view.find_dependencies(vec![app_id(0)]).await.unwrap(); assert_eq!( app_ids, Vec::from_iter([2, 3, 1, 0].into_iter().map(app_id)) ); - let app_ids = view - .find_dependencies( - vec![app_id(0), app_id(5)], - &vec![ - (app_id(5), app_description(5, vec![4])), - (app_id(4), app_description(5, vec![2])), - ] - .into_iter() - .collect(), - ) - .await - .unwrap(); - assert_eq!( - app_ids, - Vec::from_iter([2, 4, 5, 3, 1, 0].into_iter().map(app_id)) - ); } #[tokio::test] @@ -104,15 +79,9 @@ async fn test_topological_sort_with_loop() { (0, vec![1]), ])) .unwrap(); - let app_ids = view - .find_dependencies(vec![app_id(1)], &Default::default()) - .await - .unwrap(); + let app_ids = view.find_dependencies(vec![app_id(1)]).await.unwrap(); assert_eq!(app_ids, Vec::from_iter([2, 3, 1].into_iter().map(app_id))); - let app_ids = view - .find_dependencies(vec![app_id(0)], &Default::default()) - .await - .unwrap(); + let app_ids = view.find_dependencies(vec![app_id(0)]).await.unwrap(); assert_eq!( app_ids, Vec::from_iter([2, 3, 1, 0].into_iter().map(app_id))