Custom proof specs in config (#1574)

* Impl serde for ics23 proto types

* Add proof specs to client state

* Get proof specs from chain config

* Fix test compilation

* Define ProofSpecs domain type using ibc_proto::ics23 types

* Minor refactoring

* Add attrs for ics23 protos in proto-compiler

* Fix clippy errors

* Refactor Tendermint  to use domain type

* Parse config proofspecs as JSON serialized string

* Add Default impl for ProofSpecs

* Use serde(with) instead of newtype

* Get rid of unwraps

* Fix failing test

* Document proof-specs opt in config

* Minor refactoring to match style

* Create .changelog entry

* Serialilze ProofSpecs as a pretty formatted JSON string

* Set serde dep version to '1'

* Manually implement conversions b/w ProofSpec protos

* Disallow empty proof-specs

* Move custom serde serializer to its own module

Co-authored-by: Romain Ruetschi <romain@informal.systems>

.changelog/unreleased/features/ibc-relayer/1561-config-proof-specs.md

@@ -0,0 +1,2 @@
+- Allow custom proof-specs in chain config
+  ([#1561](https://github.com/informalsystems/ibc-rs/issues/1561))

config.toml

@@ -223,6 +223,60 @@ memo_prefix = ''
 #   ['transfer', 'channel-0'],
 # ]
 
+# Specify custom ICS23 proof-specs for the chain (serialized as JSON).
+# Default: [`ProofSpecs::cosmos()`](modules/src/core/ics23_commitment/specs.rs)
+proof-specs = '''
+[
+  {
+    "leaf_spec": {
+      "hash": 1,
+      "prehash_key": 0,
+      "prehash_value": 1,
+      "length": 1,
+      "prefix": [
+        0
+      ]
+    },
+    "inner_spec": {
+      "child_order": [
+        0,
+        1
+      ],
+      "child_size": 33,
+      "min_prefix_length": 4,
+      "max_prefix_length": 12,
+      "empty_child": [],
+      "hash": 1
+    },
+    "max_depth": 0,
+    "min_depth": 0
+  },
+  {
+    "leaf_spec": {
+      "hash": 1,
+      "prehash_key": 0,
+      "prehash_value": 1,
+      "length": 1,
+      "prefix": [
+        0
+      ]
+    },
+    "inner_spec": {
+      "child_order": [
+        0,
+        1
+      ],
+      "child_size": 32,
+      "min_prefix_length": 1,
+      "max_prefix_length": 1,
+      "empty_child": [],
+      "hash": 1
+    },
+    "max_depth": 0,
+    "min_depth": 0
+  }
+]
+'''
 
 [[chains]]
 id = 'ibc-1'

modules/src/clients/ics07_tendermint/client_state.rs

@@ -29,7 +29,7 @@ pub struct ClientState {
     pub max_clock_drift: Duration,
     pub frozen_height: Height,
     pub latest_height: Height,
-    // pub proof_specs: ::core::vec::Vec<super::super::super::super::ics23::ProofSpec>,
+    pub proof_specs: ProofSpecs,
     pub upgrade_path: Vec<String>,
     pub allow_update: AllowUpdate,
 }
@@ -52,6 +52,7 @@ impl ClientState {
         max_clock_drift: Duration,
         latest_height: Height,
         frozen_height: Height,
+        proof_specs: ProofSpecs,
         upgrade_path: Vec<String>,
         allow_update: AllowUpdate,
     ) -> Result<ClientState, Error> {
@@ -91,6 +92,13 @@ impl ClientState {
             ));
         }
 
+        // Disallow empty proof-specs
+        if proof_specs.is_empty() {
+            return Err(Error::validation(
+                "ClientState proof-specs cannot be empty".to_string(),
+            ));
+        }
+
         Ok(Self {
             chain_id,
             trust_level,
@@ -99,6 +107,7 @@ impl ClientState {
             max_clock_drift,
             frozen_height,
             latest_height,
+            proof_specs,
             upgrade_path,
             allow_update,
         })
@@ -228,6 +237,7 @@ impl TryFrom<RawClientState> for ClientState {
                 after_expiry: raw.allow_update_after_expiry,
                 after_misbehaviour: raw.allow_update_after_misbehaviour,
             },
+            proof_specs: raw.proof_specs.into(),
         })
     }
 }
