From e4f77654380ee55cd3fe8ba71b053d2d0a0aa16a Mon Sep 17 00:00:00 2001 From: Andreas Fackler Date: Tue, 26 Mar 2024 15:04:18 +0100 Subject: [PATCH] Fix certificate value caching. (#1828) * Fix certificate value caching. * Rename to validated_into_confirmed. * Change to validated_to_confirmed and avoid cloning. --- linera-chain/src/data_types.rs | 20 +++++++++------- linera-chain/src/manager.rs | 2 +- linera-core/src/client.rs | 2 +- linera-core/src/unit_tests/worker_tests.rs | 2 +- linera-core/src/worker.rs | 28 ++++++++++------------ 5 files changed, 26 insertions(+), 28 deletions(-) diff --git a/linera-chain/src/data_types.rs b/linera-chain/src/data_types.rs index 6091830182df..320db2e7fa05 100644 --- a/linera-chain/src/data_types.rs +++ b/linera-chain/src/data_types.rs @@ -767,16 +767,18 @@ impl HashedValue { } } - pub fn into_confirmed(self) -> Option { - match self.value { - value @ CertificateValue::ConfirmedBlock { .. } => Some(HashedValue { - hash: self.hash, - value, - }), - CertificateValue::ValidatedBlock { executed_block } => { - Some(CertificateValue::ConfirmedBlock { executed_block }.into()) + /// Returns the corresponding `ConfirmedBlock`, if this is a `ValidatedBlock`. + pub fn validated_to_confirmed(&self) -> Option { + match &self.value { + CertificateValue::ValidatedBlock { executed_block } => Some( + CertificateValue::ConfirmedBlock { + executed_block: executed_block.clone(), + } + .into(), + ), + CertificateValue::ConfirmedBlock { .. } | CertificateValue::LeaderTimeout { .. } => { + None } - CertificateValue::LeaderTimeout { .. } => None, } } diff --git a/linera-chain/src/manager.rs b/linera-chain/src/manager.rs index baeeb1766625..8e7bcb6321e3 100644 --- a/linera-chain/src/manager.rs +++ b/linera-chain/src/manager.rs @@ -388,7 +388,7 @@ impl ChainManager { if key_pair.is_some() && round < self.current_round() { return; } - let Some(value) = certificate.value.clone().into_confirmed() else { + let Some(value) = certificate.value.validated_to_confirmed() else { // Unreachable: This is only called with validated blocks. error!("Unexpected certificate; expected ValidatedBlock"); return; diff --git a/linera-core/src/client.rs b/linera-core/src/client.rs index acf272d6459e..c7da8a2d9812 100644 --- a/linera-core/src/client.rs +++ b/linera-core/src/client.rs @@ -527,7 +527,7 @@ where committee: &Committee, certificate: Certificate, ) -> Result { - let value = certificate.value.clone().into_confirmed().ok_or_else(|| { + let value = certificate.value.validated_to_confirmed().ok_or_else(|| { ChainClientError::InternalError( "Certificate for finalization must be a validated block", ) diff --git a/linera-core/src/unit_tests/worker_tests.rs b/linera-core/src/unit_tests/worker_tests.rs index 41efa5698d57..e049b5eaab2e 100644 --- a/linera-core/src/unit_tests/worker_tests.rs +++ b/linera-core/src/unit_tests/worker_tests.rs @@ -4309,6 +4309,6 @@ where Some(Box::new(certificate3)) ); let vote = response.info.manager.pending.as_ref().unwrap(); - assert_eq!(vote.value, value2.into_confirmed().unwrap().lite()); + assert_eq!(vote.value, value2.validated_to_confirmed().unwrap().lite()); assert_eq!(vote.round, Round::MultiLeader(2)); } diff --git a/linera-core/src/worker.rs b/linera-core/src/worker.rs index 1faf1024bf87..d28b09248133 100644 --- a/linera-core/src/worker.rs +++ b/linera-core/src/worker.rs @@ -764,9 +764,11 @@ where } => block, _ => panic!("Expecting a validation certificate"), }; + let chain_id = block.chain_id; + let height = block.height; // Check that the chain is active and ready for this confirmation. // Verify the certificate. Returns a catch-all error to make client code more robust. - let mut chain = self.storage.load_active_chain(block.chain_id).await?; + let mut chain = self.storage.load_active_chain(chain_id).await?; let (epoch, committee) = chain .execution_state .system @@ -775,10 +777,7 @@ where Self::check_block_epoch(epoch, block)?; certificate.check(committee)?; let mut actions = NetworkActions::default(); - if chain - .tip_state - .get() - .already_validated_block(block.height)? + if chain.tip_state.get().already_validated_block(height)? || chain.manager.get().check_validated_block(&certificate)? == ChainManagerOutcome::Skip { // If we just processed the same pending block, return the chain info unchanged. @@ -789,24 +788,21 @@ where )); } self.cache_validated(&certificate.value).await; - let current_round = chain.manager.get().current_round(); + let old_round = chain.manager.get().current_round(); chain.manager.get_mut().create_final_vote( - certificate.clone(), + certificate, self.key_pair(), self.storage.current_time(), ); let info = ChainInfoResponse::new(&chain, self.key_pair()); chain.save().await?; - if chain.manager.get().current_round() > current_round { + let round = chain.manager.get().current_round(); + if round > old_round { actions.notifications.push(Notification { - chain_id: block.chain_id, - reason: Reason::NewRound { - height: block.height, - round: chain.manager.get().current_round(), - }, + chain_id, + reason: Reason::NewRound { height, round }, }) } - self.cache_validated(&certificate.value).await; Ok((info, actions, false)) } @@ -928,7 +924,7 @@ where /// Caches the validated block and the corresponding confirmed block. async fn cache_validated(&mut self, value: &HashedValue) { if self.cache_recent_value(Cow::Borrowed(value)).await { - if let Some(value) = value.clone().into_confirmed() { + if let Some(value) = value.validated_to_confirmed() { self.cache_recent_value(Cow::Owned(value)).await; } } @@ -1125,7 +1121,7 @@ where manager.create_vote(proposal, outcome, self.key_pair(), local_time); // Cache the value we voted on, so the client doesn't have to send it again. if let Some(vote) = manager.pending() { - self.cache_recent_value(Cow::Borrowed(&vote.value)).await; + self.cache_validated(&vote.value).await; } let info = ChainInfoResponse::new(&chain, self.key_pair()); chain.save().await?;