Skip to content

Commit b3c23c2

Browse files
sergeradsergerad
authored
chore: Refactor ConversionError (#1754)
1 parent a7abf73 commit b3c23c2

File tree

18 files changed

+747
-503
lines changed

18 files changed

+747
-503
lines changed

‎crates/block-producer/src/store/mod.rs‎

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@ use std::num::NonZeroU32;
44

55
use itertools::Itertools;
66
use miden_node_proto::clients::{Builder, StoreBlockProducerClient};
7+
use miden_node_proto::decode::{ConversionResultExt, GrpcDecodeExt};
78
use miden_node_proto::domain::batch::BatchInputs;
8-
use miden_node_proto::errors::{ConversionError, MissingFieldHelper};
9-
use miden_node_proto::{AccountState, generated as proto};
9+
use miden_node_proto::errors::ConversionError;
10+
use miden_node_proto::{AccountState, decode, generated as proto};
1011
use miden_node_utils::formatting::format_opt;
1112
use miden_protocol::Word;
1213
use miden_protocol::account::AccountId;
@@ -70,21 +71,14 @@ impl TryFrom<proto::store::TransactionInputs> for TransactionInputs {
7071
type Error = ConversionError;
7172

7273
fn try_from(response: proto::store::TransactionInputs) -> Result<Self, Self::Error> {
73-
let AccountState { account_id, account_commitment } = response
74-
.account_state
75-
.ok_or(proto::store::TransactionInputs::missing_field(stringify!(account_state)))?
76-
.try_into()?;
74+
let decoder = response.decoder();
75+
let AccountState { account_id, account_commitment } =
76+
decode!(decoder, response.account_state)?;
7777

7878
let mut nullifiers = HashMap::new();
7979
for nullifier_record in response.nullifiers {
80-
let nullifier = nullifier_record
81-
.nullifier
82-
.ok_or(
83-
proto::store::transaction_inputs::NullifierTransactionInputRecord::missing_field(
84-
stringify!(nullifier),
85-
),
86-
)?
87-
.try_into()?;
80+
let decoder = nullifier_record.decoder();
81+
let nullifier = decode!(decoder, nullifier_record.nullifier)?;
8882

8983
// Note that this intentionally maps 0 to None as this is the definition used in
9084
// protobuf.
@@ -95,7 +89,8 @@ impl TryFrom<proto::store::TransactionInputs> for TransactionInputs {
9589
.found_unauthenticated_notes
9690
.into_iter()
9791
.map(Word::try_from)
98-
.collect::<Result<_, ConversionError>>()?;
92+
.collect::<Result<_, ConversionError>>()
93+
.context("found_unauthenticated_notes")?;
9994

10095
let current_block_height = response.block_height.into();
10196

@@ -148,11 +143,13 @@ impl StoreClient {
148143
.await?
149144
.into_inner()
150145
.block_header
151-
.ok_or(miden_node_proto::generated::blockchain::BlockHeader::missing_field(
152-
"block_header",
153-
))?;
146+
.ok_or_else(|| {
147+
StoreError::DeserializationError(ConversionError::missing_field::<
148+
miden_node_proto::generated::blockchain::BlockHeader,
149+
>("block_header"))
150+
})?;
154151

155-
BlockHeader::try_from(response).map_err(Into::into)
152+
BlockHeader::try_from(response).map_err(StoreError::DeserializationError)
156153
}
157154

158155
#[instrument(target = COMPONENT, name = "store.client.get_tx_inputs", skip_all, err)]
@@ -219,7 +216,7 @@ impl StoreClient {
219216

220217
let store_response = self.client.clone().get_block_inputs(request).await?.into_inner();
221218

222-
store_response.try_into().map_err(Into::into)
219+
store_response.try_into().map_err(StoreError::DeserializationError)
223220
}
224221

225222
#[instrument(target = COMPONENT, name = "store.client.get_batch_inputs", skip_all, err)]
@@ -235,7 +232,7 @@ impl StoreClient {
235232

236233
let store_response = self.client.clone().get_batch_inputs(request).await?.into_inner();
237234

238-
store_response.try_into().map_err(Into::into)
235+
store_response.try_into().map_err(StoreError::DeserializationError)
239236
}
240237

241238
#[instrument(target = COMPONENT, name = "store.client.apply_block", skip_all, err)]

‎crates/ntx-builder/src/clients/store.rs‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::ops::RangeInclusive;
33
use std::time::Duration;
44

55
use miden_node_proto::clients::{Builder, StoreNtxBuilderClient};
6+
use miden_node_proto::decode::ConversionResultExt;
67
use miden_node_proto::domain::account::{AccountDetails, AccountResponse, NetworkAccountId};
78
use miden_node_proto::errors::ConversionError;
89
use miden_node_proto::generated::rpc::BlockRange;
@@ -101,7 +102,10 @@ impl StoreClient {
101102
match response.current_block_header {
102103
// There are new blocks compared to the builder's latest state
103104
Some(block) => {
104-
let peaks = try_convert(response.current_peaks).collect::<Result<_, _>>()?;
105+
let peaks: Vec<Word> = try_convert(response.current_peaks)
106+
.collect::<Result<_, _>>()
107+
.context("current_peaks")
108+
.map_err(StoreError::DeserializationError)?;
105109
let header =
106110
BlockHeader::try_from(block).map_err(StoreError::DeserializationError)?;
107111

@@ -140,9 +144,7 @@ impl StoreClient {
140144
// which implies details being public, so OK to error otherwise
141145
let account = match store_response.map(|acc| acc.details) {
142146
Some(Some(details)) => Some(Account::read_from_bytes(&details).map_err(|err| {
143-
StoreError::DeserializationError(ConversionError::deserialization_error(
144-
"account", err,
145-
))
147+
StoreError::DeserializationError(ConversionError::from(err).context("details"))
146148
})?),
147149
_ => None,
148150
};
@@ -185,7 +187,8 @@ impl StoreClient {
185187
let account_details = account_response
186188
.details
187189
.ok_or(StoreError::MissingDetails("account details".into()))?;
188-
let partial_account = build_minimal_foreign_account(&account_details)?;
190+
let partial_account = build_minimal_foreign_account(&account_details)
191+
.map_err(StoreError::DeserializationError)?;
189192

190193
Ok(AccountInputs::new(partial_account, account_response.witness))
191194
}
@@ -216,7 +219,10 @@ impl StoreClient {
216219

217220
all_notes.reserve(resp.notes.len());
218221
for note in resp.notes {
219-
all_notes.push(AccountTargetNetworkNote::try_from(note)?);
222+
all_notes.push(
223+
AccountTargetNetworkNote::try_from(note)
224+
.map_err(StoreError::DeserializationError)?,
225+
);
220226
}
221227

222228
match resp.next_token {
@@ -317,10 +323,9 @@ impl StoreClient {
317323
.into_iter()
318324
.map(|account_id| {
319325
let account_id = AccountId::read_from_bytes(&account_id.id).map_err(|err| {
320-
StoreError::DeserializationError(ConversionError::deserialization_error(
321-
"account_id",
322-
err,
323-
))
326+
StoreError::DeserializationError(
327+
ConversionError::from(err).context("account_id"),
328+
)
324329
})?;
325330
NetworkAccountId::try_from(account_id).map_err(|_| {
326331
StoreError::MalformedResponse(
@@ -330,12 +335,9 @@ impl StoreClient {
330335
})
331336
.collect::<Result<Vec<NetworkAccountId>, StoreError>>()?;
332337

333-
let pagination_info = response.pagination_info.ok_or(
334-
ConversionError::MissingFieldInProtobufRepresentation {
335-
entity: "NetworkAccountIdList",
336-
field_name: "pagination_info",
337-
},
338-
)?;
338+
let pagination_info = response.pagination_info.ok_or(ConversionError::missing_field::<
339+
proto::store::NetworkAccountIdList,
340+
>("pagination_info"))?;
339341

340342
Ok((accounts, pagination_info))
341343
}
@@ -406,8 +408,10 @@ impl StoreClient {
406408
let smt_opening = asset_witness.proof.ok_or_else(|| {
407409
StoreError::MalformedResponse("missing proof in vault asset witness".to_string())
408410
})?;
409-
let proof: SmtProof =
410-
smt_opening.try_into().map_err(StoreError::DeserializationError)?;
411+
let proof: SmtProof = smt_opening
412+
.try_into()
413+
.context("proof")
414+
.map_err(StoreError::DeserializationError)?;
411415
let witness = AssetWitness::new(proof)
412416
.map_err(|err| StoreError::DeserializationError(ConversionError::from(err)))?;
413417

@@ -445,7 +449,10 @@ impl StoreClient {
445449
StoreError::MalformedResponse("missing proof in storage map witness".to_string())
446450
})?;
447451

448-
let proof: SmtProof = smt_opening.try_into().map_err(StoreError::DeserializationError)?;
452+
let proof: SmtProof = smt_opening
453+
.try_into()
454+
.context("proof")
455+
.map_err(StoreError::DeserializationError)?;
449456

450457
// Create the storage map witness using the proof and raw map key.
451458
let witness = StorageMapWitness::new(proof, [map_key]).map_err(|_err| {
@@ -482,10 +489,11 @@ pub fn build_minimal_foreign_account(
482489
account_details: &AccountDetails,
483490
) -> Result<PartialAccount, ConversionError> {
484491
// Derive account code.
485-
let account_code_bytes = account_details
486-
.account_code
487-
.as_ref()
488-
.ok_or(ConversionError::AccountCodeMissing)?;
492+
let account_code_bytes = account_details.account_code.as_ref().ok_or_else(|| {
493+
ConversionError::missing_field::<proto::rpc::account_response::AccountDetails>(
494+
"account_code",
495+
)
496+
})?;
489497
let account_code = AccountCode::from_bytes(account_code_bytes)?;
490498

491499
// Derive partial storage. Storage maps are not required for foreign accounts.

0 commit comments

Comments
 (0)