@@ -242,7 +252,7 @@ impl From<ClientState> for RawClientState {
             max_clock_drift: Some(value.max_clock_drift.into()),
             frozen_height: Some(value.frozen_height.into()),
             latest_height: Some(value.latest_height.into()),
-            proof_specs: ProofSpecs::cosmos().into(),
+            proof_specs: value.proof_specs.into(),
             allow_update_after_expiry: value.allow_update.after_expiry,
             allow_update_after_misbehaviour: value.allow_update.after_misbehaviour,
             upgrade_path: value.upgrade_path,
@@ -261,6 +271,7 @@ mod tests {
 
     use crate::clients::ics07_tendermint::client_state::{AllowUpdate, ClientState};
     use crate::core::ics02_client::trust_threshold::TrustThreshold;
+    use crate::core::ics23_commitment::specs::ProofSpecs;
     use crate::core::ics24_host::identifier::ChainId;
     use crate::test::test_serialization_roundtrip;
     use crate::timestamp::ZERO_DURATION;
@@ -293,6 +304,7 @@ mod tests {
             max_clock_drift: Duration,
             latest_height: Height,
             frozen_height: Height,
+            proof_specs: ProofSpecs,
             upgrade_path: Vec<String>,
             allow_update: AllowUpdate,
         }
@@ -306,6 +318,7 @@ mod tests {
             max_clock_drift: Duration::new(3, 0),
             latest_height: Height::new(0, 10),
             frozen_height: Height::default(),
+            proof_specs: ProofSpecs::default(),
             upgrade_path: vec!["".to_string()],
             allow_update: AllowUpdate {
                 after_expiry: false,
@@ -373,6 +386,7 @@ mod tests {
                 p.max_clock_drift,
                 p.latest_height,
                 p.frozen_height,
+                p.proof_specs,
                 p.upgrade_path,
                 p.allow_update,
             );
@@ -414,6 +428,7 @@ pub mod test_util {
                     u64::from(tm_header.height),
                 ),
                 Height::zero(),
+                Default::default(),
                 vec!["".to_string()],
                 AllowUpdate {
                     after_expiry: false,

modules/src/core/ics02_client/handler/create_client.rs

@@ -71,6 +71,7 @@ mod tests {
     use crate::core::ics02_client::msgs::create_client::MsgCreateAnyClient;
     use crate::core::ics02_client::msgs::ClientMsg;
     use crate::core::ics02_client::trust_threshold::TrustThreshold;
+    use crate::core::ics23_commitment::specs::ProofSpecs;
     use crate::core::ics24_host::identifier::ClientId;
     use crate::events::IbcEvent;
     use crate::handler::HandlerOutput;
@@ -230,6 +231,7 @@ mod tests {
             max_clock_drift: Duration::from_millis(3000),
             latest_height: Height::new(0, u64::from(tm_header.height)),
             frozen_height: Height::zero(),
+            proof_specs: ProofSpecs::default(),
             allow_update: AllowUpdate {
                 after_expiry: false,
                 after_misbehaviour: false,

modules/src/core/ics23_commitment/commitment.rs

@@ -157,11 +157,12 @@ impl Serialize for CommitmentPrefix {
 pub mod test_util {
     use crate::prelude::*;
     use ibc_proto::ibc::core::commitment::v1::MerkleProof as RawMerkleProof;
+    use ibc_proto::ics23::CommitmentProof;
 
     /// Returns a dummy `RawMerkleProof`, for testing only!
     pub fn get_dummy_merkle_proof() -> RawMerkleProof {
-        let parsed = ibc_proto::ics23::CommitmentProof { proof: None };
-        let mproofs: Vec<ibc_proto::ics23::CommitmentProof> = vec![parsed];
+        let parsed = CommitmentProof { proof: None };
+        let mproofs: Vec<CommitmentProof> = vec![parsed];
         RawMerkleProof { proofs: mproofs }
     }
 }

modules/src/core/ics23_commitment/specs.rs

@@ -1,6 +1,7 @@
 use crate::prelude::*;
-use ibc_proto::ics23::ProofSpec as ProtoProofSpec;
-use ics23::ProofSpec;
+use ibc_proto::ics23::{InnerSpec as IbcInnerSpec, LeafOp as IbcLeafOp, ProofSpec as IbcProofSpec};
+use ics23::{InnerSpec as Ics23InnerSpec, LeafOp as Ics23LeafOp, ProofSpec as Ics23ProofSpec};
+use serde::{Deserialize, Serialize};
 
 /// An array of proof specifications.
 ///
@@ -9,38 +10,139 @@ use ics23::ProofSpec;
 /// Additionally, this type also aids in the conversion from `ProofSpec` types from crate `ics23`
 /// into proof specifications as represented in the `ibc_proto` type; see the
 /// `From` trait(s) below.
-pub struct ProofSpecs {
-    specs: Vec<ProofSpec>,
-}
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
+pub struct ProofSpecs(Vec<ProofSpec>);
 
 impl ProofSpecs {
     /// Returns the specification for Cosmos-SDK proofs
     pub fn cosmos() -> Self {
-        Self {
-            specs: vec![
-                ics23::iavl_spec(),       // Format of proofs-iavl (iavl merkle proofs)
-                ics23::tendermint_spec(), // Format of proofs-tendermint (crypto/ merkle SimpleProof)
-            ],
+        vec![
+            ics23::iavl_spec(),       // Format of proofs-iavl (iavl merkle proofs)
+            ics23::tendermint_spec(), // Format of proofs-tendermint (crypto/ merkle SimpleProof)
+        ]
+        .into()
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+}
+
+impl Default for ProofSpecs {
+    fn default() -> Self {
+        Self::cosmos()
+    }
+}
+
+impl From<Vec<IbcProofSpec>> for ProofSpecs {
+    fn from(ibc_specs: Vec<IbcProofSpec>) -> Self {
+        Self(ibc_specs.into_iter().map(ProofSpec).collect())
+    }
+}
+
+impl From<Vec<Ics23ProofSpec>> for ProofSpecs {
+    fn from(ics23_specs: Vec<Ics23ProofSpec>) -> Self {
+        Self(
+            ics23_specs
+                .into_iter()
+                .map(|ics23_spec| ics23_spec.into())
+                .collect(),
+        )
+    }
+}
+
+impl From<ProofSpecs> for Vec<Ics23ProofSpec> {
+    fn from(specs: ProofSpecs) -> Self {
+        specs.0.into_iter().map(|spec| spec.into()).collect()
+    }
+}
+
+impl From<ProofSpecs> for Vec<IbcProofSpec> {
+    fn from(specs: ProofSpecs) -> Self {
+        specs.0.into_iter().map(|spec| spec.0).collect()
+    }
+}
+
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
+struct ProofSpec(IbcProofSpec);
+
+impl From<Ics23ProofSpec> for ProofSpec {
+    fn from(spec: Ics23ProofSpec) -> Self {
+        Self(IbcProofSpec {
+            leaf_spec: spec.leaf_spec.map(|lop| LeafOp::from(lop).0),
+            inner_spec: spec.inner_spec.map(|ispec| InnerSpec::from(ispec).0),
+            max_depth: spec.max_depth,
+            min_depth: spec.min_depth,
+        })
+    }
+}
+
+impl From<ProofSpec> for Ics23ProofSpec {
+    fn from(spec: ProofSpec) -> Self {
+        let spec = spec.0;
+        Ics23ProofSpec {
+            leaf_spec: spec.leaf_spec.map(|lop| LeafOp(lop).into()),
+            inner_spec: spec.inner_spec.map(|ispec| InnerSpec(ispec).into()),
+            max_depth: spec.max_depth,
+            min_depth: spec.min_depth,
+        }
+    }
+}
+
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
+struct LeafOp(IbcLeafOp);
+
+impl From<Ics23LeafOp> for LeafOp {
+    fn from(leaf_op: Ics23LeafOp) -> Self {
+        Self(IbcLeafOp {
+            hash: leaf_op.hash,
+            prehash_key: leaf_op.prehash_key,
+            prehash_value: leaf_op.prehash_value,
+            length: leaf_op.length,
+            prefix: leaf_op.prefix,
+        })
+    }
+}
+
+impl From<LeafOp> for Ics23LeafOp {
+    fn from(leaf_op: LeafOp) -> Self {
+        let leaf_op = leaf_op.0;
+        Ics23LeafOp {
+            hash: leaf_op.hash,
+            prehash_key: leaf_op.prehash_key,
+            prehash_value: leaf_op.prehash_value,
+            length: leaf_op.length,
+            prefix: leaf_op.prefix,
         }
     }
 }
 
-/// Converts from the domain type (which is represented as a vector of `ics23::ProofSpec`
-/// to the corresponding proto type (vector of `ibc_proto::ProofSpec`).
-/// TODO: fix with <https://github.com/informalsystems/ibc-rs/issues/853>
-impl From<ProofSpecs> for Vec<ProtoProofSpec> {
-    fn from(domain_specs: ProofSpecs) -> Self {
-        let mut raw_specs = Vec::new();
-        for ds in domain_specs.specs.iter() {
-            // Both `ProofSpec` types implement trait `prost::Message`. Convert by encoding, then
-            // decoding into the destination type.
-            // Safety note: the source and target data structures are identical, hence the
-            // encode/decode conversion here should be infallible.
-            let mut encoded = Vec::new();
-            prost::Message::encode(ds, &mut encoded).unwrap();
-            let decoded: ProtoProofSpec = prost::Message::decode(&*encoded).unwrap();
-            raw_specs.push(decoded);
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
+struct InnerSpec(IbcInnerSpec);
+
+impl From<Ics23InnerSpec> for InnerSpec {
+    fn from(inner_spec: Ics23InnerSpec) -> Self {
+        Self(IbcInnerSpec {
+            child_order: inner_spec.child_order,
+            child_size: inner_spec.child_size,
+            min_prefix_length: inner_spec.min_prefix_length,
+            max_prefix_length: inner_spec.max_prefix_length,
+            empty_child: inner_spec.empty_child,
+            hash: inner_spec.hash,
+        })
+    }
+}
+
+impl From<InnerSpec> for Ics23InnerSpec {
+    fn from(inner_spec: InnerSpec) -> Self {
+        let inner_spec = inner_spec.0;
+        Ics23InnerSpec {
+            child_order: inner_spec.child_order,
+            child_size: inner_spec.child_size,
+            min_prefix_length: inner_spec.min_prefix_length,
+            max_prefix_length: inner_spec.max_prefix_length,
+            empty_child: inner_spec.empty_child,
+            hash: inner_spec.hash,
         }
-        raw_specs
     }
 }

proto-compiler/src/cmd/clone.rs

@@ -173,4 +173,4 @@ fn checkout_tag(repo: &Repository, tag_name: &str) -> Result<(), git2::Error> {
     }
 
     Ok(())
-}
+}