Skip to content

Commit

Permalink
Make process() functions pub(crate) (#328)
Browse files Browse the repository at this point in the history
* Make Msg* structs pub(crate)

* Create 324-make-msg-structs-pub-crate.md

* fix clippy

* Remove Unused new function

* fix merge error

* Revert pub(crate) because pub trait ValidationContext

* fix test
  • Loading branch information
DaviRain-Su authored Jan 9, 2023
1 parent 9b46243 commit f1d2ac5
Show file tree
Hide file tree
Showing 41 changed files with 251 additions and 445 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Make Msg* structs pub(crate) ([#324](https://github.com/cosmos/ibc-
rs/issues/324))
44 changes: 16 additions & 28 deletions crates/ibc/src/core/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,30 +110,24 @@ mod val_exec_ctx {
Self: Sized,
{
match message {
MsgEnvelope::ClientMsg(message) => match message {
MsgEnvelope::Client(message) => match message {
ClientMsg::CreateClient(message) => create_client::validate(self, message),
ClientMsg::UpdateClient(message) => update_client::validate(self, message),
ClientMsg::Misbehaviour(message) => misbehaviour::validate(self, message),
ClientMsg::UpgradeClient(message) => upgrade_client::validate(self, message),
}
.map_err(RouterError::ContextError),
MsgEnvelope::ConnectionMsg(message) => match message {
ConnectionMsg::ConnectionOpenInit(message) => {
conn_open_init::validate(self, message)
}
ConnectionMsg::ConnectionOpenTry(message) => {
conn_open_try::validate(self, message)
}
ConnectionMsg::ConnectionOpenAck(message) => {
conn_open_ack::validate(self, message)
}
ConnectionMsg::ConnectionOpenConfirm(ref message) => {
MsgEnvelope::Connection(message) => match message {
ConnectionMsg::OpenInit(message) => conn_open_init::validate(self, message),
ConnectionMsg::OpenTry(message) => conn_open_try::validate(self, message),
ConnectionMsg::OpenAck(message) => conn_open_ack::validate(self, message),
ConnectionMsg::OpenConfirm(ref message) => {
conn_open_confirm::validate(self, message)
}
}
.map_err(RouterError::ContextError),
MsgEnvelope::ChannelMsg(_message) => todo!(),
MsgEnvelope::PacketMsg(_message) => todo!(),
MsgEnvelope::Channel(_message) => todo!(),
MsgEnvelope::Packet(_message) => todo!(),
}
}

Expand Down Expand Up @@ -335,30 +329,24 @@ mod val_exec_ctx {
Self: Sized,
{
match message {
MsgEnvelope::ClientMsg(message) => match message {
MsgEnvelope::Client(message) => match message {
ClientMsg::CreateClient(message) => create_client::execute(self, message),
ClientMsg::UpdateClient(message) => update_client::execute(self, message),
ClientMsg::Misbehaviour(message) => misbehaviour::execute(self, message),
ClientMsg::UpgradeClient(message) => upgrade_client::execute(self, message),
}
.map_err(RouterError::ContextError),
MsgEnvelope::ConnectionMsg(message) => match message {
ConnectionMsg::ConnectionOpenInit(message) => {
conn_open_init::execute(self, message)
}
ConnectionMsg::ConnectionOpenTry(message) => {
conn_open_try::execute(self, message)
}
ConnectionMsg::ConnectionOpenAck(message) => {
conn_open_ack::execute(self, message)
}
ConnectionMsg::ConnectionOpenConfirm(ref message) => {
MsgEnvelope::Connection(message) => match message {
ConnectionMsg::OpenInit(message) => conn_open_init::execute(self, message),
ConnectionMsg::OpenTry(message) => conn_open_try::execute(self, message),
ConnectionMsg::OpenAck(message) => conn_open_ack::execute(self, message),
ConnectionMsg::OpenConfirm(ref message) => {
conn_open_confirm::execute(self, message)
}
}
.map_err(RouterError::ContextError),
MsgEnvelope::ChannelMsg(_message) => todo!(),
MsgEnvelope::PacketMsg(_message) => todo!(),
MsgEnvelope::Channel(_message) => todo!(),
MsgEnvelope::Packet(_message) => todo!(),
}
}

Expand Down
5 changes: 4 additions & 1 deletion crates/ibc/src/core/ics02_client/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ pub enum ClientResult {
}

/// General entry point for processing any message related to ICS2 (client functions) protocols.
pub fn dispatch<Ctx>(ctx: &Ctx, msg: ClientMsg) -> Result<HandlerOutput<ClientResult>, ClientError>
pub(crate) fn dispatch<Ctx>(
ctx: &Ctx,
msg: ClientMsg,
) -> Result<HandlerOutput<ClientResult>, ClientError>
where
Ctx: ClientReader,
{
Expand Down
2 changes: 1 addition & 1 deletion crates/ibc/src/core/ics02_client/handler/create_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ where
Ok(())
}

pub fn process(
pub(crate) fn process(
ctx: &dyn ClientReader,
msg: MsgCreateClient,
) -> HandlerResult<ClientResult, ClientError> {
Expand Down
2 changes: 1 addition & 1 deletion crates/ibc/src/core/ics02_client/handler/misbehaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ where
ctx.store_client_state(ClientStatePath(client_id), client_state)
}

pub fn process(
pub(crate) fn process(
ctx: &dyn ClientReader,
msg: MsgSubmitMisbehaviour,
) -> HandlerResult<ClientResult, ClientError> {
Expand Down
2 changes: 1 addition & 1 deletion crates/ibc/src/core/ics02_client/handler/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ where
Ok(())
}

pub fn process<Ctx: ClientReader>(
pub(crate) fn process<Ctx: ClientReader>(
ctx: &Ctx,
msg: MsgUpdateClient,
) -> HandlerResult<ClientResult, ClientError> {
Expand Down
2 changes: 1 addition & 1 deletion crates/ibc/src/core/ics02_client/handler/upgrade_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ where
Ok(())
}

pub fn process(
pub(crate) fn process(
ctx: &dyn ClientReader,
msg: MsgUpgradeClient,
) -> HandlerResult<ClientResult, ClientError> {
Expand Down
1 change: 1 addition & 0 deletions crates/ibc/src/core/ics02_client/msgs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub mod misbehaviour;
pub mod update_client;
pub mod upgrade_client;

#[allow(dead_code)]
#[derive(Clone, Debug)]
pub enum ClientMsg {
CreateClient(MsgCreateClient),
Expand Down
22 changes: 12 additions & 10 deletions crates/ibc/src/core/ics02_client/msgs/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,6 @@ pub struct MsgUpdateClient {
pub signer: Signer,
}

impl MsgUpdateClient {
pub fn new(client_id: ClientId, header: Any, signer: Signer) -> Self {
MsgUpdateClient {
client_id,
header,
signer,
}
}
}

impl Msg for MsgUpdateClient {
type Raw = RawMsgUpdateClient;

Expand Down Expand Up @@ -71,13 +61,25 @@ mod tests {

use test_log::test;

use ibc_proto::google::protobuf::Any;
use ibc_proto::ibc::core::client::v1::MsgUpdateClient as RawMsgUpdateClient;

use crate::clients::ics07_tendermint::header::test_util::get_dummy_ics07_header;
use crate::core::ics02_client::msgs::MsgUpdateClient;
use crate::core::ics24_host::identifier::ClientId;
use crate::signer::Signer;
use crate::test_utils::get_dummy_account_id;

impl MsgUpdateClient {
pub fn new(client_id: ClientId, header: Any, signer: Signer) -> Self {
MsgUpdateClient {
client_id,
header,
signer,
}
}
}

#[test]
fn msg_update_client_serialization() {
let client_id: ClientId = "tendermint".parse().unwrap();
Expand Down
47 changes: 24 additions & 23 deletions crates/ibc/src/core/ics02_client/msgs/upgrade_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,26 +29,6 @@ pub struct MsgUpgradeClient {
pub signer: Signer,
}

impl MsgUpgradeClient {
pub fn new(
client_id: ClientId,
client_state: Any,
consensus_state: Any,
proof_upgrade_client: RawMerkleProof,
proof_upgrade_consensus_state: RawMerkleProof,
signer: Signer,
) -> Self {
MsgUpgradeClient {
client_id,
client_state,
consensus_state,
proof_upgrade_client,
proof_upgrade_consensus_state,
signer,
}
}
}

impl Msg for MsgUpgradeClient {
type Raw = RawMsgUpgradeClient;

Expand Down Expand Up @@ -114,7 +94,9 @@ impl TryFrom<RawMsgUpgradeClient> for MsgUpgradeClient {

#[cfg(test)]
pub mod test_util {
use ibc_proto::google::protobuf::Any;
use ibc_proto::ibc::core::client::v1::MsgUpgradeClient as RawMsgUpgradeClient;
use ibc_proto::ibc::core::commitment::v1::MerkleProof as RawMerkleProof;

use crate::{
core::{ics02_client::height::Height, ics24_host::identifier::ClientId},
Expand All @@ -125,13 +107,32 @@ pub mod test_util {
};

use super::MsgUpgradeClient;
use crate::signer::Signer;

/// Extends the implementation with additional helper methods.
impl MsgUpgradeClient {
/// Setter for `client_id`. Amenable to chaining, since it consumes the input message.
pub fn with_client_id(self, client_id: ClientId) -> Self {
MsgUpgradeClient { client_id, ..self }
pub fn new(
client_id: ClientId,
client_state: Any,
consensus_state: Any,
proof_upgrade_client: RawMerkleProof,
proof_upgrade_consensus_state: RawMerkleProof,
signer: Signer,
) -> Self {
MsgUpgradeClient {
client_id,
client_state,
consensus_state,
proof_upgrade_client,
proof_upgrade_consensus_state,
signer,
}
}

// /// Setter for `client_id`. Amenable to chaining, since it consumes the input message.
// pub fn with_client_id(self, client_id: ClientId) -> Self {
// MsgUpgradeClient { client_id, ..self }
// }
}

/// Returns a dummy `RawMsgUpgradeClient`, for testing only!
Expand Down
10 changes: 5 additions & 5 deletions crates/ibc/src/core/ics03_connection/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,17 @@ pub struct ConnectionResult {

/// General entry point for processing any type of message related to the ICS3 connection open
/// handshake protocol.
pub fn dispatch<Ctx>(
pub(crate) fn dispatch<Ctx>(
ctx: &Ctx,
msg: ConnectionMsg,
) -> Result<HandlerOutput<ConnectionResult>, ConnectionError>
where
Ctx: ConnectionReader,
{
match msg {
ConnectionMsg::ConnectionOpenInit(msg) => conn_open_init::process(ctx, msg),
ConnectionMsg::ConnectionOpenTry(msg) => conn_open_try::process(ctx, msg),
ConnectionMsg::ConnectionOpenAck(msg) => conn_open_ack::process(ctx, msg),
ConnectionMsg::ConnectionOpenConfirm(msg) => conn_open_confirm::process(ctx, msg),
ConnectionMsg::OpenInit(msg) => conn_open_init::process(ctx, msg),
ConnectionMsg::OpenTry(msg) => conn_open_try::process(ctx, msg),
ConnectionMsg::OpenAck(msg) => conn_open_ack::process(ctx, msg),
ConnectionMsg::OpenConfirm(msg) => conn_open_confirm::process(ctx, msg),
}
}
8 changes: 4 additions & 4 deletions crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,15 +416,15 @@ mod tests {
.clone()
.with_client(&client_id, proof_height)
.with_connection(conn_id.clone(), default_conn_end),
msg: ConnectionMsg::ConnectionOpenAck(msg_ack.clone()),
msg: ConnectionMsg::OpenAck(msg_ack.clone()),
want_pass: true,
match_error: Box::new(|_| panic!("should not have error")),
},
Test {
name: "Processing fails because the connection does not exist in the context"
.to_string(),
ctx: default_context.clone(),
msg: ConnectionMsg::ConnectionOpenAck(msg_ack.clone()),
msg: ConnectionMsg::OpenAck(msg_ack.clone()),
want_pass: false,
match_error: {
let right_connection_id = conn_id.clone();
Expand All @@ -444,7 +444,7 @@ mod tests {
ctx: default_context
.with_client(&client_id, proof_height)
.with_connection(conn_id.clone(), conn_end_open),
msg: ConnectionMsg::ConnectionOpenAck(msg_ack),
msg: ConnectionMsg::OpenAck(msg_ack),
want_pass: false,
match_error: {
let right_connection_id = conn_id;
Expand Down Expand Up @@ -476,7 +476,7 @@ mod tests {
{
let res = ValidationContext::validate(
&test.ctx,
MsgEnvelope::ConnectionMsg(test.msg.clone()),
MsgEnvelope::Connection(test.msg.clone()),
);

match res {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ mod tests {
Test {
name: "Processing fails due to missing connection in context".to_string(),
ctx: context.clone(),
msg: ConnectionMsg::ConnectionOpenConfirm(msg_confirm.clone()),
msg: ConnectionMsg::OpenConfirm(msg_confirm.clone()),
want_pass: false,
},
Test {
Expand All @@ -325,15 +325,15 @@ mod tests {
.clone()
.with_client(&client_id, Height::new(0, 10).unwrap())
.with_connection(msg_confirm.conn_id_on_b.clone(), incorrect_conn_end_state),
msg: ConnectionMsg::ConnectionOpenConfirm(msg_confirm.clone()),
msg: ConnectionMsg::OpenConfirm(msg_confirm.clone()),
want_pass: false,
},
Test {
name: "Processing successful".to_string(),
ctx: context
.with_client(&client_id, Height::new(0, 10).unwrap())
.with_connection(msg_confirm.conn_id_on_b.clone(), correct_conn_end),
msg: ConnectionMsg::ConnectionOpenConfirm(msg_confirm),
msg: ConnectionMsg::OpenConfirm(msg_confirm),
want_pass: true,
},
]
Expand All @@ -345,7 +345,7 @@ mod tests {
{
let res = ValidationContext::validate(
&test.ctx,
MsgEnvelope::ConnectionMsg(test.msg.clone()),
MsgEnvelope::Connection(test.msg.clone()),
);

match res {
Expand Down
10 changes: 5 additions & 5 deletions crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,28 +211,28 @@ mod tests {
Test {
name: "Processing fails because no client exists in the context".to_string(),
ctx: default_context,
msg: ConnectionMsg::ConnectionOpenInit(msg_conn_init_default.clone()),
msg: ConnectionMsg::OpenInit(msg_conn_init_default.clone()),
expected_versions: vec![msg_conn_init_default.version.clone().unwrap()],
want_pass: false,
},
Test {
name: "Incompatible version in MsgConnectionOpenInit msg".to_string(),
ctx: good_context.clone(),
msg: ConnectionMsg::ConnectionOpenInit(msg_conn_init_bad_version),
msg: ConnectionMsg::OpenInit(msg_conn_init_bad_version),
expected_versions: vec![],
want_pass: false,
},
Test {
name: "No version in MsgConnectionOpenInit msg".to_string(),
ctx: good_context.clone(),
msg: ConnectionMsg::ConnectionOpenInit(msg_conn_init_no_version),
msg: ConnectionMsg::OpenInit(msg_conn_init_no_version),
expected_versions: ConnectionReader::get_compatible_versions(&good_context),
want_pass: true,
},
Test {
name: "Good parameters".to_string(),
ctx: good_context,
msg: ConnectionMsg::ConnectionOpenInit(msg_conn_init_default.clone()),
msg: ConnectionMsg::OpenInit(msg_conn_init_default.clone()),
expected_versions: vec![msg_conn_init_default.version.unwrap()],
want_pass: true,
},
Expand All @@ -245,7 +245,7 @@ mod tests {
{
let res = ValidationContext::validate(
&test.ctx,
MsgEnvelope::ConnectionMsg(test.msg.clone()),
MsgEnvelope::Connection(test.msg.clone()),
);

match res {
Expand Down
Loading

0 comments on commit f1d2ac5

Please sign in to comment.