diff --git a/crates/block-producer/src/store/mod.rs b/crates/block-producer/src/store/mod.rs index fb20bc160e..e4eae10701 100644 --- a/crates/block-producer/src/store/mod.rs +++ b/crates/block-producer/src/store/mod.rs @@ -5,7 +5,7 @@ use std::num::NonZeroU32; use itertools::Itertools; use miden_node_proto::clients::{Builder, StoreBlockProducerClient}; use miden_node_proto::domain::batch::BatchInputs; -use miden_node_proto::errors::{ConversionError, MissingFieldHelper}; +use miden_node_proto::errors::{ConversionError, ConversionResultExt}; use miden_node_proto::{AccountState, generated as proto}; use miden_node_utils::formatting::format_opt; use miden_protocol::Word; @@ -72,19 +72,21 @@ impl TryFrom for TransactionInputs { fn try_from(response: proto::store::TransactionInputs) -> Result { let AccountState { account_id, account_commitment } = response .account_state - .ok_or(proto::store::TransactionInputs::missing_field(stringify!(account_state)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + account_state + )))? + .try_into() + .context("account_state")?; let mut nullifiers = HashMap::new(); for nullifier_record in response.nullifiers { let nullifier = nullifier_record .nullifier - .ok_or( - proto::store::transaction_inputs::NullifierTransactionInputRecord::missing_field( - stringify!(nullifier), - ), - )? - .try_into()?; + .ok_or(ConversionError::missing_field::< + proto::store::transaction_inputs::NullifierTransactionInputRecord, + >(stringify!(nullifier)))? + .try_into() + .context("nullifier")?; // Note that this intentionally maps 0 to None as this is the definition used in // protobuf. @@ -95,7 +97,8 @@ impl TryFrom for TransactionInputs { .found_unauthenticated_notes .into_iter() .map(Word::try_from) - .collect::>()?; + .collect::>() + .context("found_unauthenticated_notes")?; let current_block_height = response.block_height.into(); @@ -148,11 +151,13 @@ impl StoreClient { .await? .into_inner() .block_header - .ok_or(miden_node_proto::generated::blockchain::BlockHeader::missing_field( - "block_header", - ))?; + .ok_or_else(|| { + StoreError::DeserializationError(ConversionError::missing_field::< + miden_node_proto::generated::blockchain::BlockHeader, + >("block_header")) + })?; - BlockHeader::try_from(response).map_err(Into::into) + BlockHeader::try_from(response).map_err(StoreError::DeserializationError) } #[instrument(target = COMPONENT, name = "store.client.get_tx_inputs", skip_all, err)] @@ -219,7 +224,7 @@ impl StoreClient { let store_response = self.client.clone().get_block_inputs(request).await?.into_inner(); - store_response.try_into().map_err(Into::into) + store_response.try_into().map_err(StoreError::DeserializationError) } #[instrument(target = COMPONENT, name = "store.client.get_batch_inputs", skip_all, err)] @@ -235,7 +240,7 @@ impl StoreClient { let store_response = self.client.clone().get_batch_inputs(request).await?.into_inner(); - store_response.try_into().map_err(Into::into) + store_response.try_into().map_err(StoreError::DeserializationError) } #[instrument(target = COMPONENT, name = "store.client.apply_block", skip_all, err)] diff --git a/crates/ntx-builder/src/clients/store.rs b/crates/ntx-builder/src/clients/store.rs index 1f8c7b5f72..0d11dc1fc6 100644 --- a/crates/ntx-builder/src/clients/store.rs +++ b/crates/ntx-builder/src/clients/store.rs @@ -4,7 +4,7 @@ use std::time::Duration; use miden_node_proto::clients::{Builder, StoreNtxBuilderClient}; use miden_node_proto::domain::account::{AccountDetails, AccountResponse, NetworkAccountId}; -use miden_node_proto::errors::ConversionError; +use miden_node_proto::errors::{ConversionError, ConversionResultExt}; use miden_node_proto::generated::rpc::BlockRange; use miden_node_proto::generated::{self as proto}; use miden_node_proto::try_convert; @@ -101,7 +101,10 @@ impl StoreClient { match response.current_block_header { // There are new blocks compared to the builder's latest state Some(block) => { - let peaks = try_convert(response.current_peaks).collect::>()?; + let peaks: Vec = try_convert(response.current_peaks) + .collect::>() + .context("current_peaks") + .map_err(StoreError::DeserializationError)?; let header = BlockHeader::try_from(block).map_err(StoreError::DeserializationError)?; @@ -140,9 +143,7 @@ impl StoreClient { // which implies details being public, so OK to error otherwise let account = match store_response.map(|acc| acc.details) { Some(Some(details)) => Some(Account::read_from_bytes(&details).map_err(|err| { - StoreError::DeserializationError(ConversionError::deserialization_error( - "account", err, - )) + StoreError::DeserializationError(ConversionError::from(err).context("details")) })?), _ => None, }; @@ -185,7 +186,8 @@ impl StoreClient { let account_details = account_response .details .ok_or(StoreError::MissingDetails("account details".into()))?; - let partial_account = build_minimal_foreign_account(&account_details)?; + let partial_account = build_minimal_foreign_account(&account_details) + .map_err(StoreError::DeserializationError)?; Ok(AccountInputs::new(partial_account, account_response.witness)) } @@ -216,7 +218,10 @@ impl StoreClient { all_notes.reserve(resp.notes.len()); for note in resp.notes { - all_notes.push(AccountTargetNetworkNote::try_from(note)?); + all_notes.push( + AccountTargetNetworkNote::try_from(note) + .map_err(StoreError::DeserializationError)?, + ); } match resp.next_token { @@ -317,10 +322,9 @@ impl StoreClient { .into_iter() .map(|account_id| { let account_id = AccountId::read_from_bytes(&account_id.id).map_err(|err| { - StoreError::DeserializationError(ConversionError::deserialization_error( - "account_id", - err, - )) + StoreError::DeserializationError( + ConversionError::from(err).context("account_id"), + ) })?; NetworkAccountId::try_from(account_id).map_err(|_| { StoreError::MalformedResponse( @@ -330,12 +334,9 @@ impl StoreClient { }) .collect::, StoreError>>()?; - let pagination_info = response.pagination_info.ok_or( - ConversionError::MissingFieldInProtobufRepresentation { - entity: "NetworkAccountIdList", - field_name: "pagination_info", - }, - )?; + let pagination_info = response.pagination_info.ok_or(ConversionError::missing_field::< + proto::store::NetworkAccountIdList, + >("pagination_info"))?; Ok((accounts, pagination_info)) } @@ -406,8 +407,10 @@ impl StoreClient { let smt_opening = asset_witness.proof.ok_or_else(|| { StoreError::MalformedResponse("missing proof in vault asset witness".to_string()) })?; - let proof: SmtProof = - smt_opening.try_into().map_err(StoreError::DeserializationError)?; + let proof: SmtProof = smt_opening + .try_into() + .context("proof") + .map_err(StoreError::DeserializationError)?; let witness = AssetWitness::new(proof) .map_err(|err| StoreError::DeserializationError(ConversionError::from(err)))?; @@ -445,7 +448,10 @@ impl StoreClient { StoreError::MalformedResponse("missing proof in storage map witness".to_string()) })?; - let proof: SmtProof = smt_opening.try_into().map_err(StoreError::DeserializationError)?; + let proof: SmtProof = smt_opening + .try_into() + .context("proof") + .map_err(StoreError::DeserializationError)?; // Create the storage map witness using the proof and raw map key. let witness = StorageMapWitness::new(proof, [map_key]).map_err(|_err| { @@ -482,10 +488,11 @@ pub fn build_minimal_foreign_account( account_details: &AccountDetails, ) -> Result { // Derive account code. - let account_code_bytes = account_details - .account_code - .as_ref() - .ok_or(ConversionError::AccountCodeMissing)?; + let account_code_bytes = account_details.account_code.as_ref().ok_or_else(|| { + ConversionError::missing_field::( + "account_code", + ) + })?; let account_code = AccountCode::from_bytes(account_code_bytes)?; // Derive partial storage. Storage maps are not required for foreign accounts. diff --git a/crates/proto/src/domain/account.rs b/crates/proto/src/domain/account.rs index aeec888328..5c4b643b4a 100644 --- a/crates/proto/src/domain/account.rs +++ b/crates/proto/src/domain/account.rs @@ -25,7 +25,7 @@ use miden_standards::note::{NetworkAccountTarget, NetworkAccountTargetError}; use thiserror::Error; use super::try_convert; -use crate::errors::{ConversionError, MissingFieldHelper}; +use crate::errors::{ConversionError, ConversionResultExt}; use crate::generated::{self as proto}; #[cfg(test)] @@ -57,7 +57,8 @@ impl TryFrom for AccountId { type Error = ConversionError; fn try_from(account_id: proto::account::AccountId) -> Result { - AccountId::read_from_bytes(&account_id.id).map_err(|_| ConversionError::NotAValidFelt) + AccountId::read_from_bytes(&account_id.id) + .map_err(|_| ConversionError::message("value is not in the range 0..MODULUS")) } } @@ -125,11 +126,15 @@ impl TryFrom for AccountStorageHeader { .map(|slot| { let slot_name = StorageSlotName::new(slot.slot_name)?; let slot_type = storage_slot_type_from_raw(slot.slot_type)?; - let commitment = - slot.commitment.ok_or(ConversionError::NotAValidFelt)?.try_into()?; + let commitment = slot + .commitment + .ok_or(ConversionError::message("value is not in the range 0..MODULUS"))? + .try_into() + .context("commitment")?; Ok(StorageSlotHeader::new(slot_name, slot_type, commitment)) }) - .collect::, ConversionError>>()?; + .collect::, ConversionError>>() + .context("slots")?; Ok(AccountStorageHeader::new(slot_headers)?) } @@ -153,11 +158,14 @@ impl TryFrom for AccountRequest { let proto::rpc::AccountRequest { account_id, block_num, details } = value; let account_id = account_id - .ok_or(proto::rpc::AccountRequest::missing_field(stringify!(account_id)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + account_id + )))? + .try_into() + .context("account_id")?; let block_num = block_num.map(Into::into); - let details = details.map(TryFrom::try_from).transpose()?; + let details = details.map(TryFrom::try_from).transpose().context("details")?; Ok(AccountRequest { account_id, block_num, details }) } @@ -182,9 +190,14 @@ impl TryFrom for AccountDetai storage_maps, } = value; - let code_commitment = code_commitment.map(TryFrom::try_from).transpose()?; - let asset_vault_commitment = asset_vault_commitment.map(TryFrom::try_from).transpose()?; - let storage_requests = try_convert(storage_maps).collect::>()?; + let code_commitment = + code_commitment.map(TryFrom::try_from).transpose().context("code_commitment")?; + let asset_vault_commitment = asset_vault_commitment + .map(TryFrom::try_from) + .transpose() + .context("asset_vault_commitment")?; + let storage_requests = + try_convert(storage_maps).collect::>().context("storage_maps")?; Ok(AccountDetailRequest { code_commitment, @@ -213,8 +226,13 @@ impl TryFrom(stringify!(slot_data)))? + .try_into() + .context("slot_data")?; Ok(StorageMapRequest { slot_name, slot_data }) } @@ -242,7 +260,7 @@ impl Ok(match value { ProtoSlotData::AllEntries(true) => SlotData::All, ProtoSlotData::AllEntries(false) => { - return Err(ConversionError::EnumDiscriminantOutOfRange); + return Err(ConversionError::message("enum variant discriminant out of range")); }, ProtoSlotData::MapKeys(keys) => { let keys = try_convert(keys.map_keys).collect::, _>>()?; @@ -268,18 +286,30 @@ impl TryFrom for AccountHeader { } = value; let account_id = account_id - .ok_or(proto::account::AccountHeader::missing_field(stringify!(account_id)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + account_id + )))? + .try_into() + .context("account_id")?; let vault_root = vault_root - .ok_or(proto::account::AccountHeader::missing_field(stringify!(vault_root)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + vault_root + )))? + .try_into() + .context("vault_root")?; let storage_commitment = storage_commitment - .ok_or(proto::account::AccountHeader::missing_field(stringify!(storage_commitment)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + storage_commitment + )))? + .try_into() + .context("storage_commitment")?; let code_commitment = code_commitment - .ok_or(proto::account::AccountHeader::missing_field(stringify!(code_commitment)))? - .try_into()?; - let nonce = nonce.try_into().map_err(|_e| ConversionError::NotAValidFelt)?; + .ok_or(ConversionError::missing_field::(stringify!( + code_commitment + )))? + .try_into() + .context("code_commitment")?; + let nonce = nonce.try_into().map_err(ConversionError::message).context("nonce")?; Ok(AccountHeader::new( account_id, @@ -375,12 +405,13 @@ impl TryFrom for AccountVaultDetails { } else { let parsed_assets = Result::, ConversionError>::from_iter(assets.into_iter().map(|asset| { - let asset = asset - .asset - .ok_or(proto::primitives::Asset::missing_field(stringify!(asset)))?; - let asset = Word::try_from(asset)?; - Asset::try_from(asset).map_err(ConversionError::AssetError) - }))?; + let asset = asset.asset.ok_or(ConversionError::missing_field::< + proto::primitives::Asset, + >(stringify!(asset)))?; + let asset = Word::try_from(asset).context("asset")?; + Asset::try_from(asset).map_err(ConversionError::from) + })) + .context("assets")?; Ok(Self::Assets(parsed_assets)) } } @@ -535,18 +566,16 @@ impl TryFrom entries, } = value; - let slot_name = StorageSlotName::new(slot_name)?; + let slot_name = StorageSlotName::new(slot_name).context("slot_name")?; let entries = if too_many_entries { StorageMapEntries::LimitExceeded } else { match entries { None => { - return Err( - proto::rpc::account_storage_details::AccountStorageMapDetails::missing_field( - stringify!(entries), - ), - ); + return Err(ConversionError::missing_field::< + proto::rpc::account_storage_details::AccountStorageMapDetails, + >(stringify!(entries))); }, Some(ProtoEntries::AllEntries(AllMapEntries { entries })) => { let entries = entries @@ -554,28 +583,39 @@ impl TryFrom .map(|entry| { let key = entry .key - .ok_or(StorageMapEntry::missing_field(stringify!(key)))? + .ok_or(ConversionError::missing_field::( + stringify!(key), + ))? .try_into() - .map(StorageMapKey::new)?; + .map(StorageMapKey::new) + .context("key")?; let value = entry .value - .ok_or(StorageMapEntry::missing_field(stringify!(value)))? - .try_into()?; + .ok_or(ConversionError::missing_field::( + stringify!(value), + ))? + .try_into() + .context("value")?; Ok((key, value)) }) - .collect::, ConversionError>>()?; + .collect::, ConversionError>>() + .context("entries")?; StorageMapEntries::AllEntries(entries) }, Some(ProtoEntries::EntriesWithProofs(MapEntriesWithProofs { entries })) => { let proofs = entries .into_iter() .map(|entry| { - let smt_opening = entry.proof.ok_or( - StorageMapEntryWithProof::missing_field(stringify!(proof)), - )?; - SmtProof::try_from(smt_opening) + let smt_opening = + entry.proof.ok_or(ConversionError::missing_field::< + StorageMapEntryWithProof, + >(stringify!( + proof + )))?; + SmtProof::try_from(smt_opening).context("proof") }) - .collect::, ConversionError>>()?; + .collect::, ConversionError>>() + .context("entries")?; StorageMapEntries::EntriesWithProofs(proofs) }, } @@ -674,10 +714,14 @@ impl TryFrom for AccountStorageDetails { let proto::rpc::AccountStorageDetails { header, map_details } = value; let header = header - .ok_or(proto::rpc::AccountStorageDetails::missing_field(stringify!(header)))? - .try_into()?; + .ok_or(ConversionError::missing_field::( + stringify!(header), + ))? + .try_into() + .context("header")?; - let map_details = try_convert(map_details).collect::, _>>()?; + let map_details = + try_convert(map_details).collect::, _>>().context("map_details")?; Ok(Self { header, map_details }) } @@ -694,11 +738,11 @@ impl From for proto::rpc::AccountStorageDetails { } } -const fn storage_slot_type_from_raw(slot_type: u32) -> Result { +fn storage_slot_type_from_raw(slot_type: u32) -> Result { Ok(match slot_type { 0 => StorageSlotType::Value, 1 => StorageSlotType::Map, - _ => return Err(ConversionError::EnumDiscriminantOutOfRange), + _ => return Err(ConversionError::message("enum variant discriminant out of range")), }) } @@ -726,14 +770,19 @@ impl TryFrom for AccountResponse { let proto::rpc::AccountResponse { block_num, witness, details } = value; let block_num = block_num - .ok_or(proto::rpc::AccountResponse::missing_field(stringify!(block_num)))? + .ok_or(ConversionError::missing_field::(stringify!( + block_num + )))? .into(); let witness = witness - .ok_or(proto::rpc::AccountResponse::missing_field(stringify!(witness)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + witness + )))? + .try_into() + .context("witness")?; - let details = details.map(TryFrom::try_from).transpose()?; + let details = details.map(TryFrom::try_from).transpose().context("details")?; Ok(AccountResponse { block_num, witness, details }) } @@ -792,20 +841,25 @@ impl TryFrom for AccountDetails { } = value; let account_header = header - .ok_or(proto::rpc::account_response::AccountDetails::missing_field(stringify!(header)))? - .try_into()?; + .ok_or(ConversionError::missing_field::( + stringify!(header), + ))? + .try_into() + .context("header")?; let storage_details = storage_details - .ok_or(proto::rpc::account_response::AccountDetails::missing_field(stringify!( - storage_details - )))? - .try_into()?; + .ok_or(ConversionError::missing_field::( + stringify!(storage_details), + ))? + .try_into() + .context("storage_details")?; let vault_details = vault_details - .ok_or(proto::rpc::account_response::AccountDetails::missing_field(stringify!( - vault_details - )))? - .try_into()?; + .ok_or(ConversionError::missing_field::( + stringify!(vault_details), + ))? + .try_into() + .context("vault_details")?; let account_code = code; Ok(AccountDetails { @@ -849,19 +903,28 @@ impl TryFrom for AccountWitness { fn try_from(account_witness: proto::account::AccountWitness) -> Result { let witness_id = account_witness .witness_id - .ok_or(proto::account::AccountWitness::missing_field(stringify!(witness_id)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + witness_id + )))? + .try_into() + .context("witness_id")?; let commitment = account_witness .commitment - .ok_or(proto::account::AccountWitness::missing_field(stringify!(commitment)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + commitment + )))? + .try_into() + .context("commitment")?; let path = account_witness .path - .ok_or(proto::account::AccountWitness::missing_field(stringify!(path)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + path + )))? + .try_into() + .context("path")?; AccountWitness::new(witness_id, commitment, path).map_err(|err| { - ConversionError::deserialization_error( + ConversionError::deserialization( "AccountWitness", DeserializationError::InvalidValue(err.to_string()), ) @@ -897,21 +960,30 @@ impl TryFrom for AccountWitnessRecord { ) -> Result { let witness_id = account_witness_record .witness_id - .ok_or(proto::account::AccountWitness::missing_field(stringify!(witness_id)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + witness_id + )))? + .try_into() + .context("witness_id")?; let commitment = account_witness_record .commitment - .ok_or(proto::account::AccountWitness::missing_field(stringify!(commitment)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + commitment + )))? + .try_into() + .context("commitment")?; let path: SparseMerklePath = account_witness_record .path .as_ref() - .ok_or(proto::account::AccountWitness::missing_field(stringify!(path)))? + .ok_or(ConversionError::missing_field::(stringify!( + path + )))? .clone() - .try_into()?; + .try_into() + .context("path")?; let witness = AccountWitness::new(witness_id, commitment, path).map_err(|err| { - ConversionError::deserialization_error( + ConversionError::deserialization( "AccountWitness", DeserializationError::InvalidValue(err.to_string()), ) @@ -920,8 +992,11 @@ impl TryFrom for AccountWitnessRecord { Ok(Self { account_id: account_witness_record .account_id - .ok_or(proto::account::AccountWitness::missing_field(stringify!(account_id)))? - .try_into()?, + .ok_or(ConversionError::missing_field::( + stringify!(account_id), + ))? + .try_into() + .context("account_id")?, witness, }) } @@ -968,17 +1043,19 @@ impl TryFrom fo ) -> Result { let account_id = from .account_id - .ok_or(proto::store::transaction_inputs::AccountTransactionInputRecord::missing_field( - stringify!(account_id), - ))? - .try_into()?; + .ok_or(ConversionError::missing_field::< + proto::store::transaction_inputs::AccountTransactionInputRecord, + >(stringify!(account_id)))? + .try_into() + .context("account_id")?; let account_commitment = from .account_commitment - .ok_or(proto::store::transaction_inputs::AccountTransactionInputRecord::missing_field( - stringify!(account_commitment), - ))? - .try_into()?; + .ok_or(ConversionError::missing_field::< + proto::store::transaction_inputs::AccountTransactionInputRecord, + >(stringify!(account_commitment)))? + .try_into() + .context("account_commitment")?; // If the commitment is equal to `Word::empty()`, it signifies that this is a new // account which is not yet present in the Store. @@ -1008,10 +1085,12 @@ impl TryFrom for Asset { type Error = ConversionError; fn try_from(value: proto::primitives::Asset) -> Result { - let inner = value.asset.ok_or(proto::primitives::Asset::missing_field("asset"))?; - let word = Word::try_from(inner)?; + let inner = value + .asset + .ok_or(ConversionError::missing_field::("asset"))?; + let word = Word::try_from(inner).context("asset")?; - Asset::try_from(word).map_err(ConversionError::AssetError) + Asset::try_from(word).map_err(ConversionError::from) } } diff --git a/crates/proto/src/domain/batch.rs b/crates/proto/src/domain/batch.rs index 1cccf6ab8b..af30b8a157 100644 --- a/crates/proto/src/domain/batch.rs +++ b/crates/proto/src/domain/batch.rs @@ -5,7 +5,7 @@ use miden_protocol::note::{NoteId, NoteInclusionProof}; use miden_protocol::transaction::PartialBlockchain; use miden_protocol::utils::{Deserializable, Serializable}; -use crate::errors::{ConversionError, MissingFieldHelper}; +use crate::errors::{ConversionError, ConversionResultExt}; use crate::generated as proto; /// Data required for a transaction batch. @@ -33,17 +33,17 @@ impl TryFrom for BatchInputs { let result = Self { batch_reference_block_header: response .batch_reference_block_header - .ok_or(proto::store::BatchInputs::missing_field("block_header"))? - .try_into()?, + .ok_or(ConversionError::missing_field::("block_header"))? + .try_into() + .context("batch_reference_block_header")?, note_proofs: response .note_proofs .iter() .map(<(NoteId, NoteInclusionProof)>::try_from) - .collect::>()?, + .collect::>() + .context("note_proofs")?, partial_block_chain: PartialBlockchain::read_from_bytes(&response.partial_block_chain) - .map_err(|source| { - ConversionError::deserialization_error("PartialBlockchain", source) - })?, + .map_err(|source| ConversionError::deserialization("PartialBlockchain", source))?, }; Ok(result) diff --git a/crates/proto/src/domain/block.rs b/crates/proto/src/domain/block.rs index 112f84e50b..7d87adb3c2 100644 --- a/crates/proto/src/domain/block.rs +++ b/crates/proto/src/domain/block.rs @@ -17,7 +17,7 @@ use miden_protocol::transaction::PartialBlockchain; use miden_protocol::utils::{Deserializable, Serializable}; use thiserror::Error; -use crate::errors::{ConversionError, MissingFieldHelper}; +use crate::errors::{ConversionError, ConversionResultExt}; use crate::{AccountWitnessRecord, NullifierWitnessRecord, generated as proto}; // BLOCK NUMBER @@ -79,44 +79,67 @@ impl TryFrom for BlockHeader { value.version, value .prev_block_commitment - .ok_or(proto::blockchain::BlockHeader::missing_field(stringify!( - prev_block_commitment - )))? - .try_into()?, + .ok_or(ConversionError::missing_field::( + stringify!(prev_block_commitment), + ))? + .try_into() + .context("prev_block_commitment")?, value.block_num.into(), value .chain_commitment - .ok_or(proto::blockchain::BlockHeader::missing_field(stringify!(chain_commitment)))? - .try_into()?, + .ok_or(ConversionError::missing_field::( + stringify!(chain_commitment), + ))? + .try_into() + .context("chain_commitment")?, value .account_root - .ok_or(proto::blockchain::BlockHeader::missing_field(stringify!(account_root)))? - .try_into()?, + .ok_or(ConversionError::missing_field::( + stringify!(account_root), + ))? + .try_into() + .context("account_root")?, value .nullifier_root - .ok_or(proto::blockchain::BlockHeader::missing_field(stringify!(nullifier_root)))? - .try_into()?, + .ok_or(ConversionError::missing_field::( + stringify!(nullifier_root), + ))? + .try_into() + .context("nullifier_root")?, value .note_root - .ok_or(proto::blockchain::BlockHeader::missing_field(stringify!(note_root)))? - .try_into()?, + .ok_or(ConversionError::missing_field::( + stringify!(note_root), + ))? + .try_into() + .context("note_root")?, value .tx_commitment - .ok_or(proto::blockchain::BlockHeader::missing_field(stringify!(tx_commitment)))? - .try_into()?, + .ok_or(ConversionError::missing_field::( + stringify!(tx_commitment), + ))? + .try_into() + .context("tx_commitment")?, value .tx_kernel_commitment - .ok_or(proto::blockchain::BlockHeader::missing_field(stringify!( - tx_kernel_commitment - )))? - .try_into()?, + .ok_or(ConversionError::missing_field::( + stringify!(tx_kernel_commitment), + ))? + .try_into() + .context("tx_kernel_commitment")?, value .validator_key - .ok_or(proto::blockchain::BlockHeader::missing_field(stringify!(validator_key)))? - .try_into()?, + .ok_or(ConversionError::missing_field::( + stringify!(validator_key), + ))? + .try_into() + .context("validator_key")?, FeeParameters::try_from(value.fee_parameters.ok_or( - proto::blockchain::FeeParameters::missing_field(stringify!(fee_parameters)), - )?)?, + ConversionError::missing_field::(stringify!( + fee_parameters + )), + )?) + .context("fee_parameters")?, value.timestamp, )) } @@ -149,7 +172,7 @@ impl TryFrom for BlockBody { type Error = ConversionError; fn try_from(value: proto::blockchain::BlockBody) -> Result { BlockBody::read_from_bytes(&value.block_body) - .map_err(|source| ConversionError::deserialization_error("BlockBody", source)) + .map_err(|source| ConversionError::deserialization("BlockBody", source)) } } @@ -185,16 +208,25 @@ impl TryFrom for SignedBlock { fn try_from(value: proto::blockchain::SignedBlock) -> Result { let header = value .header - .ok_or(proto::blockchain::SignedBlock::missing_field(stringify!(header)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + header + )))? + .try_into() + .context("header")?; let body = value .body - .ok_or(proto::blockchain::SignedBlock::missing_field(stringify!(body)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + body + )))? + .try_into() + .context("body")?; let signature = value .signature - .ok_or(proto::blockchain::SignedBlock::missing_field(stringify!(signature)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + signature + )))? + .try_into() + .context("signature")?; Ok(SignedBlock::new_unchecked(header, body, signature)) } @@ -241,8 +273,11 @@ impl TryFrom for BlockInputs { fn try_from(response: proto::store::BlockInputs) -> Result { let latest_block_header: BlockHeader = response .latest_block_header - .ok_or(proto::blockchain::BlockHeader::missing_field("block_header"))? - .try_into()?; + .ok_or(ConversionError::missing_field::( + "block_header", + ))? + .try_into() + .context("latest_block_header")?; let account_witnesses = response .account_witnesses @@ -251,7 +286,8 @@ impl TryFrom for BlockInputs { let witness_record: AccountWitnessRecord = entry.try_into()?; Ok((witness_record.account_id, witness_record.witness)) }) - .collect::, ConversionError>>()?; + .collect::, ConversionError>>() + .context("account_witnesses")?; let nullifier_witnesses = response .nullifier_witnesses @@ -260,18 +296,18 @@ impl TryFrom for BlockInputs { let witness: NullifierWitnessRecord = entry.try_into()?; Ok((witness.nullifier, NullifierWitness::new(witness.proof))) }) - .collect::, ConversionError>>()?; + .collect::, ConversionError>>() + .context("nullifier_witnesses")?; let unauthenticated_note_proofs = response .unauthenticated_note_proofs .iter() .map(<(NoteId, NoteInclusionProof)>::try_from) - .collect::>()?; + .collect::>() + .context("unauthenticated_note_proofs")?; let partial_block_chain = PartialBlockchain::read_from_bytes(&response.partial_block_chain) - .map_err(|source| { - ConversionError::deserialization_error("PartialBlockchain", source) - })?; + .map_err(|source| ConversionError::deserialization("PartialBlockchain", source))?; Ok(BlockInputs::new( latest_block_header, @@ -290,7 +326,7 @@ impl TryFrom for PublicKey { type Error = ConversionError; fn try_from(public_key: proto::blockchain::ValidatorPublicKey) -> Result { PublicKey::read_from_bytes(&public_key.validator_key) - .map_err(|source| ConversionError::deserialization_error("PublicKey", source)) + .map_err(|source| ConversionError::deserialization("PublicKey", source)) } } @@ -313,7 +349,7 @@ impl TryFrom for Signature { type Error = ConversionError; fn try_from(signature: proto::blockchain::BlockSignature) -> Result { Signature::read_from_bytes(&signature.signature) - .map_err(|source| ConversionError::deserialization_error("Signature", source)) + .map_err(|source| ConversionError::deserialization("Signature", source)) } } @@ -335,9 +371,13 @@ impl From<&Signature> for proto::blockchain::BlockSignature { impl TryFrom for FeeParameters { type Error = ConversionError; fn try_from(fee_params: proto::blockchain::FeeParameters) -> Result { - let native_asset_id = fee_params.native_asset_id.map(AccountId::try_from).ok_or( - proto::blockchain::FeeParameters::missing_field(stringify!(native_asset_id)), - )??; + let native_asset_id = fee_params + .native_asset_id + .map(AccountId::try_from) + .ok_or(ConversionError::missing_field::(stringify!( + native_asset_id + )))? + .context("native_asset_id")?; let fee_params = FeeParameters::new(native_asset_id, fee_params.verification_base_fee)?; Ok(fee_params) } diff --git a/crates/proto/src/domain/digest.rs b/crates/proto/src/domain/digest.rs index 08d8c3f9a1..2815cef093 100644 --- a/crates/proto/src/domain/digest.rs +++ b/crates/proto/src/domain/digest.rs @@ -65,12 +65,12 @@ impl FromHex for proto::primitives::Digest { let data = hex::decode(hex)?; match data.len() { - size if size < DIGEST_DATA_SIZE => { - Err(ConversionError::InsufficientData { expected: DIGEST_DATA_SIZE, got: size }) - }, - size if size > DIGEST_DATA_SIZE => { - Err(ConversionError::TooMuchData { expected: DIGEST_DATA_SIZE, got: size }) - }, + size if size < DIGEST_DATA_SIZE => Err(ConversionError::message(format!( + "not enough data, expected {DIGEST_DATA_SIZE}, got {size}" + ))), + size if size > DIGEST_DATA_SIZE => Err(ConversionError::message(format!( + "too much data, expected {DIGEST_DATA_SIZE}, got {size}" + ))), _ => { let d0 = u64::from_be_bytes(data[..8].try_into().unwrap()); let d1 = u64::from_be_bytes(data[8..16].try_into().unwrap()); @@ -178,7 +178,7 @@ impl TryFrom for [Felt; 4] { .iter() .any(|v| *v >= ::MODULUS) { - return Err(ConversionError::NotAValidFelt); + return Err(ConversionError::message("value is not in the range 0..MODULUS")); } Ok([ diff --git a/crates/proto/src/domain/mempool.rs b/crates/proto/src/domain/mempool.rs index c9bf76bfc9..32fdb5b7f8 100644 --- a/crates/proto/src/domain/mempool.rs +++ b/crates/proto/src/domain/mempool.rs @@ -7,7 +7,7 @@ use miden_protocol::transaction::TransactionId; use miden_protocol::utils::{Deserializable, Serializable}; use miden_standards::note::AccountTargetNetworkNote; -use crate::errors::{ConversionError, MissingFieldHelper}; +use crate::errors::{ConversionError, ConversionResultExt}; use crate::generated as proto; #[derive(Debug, Clone)] @@ -82,30 +82,37 @@ impl TryFrom for MempoolEvent { type Error = ConversionError; fn try_from(event: proto::block_producer::MempoolEvent) -> Result { - let event = - event.event.ok_or(proto::block_producer::MempoolEvent::missing_field("event"))?; + let event = event.event.ok_or(ConversionError::missing_field::< + proto::block_producer::MempoolEvent, + >("event"))?; match event { proto::block_producer::mempool_event::Event::TransactionAdded(tx) => { let id = tx .id - .ok_or(proto::block_producer::mempool_event::TransactionAdded::missing_field( - "id", - ))? - .try_into()?; - let nullifiers = - tx.nullifiers.into_iter().map(TryInto::try_into).collect::>()?; + .ok_or(ConversionError::missing_field::< + proto::block_producer::mempool_event::TransactionAdded, + >("id"))? + .try_into() + .context("id")?; + let nullifiers = tx + .nullifiers + .into_iter() + .map(Nullifier::try_from) + .collect::>() + .context("nullifiers")?; let network_notes = tx .network_notes .into_iter() - .map(TryInto::try_into) - .collect::>()?; + .map(AccountTargetNetworkNote::try_from) + .collect::>() + .context("network_notes")?; let account_delta = tx .network_account_delta .as_deref() .map(AccountUpdateDetails::read_from_bytes) .transpose() - .map_err(|err| ConversionError::deserialization_error("account_delta", err))?; + .map_err(|err| ConversionError::deserialization("account_delta", err))?; Ok(Self::TransactionAdded { id, @@ -117,16 +124,18 @@ impl TryFrom for MempoolEvent { proto::block_producer::mempool_event::Event::BlockCommitted(block_committed) => { let header = block_committed .block_header - .ok_or(proto::block_producer::mempool_event::BlockCommitted::missing_field( - "block_header", - ))? - .try_into()?; + .ok_or(ConversionError::missing_field::< + proto::block_producer::mempool_event::BlockCommitted, + >("block_header"))? + .try_into() + .context("block_header")?; let header = Box::new(header); let txs = block_committed .transactions .into_iter() .map(TransactionId::try_from) - .collect::>()?; + .collect::>() + .context("transactions")?; Ok(Self::BlockCommitted { header, txs }) }, @@ -135,7 +144,8 @@ impl TryFrom for MempoolEvent { .reverted .into_iter() .map(TransactionId::try_from) - .collect::>()?; + .collect::>() + .context("reverted")?; Ok(Self::TransactionsReverted(txs)) }, diff --git a/crates/proto/src/domain/merkle.rs b/crates/proto/src/domain/merkle.rs index ed14d523ba..040fd700ae 100644 --- a/crates/proto/src/domain/merkle.rs +++ b/crates/proto/src/domain/merkle.rs @@ -4,7 +4,7 @@ use miden_protocol::crypto::merkle::smt::{LeafIndex, SmtLeaf, SmtProof}; use miden_protocol::crypto::merkle::{MerklePath, SparseMerklePath}; use crate::domain::{convert, try_convert}; -use crate::errors::{ConversionError, MissingFieldHelper}; +use crate::errors::{ConversionError, ConversionResultExt}; use crate::generated as proto; // MERKLE PATH @@ -62,7 +62,8 @@ impl TryFrom for SparseMerklePath { .siblings .into_iter() .map(Word::try_from) - .collect::, _>>()?, + .collect::, _>>() + .context("siblings")?, )?) } } @@ -84,12 +85,16 @@ impl TryFrom for MmrDelta { type Error = ConversionError; fn try_from(value: proto::primitives::MmrDelta) -> Result { - let data: Result, ConversionError> = - value.data.into_iter().map(Word::try_from).collect(); + let data: Vec<_> = value + .data + .into_iter() + .map(Word::try_from) + .collect::>() + .context("data")?; Ok(MmrDelta { forest: Forest::new(value.forest as usize), - data: data?, + data, }) } } @@ -104,20 +109,22 @@ impl TryFrom for SmtLeaf { type Error = ConversionError; fn try_from(value: proto::primitives::SmtLeaf) -> Result { - let leaf = value.leaf.ok_or(proto::primitives::SmtLeaf::missing_field(stringify!(leaf)))?; + let leaf = value.leaf.ok_or( + ConversionError::missing_field::(stringify!(leaf)), + )?; match leaf { proto::primitives::smt_leaf::Leaf::EmptyLeafIndex(leaf_index) => { Ok(Self::new_empty(LeafIndex::new_max_depth(leaf_index))) }, proto::primitives::smt_leaf::Leaf::Single(entry) => { - let (key, value): (Word, Word) = entry.try_into()?; + let (key, value): (Word, Word) = entry.try_into().context("single")?; Ok(SmtLeaf::new_single(key, value)) }, proto::primitives::smt_leaf::Leaf::Multiple(entries) => { let domain_entries: Vec<(Word, Word)> = - try_convert(entries.entries).collect::>()?; + try_convert(entries.entries).collect::>().context("multiple")?; Ok(SmtLeaf::new_multiple(domain_entries)?) }, @@ -150,12 +157,18 @@ impl TryFrom for (Word, Word) { fn try_from(entry: proto::primitives::SmtLeafEntry) -> Result { let key: Word = entry .key - .ok_or(proto::primitives::SmtLeafEntry::missing_field(stringify!(key)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + key + )))? + .try_into() + .context("key")?; let value: Word = entry .value - .ok_or(proto::primitives::SmtLeafEntry::missing_field(stringify!(value)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + value + )))? + .try_into() + .context("value")?; Ok((key, value)) } @@ -179,12 +192,18 @@ impl TryFrom for SmtProof { fn try_from(opening: proto::primitives::SmtOpening) -> Result { let path: SparseMerklePath = opening .path - .ok_or(proto::primitives::SmtOpening::missing_field(stringify!(path)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + path + )))? + .try_into() + .context("path")?; let leaf: SmtLeaf = opening .leaf - .ok_or(proto::primitives::SmtOpening::missing_field(stringify!(leaf)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + leaf + )))? + .try_into() + .context("leaf")?; Ok(SmtProof::new(path, leaf)?) } diff --git a/crates/proto/src/domain/note.rs b/crates/proto/src/domain/note.rs index 6a750a6582..08ad922c22 100644 --- a/crates/proto/src/domain/note.rs +++ b/crates/proto/src/domain/note.rs @@ -17,7 +17,7 @@ use miden_protocol::utils::{Deserializable, Serializable}; use miden_protocol::{MastForest, MastNodeId, Word}; use miden_standards::note::AccountTargetNetworkNote; -use crate::errors::{ConversionError, MissingFieldHelper}; +use crate::errors::{ConversionError, ConversionResultExt}; use crate::generated as proto; // NOTE TYPE @@ -39,7 +39,9 @@ impl TryFrom for NoteType { match note_type { proto::note::NoteType::Public => Ok(NoteType::Public), proto::note::NoteType::Private => Ok(NoteType::Private), - proto::note::NoteType::Unspecified => Err(ConversionError::EnumDiscriminantOutOfRange), + proto::note::NoteType::Unspecified => { + Err(ConversionError::message("enum variant discriminant out of range")) + }, } } } @@ -53,11 +55,15 @@ impl TryFrom for NoteMetadata { fn try_from(value: proto::note::NoteMetadata) -> Result { let sender = value .sender - .ok_or_else(|| proto::note::NoteMetadata::missing_field(stringify!(sender)))? - .try_into()?; + .ok_or_else(|| { + ConversionError::missing_field::(stringify!(sender)) + })? + .try_into() + .context("sender")?; let note_type = proto::note::NoteType::try_from(value.note_type) - .map_err(|_| ConversionError::EnumDiscriminantOutOfRange)? - .try_into()?; + .map_err(|_| ConversionError::message("enum variant discriminant out of range"))? + .try_into() + .context("note_type")?; let tag = NoteTag::new(value.tag); // Deserialize attachment if present @@ -65,7 +71,7 @@ impl TryFrom for NoteMetadata { NoteAttachment::default() } else { NoteAttachment::read_from_bytes(&value.attachment) - .map_err(|err| ConversionError::deserialization_error("NoteAttachment", err))? + .map_err(|err| ConversionError::deserialization("NoteAttachment", err))? }; Ok(NoteMetadata::new(sender, note_type).with_tag(tag).with_attachment(attachment)) @@ -105,14 +111,17 @@ impl TryFrom for AccountTargetNetworkNote { fn try_from(value: proto::note::NetworkNote) -> Result { let details = NoteDetails::read_from_bytes(&value.details) - .map_err(|err| ConversionError::deserialization_error("NoteDetails", err))?; + .map_err(|err| ConversionError::deserialization("NoteDetails", err))?; let (assets, recipient) = details.into_parts(); let metadata: NoteMetadata = value .metadata - .ok_or_else(|| proto::note::NetworkNote::missing_field(stringify!(metadata)))? - .try_into()?; + .ok_or_else(|| { + ConversionError::missing_field::(stringify!(metadata)) + })? + .try_into() + .context("metadata")?; let note = Note::new(assets, metadata, recipient); - AccountTargetNetworkNote::new(note).map_err(ConversionError::NetworkNoteError) + AccountTargetNetworkNote::new(note).map_err(ConversionError::from) } } @@ -140,7 +149,7 @@ impl TryFrom for Word { note_id .id .as_ref() - .ok_or(proto::note::NoteId::missing_field(stringify!(id)))? + .ok_or(ConversionError::missing_field::(stringify!(id)))? .try_into() } } @@ -172,27 +181,31 @@ impl TryFrom<&proto::note::NoteInclusionInBlockProof> for (NoteId, NoteInclusion proof .inclusion_path .as_ref() - .ok_or(proto::note::NoteInclusionInBlockProof::missing_field(stringify!( - inclusion_path - )))? + .ok_or(ConversionError::missing_field::( + stringify!(inclusion_path), + ))? .clone(), - )?; + ) + .context("inclusion_path")?; let note_id = Word::try_from( proof .note_id .as_ref() - .ok_or(proto::note::NoteInclusionInBlockProof::missing_field(stringify!(note_id)))? + .ok_or(ConversionError::missing_field::( + stringify!(note_id), + ))? .id .as_ref() - .ok_or(proto::note::NoteId::missing_field(stringify!(id)))?, - )?; + .ok_or(ConversionError::missing_field::(stringify!(id)))?, + ) + .context("note_id")?; Ok(( NoteId::from_raw(note_id), NoteInclusionProof::new( proof.block_num.into(), - proof.note_index_in_block.try_into()?, + proof.note_index_in_block.try_into().context("note_index_in_block")?, inclusion_path, )?, )) @@ -205,15 +218,16 @@ impl TryFrom for Note { fn try_from(proto_note: proto::note::Note) -> Result { let metadata: NoteMetadata = proto_note .metadata - .ok_or(proto::note::Note::missing_field(stringify!(metadata)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!(metadata)))? + .try_into() + .context("metadata")?; let details = proto_note .details - .ok_or(proto::note::Note::missing_field(stringify!(details)))?; + .ok_or(ConversionError::missing_field::(stringify!(details)))?; let note_details = NoteDetails::read_from_bytes(&details) - .map_err(|err| ConversionError::deserialization_error("NoteDetails", err))?; + .map_err(|err| ConversionError::deserialization("NoteDetails", err))?; let (assets, recipient) = note_details.into_parts(); Ok(Note::new(assets, metadata, recipient)) @@ -238,12 +252,18 @@ impl TryFrom for NoteHeader { fn try_from(value: proto::note::NoteHeader) -> Result { let note_id_word: Word = value .note_id - .ok_or_else(|| proto::note::NoteHeader::missing_field(stringify!(note_id)))? - .try_into()?; + .ok_or_else(|| { + ConversionError::missing_field::(stringify!(note_id)) + })? + .try_into() + .context("note_id")?; let metadata: NoteMetadata = value .metadata - .ok_or_else(|| proto::note::NoteHeader::missing_field(stringify!(metadata)))? - .try_into()?; + .ok_or_else(|| { + ConversionError::missing_field::(stringify!(metadata)) + })? + .try_into() + .context("metadata")?; Ok(NoteHeader::new(NoteId::from_raw(note_id_word), metadata)) } @@ -268,9 +288,9 @@ impl TryFrom for NoteScript { let proto::note::NoteScript { entrypoint, mast } = value; let mast = MastForest::read_from_bytes(&mast) - .map_err(|err| Self::Error::deserialization_error("note_script.mast", err))?; + .map_err(|err| ConversionError::deserialization("note_script.mast", err))?; let entrypoint = MastNodeId::from_u32_safe(entrypoint, &mast) - .map_err(|err| Self::Error::deserialization_error("note_script.entrypoint", err))?; + .map_err(|err| ConversionError::deserialization("note_script.entrypoint", err))?; Ok(Self::from_parts(Arc::new(mast), entrypoint)) } diff --git a/crates/proto/src/domain/nullifier.rs b/crates/proto/src/domain/nullifier.rs index 3ccdf88bae..0277d946a7 100644 --- a/crates/proto/src/domain/nullifier.rs +++ b/crates/proto/src/domain/nullifier.rs @@ -2,7 +2,7 @@ use miden_protocol::Word; use miden_protocol::crypto::merkle::smt::SmtProof; use miden_protocol::note::Nullifier; -use crate::errors::{ConversionError, MissingFieldHelper}; +use crate::errors::{ConversionError, ConversionResultExt}; use crate::generated as proto; // FROM NULLIFIER @@ -50,16 +50,18 @@ impl TryFrom for NullifierWitnessR Ok(Self { nullifier: nullifier_witness_record .nullifier - .ok_or(proto::store::block_inputs::NullifierWitness::missing_field(stringify!( - nullifier - )))? - .try_into()?, + .ok_or(ConversionError::missing_field::< + proto::store::block_inputs::NullifierWitness, + >(stringify!(nullifier)))? + .try_into() + .context("nullifier")?, proof: nullifier_witness_record .opening - .ok_or(proto::store::block_inputs::NullifierWitness::missing_field(stringify!( - opening - )))? - .try_into()?, + .ok_or(ConversionError::missing_field::< + proto::store::block_inputs::NullifierWitness, + >(stringify!(opening)))? + .try_into() + .context("opening")?, }) } } diff --git a/crates/proto/src/domain/transaction.rs b/crates/proto/src/domain/transaction.rs index 62052eb633..e7b538d5c5 100644 --- a/crates/proto/src/domain/transaction.rs +++ b/crates/proto/src/domain/transaction.rs @@ -3,7 +3,7 @@ use miden_protocol::note::Nullifier; use miden_protocol::transaction::{InputNoteCommitment, TransactionId}; use miden_protocol::utils::{Deserializable, Serializable}; -use crate::errors::{ConversionError, MissingFieldHelper}; +use crate::errors::{ConversionError, ConversionResultExt}; use crate::generated as proto; // FROM TRANSACTION ID @@ -51,11 +51,9 @@ impl TryFrom for TransactionId { fn try_from(value: proto::transaction::TransactionId) -> Result { value .id - .ok_or(ConversionError::MissingFieldInProtobufRepresentation { - entity: "TransactionId", - field_name: "id", - })? + .ok_or(ConversionError::missing_field::("id"))? .try_into() + .context("id") } } @@ -78,12 +76,15 @@ impl TryFrom for InputNoteCommitment { let nullifier: Nullifier = value .nullifier .ok_or_else(|| { - proto::transaction::InputNoteCommitment::missing_field(stringify!(nullifier)) + ConversionError::missing_field::( + stringify!(nullifier), + ) })? - .try_into()?; + .try_into() + .context("nullifier")?; let header: Option = - value.header.map(TryInto::try_into).transpose()?; + value.header.map(TryInto::try_into).transpose().context("header")?; // TODO: https://github.com/0xMiden/node/issues/1783 // InputNoteCommitment has private fields, so we reconstruct it via @@ -92,6 +93,6 @@ impl TryFrom for InputNoteCommitment { nullifier.write_into(&mut bytes); header.write_into(&mut bytes); InputNoteCommitment::read_from_bytes(&bytes) - .map_err(|err| ConversionError::deserialization_error("InputNoteCommitment", err)) + .map_err(|err| ConversionError::deserialization("InputNoteCommitment", err)) } } diff --git a/crates/proto/src/errors/mod.rs b/crates/proto/src/errors/mod.rs index 04493e6960..a005d1ad99 100644 --- a/crates/proto/src/errors/mod.rs +++ b/crates/proto/src/errors/mod.rs @@ -1,79 +1,96 @@ use std::any::type_name; -use std::num::TryFromIntError; +use std::fmt; // Re-export the GrpcError derive macro for convenience pub use miden_node_grpc_error_macro::GrpcError; -use miden_protocol::crypto::merkle::smt::{SmtLeafError, SmtProofError}; -use miden_protocol::errors::{AccountError, AssetError, FeeError, NoteError, StorageSlotNameError}; use miden_protocol::utils::DeserializationError; -use miden_standards::note::NetworkAccountTargetError; -use thiserror::Error; #[cfg(test)] mod test_macro; -#[derive(Debug, Error)] -pub enum ConversionError { - #[error("asset error")] - AssetError(#[from] AssetError), - #[error("account code missing")] - AccountCodeMissing, - #[error("account error")] - AccountError(#[from] AccountError), - #[error("fee parameters error")] - FeeError(#[from] FeeError), - #[error("hex error")] - HexError(#[from] hex::FromHexError), - #[error("note error")] - NoteError(#[from] NoteError), - #[error("network note error")] - NetworkNoteError(#[source] NetworkAccountTargetError), - #[error("SMT leaf error")] - SmtLeafError(#[from] SmtLeafError), - #[error("SMT proof error")] - SmtProofError(#[from] SmtProofError), - #[error("storage slot name error")] - StorageSlotNameError(#[from] StorageSlotNameError), - #[error("integer conversion error: {0}")] - TryFromIntError(#[from] TryFromIntError), - #[error("too much data, expected {expected}, got {got}")] - TooMuchData { expected: usize, got: usize }, - #[error("not enough data, expected {expected}, got {got}")] - InsufficientData { expected: usize, got: usize }, - #[error("value is not in the range 0..MODULUS")] - NotAValidFelt, - #[error("merkle error")] - MerkleError(#[from] miden_protocol::crypto::merkle::MerkleError), - #[error("field `{entity}::{field_name}` is missing")] - MissingFieldInProtobufRepresentation { - entity: &'static str, - field_name: &'static str, - }, - #[error("failed to deserialize {entity}")] - DeserializationError { - entity: &'static str, - source: DeserializationError, - }, - #[error("enum variant discriminant out of range")] - EnumDiscriminantOutOfRange, +// CONVERSION ERROR +// ================================================================================================ + +/// Opaque error for protobuf-to-domain conversions. +/// +/// Captures an underlying error plus an optional field path stack that describes which nested +/// field caused the error (e.g. `"block.header.account_root: value is not in range 0..MODULUS"`). +/// +/// Always maps to [`tonic::Status::invalid_argument()`]. +#[derive(Debug)] +pub struct ConversionError { + path: Vec<&'static str>, + source: Box, } impl ConversionError { - pub fn deserialization_error(entity: &'static str, source: DeserializationError) -> Self { - Self::DeserializationError { entity, source } + /// Create a new `ConversionError` wrapping any error source. + pub fn new(source: impl std::error::Error + Send + Sync + 'static) -> Self { + Self { + path: Vec::new(), + source: Box::new(source), + } + } + + /// Add field context to the error path. + /// + /// Called from inner to outer, so the path accumulates in reverse + /// (outermost field pushed last). + /// + /// Use this to annotate errors from `try_into()` / `try_from()` where the underlying + /// error has no knowledge of which field it originated from. Do not use it with + /// [`missing_field`](Self::missing_field) which already embeds the field name in its + /// message. + #[must_use] + pub fn context(mut self, field: &'static str) -> Self { + self.path.push(field); + self + } + + /// Create a "missing field" error for a protobuf message type `T`. + pub fn missing_field(field_name: &'static str) -> Self { + Self { + path: Vec::new(), + source: Box::new(MissingFieldError { entity: type_name::(), field_name }), + } } -} -pub trait MissingFieldHelper { - fn missing_field(field_name: &'static str) -> ConversionError; + /// Create a deserialization error for a named entity. + pub fn deserialization(entity: &'static str, source: DeserializationError) -> Self { + Self { + path: Vec::new(), + source: Box::new(DeserializationErrorWrapper { entity, source }), + } + } + + /// Create a `ConversionError` from an ad-hoc error message. + pub fn message(msg: impl Into) -> Self { + Self { + path: Vec::new(), + source: Box::new(StringError(msg.into())), + } + } } -impl MissingFieldHelper for T { - fn missing_field(field_name: &'static str) -> ConversionError { - ConversionError::MissingFieldInProtobufRepresentation { - entity: type_name::(), - field_name, +impl fmt::Display for ConversionError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if !self.path.is_empty() { + // Path was pushed inner-to-outer, so reverse for display. + for (i, segment) in self.path.iter().rev().enumerate() { + if i > 0 { + f.write_str(".")?; + } + f.write_str(segment)?; + } + f.write_str(": ")?; } + write!(f, "{}", self.source) + } +} + +impl std::error::Error for ConversionError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&*self.source) } } @@ -82,3 +99,155 @@ impl From for tonic::Status { tonic::Status::invalid_argument(value.to_string()) } } + +// INTERNAL HELPER ERROR TYPES +// ================================================================================================ + +#[derive(Debug)] +struct MissingFieldError { + entity: &'static str, + field_name: &'static str, +} + +impl fmt::Display for MissingFieldError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "field `{}::{}` is missing", self.entity, self.field_name) + } +} + +impl std::error::Error for MissingFieldError {} + +#[derive(Debug)] +struct DeserializationErrorWrapper { + entity: &'static str, + source: DeserializationError, +} + +impl fmt::Display for DeserializationErrorWrapper { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "failed to deserialize {}: {}", self.entity, self.source) + } +} + +impl std::error::Error for DeserializationErrorWrapper { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.source) + } +} + +#[derive(Debug)] +struct StringError(String); + +impl fmt::Display for StringError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.0) + } +} + +impl std::error::Error for StringError {} + +// CONVERSION RESULT EXTENSION TRAIT +// ================================================================================================ + +/// Extension trait to ergonomically add field context to [`ConversionError`] results. +/// +/// This makes it easy to inject field names into the error path at each `?` site: +/// +/// ```rust,ignore +/// let account_root = value.account_root +/// .ok_or(ConversionError::missing_field::("account_root"))? +/// .try_into() +/// .context("account_root")?; +/// ``` +/// +/// The context stacks automatically through nested conversions, producing error paths like +/// `"header.account_root: value is not in range 0..MODULUS"`. +pub trait ConversionResultExt { + /// Add field context to the error, wrapping it in a [`ConversionError`] if needed. + fn context(self, field: &'static str) -> Result; +} + +impl> ConversionResultExt for Result { + fn context(self, field: &'static str) -> Result { + self.map_err(|e| e.into().context(field)) + } +} + +// FROM IMPLS FOR EXTERNAL ERROR TYPES +// ================================================================================================ + +macro_rules! impl_from_for_conversion_error { + ($($ty:ty),* $(,)?) => { + $( + impl From<$ty> for ConversionError { + fn from(e: $ty) -> Self { + Self::new(e) + } + } + )* + }; +} + +impl_from_for_conversion_error!( + hex::FromHexError, + miden_protocol::errors::AccountError, + miden_protocol::errors::AssetError, + miden_protocol::errors::AssetVaultError, + miden_protocol::errors::FeeError, + miden_protocol::errors::NoteError, + miden_protocol::errors::StorageSlotNameError, + miden_protocol::crypto::merkle::MerkleError, + miden_protocol::crypto::merkle::smt::SmtLeafError, + miden_protocol::crypto::merkle::smt::SmtProofError, + miden_standards::note::NetworkAccountTargetError, + std::num::TryFromIntError, + DeserializationError, +); + +#[cfg(test)] +mod tests { + use super::*; + + /// Simulates a deeply nested conversion where each layer adds its field context. + fn inner_conversion() -> Result<(), ConversionError> { + Err(ConversionError::message("value is not in range 0..MODULUS")) + } + + fn outer_conversion() -> Result<(), ConversionError> { + inner_conversion().context("account_root").context("header") + } + + #[test] + fn test_context_builds_dotted_field_path() { + let err = outer_conversion().unwrap_err(); + assert_eq!(err.to_string(), "header.account_root: value is not in range 0..MODULUS"); + } + + #[test] + fn test_context_single_field() { + let err = inner_conversion().context("nullifier").unwrap_err(); + assert_eq!(err.to_string(), "nullifier: value is not in range 0..MODULUS"); + } + + #[test] + fn test_context_deep_nesting() { + let err = outer_conversion().context("block").context("response").unwrap_err(); + assert_eq!( + err.to_string(), + "response.block.header.account_root: value is not in range 0..MODULUS" + ); + } + + #[test] + fn test_no_context_shows_source_only() { + let err = inner_conversion().unwrap_err(); + assert_eq!(err.to_string(), "value is not in range 0..MODULUS"); + } + + #[test] + fn test_context_on_external_error_type() { + let result: Result = u8::try_from(256u16); + let err = result.context("fee_amount").unwrap_err(); + assert!(err.to_string().starts_with("fee_amount: "), "expected field prefix, got: {err}",); + } +} diff --git a/crates/rpc/src/server/api.rs b/crates/rpc/src/server/api.rs index a0ec88859a..1f887175d6 100644 --- a/crates/rpc/src/server/api.rs +++ b/crates/rpc/src/server/api.rs @@ -534,11 +534,13 @@ fn endpoint_limits(params: &[(&str, usize)]) -> proto::rpc::EndpointLimits { /// Cached RPC query parameter limits. static RPC_LIMITS: LazyLock = LazyLock::new(|| { - use QueryParamAccountIdLimit as AccountId; - use QueryParamNoteIdLimit as NoteId; - use QueryParamNoteTagLimit as NoteTag; - use QueryParamNullifierLimit as Nullifier; - use QueryParamStorageMapKeyTotalLimit as StorageMapKeyTotal; + use { + QueryParamAccountIdLimit as AccountId, + QueryParamNoteIdLimit as NoteId, + QueryParamNoteTagLimit as NoteTag, + QueryParamNullifierLimit as Nullifier, + QueryParamStorageMapKeyTotalLimit as StorageMapKeyTotal, + }; proto::rpc::RpcLimits { endpoints: std::collections::HashMap::from([ diff --git a/crates/store/src/server/api.rs b/crates/store/src/server/api.rs index 56bfcafb49..a076b3fad6 100644 --- a/crates/store/src/server/api.rs +++ b/crates/store/src/server/api.rs @@ -1,7 +1,7 @@ use std::collections::BTreeSet; use std::sync::Arc; -use miden_node_proto::errors::ConversionError; +use miden_node_proto::errors::{ConversionError, ConversionResultExt}; use miden_node_proto::generated as proto; use miden_node_utils::ErrorReport; use miden_protocol::Word; @@ -109,8 +109,7 @@ where E: From, { block_range.ok_or_else(|| { - ConversionError::MissingFieldInProtobufRepresentation { entity, field_name: "block_range" } - .into() + ConversionError::message(format!("{entity}: missing field `block_range`")).into() }) } @@ -123,12 +122,10 @@ pub fn read_root( where E: From, { - root.ok_or_else(|| ConversionError::MissingFieldInProtobufRepresentation { - entity, - field_name: "root", - })? - .try_into() - .map_err(Into::into) + root.ok_or_else(|| ConversionError::message(format!("{entity}: missing field `root`")))? + .try_into() + .context("root") + .map_err(|e: ConversionError| e.into()) } /// Converts a collection of proto primitives to Words, returning a specific error type if @@ -143,6 +140,7 @@ where .into_iter() .map(TryInto::try_into) .collect::, ConversionError>>() + .context("digests") .map_err(Into::into) } @@ -156,6 +154,7 @@ where .cloned() .map(AccountId::try_from) .collect::>() + .context("account_ids") .map_err(Into::into) } @@ -163,16 +162,10 @@ pub fn read_account_id(id: Option) -> Result, { - id.ok_or_else(|| { - ConversionError::deserialization_error( - "AccountId", - miden_protocol::crypto::utils::DeserializationError::InvalidValue( - "Missing account ID".to_string(), - ), - ) - })? - .try_into() - .map_err(Into::into) + id.ok_or_else(|| ConversionError::message("missing account ID"))? + .try_into() + .context("account_id") + .map_err(|e: ConversionError| e.into()) } #[instrument( @@ -189,8 +182,9 @@ where nullifiers .iter() .copied() - .map(TryInto::try_into) + .map(Nullifier::try_from) .collect::>() + .context("nullifiers") .map_err(Into::into) } diff --git a/crates/store/src/server/block_producer.rs b/crates/store/src/server/block_producer.rs index 25f6b05f60..e4014bcfb2 100644 --- a/crates/store/src/server/block_producer.rs +++ b/crates/store/src/server/block_producer.rs @@ -2,7 +2,7 @@ use std::convert::Infallible; use futures::TryFutureExt; use miden_crypto::dsa::ecdsa_k256_keccak::Signature; -use miden_node_proto::errors::MissingFieldHelper; +use miden_node_proto::errors::{ConversionError, ConversionResultExt}; use miden_node_proto::generated::store::block_producer_server; use miden_node_proto::generated::{self as proto}; use miden_node_proto::try_convert; @@ -55,24 +55,33 @@ impl block_producer_server::BlockProducer for StoreApi { ) })?; // Read block. - let block = request - .block - .ok_or(proto::store::ApplyBlockRequest::missing_field(stringify!(block)))?; + let block = request.block.ok_or(ConversionError::missing_field::< + proto::store::ApplyBlockRequest, + >(stringify!(block)))?; // Read block header. let header: BlockHeader = block .header - .ok_or(proto::blockchain::SignedBlock::missing_field(stringify!(header)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + header + )))? + .try_into() + .context("header")?; // Read block body. let body: BlockBody = block .body - .ok_or(proto::blockchain::SignedBlock::missing_field(stringify!(body)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + body + )))? + .try_into() + .context("body")?; // Read signature. let signature: Signature = block .signature - .ok_or(proto::blockchain::SignedBlock::missing_field(stringify!(signature)))? - .try_into()?; + .ok_or(ConversionError::missing_field::(stringify!( + signature + )))? + .try_into() + .context("signature")?; // Get block inputs from ordered batches. let block_inputs = diff --git a/crates/store/src/server/rpc_api.rs b/crates/store/src/server/rpc_api.rs index a12fcafae7..4b8d6d6780 100644 --- a/crates/store/src/server/rpc_api.rs +++ b/crates/store/src/server/rpc_api.rs @@ -1,6 +1,6 @@ use miden_node_proto::convert; use miden_node_proto::domain::block::InvalidBlockRange; -use miden_node_proto::errors::MissingFieldHelper; +use miden_node_proto::errors::ConversionError; use miden_node_proto::generated::store::rpc_server; use miden_node_proto::generated::{self as proto}; use miden_node_utils::limiter::{ @@ -163,7 +163,11 @@ impl rpc_server::Rpc for StoreApi { let block_range = request .block_range - .ok_or_else(|| proto::rpc::SyncChainMmrRequest::missing_field(stringify!(block_range))) + .ok_or_else(|| { + ConversionError::missing_field::(stringify!( + block_range + )) + }) .map_err(SyncChainMmrError::DeserializationFailed)?; let block_from = BlockNumber::from(block_range.block_from);