From 42ad0ee36ded0a0f28c0463dc8a109b5c351f932 Mon Sep 17 00:00:00 2001
From: Jun Kimura <jun.kimura@datachain.jp>
Date: Wed, 7 Feb 2024 19:42:28 +0900
Subject: [PATCH] fix to return an error for missing proto value instead of
 calling unwrap

Signed-off-by: Jun Kimura <jun.kimura@datachain.jp>
---
 crates/ibc/src/client_state.rs | 17 ++++++++----
 crates/ibc/src/errors.rs       |  8 ++++++
 crates/ibc/src/header.rs       | 16 ++++++++---
 crates/ibc/src/misbehaviour.rs | 26 +++++++++++++-----
 crates/ibc/src/types.rs        | 49 ++++++++++++++++++++++++----------
 crates/ibc/src/update.rs       |  1 -
 6 files changed, 87 insertions(+), 30 deletions(-)

diff --git a/crates/ibc/src/client_state.rs b/crates/ibc/src/client_state.rs
index 75d80eb..e93ca65 100644
--- a/crates/ibc/src/client_state.rs
+++ b/crates/ibc/src/client_state.rs
@@ -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
@@ -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(),
@@ -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(),
         })
diff --git a/crates/ibc/src/errors.rs b/crates/ibc/src/errors.rs
index 1297540..443dbfe 100644
--- a/crates/ibc/src/errors.rs
+++ b/crates/ibc/src/errors.rs
@@ -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 {
diff --git a/crates/ibc/src/header.rs b/crates/ibc/src/header.rs
index 3e6d6c6..986402e 100644
--- a/crates/ibc/src/header.rs
+++ b/crates/ibc/src/header.rs
@@ -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()?,
diff --git a/crates/ibc/src/misbehaviour.rs b/crates/ibc/src/misbehaviour.rs
index 375a0a4..1acf42e 100644
--- a/crates/ibc/src/misbehaviour.rs
+++ b/crates/ibc/src/misbehaviour.rs
@@ -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"))?,
                 )?,
             }),
         })
@@ -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"))?,
                 )?,
             }),
         })
diff --git a/crates/ibc/src/types.rs b/crates/ibc/src/types.rs
index 8fa9433..b2a0b67 100644
--- a/crates/ibc/src/types.rs
+++ b/crates/ibc/src/types.rs
@@ -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,
@@ -250,10 +258,18 @@ 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,
@@ -261,7 +277,7 @@ pub(crate) fn convert_proto_to_consensus_update<const SYNC_COMMITTEE_SIZE: usize
             || 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()
@@ -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,
                     )?,
                 },
@@ -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(),
     };
 
diff --git a/crates/ibc/src/update.rs b/crates/ibc/src/update.rs
index 04d0e5c..ecf92e9 100644
--- a/crates/ibc/src/update.rs
+++ b/crates/ibc/src/update.rs
@@ -108,7 +108,6 @@ pub fn apply_updates<
             )?,
             current_sync_committee: trusted_consensus_state
                 .next_sync_committee()
-                .as_ref()
                 .unwrap()
                 .aggregate_pubkey
                 .clone(),