Skip to content

Get rid of get_blobs_not_found and unify blob errors #2649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions linera-chain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use data_types::{MessageBundle, Origin, PostedMessage};
use linera_base::{
crypto::{CryptoError, CryptoHash},
data_types::{ArithmeticError, BlockHeight, Round, Timestamp},
identifiers::{ApplicationId, ChainId},
identifiers::{ApplicationId, BlobId, ChainId},
};
use linera_execution::ExecutionError;
use linera_views::views::ViewError;
Expand All @@ -39,7 +39,7 @@ pub enum ChainError {
#[error("Arithmetic error: {0}")]
ArithmeticError(#[from] ArithmeticError),
#[error("Error in view operation: {0}")]
ViewError(#[from] ViewError),
ViewError(ViewError),
#[error("Execution error: {0} during {1:?}")]
ExecutionError(Box<ExecutionError>, ChainExecutionContext),

Expand Down Expand Up @@ -157,6 +157,17 @@ pub enum ChainError {
expected: CryptoHash,
actual: CryptoHash,
},
#[error("Blobs not found: {0:?}")]
BlobsNotFound(Vec<BlobId>),
}

impl From<ViewError> for ChainError {
fn from(error: ViewError) -> Self {
match error {
ViewError::BlobsNotFound(blob_ids) => ChainError::BlobsNotFound(blob_ids),
error => ChainError::ViewError(error),
}
}
}

#[derive(Copy, Clone, Debug)]
Expand Down
67 changes: 47 additions & 20 deletions linera-core/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,12 +383,8 @@ where
.handle_certificate(certificate.clone().into(), vec![])
.await;

if let Some(blob_ids) = result
.as_ref()
.err()
.and_then(LocalNodeError::get_blobs_not_found)
{
if let Some(blobs) = remote_node.try_download_blobs(&blob_ids).await {
if let Err(LocalNodeError::BlobsNotFound(blob_ids)) = &result {
if let Some(blobs) = remote_node.try_download_blobs(blob_ids).await {
result = self.handle_certificate(certificate.into(), blobs).await;
}
}
Expand Down Expand Up @@ -530,7 +526,7 @@ where
#[derive(Debug, Error)]
pub enum ChainClientError {
#[error("Local node operation failed: {0}")]
LocalNodeError(#[from] LocalNodeError),
LocalNodeError(LocalNodeError),

#[error("Remote node operation failed: {0}")]
RemoteNodeError(#[from] NodeError),
Expand All @@ -542,7 +538,7 @@ pub enum ChainClientError {
JsonError(#[from] serde_json::Error),

#[error("Chain operation failed: {0}")]
ChainError(#[from] ChainError),
ChainError(ChainError),

#[error(transparent)]
CommunicationError(#[from] CommunicationError<NodeError>),
Expand Down Expand Up @@ -578,7 +574,7 @@ pub enum ChainClientError {
FoundMultipleKeysForChain(ChainId),

#[error(transparent)]
ViewError(#[from] ViewError),
ViewError(ViewError),

#[error("Blobs not found: {0:?}")]
BlobsNotFound(Vec<BlobId>),
Expand All @@ -593,6 +589,40 @@ pub enum ChainClientError {
},
}

impl From<ViewError> for ChainClientError {
fn from(error: ViewError) -> Self {
match error {
ViewError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
error => Self::ViewError(error),
}
}
}

impl From<LocalNodeError> for ChainClientError {
fn from(error: LocalNodeError) -> Self {
match error {
LocalNodeError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
error => Self::LocalNodeError(error),
}
}
}

impl From<ChainError> for ChainClientError {
fn from(error: ChainError) -> Self {
match error {
ChainError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
ChainError::ExecutionError(execution_error, context) => {
if let ExecutionError::BlobsNotFound(blob_ids) = *execution_error {
Self::BlobsNotFound(blob_ids)
} else {
Self::ChainError(ChainError::ExecutionError(execution_error, context))
}
}
error => Self::ChainError(error),
}
}
}

impl From<Infallible> for ChainClientError {
fn from(infallible: Infallible) -> Self {
match infallible {}
Expand Down Expand Up @@ -1229,7 +1259,7 @@ where
.await
{
match &err {
LocalNodeError::WorkerError(WorkerError::BlobsNotFound(blob_ids)) => {
LocalNodeError::BlobsNotFound(blob_ids) => {
let blobs = RemoteNode::download_blobs(blob_ids, &nodes)
.await
.ok_or(err)?;
Expand Down Expand Up @@ -1727,8 +1757,8 @@ where
.handle_block_proposal(*proposal.clone())
.await
{
if let Some(blob_ids) = original_err.get_blobs_not_found() {
self.update_local_node_with_blobs_from(blob_ids, remote_node)
if let LocalNodeError::BlobsNotFound(blob_ids) = &original_err {
self.update_local_node_with_blobs_from(blob_ids.clone(), remote_node)
.await?;
continue; // We found the missing blobs: retry.
}
Expand All @@ -1748,9 +1778,7 @@ where
.handle_certificate(Certificate::from(*cert.clone()), blobs)
.await
{
if let LocalNodeError::WorkerError(WorkerError::BlobsNotFound(blob_ids)) =
&original_err
{
if let LocalNodeError::BlobsNotFound(blob_ids) = &original_err {
if let Some(new_blobs) = remote_node
.find_missing_blobs(blob_ids.clone(), chain_id)
.await?
Expand Down Expand Up @@ -1887,11 +1915,10 @@ where
.local_node
.stage_block_execution(block.clone())
.await;
if let Err(err) = &result {
if let Some(blob_ids) = err.get_blobs_not_found() {
self.receive_certificates_for_blobs(blob_ids).await?;
continue; // We found the missing blob: retry.
}
if let Err(LocalNodeError::BlobsNotFound(blob_ids)) = &result {
self.receive_certificates_for_blobs(blob_ids.clone())
.await?;
continue; // We found the missing blob: retry.
}
return Ok(result?);
}
Expand Down
46 changes: 21 additions & 25 deletions linera-core/src/local_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ use linera_base::{
use linera_chain::{
data_types::{Block, BlockProposal, ExecutedBlock},
types::{Certificate, CertificateValue, ConfirmedBlockCertificate, LiteCertificate},
ChainError, ChainStateView,
ChainStateView,
};
use linera_execution::{ExecutionError, Query, Response, SystemExecutionError};
use linera_execution::{Query, Response};
use linera_storage::Storage;
use linera_views::views::ViewError;
use thiserror::Error;
Expand Down Expand Up @@ -54,10 +54,10 @@ pub enum LocalNodeError {
ArithmeticError(#[from] ArithmeticError),

#[error(transparent)]
ViewError(#[from] linera_views::views::ViewError),
ViewError(ViewError),

#[error("Local node operation failed: {0}")]
WorkerError(#[from] WorkerError),
WorkerError(WorkerError),

#[error("Failed to read blob {blob_id:?} of chain {chain_id:?}")]
CannotReadLocalBlob { chain_id: ChainId, blob_id: BlobId },
Expand All @@ -67,29 +67,25 @@ pub enum LocalNodeError {

#[error("The chain info response received from the local node is invalid")]
InvalidChainInfoResponse,

#[error("Blobs not found: {0:?}")]
BlobsNotFound(Vec<BlobId>),
}

impl LocalNodeError {
pub fn get_blobs_not_found(&self) -> Option<Vec<BlobId>> {
match self {
LocalNodeError::WorkerError(WorkerError::ChainError(chain_error)) => {
match &**chain_error {
ChainError::ExecutionError(execution_error, _) => match **execution_error {
ExecutionError::SystemError(SystemExecutionError::BlobNotFoundOnRead(
blob_id,
))
| ExecutionError::ViewError(ViewError::BlobNotFoundOnRead(blob_id)) => {
Some(vec![blob_id])
}
_ => None,
},
_ => None,
}
}
LocalNodeError::WorkerError(WorkerError::BlobsNotFound(blob_ids)) => {
Some(blob_ids.clone())
}
_ => None,
impl From<WorkerError> for LocalNodeError {
fn from(error: WorkerError) -> Self {
match error {
WorkerError::BlobsNotFound(blob_ids) => LocalNodeError::BlobsNotFound(blob_ids),
error => LocalNodeError::WorkerError(error),
}
}
}

impl From<ViewError> for LocalNodeError {
fn from(error: ViewError) -> Self {
match error {
ViewError::BlobsNotFound(blob_ids) => LocalNodeError::BlobsNotFound(blob_ids),
error => LocalNodeError::ViewError(error),
}
}
}
Expand Down
33 changes: 17 additions & 16 deletions linera-core/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use linera_chain::{
};
use linera_execution::{
committee::{Committee, ValidatorName},
ExecutionError, SystemExecutionError,
ExecutionError,
};
use linera_version::VersionInfo;
use linera_views::views::ViewError;
Expand Down Expand Up @@ -182,7 +182,7 @@ pub enum NodeError {
height: BlockHeight,
},

#[error("The following blobs are missing: {0:?}.")]
#[error("Blobs not found: {0:?}")]
BlobsNotFound(Vec<BlobId>),

// This error must be normalized during conversions.
Expand Down Expand Up @@ -220,8 +220,6 @@ pub enum NodeError {
#[error("Failed to make a chain info query on the local node: {error}")]
LocalNodeQuery { error: String },

#[error("Blob not found on storage read: {0}")]
BlobNotFoundOnRead(BlobId),
#[error("Node failed to provide a 'last used by' certificate for the blob")]
InvalidCertificateForBlob(BlobId),
#[error("Local error handling validator response")]
Expand Down Expand Up @@ -255,8 +253,11 @@ impl CrossChainMessageDelivery {

impl From<ViewError> for NodeError {
fn from(error: ViewError) -> Self {
Self::ViewError {
error: error.to_string(),
match error {
ViewError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
error => Self::ViewError {
error: error.to_string(),
},
}
}
}
Expand Down Expand Up @@ -290,16 +291,16 @@ impl From<ChainError> for NodeError {
height,
},
ChainError::InactiveChain(chain_id) => Self::InactiveChain(chain_id),
ChainError::ExecutionError(execution_error, context) => match *execution_error {
ExecutionError::SystemError(SystemExecutionError::BlobNotFoundOnRead(blob_id))
| ExecutionError::ViewError(ViewError::BlobNotFoundOnRead(blob_id)) => {
Self::BlobNotFoundOnRead(blob_id)
ChainError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
ChainError::ExecutionError(execution_error, context) => {
if let ExecutionError::BlobsNotFound(blob_ids) = *execution_error {
Self::BlobsNotFound(blob_ids)
} else {
Self::ChainError {
error: ChainError::ExecutionError(execution_error, context).to_string(),
}
}
execution_error => Self::ChainError {
error: ChainError::ExecutionError(Box::new(execution_error), context)
.to_string(),
},
},
}
error => Self::ChainError {
error: error.to_string(),
},
Expand All @@ -312,7 +313,7 @@ impl From<WorkerError> for NodeError {
match error {
WorkerError::ChainError(error) => (*error).into(),
WorkerError::MissingCertificateValue => Self::MissingCertificateValue,
WorkerError::BlobsNotFound(blob_ids) => NodeError::BlobsNotFound(blob_ids),
WorkerError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
error => Self::WorkerError {
error: error.to_string(),
},
Expand Down
4 changes: 2 additions & 2 deletions linera-core/src/unit_tests/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ where
let handle_block_proposal_result =
Self::handle_block_proposal(proposal, &mut validator).await;
let result = match handle_block_proposal_result {
Some(Err(NodeError::BlobsNotFound(_) | NodeError::BlobNotFoundOnRead(_))) => {
Some(Err(NodeError::BlobsNotFound(_))) => {
handle_block_proposal_result.expect("handle_block_proposal_result should be Some")
}
_ => match validator.fault_type {
Expand Down Expand Up @@ -367,7 +367,7 @@ where
let handle_certificate_result =
Self::handle_certificate(certificate, validator, blobs).await;
match handle_certificate_result {
Some(Err(NodeError::BlobsNotFound(_) | NodeError::BlobNotFoundOnRead(_))) => {
Some(Err(NodeError::BlobsNotFound(_))) => {
handle_certificate_result.expect("handle_certificate_result should be Some")
}
_ => match validator.fault_type {
Expand Down
4 changes: 2 additions & 2 deletions linera-core/src/updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,8 @@ where
// synchronize them now and retry.
self.send_chain_information_for_senders(chain_id).await?;
}
Err(NodeError::BlobNotFoundOnRead(_)) if !blob_ids.is_empty() => {
// For `BlobNotFoundOnRead`, we assume that the local node should already be
Err(NodeError::BlobsNotFound(_)) if !blob_ids.is_empty() => {
// For `BlobsNotFound`, we assume that the local node should already be
// updated with the needed blobs, so sending the chain information about the
// certificates that last used the blobs to the validator node should be enough.
let missing_blob_ids = stream::iter(mem::take(&mut blob_ids))
Expand Down
Loading
Loading