From d60e1f57726a1b78f8de9ef74a60eb0d08adfdbf 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 | 22 ++--- linera-core/src/data_types.rs | 15 +--- linera-core/src/local_node.rs | 84 +++++++++---------- 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 | 2 +- 8 files changed, 55 insertions(+), 93 deletions(-) diff --git a/linera-core/src/chain_worker/state.rs b/linera-core/src/chain_worker/state.rs index ce843c825d88..e8b2b7c9701f 100644 --- a/linera-core/src/chain_worker/state.rs +++ b/linera-core/src/chain_worker/state.rs @@ -741,10 +741,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 782875455046..d64ce9b70d29 100644 --- a/linera-core/src/client.rs +++ b/linera-core/src/client.rs @@ -736,18 +736,14 @@ where &self, locations: &[BytecodeLocation], nodes: &[(ValidatorName,

::Node)], - chain_id: ChainId, ) -> 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() .flatten() + .flatten() .collect::>() } @@ -813,7 +809,7 @@ where 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); @@ -832,7 +828,7 @@ where blob_ids, )) => { let values = self - .find_missing_application_bytecodes(locations, &nodes, block.chain_id) + .find_missing_application_bytecodes(locations, &nodes) .await; let blobs = self.find_missing_blobs(blob_ids, &nodes).await; @@ -1264,11 +1260,15 @@ 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)?; 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 bc77c622a534..b00466fa4a2f 100644 --- a/linera-core/src/local_node.rs +++ b/linera-core/src/local_node.rs @@ -175,7 +175,6 @@ where async fn find_missing_application_bytecodes( &self, - chain_id: ChainId, locations: &[BytecodeLocation], node: &mut A, name: ValidatorName, @@ -186,15 +185,13 @@ where 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 + Self::try_download_hashed_certificate_value_from(&mut node, name, *location).await } })) .await .into_iter() .flatten() + .flatten() .collect::>() } @@ -218,7 +215,7 @@ where } async fn try_process_certificates( - &self, + &mut self, name: ValidatorName, node: &mut A, chain_id: ChainId, @@ -245,12 +242,7 @@ where locations, ))) => { let values = self - .find_missing_application_bytecodes( - certificate.value().chain_id(), - locations, - node, - name, - ) + .find_missing_application_bytecodes(locations, node, name) .await; if values.len() != locations.len() { @@ -272,9 +264,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, name) .await; let blobs = self.find_missing_blobs(blob_ids, node, name).await; if values.len() != locations.len() || blobs.len() != blob_ids.len() { @@ -362,7 +353,7 @@ where } pub async fn download_certificates( - &self, + &mut self, mut validators: Vec<(ValidatorName, A)>, chain_id: ChainId, target_next_block_height: BlockHeight, @@ -403,9 +394,9 @@ 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( - &self, + &mut self, validators: Vec<(ValidatorName, A)>, - hashed_certificate_value_locations: impl IntoIterator, + hashed_certificate_value_locations: impl IntoIterator, ) -> Result, LocalNodeError> where A: LocalValidatorNode + Clone + 'static, @@ -413,7 +404,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) @@ -424,7 +415,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, )); } } @@ -448,7 +439,6 @@ where pub async fn read_or_download_hashed_certificate_value( storage: S, validators: Vec<(ValidatorName, A)>, - chain_id: ChainId, location: BytecodeLocation, ) -> Result, LocalNodeError> where @@ -462,7 +452,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) @@ -492,7 +482,7 @@ where } async fn try_download_certificates_from( - &self, + &mut self, name: ValidatorName, mut node: A, chain_id: ChainId, @@ -570,7 +560,7 @@ where let mut futures = vec![]; for (name, node) in validators { - let client = self.clone(); + let mut client = self.clone(); let mut notifications = vec![]; futures.push(async move { ( @@ -594,7 +584,7 @@ where } pub async fn try_synchronize_chain_state_from( - &self, + &mut self, name: ValidatorName, mut node: A, chain_id: ChainId, @@ -661,24 +651,21 @@ where pub async fn download_hashed_certificate_value( mut validators: Vec<(ValidatorName, A)>, - chain_id: ChainId, location: BytecodeLocation, - ) -> Option + ) -> Result, LocalNodeError> where A: LocalValidatorNode + Clone + 'static, { - // Sequentially try each validator in random order. + // Sequentially try each validator in random order, to improve efficiency. 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 + if let Some(value) = + Self::try_download_hashed_certificate_value_from(&mut node, name, location).await? { - return Some(value); + return Ok(Some(value)); } } - None + Ok(None) } pub async fn download_blob( @@ -720,21 +707,34 @@ where } async fn try_download_hashed_certificate_value_from( - name: ValidatorName, node: &mut A, - chain_id: ChainId, + name: ValidatorName, location: BytecodeLocation, - ) -> Option + ) -> Result, LocalNodeError> 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; + match node + .download_certificate_value(location.certificate_hash) + .await + { + Ok(certificate) => { + let hashed_certificate_value: HashedCertificateValue = certificate.into(); + if hashed_certificate_value.hash() == location.certificate_hash { + Ok(Some(hashed_certificate_value)) + } else { + tracing::info!( + "Validator {name} sent an invalid certificate value {location:?}." + ); + Ok(None) + } + } + Err(error) => { + tracing::debug!( + "Failed to fetch certificate value {location:?} from validator {name}: {error}" + ); + Ok(None) } } - None } } diff --git a/linera-rpc/proto/rpc.proto b/linera-rpc/proto/rpc.proto index c1dd2ec0fcfa..f9dec4071ab9 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 ea86df575be7..1c365a46fd86 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 d2371ae34e2c..1c227f1a1ef7 100644 --- a/linera-rpc/tests/snapshots/format__format.yaml.snap +++ b/linera-rpc/tests/snapshots/format__format.yaml.snap @@ -213,9 +213,6 @@ ChainInfo: - requested_received_log: SEQ: TYPENAME: ChainAndHeight - - requested_hashed_certificate_value: - OPTION: - TYPENAME: CertificateValue ChainInfoQuery: STRUCT: - chain_id: @@ -236,9 +233,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 817dd91d0db1..1bbca46f25d1 100644 --- a/linera-service/src/linera/main.rs +++ b/linera-service/src/linera/main.rs @@ -1094,7 +1094,7 @@ impl Job { let state = WorkerState::new("Local node".to_string(), None, storage) .with_allow_inactive_chains(true) .with_allow_messages_from_deprecated_epochs(true); - let node_client = LocalNodeClient::new(state); + let mut node_client = LocalNodeClient::new(state); // Take the latest committee we know of. let admin_chain_id = context.wallet().genesis_admin_chain();