Skip to content

Commit

Permalink
Several corrections to linera-execution code. (#3231)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
MathieuDutSik authored Feb 3, 2025
1 parent d1919f2 commit 1925ccd
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 66 deletions.
18 changes: 4 additions & 14 deletions linera-execution/src/applications.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -104,7 +104,6 @@ where
pub async fn find_dependencies(
&self,
mut stack: Vec<UserApplicationId>,
registered_apps: &HashMap<UserApplicationId, UserApplicationDescription>,
) -> Result<Vec<UserApplicationId>, SystemExecutionError> {
// What we return at the end.
let mut result = Vec::new();
Expand All @@ -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);
Expand All @@ -147,16 +142,11 @@ where
pub async fn describe_applications_with_dependencies(
&self,
ids: Vec<UserApplicationId>,
extra_registered_apps: &HashMap<UserApplicationId, UserApplicationDescription>,
) -> Result<Vec<UserApplicationDescription>, 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)
Expand Down
3 changes: 1 addition & 2 deletions linera-execution/src/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

use std::{
collections::{BTreeMap, BTreeSet, HashMap},
collections::{BTreeMap, BTreeSet},
mem, vec,
};

Expand Down Expand Up @@ -289,7 +289,6 @@ where
.registry
.describe_applications_with_dependencies(
applications_to_describe.into_iter().collect(),
&HashMap::new(),
)
.await?;

This comment has been minimized.

Copy link
@dukalis666777

dukalis666777 Feb 3, 2025

linera-execution/src/applications.rs

This comment has been minimized.

Copy link
@dukalis666777

dukalis666777 Feb 3, 2025

its ok

Expand Down
8 changes: 4 additions & 4 deletions linera-execution/src/execution_state_actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,14 +499,14 @@ pub enum ExecutionRequest {
CloseChain {
application_id: UserApplicationId,
#[debug(skip)]
callback: oneshot::Sender<Result<(), ExecutionError>>,
callback: Sender<Result<(), ExecutionError>>,
},

ChangeApplicationPermissions {
application_id: UserApplicationId,
application_permissions: ApplicationPermissions,
#[debug(skip)]
callback: oneshot::Sender<Result<(), ExecutionError>>,
callback: Sender<Result<(), ExecutionError>>,
},

CreateApplication {
Expand All @@ -515,7 +515,7 @@ pub enum ExecutionRequest {
parameters: Vec<u8>,
required_application_ids: Vec<UserApplicationId>,
#[debug(skip)]
callback: oneshot::Sender<Result<CreateApplicationResult, ExecutionError>>,
callback: Sender<Result<CreateApplicationResult, ExecutionError>>,
},

FetchUrl {
Expand All @@ -530,7 +530,7 @@ pub enum ExecutionRequest {
#[debug(with = hex_debug)]
payload: Vec<u8>,
#[debug(skip)]
callback: oneshot::Sender<Vec<u8>>,
callback: Sender<Vec<u8>>,
},

ReadBlobContent {
Expand Down
2 changes: 2 additions & 0 deletions linera-execution/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1153,6 +1153,7 @@ impl Operation {
}

/// Creates a new user application operation following the `application_id`'s [`Abi`].
#[cfg(with_testing)]
pub fn user<A: Abi>(
application_id: UserApplicationId<A>,
operation: &A::Operation,
Expand All @@ -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,
Expand Down
15 changes: 5 additions & 10 deletions linera-execution/src/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
}
}
Expand All @@ -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),
Expand Down Expand Up @@ -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 {
Expand Down
41 changes: 5 additions & 36 deletions linera-execution/src/unit_tests/applications_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,41 +57,16 @@ fn registry(graph: impl IntoIterator<Item = (u32, Vec<u32>)>) -> 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]
Expand All @@ -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))
Expand Down

1 comment on commit 1925ccd

@Magnitico

This comment was marked as spam.

Please sign in to comment.