From ab42c36013cdb6453e372050c8eba2833481ade8 Mon Sep 17 00:00:00 2001 From: Andre da Silva Date: Wed, 29 May 2024 20:47:17 -0300 Subject: [PATCH] Replace certificate chain info query usage with certificate download --- linera-core/src/chain_worker/state.rs | 4 - linera-core/src/client.rs | 51 ++++++------- linera-core/src/data_types.rs | 15 +--- linera-core/src/local_node.rs | 73 ++++++------------- linera-core/src/node.rs | 21 ++++++ linera-rpc/proto/rpc.proto | 3 - linera-rpc/src/grpc/conversions.rs | 12 --- .../tests/snapshots/format__format.yaml.snap | 6 -- linera-service/src/linera/main.rs | 4 +- 9 files changed, 70 insertions(+), 119 deletions(-) diff --git a/linera-core/src/chain_worker/state.rs b/linera-core/src/chain_worker/state.rs index 48900f8ed731..8ea8ef6be60e 100644 --- a/linera-core/src/chain_worker/state.rs +++ b/linera-core/src/chain_worker/state.rs @@ -709,10 +709,6 @@ where let start = usize::try_from(start).map_err(|_| ArithmeticError::Overflow)?; info.requested_received_log = self.chain.received_log.read(start..).await?; } - if let Some(hash) = query.request_hashed_certificate_value { - info.requested_hashed_certificate_value = - Some(self.storage.read_hashed_certificate_value(hash).await?); - } if query.request_manager_values { info.manager.add_values(self.chain.manager.get()); } diff --git a/linera-core/src/client.rs b/linera-core/src/client.rs index 21b5c43c5104..d8e79195feed 100644 --- a/linera-core/src/client.rs +++ b/linera-core/src/client.rs @@ -729,15 +729,10 @@ where async fn find_missing_application_bytecodes( &self, locations: &[BytecodeLocation], - nodes: &[(ValidatorName,

::Node)], - chain_id: ChainId, + nodes: &[

::Node], ) -> Vec { future::join_all(locations.iter().map(|location| { - LocalNodeClient::::download_hashed_certificate_value( - nodes.to_owned(), - chain_id, - *location, - ) + LocalNodeClient::::download_hashed_certificate_value(nodes.to_owned(), *location) })) .await .into_iter() @@ -795,12 +790,17 @@ where .process_certificate(certificate.clone(), vec![], vec![]) .await { + let nodes = nodes + .into_iter() + .map(|(_name, node)| node) + .collect::>(); + match &err { LocalNodeError::WorkerError(WorkerError::ApplicationBytecodesNotFound( locations, )) => { let values = self - .find_missing_application_bytecodes(locations, &nodes, block.chain_id) + .find_missing_application_bytecodes(locations, &nodes) .await; ensure!(values.len() == locations.len(), err); @@ -808,15 +808,7 @@ where .await?; } LocalNodeError::WorkerError(WorkerError::BlobsNotFound(blob_ids)) => { - let blobs = self - .find_missing_blobs( - blob_ids, - &nodes - .into_iter() - .map(|(_name, node)| node) - .collect::>(), - ) - .await; + let blobs = self.find_missing_blobs(blob_ids, &nodes).await; ensure!(blobs.len() == blob_ids.len(), err); self.process_certificate(certificate.clone(), vec![], blobs) @@ -827,17 +819,9 @@ where blob_ids, )) => { let values = self - .find_missing_application_bytecodes(locations, &nodes, block.chain_id) - .await; - let blobs = self - .find_missing_blobs( - blob_ids, - &nodes - .into_iter() - .map(|(_name, node)| node) - .collect::>(), - ) + .find_missing_application_bytecodes(locations, &nodes) .await; + let blobs = self.find_missing_blobs(blob_ids, &nodes).await; ensure!( blobs.len() == blob_ids.len() && values.len() == locations.len(), @@ -1254,11 +1238,18 @@ where }; // Collect the hashed certificate values required for execution. let committee = self.local_committee().await?; - let nodes: Vec<(ValidatorName, P::Node)> = - self.validator_node_provider.make_nodes(&committee)?; + let nodes = self + .validator_node_provider + .make_nodes::>(&committee)? + .into_iter() + .map(|(_name, node)| node) + .collect(); let values = self .node_client - .read_or_download_hashed_certificate_values(nodes.clone(), block.bytecode_locations()) + .read_or_download_hashed_certificate_values( + nodes, + block.bytecode_locations().into_keys(), + ) .await?; let hashed_blobs = self.read_local_blobs(block.blob_ids()).await?; // Create the final block proposal. diff --git a/linera-core/src/data_types.rs b/linera-core/src/data_types.rs index e23a058812c8..0f2032c77b97 100644 --- a/linera-core/src/data_types.rs +++ b/linera-core/src/data_types.rs @@ -10,9 +10,7 @@ use linera_base::{ identifiers::{ChainDescription, ChainId, Owner}, }; use linera_chain::{ - data_types::{ - Certificate, ChainAndHeight, HashedCertificateValue, IncomingMessage, Medium, MessageBundle, - }, + data_types::{Certificate, ChainAndHeight, IncomingMessage, Medium, MessageBundle}, manager::ChainManagerInfo, ChainStateView, }; @@ -68,8 +66,6 @@ pub struct ChainInfoQuery { pub request_leader_timeout: bool, /// Include a vote to switch to fallback mode, if appropriate. pub request_fallback: bool, - /// Query a certificate value that contains a binary blob (e.g. bytecode) required by this chain. - pub request_hashed_certificate_value: Option, } impl ChainInfoQuery { @@ -85,7 +81,6 @@ impl ChainInfoQuery { request_manager_values: false, request_leader_timeout: false, request_fallback: false, - request_hashed_certificate_value: None, } } @@ -133,11 +128,6 @@ impl ChainInfoQuery { self.request_fallback = true; self } - - pub fn with_hashed_certificate_value(mut self, hash: CryptoHash) -> Self { - self.request_hashed_certificate_value = Some(hash); - self - } } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -173,8 +163,6 @@ pub struct ChainInfo { pub count_received_log: usize, /// The response to `request_received_certificates_excluding_first_nth` pub requested_received_log: Vec, - /// The requested hashed certificate value, if any. - pub requested_hashed_certificate_value: Option, } /// The response to an `ChainInfoQuery` @@ -253,7 +241,6 @@ where requested_sent_certificates: Vec::new(), count_received_log: view.received_log.count(), requested_received_log: Vec::new(), - requested_hashed_certificate_value: None, } } } diff --git a/linera-core/src/local_node.rs b/linera-core/src/local_node.rs index b015956ba2d3..8e01fbd764de 100644 --- a/linera-core/src/local_node.rs +++ b/linera-core/src/local_node.rs @@ -185,23 +185,20 @@ where async fn find_missing_application_bytecodes( &self, - chain_id: ChainId, locations: &[BytecodeLocation], node: &mut A, - name: ValidatorName, ) -> Vec where A: LocalValidatorNode + Clone + 'static, { - future::join_all(locations.iter().map(|location| { - let mut node = node.clone(); - async move { - Self::try_download_hashed_certificate_value_from( - name, &mut node, chain_id, *location, - ) - .await - } - })) + future::join_all( + locations.iter().map(|location| { + let mut node = node.clone(); + async move { + Self::try_download_hashed_certificate_value_from(&mut node, *location).await + } + }), + ) .await .into_iter() .flatten() @@ -224,7 +221,6 @@ where async fn try_process_certificates( &mut self, - name: ValidatorName, node: &mut A, chain_id: ChainId, certificates: Vec, @@ -249,12 +245,7 @@ where locations, ))) => { let values = self - .find_missing_application_bytecodes( - certificate.value().chain_id(), - locations, - node, - name, - ) + .find_missing_application_bytecodes(locations, node) .await; if values.len() != locations.len() { @@ -275,9 +266,8 @@ where Err(LocalNodeError::WorkerError( WorkerError::ApplicationBytecodesAndBlobsNotFound(locations, blob_ids), )) => { - let chain_id = certificate.value().chain_id(); let values = self - .find_missing_application_bytecodes(chain_id, locations, node, name) + .find_missing_application_bytecodes(locations, node) .await; let blobs = self.find_missing_blobs(blob_ids, node).await; @@ -392,8 +382,8 @@ where /// Does not fail if a hashed certificate value can't be downloaded; it just gets omitted from the result. pub async fn read_or_download_hashed_certificate_values( &mut self, - validators: Vec<(ValidatorName, A)>, - hashed_certificate_value_locations: impl IntoIterator, + validators: Vec, + hashed_certificate_value_locations: impl IntoIterator, ) -> Result, LocalNodeError> where A: LocalValidatorNode + Clone + 'static, @@ -401,7 +391,7 @@ where let mut values = vec![]; let mut tasks = vec![]; let mut node = self.node.lock().await; - for (location, chain_id) in hashed_certificate_value_locations { + for location in hashed_certificate_value_locations { if let Some(value) = node .state .recent_hashed_certificate_value(&location.certificate_hash) @@ -412,7 +402,7 @@ where let validators = validators.clone(); let storage = node.state.storage_client().clone(); tasks.push(Self::read_or_download_hashed_certificate_value( - storage, validators, chain_id, location, + storage, validators, location, )); } } @@ -435,8 +425,7 @@ where pub async fn read_or_download_hashed_certificate_value( storage: S, - validators: Vec<(ValidatorName, A)>, - chain_id: ChainId, + validators: Vec, location: BytecodeLocation, ) -> Result, LocalNodeError> where @@ -450,7 +439,7 @@ where Err(ViewError::NotFound(..)) => {} Err(err) => Err(err)?, } - match Self::download_hashed_certificate_value(validators, chain_id, location).await { + match Self::download_hashed_certificate_value(validators, location).await { Some(hashed_certificate_value) => { storage .write_hashed_certificate_value(&hashed_certificate_value) @@ -503,7 +492,7 @@ where break; }; let Some(info) = self - .try_process_certificates(name, &mut node, chain_id, certificates) + .try_process_certificates(&mut node, chain_id, certificates) .await else { break; @@ -600,12 +589,7 @@ where }; if !info.requested_sent_certificates.is_empty() && self - .try_process_certificates( - name, - &mut node, - chain_id, - info.requested_sent_certificates, - ) + .try_process_certificates(&mut node, chain_id, info.requested_sent_certificates) .await .is_none() { @@ -631,8 +615,7 @@ where } pub async fn download_hashed_certificate_value( - mut validators: Vec<(ValidatorName, A)>, - chain_id: ChainId, + mut validators: Vec, location: BytecodeLocation, ) -> Option where @@ -640,11 +623,9 @@ where { // Sequentially try each validator in random order. validators.shuffle(&mut rand::thread_rng()); - for (name, mut node) in validators { - if let Some(value) = Self::try_download_hashed_certificate_value_from( - name, &mut node, chain_id, location, - ) - .await + for mut node in validators { + if let Some(value) = + Self::try_download_hashed_certificate_value_from(&mut node, location).await { return Some(value); } @@ -677,20 +658,14 @@ where } async fn try_download_hashed_certificate_value_from( - name: ValidatorName, node: &mut A, - chain_id: ChainId, location: BytecodeLocation, ) -> Option where A: LocalValidatorNode + Clone + 'static, { - let query = - ChainInfoQuery::new(chain_id).with_hashed_certificate_value(location.certificate_hash); - if let Ok(response) = node.handle_chain_info_query(query).await { - if response.check(name).is_ok() { - return response.info.requested_hashed_certificate_value; - } + if let Ok(certificate) = node.download_certificate(location.certificate_hash).await { + return Some(certificate.into()); } None } diff --git a/linera-core/src/node.rs b/linera-core/src/node.rs index 5118b46ae8f9..6b0979f9bd33 100644 --- a/linera-core/src/node.rs +++ b/linera-core/src/node.rs @@ -99,6 +99,27 @@ pub trait LocalValidatorNodeProvider { fn make_node(&self, address: &str) -> Result; + // fn make_nodes(&self, committee: &Committee) -> Result + // where + // I: FromIterator, + // { + // self.make_nodes_from_list(committee.validator_addresses()) + // } + + // fn make_nodes_from_list( + // &self, + // validators: impl IntoIterator, + // ) -> Result + // where + // I: FromIterator, + // A: AsRef, + // { + // validators + // .into_iter() + // .map(|(_name, address)| self.make_node(address.as_ref())) + // .collect() + // } + fn make_nodes(&self, committee: &Committee) -> Result where I: FromIterator<(ValidatorName, Self::Node)>, diff --git a/linera-rpc/proto/rpc.proto b/linera-rpc/proto/rpc.proto index 0a1704599ed2..eff8cdc6f934 100644 --- a/linera-rpc/proto/rpc.proto +++ b/linera-rpc/proto/rpc.proto @@ -145,9 +145,6 @@ message ChainInfoQuery { // Request a signed vote for a leader timeout. bool request_leader_timeout = 8; - // Query a value that contains a binary hashed certificate value (e.g. bytecode) required by this chain. - optional bytes request_hashed_certificate_value = 9; - // Query the balance of a given owner. optional Owner request_owner_balance = 10; diff --git a/linera-rpc/src/grpc/conversions.rs b/linera-rpc/src/grpc/conversions.rs index 34470bc211ce..c0b55f3a9ec8 100644 --- a/linera-rpc/src/grpc/conversions.rs +++ b/linera-rpc/src/grpc/conversions.rs @@ -356,10 +356,6 @@ impl TryFrom for ChainInfoQuery { .request_sent_certificates_in_range .map(|range| bincode::deserialize(&range)) .transpose()?; - let request_hashed_certificate_value = chain_info_query - .request_hashed_certificate_value - .map(|bytes| bincode::deserialize(&bytes)) - .transpose()?; Ok(Self { request_committees: chain_info_query.request_committees, @@ -376,7 +372,6 @@ impl TryFrom for ChainInfoQuery { request_manager_values: chain_info_query.request_manager_values, request_leader_timeout: chain_info_query.request_leader_timeout, request_fallback: chain_info_query.request_fallback, - request_hashed_certificate_value, }) } } @@ -389,10 +384,6 @@ impl TryFrom for api::ChainInfoQuery { .request_sent_certificates_in_range .map(|range| bincode::serialize(&range)) .transpose()?; - let request_hashed_certificate_value = chain_info_query - .request_hashed_certificate_value - .map(|hash| bincode::serialize(&hash)) - .transpose()?; Ok(Self { chain_id: Some(chain_info_query.chain_id.into()), @@ -406,7 +397,6 @@ impl TryFrom for api::ChainInfoQuery { request_manager_values: chain_info_query.request_manager_values, request_leader_timeout: chain_info_query.request_leader_timeout, request_fallback: chain_info_query.request_fallback, - request_hashed_certificate_value, }) } } @@ -757,7 +747,6 @@ pub mod tests { requested_sent_certificates: vec![], count_received_log: 0, requested_received_log: vec![], - requested_hashed_certificate_value: None, }); let chain_info_response_none = ChainInfoResponse { @@ -794,7 +783,6 @@ pub mod tests { request_manager_values: false, request_leader_timeout: false, request_fallback: true, - request_hashed_certificate_value: None, }; round_trip_check::<_, api::ChainInfoQuery>(chain_info_query_some); } diff --git a/linera-rpc/tests/snapshots/format__format.yaml.snap b/linera-rpc/tests/snapshots/format__format.yaml.snap index b0f4acaaef48..480c4b3d771f 100644 --- a/linera-rpc/tests/snapshots/format__format.yaml.snap +++ b/linera-rpc/tests/snapshots/format__format.yaml.snap @@ -220,9 +220,6 @@ ChainInfo: - requested_received_log: SEQ: TYPENAME: ChainAndHeight - - requested_hashed_certificate_value: - OPTION: - TYPENAME: CertificateValue ChainInfoQuery: STRUCT: - chain_id: @@ -243,9 +240,6 @@ ChainInfoQuery: - request_manager_values: BOOL - request_leader_timeout: BOOL - request_fallback: BOOL - - request_hashed_certificate_value: - OPTION: - TYPENAME: CryptoHash ChainInfoResponse: STRUCT: - info: diff --git a/linera-service/src/linera/main.rs b/linera-service/src/linera/main.rs index 64584daa6a77..abe1589181c0 100644 --- a/linera-service/src/linera/main.rs +++ b/linera-service/src/linera/main.rs @@ -1102,7 +1102,9 @@ impl Job { let committee = info .latest_committee() .context("Invalid chain info response; missing latest committee")?; - context.make_node_provider().make_nodes(committee)? + context + .make_node_provider() + .make_nodes(committee)? }; // Download the parent chain.