Skip to content

Commit

Permalink
fix to return an error for missing proto value instead of calling unwrap
Browse files Browse the repository at this point in the history 
Signed-off-by: Jun Kimura <[email protected]>
  • Loading branch information
@bluele
bluele committed Feb 7, 2024
1 parent 5d81fa9 commit 42ad0ee
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 30 deletions.
17 changes: 12 additions & 5 deletions crates/ibc/src/client_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,9 @@ impl<const SYNC_COMMITTEE_SIZE: usize, const EXECUTION_PAYLOAD_TREE_DEPTH: usize
version
}

let raw_fork_parameters = value.fork_parameters.unwrap();
let raw_fork_parameters = value
.fork_parameters
.ok_or(Error::proto_missing("fork_parameters"))?;
let fork_parameters = ForkParameters::new(
bytes_to_version(raw_fork_parameters.genesis_fork_version),
raw_fork_parameters
Expand All @@ -634,7 +636,14 @@ impl<const SYNC_COMMITTEE_SIZE: usize, const EXECUTION_PAYLOAD_TREE_DEPTH: usize
.map(|f| ForkParameter::new(bytes_to_version(f.version), f.epoch.into()))
.collect(),
);
let trust_level = value.trust_level.unwrap();
let trust_level = value
.trust_level
.ok_or(Error::proto_missing("trust_level"))?;
let frozen_height = if let Some(h) = value.frozen_height {
Some(Height::new(h.revision_number, h.revision_height)?)
} else {
None
};
Ok(Self {
genesis_validators_root: H256::from_slice(&value.genesis_validators_root),
min_sync_committee_participants: value.min_sync_committee_participants.into(),
Expand All @@ -658,9 +667,7 @@ impl<const SYNC_COMMITTEE_SIZE: usize, const EXECUTION_PAYLOAD_TREE_DEPTH: usize
.map_err(|_| Error::NegativeMaxClockDrift)?,
latest_slot: value.latest_slot.into(),
latest_execution_block_number: value.latest_execution_block_number.into(),
frozen_height: value
.frozen_height
.map(|h| Height::new(h.revision_number, h.revision_height).unwrap()),
frozen_height,
consensus_verifier: Default::default(),
execution_verifier: Default::default(),
})
Expand Down
8 changes: 8 additions & 0 deletions crates/ibc/src/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ pub enum Error {
sync_committee_size: usize,
sync_committee_bits: Vec<u8>,
},
/// proto missing field error: `{0}`
ProtoMissingFieldError(String),
}

impl Error {
pub fn proto_missing(s: &str) -> Self {
Error::ProtoMissingFieldError(s.to_string())
}
}

