Skip to content

Commit

Permalink
Merge branch 'alic/refactor_proto_consensus' into 'master'
Browse files Browse the repository at this point in the history
chore(consensus): Use ProxyDecodeError for deserialization (RandomBeacon)

We have a dedicated error type related to protobuf (de)serialization, called `ProxyDecodeError`. This is cleaner than using Strings. 

See merge request dfinity-lab/public/ic!11535
  • Loading branch information
dist1ll committed Mar 31, 2023
2 parents 3cd97e3 + 5c19927 commit 8997799
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 10 deletions.
3 changes: 3 additions & 0 deletions rs/replay/src/backup.rs
Expand Up @@ -292,6 +292,8 @@ pub(crate) fn deserialize_consensus_artifacts(
RandomBeacon::try_from(
pb::RandomBeacon::decode(buffer.as_slice()).expect("Protobuf decoding failed"),
)
// TODO: Remove after switching to ProxyDecodeError
.map_err(|e| format!("{}", e))
.unwrap_or_else(|err| panic!("{}", deserialization_error(&rb_path, err)))
.into_message(),
);
Expand Down Expand Up @@ -381,6 +383,7 @@ pub(crate) fn deserialize_consensus_artifacts(
}
}

// TODO: Replace String with ProxyDecodeError once structures have been migrated
fn deserialization_error(file: &Path, err: String) -> String {
format!("Couldn't deserialize artifact {:?}: {}", file, err)
}
16 changes: 7 additions & 9 deletions rs/types/types/src/consensus.rs
Expand Up @@ -8,8 +8,11 @@ use crate::{
signature::*,
*,
};
use ic_protobuf::log::block_log_entry::v1::BlockLogEntry;
use ic_protobuf::types::v1 as pb;
use ic_protobuf::{
log::block_log_entry::v1::BlockLogEntry,
proxy::{try_from_option_field, ProxyDecodeError},
};
use serde::{Deserialize, Serialize};
use std::cmp::PartialOrd;
use std::convert::TryInto;
Expand Down Expand Up @@ -562,23 +565,18 @@ impl From<&RandomBeacon> for pb::RandomBeacon {
}

impl TryFrom<pb::RandomBeacon> for RandomBeacon {
type Error = String;
type Error = ProxyDecodeError;
fn try_from(beacon: pb::RandomBeacon) -> Result<Self, Self::Error> {
Ok(Signed {
content: RandomBeaconContent {
version: ReplicaVersion::try_from(beacon.version.as_str())
.map_err(|e| format!("RandomBeacon replica version failed to parse {:?}", e))?,
.map_err(|e| ProxyDecodeError::ReplicaVersionParseError(Box::new(e)))?,
height: Height::from(beacon.height),
parent: CryptoHashOf::from(CryptoHash(beacon.parent)),
},
signature: ThresholdSignature {
signature: CombinedThresholdSigOf::new(CombinedThresholdSig(beacon.signature)),
signer: NiDkgId::try_from(
beacon
.signer
.ok_or_else(|| String::from("Error: RandomBeacon signer not present"))?,
)
.map_err(|e| format!("Unable to decode Random beacon signer {:?}", e))?,
signer: try_from_option_field(beacon.signer, "RandomBeacon::signer")?,
},
})
}
Expand Down
5 changes: 4 additions & 1 deletion rs/types/types/src/consensus/catchup.rs
Expand Up @@ -99,7 +99,10 @@ impl TryFrom<pb::CatchUpContent> for CatchUpContent {
content
.random_beacon
.ok_or_else(|| String::from("Error: CUP missing block"))?,
)?;
)
// TODO: Remove after switching to ProxyDecodeError
.map_err(|e| format!("{}", e))?;

Ok(Self::new(
HashedBlock {
hash: CryptoHashOf::from(CryptoHash(content.block_hash)),
Expand Down

0 comments on commit 8997799

Please sign in to comment.