Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom proof specs in config #1574

Merged
merged 23 commits into from
Nov 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d15f6a2
Impl serde for ics23 proto types
hu55a1n1 Nov 17, 2021
bd9ff26
Add proof specs to client state
hu55a1n1 Nov 17, 2021
6750982
Get proof specs from chain config
hu55a1n1 Nov 17, 2021
ee32e66
Fix test compilation
hu55a1n1 Nov 18, 2021
f0fd102
Define ProofSpecs domain type using ibc_proto::ics23 types
hu55a1n1 Nov 18, 2021
67c8991
Minor refactoring
hu55a1n1 Nov 18, 2021
623e766
Add attrs for ics23 protos in proto-compiler
hu55a1n1 Nov 18, 2021
d55f57c
Fix clippy errors
hu55a1n1 Nov 18, 2021
f98cf85
Refactor Tendermint to use domain type
hu55a1n1 Nov 19, 2021
2c109ed
Parse config proofspecs as JSON serialized string
hu55a1n1 Nov 19, 2021
3873c11
Add Default impl for ProofSpecs
hu55a1n1 Nov 19, 2021
c7aaa03
Use serde(with) instead of newtype
hu55a1n1 Nov 19, 2021
e4b8ea3
Get rid of unwraps
hu55a1n1 Nov 19, 2021
4ab7308
Fix failing test
hu55a1n1 Nov 19, 2021
0822feb
Document proof-specs opt in config
hu55a1n1 Nov 19, 2021
1803828
Minor refactoring to match style
hu55a1n1 Nov 19, 2021
9347949
Create .changelog entry
hu55a1n1 Nov 19, 2021
0b9a709
Serialilze ProofSpecs as a pretty formatted JSON string
hu55a1n1 Nov 24, 2021
a72ab39
Set serde dep version to '1'
hu55a1n1 Nov 24, 2021
12a46a4
Manually implement conversions b/w ProofSpec protos
hu55a1n1 Nov 24, 2021
a229708
Disallow empty proof-specs
hu55a1n1 Nov 24, 2021
450992f
Merge branch 'master' into hu55a1n1/1561-custom-proof-specs
hu55a1n1 Nov 24, 2021
9ab79a1
Move custom serde serializer to its own module
romac Nov 24, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Allow custom proof-specs in chain config
([#1561](https://github.com/informalsystems/ibc-rs/issues/1561))
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 54 additions & 0 deletions config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a more operator-friendly manner to specify this? I tried to give an alternative suggestion in #1630 .

[
{
"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'
Expand Down
19 changes: 17 additions & 2 deletions modules/src/clients/ics07_tendermint/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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> {
Expand Down Expand Up @@ -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,
Expand All @@ -99,6 +107,7 @@ impl ClientState {
max_clock_drift,
frozen_height,
latest_height,
proof_specs,
hu55a1n1 marked this conversation as resolved.
Show resolved Hide resolved
upgrade_path,
allow_update,
})
Expand Down Expand Up @@ -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(),
})
}
}
Expand All @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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,
}
Expand All @@ -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,
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -414,6 +428,7 @@ pub mod test_util {
u64::from(tm_header.height),
),
Height::zero(),
Default::default(),
vec!["".to_string()],
AllowUpdate {
after_expiry: false,
Expand Down
2 changes: 2 additions & 0 deletions modules/src/core/ics02_client/handler/create_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions modules/src/core/ics23_commitment/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}
154 changes: 128 additions & 26 deletions modules/src/core/ics23_commitment/specs.rs
Original file line number Diff line number Diff line change
@@ -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.
///
Expand All @@ -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
}
}
2 changes: 1 addition & 1 deletion proto-compiler/src/cmd/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,4 @@ fn checkout_tag(repo: &Repository, tag_name: &str) -> Result<(), git2::Error> {
}

Ok(())
}
}
Loading