impl From<Error> for ClientError {
Expand Down
16 changes: 12 additions & 4 deletions crates/ibc/src/header.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,18 @@ impl<const SYNC_COMMITTEE_SIZE: usize> Protobuf<RawHeader> for Header<SYNC_COMMI
impl<const SYNC_COMMITTEE_SIZE: usize> TryFrom<RawHeader> for Header<SYNC_COMMITTEE_SIZE> {
type Error = Error;
fn try_from(value: RawHeader) -> Result<Self, Self::Error> {
let trusted_sync_committee = value.trusted_sync_committee.unwrap();
let consensus_update = value.consensus_update.unwrap();
let execution_update = value.execution_update.unwrap();
let account_update = value.account_update.unwrap();
let trusted_sync_committee = value
.trusted_sync_committee
.ok_or(Error::proto_missing("trusted_sync_committee"))?;
let consensus_update = value
.consensus_update
.ok_or(Error::proto_missing("consensus_update"))?;
let execution_update = value
.execution_update
.ok_or(Error::proto_missing("execution_update"))?;
let account_update = value
.account_update
.ok_or(Error::proto_missing("account_update"))?;

Ok(Self {
trusted_sync_committee: trusted_sync_committee.try_into()?,
Expand Down
26 changes: 20 additions & 6 deletions crates/ibc/src/misbehaviour.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,20 @@ impl<const SYNC_COMMITTEE_SIZE: usize> TryFrom<RawFinalizedHeaderMisbehaviour>
fn try_from(value: RawFinalizedHeaderMisbehaviour) -> Result<Self, Self::Error> {
Ok(Self {
client_id: ClientId::from_str(&value.client_id)?,
trusted_sync_committee: value.trusted_sync_committee.unwrap().try_into()?,
trusted_sync_committee: value
.trusted_sync_committee
.ok_or(Error::proto_missing("trusted_sync_committee"))?
.try_into()?,
data: MisbehaviourData::FinalizedHeader(FinalizedHeaderMisbehaviour {
consensus_update_1: convert_proto_to_consensus_update(
value.consensus_update_1.unwrap(),
value
.consensus_update_1
.ok_or(Error::proto_missing("consensus_update_1"))?,
)?,
consensus_update_2: convert_proto_to_consensus_update(
value.consensus_update_2.unwrap(),
value
.consensus_update_2
.ok_or(Error::proto_missing("consensus_update_2"))?,
)?,
}),
})
Expand All @@ -83,13 +90,20 @@ impl<const SYNC_COMMITTEE_SIZE: usize> TryFrom<RawNextSyncCommitteeMisbehaviour>
fn try_from(value: RawNextSyncCommitteeMisbehaviour) -> Result<Self, Self::Error> {
Ok(Self {
client_id: ClientId::from_str(&value.client_id)?,
trusted_sync_committee: value.trusted_sync_committee.unwrap().try_into()?,
trusted_sync_committee: value
.trusted_sync_committee
.ok_or(Error::proto_missing("trusted_sync_committee"))?
.try_into()?,
data: MisbehaviourData::NextSyncCommittee(NextSyncCommitteeMisbehaviour {
consensus_update_1: convert_proto_to_consensus_update(
value.consensus_update_1.unwrap(),
value
.consensus_update_1
.ok_or(Error::proto_missing("consensus_update_1"))?,
)?,
consensus_update_2: convert_proto_to_consensus_update(
value.consensus_update_2.unwrap(),
value
.consensus_update_2
.ok_or(Error::proto_missing("consensus_update_2"))?,
)?,
}),
})
Expand Down
49 changes: 35 additions & 14 deletions crates/ibc/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,24 +35,32 @@ impl<const SYNC_COMMITTEE_SIZE: usize> TryFrom<ProtoTrustedSyncCommittee>
type Error = Error;

fn try_from(value: ProtoTrustedSyncCommittee) -> Result<Self, Error> {
let trusted_height = value
.trusted_height
.as_ref()
.ok_or(Error::proto_missing("trusted_height"))?;
Ok(TrustedSyncCommittee {
height: Height::new(
value.trusted_height.as_ref().unwrap().revision_number,
value.trusted_height.as_ref().unwrap().revision_height,
trusted_height.revision_number,
trusted_height.revision_height,
)?,
sync_committee: SyncCommittee {
pubkeys: Vector::<PublicKey, SYNC_COMMITTEE_SIZE>::from_iter(
value
.sync_committee
.as_ref()
.unwrap()
.ok_or(Error::proto_missing("sync_committee"))?
.pubkeys
.clone()
.into_iter()
.map(|pk| PublicKey::try_from(pk).unwrap()),
.map(|pk| pk.try_into())
.collect::<Result<Vec<PublicKey>, _>>()?,
),
aggregate_pubkey: PublicKey::try_from(
value.sync_committee.unwrap().aggregate_pubkey,
value
.sync_committee
.ok_or(Error::proto_missing("sync_committee"))?
.aggregate_pubkey,
)?,
},
is_next: value.is_next,
Expand Down Expand Up @@ -250,18 +258,26 @@ pub(crate) fn convert_consensus_update_to_proto<const SYNC_COMMITTEE_SIZE: usize
pub(crate) fn convert_proto_to_consensus_update<const SYNC_COMMITTEE_SIZE: usize>(
consensus_update: ProtoLightClientUpdate,
) -> Result<ConsensusUpdateInfo<SYNC_COMMITTEE_SIZE>, Error> {
let attested_header =
convert_proto_to_header(consensus_update.attested_header.as_ref().unwrap())?;
let finalized_header =
convert_proto_to_header(consensus_update.finalized_header.as_ref().unwrap())?;
let attested_header = convert_proto_to_header(
consensus_update
.attested_header
.as_ref()
.ok_or(Error::proto_missing("attested_header"))?,
)?;
let finalized_header = convert_proto_to_header(
consensus_update
.finalized_header
.as_ref()
.ok_or(Error::proto_missing("finalized_header"))?,
)?;

let light_client_update = LightClientUpdate {
attested_header,
next_sync_committee: if consensus_update.next_sync_committee.is_none()
|| consensus_update
.next_sync_committee
.as_ref()
.unwrap()
.ok_or(Error::proto_missing("next_sync_committee"))?
.pubkeys
.is_empty()
|| consensus_update.next_sync_committee_branch.is_empty()
Expand All @@ -274,15 +290,16 @@ pub(crate) fn convert_proto_to_consensus_update<const SYNC_COMMITTEE_SIZE: usize
consensus_update
.next_sync_committee
.clone()
.unwrap()
.ok_or(Error::proto_missing("next_sync_committee"))?
.pubkeys
.into_iter()
.map(|pk| PublicKey::try_from(pk).unwrap()),
.map(|pk| pk.try_into())
.collect::<Result<Vec<PublicKey>, _>>()?,
),
aggregate_pubkey: PublicKey::try_from(
consensus_update
.next_sync_committee
.unwrap()
.ok_or(Error::proto_missing("next_sync_committee"))?
.aggregate_pubkey,
)?,
},
Expand All @@ -293,7 +310,11 @@ pub(crate) fn convert_proto_to_consensus_update<const SYNC_COMMITTEE_SIZE: usize
finalized_header,
decode_branch(consensus_update.finalized_header_branch),
),
sync_aggregate: convert_proto_sync_aggregate(consensus_update.sync_aggregate.unwrap())?,
sync_aggregate: convert_proto_sync_aggregate(
consensus_update
.sync_aggregate
.ok_or(Error::proto_missing("sync_aggregate"))?,
)?,
signature_slot: consensus_update.signature_slot.into(),
};

Expand Down
1 change: 0 additions & 1 deletion crates/ibc/src/update.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ pub fn apply_updates<
)?,
current_sync_committee: trusted_consensus_state
.next_sync_committee()
.as_ref()
.unwrap()
.aggregate_pubkey
.clone(),
Expand Down

0 comments on commit 42ad0ee

Please sign in to comment.