Skip to content

Commit

Permalink
Review after #3005 (#3023)
Browse files Browse the repository at this point in the history
* Rename cache by dropping rename_

* Shorten the code

* Use ==

* Rename VoteValueAndHash to VoteValue

* Update README
  • Loading branch information
deuszx authored Dec 10, 2024
1 parent a9ce930 commit a3cce00
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 47 deletions.
2 changes: 1 addition & 1 deletion examples/counter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Installing and starting the web server:
cd examples/counter/web-frontend
npm install --no-save

# Start the server but not open the web page right away.
# Start the server but do not open the web page right away.
BROWSER=none npm start &
```

Expand Down
2 changes: 1 addition & 1 deletion linera-chain/src/certificate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl CertificateValueT for ValidatedBlock {
}

fn required_blob_ids(&self) -> HashSet<BlobId> {
self.executed_block().required_blob_ids().clone()
self.executed_block().required_blob_ids()
}
}

Expand Down
14 changes: 7 additions & 7 deletions linera-chain/src/data_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ pub struct LiteValue {
}

#[derive(Debug, PartialEq, Eq, Hash, Clone, Serialize, Deserialize)]
struct ValueHashAndRound(CryptoHash, Round, CertificateKind);
struct VoteValue(CryptoHash, Round, CertificateKind);

/// A vote on a statement from a validator.
#[derive(Clone, Debug, Serialize, Deserialize)]
Expand All @@ -458,7 +458,7 @@ impl<T> Vote<T> {
where
T: CertificateValueT,
{
let hash_and_round = ValueHashAndRound(value.hash(), round, T::KIND);
let hash_and_round = VoteValue(value.hash(), round, T::KIND);
let signature = Signature::new(&hash_and_round, key_pair);
Self {
value,
Expand Down Expand Up @@ -807,7 +807,7 @@ impl BlockProposal {
impl LiteVote {
/// Uses the signing key to create a signed object.
pub fn new(value: LiteValue, round: Round, key_pair: &KeyPair) -> Self {
let hash_and_round = ValueHashAndRound(value.value_hash, round, value.kind);
let hash_and_round = VoteValue(value.value_hash, round, value.kind);
let signature = Signature::new(&hash_and_round, key_pair);
Self {
value,
Expand All @@ -819,7 +819,7 @@ impl LiteVote {

/// Verifies the signature in the vote.
pub fn check(&self) -> Result<(), ChainError> {
let hash_and_round = ValueHashAndRound(self.value.value_hash, self.round, self.value.kind);
let hash_and_round = VoteValue(self.value.value_hash, self.round, self.value.kind);
Ok(self.signature.check(&hash_and_round, self.validator.0)?)
}
}
Expand Down Expand Up @@ -853,7 +853,7 @@ impl<'a, T> SignatureAggregator<'a, T> {
where
T: CertificateValueT,
{
let hash_and_round = ValueHashAndRound(self.partial.hash(), self.partial.round, T::KIND);
let hash_and_round = VoteValue(self.partial.hash(), self.partial.round, T::KIND);
signature.check(&hash_and_round, validator.0)?;
// Check that each validator only appears once.
ensure!(
Expand Down Expand Up @@ -911,14 +911,14 @@ pub(crate) fn check_signatures(
ChainError::CertificateRequiresQuorum
);
// All that is left is checking signatures!
let hash_and_round = ValueHashAndRound(value_hash, round, certificate_kind);
let hash_and_round = VoteValue(value_hash, round, certificate_kind);
Signature::verify_batch(&hash_and_round, signatures.iter().map(|(v, s)| (&v.0, s)))?;
Ok(())
}

impl BcsSignable for ProposalContent {}

impl BcsSignable for ValueHashAndRound {}
impl BcsSignable for VoteValue {}

doc_scalar!(
MessageAction,
Expand Down
8 changes: 4 additions & 4 deletions linera-core/src/chain_worker/state/attempted_changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,13 @@ where
// Cache the value we voted on, so the client doesn't have to send it again.
Some(Either::Left(vote)) => {
self.state
.recent_executed_block_values
.executed_block_values
.insert(Cow::Borrowed(vote.value.inner().inner()))
.await;
}
Some(Either::Right(vote)) => {
self.state
.recent_executed_block_values
.executed_block_values
.insert(Cow::Borrowed(vote.value.inner().inner()))
.await;
}
Expand Down Expand Up @@ -208,7 +208,7 @@ where
}

self.state
.recent_executed_block_values
.executed_block_values
.insert(Cow::Borrowed(certificate.inner().inner()))
.await;
let required_blob_ids = executed_block.required_blob_ids();
Expand Down Expand Up @@ -370,7 +370,7 @@ where
self.save().await?;

self.state
.recent_executed_block_values
.executed_block_values
.insert(Cow::Owned(certificate.into_inner().inner().clone())) // TODO: Remove clone
.await;

Expand Down
6 changes: 3 additions & 3 deletions linera-core/src/chain_worker/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ where
chain: ChainStateView<StorageClient::Context>,
shared_chain_view: Option<Arc<RwLock<ChainStateView<StorageClient::Context>>>>,
service_runtime_endpoint: Option<ServiceRuntimeEndpoint>,
recent_executed_block_values: Arc<ValueCache<CryptoHash, Hashed<ExecutedBlock>>>,
executed_block_values: Arc<ValueCache<CryptoHash, Hashed<ExecutedBlock>>>,
tracked_chains: Option<Arc<sync::RwLock<HashSet<ChainId>>>>,
delivery_notifier: DeliveryNotifier,
knows_chain_is_active: bool,
Expand All @@ -68,7 +68,7 @@ where
pub async fn load(
config: ChainWorkerConfig,
storage: StorageClient,
executed_block_cache: Arc<ValueCache<CryptoHash, Hashed<ExecutedBlock>>>,
executed_block_values: Arc<ValueCache<CryptoHash, Hashed<ExecutedBlock>>>,
tracked_chains: Option<Arc<sync::RwLock<HashSet<ChainId>>>>,
delivery_notifier: DeliveryNotifier,
chain_id: ChainId,
Expand All @@ -82,7 +82,7 @@ where
chain,
shared_chain_view: None,
service_runtime_endpoint,
recent_executed_block_values: executed_block_cache,
executed_block_values,
tracked_chains,
delivery_notifier,
knows_chain_is_active: false,
Expand Down
6 changes: 2 additions & 4 deletions linera-core/src/unit_tests/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,7 @@ where
blobs: Vec<Blob>,
) -> Option<Result<ChainInfoResponse, NodeError>> {
match validator.fault_type {
FaultType::DontProcessValidated if matches!(T::KIND, CertificateKind::Validated) => {
None
}
FaultType::DontProcessValidated if T::KIND == CertificateKind::Validated => None,
FaultType::Honest
| FaultType::DontSendConfirmVote
| FaultType::Malicious
Expand Down Expand Up @@ -392,7 +390,7 @@ where
}
_ => match validator.fault_type {
FaultType::DontSendConfirmVote | FaultType::DontProcessValidated
if matches!(T::KIND, CertificateKind::Validated) =>
if T::KIND == CertificateKind::Validated =>
{
Err(NodeError::ClientIoError {
error: "refusing to confirm".to_string(),
Expand Down
52 changes: 25 additions & 27 deletions linera-core/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ where
storage: StorageClient,
/// Configuration options for the [`ChainWorker`]s.
chain_worker_config: ChainWorkerConfig,
recent_executed_block_cache: Arc<ValueCache<CryptoHash, Hashed<ExecutedBlock>>>,
executed_block_cache: Arc<ValueCache<CryptoHash, Hashed<ExecutedBlock>>>,
/// Chain IDs that should be tracked by a worker.
tracked_chains: Option<Arc<RwLock<HashSet<ChainId>>>>,
/// One-shot channels to notify callers when messages of a particular chain have been
Expand Down Expand Up @@ -296,7 +296,7 @@ where
nickname,
storage,
chain_worker_config: ChainWorkerConfig::default().with_key_pair(key_pair),
recent_executed_block_cache: Arc::new(ValueCache::default()),
executed_block_cache: Arc::new(ValueCache::default()),
tracked_chains: None,
delivery_notifiers: Arc::default(),
chain_worker_tasks: Arc::default(),
Expand All @@ -315,7 +315,7 @@ where
nickname,
storage,
chain_worker_config: ChainWorkerConfig::default(),
recent_executed_block_cache: Arc::new(ValueCache::default()),
executed_block_cache: Arc::new(ValueCache::default()),
tracked_chains: Some(tracked_chains),
delivery_notifiers: Arc::default(),
chain_worker_tasks: Arc::default(),
Expand Down Expand Up @@ -395,32 +395,30 @@ where
&self,
certificate: LiteCertificate<'_>,
) -> Result<Either<ConfirmedBlockCertificate, ValidatedBlockCertificate>, WorkerError> {
if let Some(executed_block) = self
.recent_executed_block_cache
let executed_block = self
.executed_block_cache
.get(&certificate.value.value_hash)
.await
{
match certificate.value.kind {
linera_chain::types::CertificateKind::Confirmed => {
let value = ConfirmedBlock::from_hashed(executed_block);
Ok(Either::Left(
certificate
.with_value(Hashed::new(value))
.ok_or(WorkerError::InvalidLiteCertificate)?,
))
}
linera_chain::types::CertificateKind::Validated => {
let value = ValidatedBlock::from_hashed(executed_block);
Ok(Either::Right(
certificate
.with_value(Hashed::new(value))
.ok_or(WorkerError::InvalidLiteCertificate)?,
))
}
_ => return Err(WorkerError::InvalidLiteCertificate), // TODO: different error?
.ok_or(WorkerError::MissingCertificateValue)?;

match certificate.value.kind {
linera_chain::types::CertificateKind::Confirmed => {
let value = ConfirmedBlock::from_hashed(executed_block);
Ok(Either::Left(
certificate
.with_value(Hashed::new(value))
.ok_or(WorkerError::InvalidLiteCertificate)?,
))
}
} else {
Err(WorkerError::MissingCertificateValue)
linera_chain::types::CertificateKind::Validated => {
let value = ValidatedBlock::from_hashed(executed_block);
Ok(Either::Right(
certificate
.with_value(Hashed::new(value))
.ok_or(WorkerError::InvalidLiteCertificate)?,
))
}
_ => return Err(WorkerError::InvalidLiteCertificate),
}
}
}
Expand Down Expand Up @@ -706,7 +704,7 @@ where
let actor = ChainWorkerActor::load(
self.chain_worker_config.clone(),
self.storage.clone(),
self.recent_executed_block_cache.clone(),
self.executed_block_cache.clone(),
self.tracked_chains.clone(),
delivery_notifier,
chain_id,
Expand Down

0 comments on commit a3cce00

Please sign in to comment.