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.

Sign up for GitHub

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

Apply validation/execution separation on Module and Router #469

Closed
wants to merge 2 commits into from
Closed

Apply validation/execution separation on Module and Router #469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd0c8b4
Apply validation/execution separation on Module and Router
Farhad-Shabani Feb 24, 2023
fef3fab
Merge branch 'main' into farhad/val-exec-on-module
Farhad-Shabani Feb 24, 2023
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
2 changes: 2 additions & 0 deletions ...log/unreleased/breaking-changes/465-exec-val-separation-on-module-and-router.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Apply validation/execution separation on Module and Router
([#465](https://github.com/cosmos/ibc-rs/issues/465))
61 changes: 7 additions & 54 deletions crates/ibc/src/core/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use self::acknowledgement::{acknowledgement_packet_execute, acknowledgement_pack
use self::recv_packet::{recv_packet_execute, recv_packet_validate};
use self::timeout::{timeout_packet_execute, timeout_packet_validate, TimeoutMsgType};

use super::ics26_routing::router::{ExecutionRouter, ValidationRouter};
use super::{
ics02_client::error::ClientError,
ics03_connection::error::ConnectionError,
Expand All @@ -45,15 +46,13 @@ use crate::core::ics04_channel::msgs::acknowledgement::Acknowledgement;
use crate::core::ics04_channel::msgs::{ChannelMsg, PacketMsg};
use crate::core::ics04_channel::packet::{Receipt, Sequence};
use crate::core::ics04_channel::timeout::TimeoutHeight;
use crate::core::ics05_port::error::PortError::UnknownPort;
use crate::core::ics23_commitment::commitment::CommitmentPrefix;
use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId};
use crate::core::ics24_host::path::{
AckPath, ChannelEndPath, ClientConnectionPath, ClientConsensusStatePath, ClientStatePath,
ClientTypePath, CommitmentPath, ConnectionPath, ReceiptPath, SeqAckPath, SeqRecvPath,
SeqSendPath,
};
use crate::core::ics26_routing::context::{Module, ModuleId};
use crate::core::{
ics02_client::{
handler::{create_client, misbehaviour, update_client, upgrade_client},
Expand Down Expand Up @@ -120,53 +119,7 @@ impl std::error::Error for ContextError {
}
}

pub trait Router {
/// Returns a reference to a `Module` registered against the specified `ModuleId`
fn get_route(&self, module_id: &ModuleId) -> Option<&dyn Module>;

/// Returns a mutable reference to a `Module` registered against the specified `ModuleId`
fn get_route_mut(&mut self, module_id: &ModuleId) -> Option<&mut dyn Module>;

/// Returns true if the `Router` has a `Module` registered against the specified `ModuleId`
fn has_route(&self, module_id: &ModuleId) -> bool;

/// Return the module_id associated with a given port_id
fn lookup_module_by_port(&self, port_id: &PortId) -> Option<ModuleId>;

fn lookup_module_channel(&self, msg: &ChannelMsg) -> Result<ModuleId, ChannelError> {
let port_id = match msg {
ChannelMsg::OpenInit(msg) => &msg.port_id_on_a,
ChannelMsg::OpenTry(msg) => &msg.port_id_on_b,
ChannelMsg::OpenAck(msg) => &msg.port_id_on_a,
ChannelMsg::OpenConfirm(msg) => &msg.port_id_on_b,
ChannelMsg::CloseInit(msg) => &msg.port_id_on_a,
ChannelMsg::CloseConfirm(msg) => &msg.port_id_on_b,
};
let module_id = self
.lookup_module_by_port(port_id)
.ok_or(ChannelError::Port(UnknownPort {
port_id: port_id.clone(),
}))?;
Ok(module_id)
}

fn lookup_module_packet(&self, msg: &PacketMsg) -> Result<ModuleId, ChannelError> {
let port_id = match msg {
PacketMsg::Recv(msg) => &msg.packet.port_on_b,
PacketMsg::Ack(msg) => &msg.packet.port_on_a,
PacketMsg::Timeout(msg) => &msg.packet.port_on_a,
PacketMsg::TimeoutOnClose(msg) => &msg.packet.port_on_a,
};
let module_id = self
.lookup_module_by_port(port_id)
.ok_or(ChannelError::Port(UnknownPort {
port_id: port_id.clone(),
}))?;
Ok(module_id)
}
}

pub trait ValidationContext: Router {
pub trait ValidationContext: ValidationRouter {
/// Validation entrypoint.
fn validate(&self, msg: MsgEnvelope) -> Result<(), RouterError>
where
Expand All @@ -191,7 +144,7 @@ pub trait ValidationContext: Router {
let module_id = self
.lookup_module_channel(&msg)
.map_err(ContextError::from)?;
if !self.has_route(&module_id) {
if !self.has_validation_route(&module_id) {
return Err(ChannelError::RouteNotFound)
.map_err(ContextError::ChannelError)
.map_err(RouterError::ContextError);
Expand All @@ -215,7 +168,7 @@ pub trait ValidationContext: Router {
let module_id = self
.lookup_module_packet(&msg)
.map_err(ContextError::from)?;
if !self.has_route(&module_id) {
if !self.has_validation_route(&module_id) {
return Err(ChannelError::RouteNotFound)
.map_err(ContextError::ChannelError)
.map_err(RouterError::ContextError);
Expand Down Expand Up @@ -412,7 +365,7 @@ pub trait ValidationContext: Router {
}
}

pub trait ExecutionContext: ValidationContext {
pub trait ExecutionContext: ValidationContext + ExecutionRouter {
/// Execution entrypoint
fn execute(&mut self, msg: MsgEnvelope) -> Result<(), RouterError>
where
Expand All @@ -437,7 +390,7 @@ pub trait ExecutionContext: ValidationContext {
let module_id = self
.lookup_module_channel(&msg)
.map_err(ContextError::from)?;
if !self.has_route(&module_id) {
if !self.has_execution_route(&module_id) {
return Err(ChannelError::RouteNotFound)
.map_err(ContextError::ChannelError)
.map_err(RouterError::ContextError);
Expand All @@ -459,7 +412,7 @@ pub trait ExecutionContext: ValidationContext {
let module_id = self
.lookup_module_packet(&msg)
.map_err(ContextError::from)?;
if !self.has_route(&module_id) {
if !self.has_execution_route(&module_id) {
return Err(ChannelError::RouteNotFound)
.map_err(ContextError::ChannelError)
.map_err(RouterError::ContextError);
Expand Down
8 changes: 4 additions & 4 deletions crates/ibc/src/core/context/acknowledgement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
channel::Order, error::ChannelError, events::AcknowledgePacket,
handler::acknowledgement, msgs::acknowledgement::MsgAcknowledgement,
},
ics26_routing::context::ModuleId,
ics26_routing::module::ModuleId,
},
events::IbcEvent,
};
Expand All @@ -25,7 +25,7 @@ where
acknowledgement::validate(ctx_a, &msg)?;

let module = ctx_a
.get_route(&module_id)
.get_validation_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;

module
Expand Down Expand Up @@ -68,7 +68,7 @@ where
};

let module = ctx_a
.get_route_mut(&module_id)
.get_execution_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;

let (extras, cb_result) =
Expand Down Expand Up @@ -156,7 +156,7 @@ mod tests {

let module_id: ModuleId = MODULE_ID_STR.parse().unwrap();
let module = DummyTransferModule::new(ctx.ibc_store_share());
ctx.add_route(module_id.clone(), module).unwrap();
ctx.add_exec_route(module_id.clone(), module).unwrap();

let msg = MsgAcknowledgement::try_from(get_dummy_raw_msg_acknowledgement(
client_height.revision_height(),
Expand Down
10 changes: 5 additions & 5 deletions crates/ibc/src/core/context/chan_close_confirm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::core::ics04_channel::error::ChannelError;
use crate::core::ics04_channel::events::CloseConfirm;
use crate::core::ics04_channel::handler::chan_close_confirm;
use crate::core::ics04_channel::msgs::chan_close_confirm::MsgChannelCloseConfirm;
use crate::core::ics26_routing::context::ModuleId;
use crate::core::ics26_routing::module::ModuleId;

use crate::events::IbcEvent;

Expand All @@ -22,7 +22,7 @@ where
chan_close_confirm::validate(ctx_b, &msg)?;

let module = ctx_b
.get_route(&module_id)
.get_validation_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;
module.on_chan_close_confirm_validate(&msg.port_id_on_b, &msg.chan_id_on_b)?;

Expand All @@ -38,7 +38,7 @@ where
ExecCtx: ExecutionContext,
{
let module = ctx_b
.get_route_mut(&module_id)
.get_execution_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;
let extras = module.on_chan_close_confirm_execute(&msg.port_id_on_b, &msg.chan_id_on_b)?;
let chan_end_path_on_b = ChannelEndPath::new(&msg.port_id_on_b, &msg.chan_id_on_b);
Expand Down Expand Up @@ -99,7 +99,7 @@ mod tests {
use crate::core::context::chan_close_confirm::chan_close_confirm_execute;
use crate::core::ics04_channel::msgs::chan_close_confirm::test_util::get_dummy_raw_msg_chan_close_confirm;
use crate::core::ics04_channel::msgs::chan_close_confirm::MsgChannelCloseConfirm;
use crate::core::ics26_routing::context::ModuleId;
use crate::core::ics26_routing::module::ModuleId;
use crate::core::ValidationContext;
use crate::events::IbcEvent;
use crate::prelude::*;
Expand Down Expand Up @@ -162,7 +162,7 @@ mod tests {

let module = DummyTransferModule::new(context.ibc_store_share());
let module_id: ModuleId = MODULE_ID_STR.parse().unwrap();
context.add_route(module_id.clone(), module).unwrap();
context.add_exec_route(module_id.clone(), module).unwrap();

let res = chan_close_confirm_execute(&mut context, module_id, msg_chan_close_confirm);
assert!(res.is_ok(), "Execution success: happy path");
Expand Down
10 changes: 5 additions & 5 deletions crates/ibc/src/core/context/chan_close_init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{core::ics04_channel::msgs::chan_close_init::MsgChannelCloseInit, pre

use crate::core::ics04_channel::channel::State;
use crate::core::ics04_channel::error::ChannelError;
use crate::core::ics26_routing::context::ModuleId;
use crate::core::ics26_routing::module::ModuleId;

use crate::events::IbcEvent;

Expand All @@ -21,7 +21,7 @@ where
chan_close_init::validate(ctx_a, &msg)?;

let module = ctx_a
.get_route(&module_id)
.get_validation_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;
module.on_chan_close_init_validate(&msg.port_id_on_a, &msg.chan_id_on_a)?;

Expand All @@ -37,7 +37,7 @@ where
ExecCtx: ExecutionContext,
{
let module = ctx_a
.get_route_mut(&module_id)
.get_execution_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;
let extras = module.on_chan_close_init_execute(&msg.port_id_on_a, &msg.chan_id_on_a)?;
let chan_end_path_on_a = ChannelEndPath::new(&msg.port_id_on_a, &msg.chan_id_on_a);
Expand Down Expand Up @@ -98,7 +98,7 @@ mod tests {
use crate::applications::transfer::MODULE_ID_STR;
use crate::core::ics04_channel::msgs::chan_close_init::test_util::get_dummy_raw_msg_chan_close_init;
use crate::core::ics04_channel::msgs::chan_close_init::MsgChannelCloseInit;
use crate::core::ics26_routing::context::ModuleId;
use crate::core::ics26_routing::module::ModuleId;
use crate::core::ValidationContext;
use crate::events::IbcEvent;
use crate::prelude::*;
Expand Down Expand Up @@ -153,7 +153,7 @@ mod tests {

let module = DummyTransferModule::new(default_context.ibc_store_share());
let module_id: ModuleId = MODULE_ID_STR.parse().unwrap();
default_context.add_route(module_id, module).unwrap();
default_context.add_exec_route(module_id, module).unwrap();

default_context
.with_client(&client_id, client_consensus_state_height)
Expand Down
10 changes: 5 additions & 5 deletions crates/ibc/src/core/context/chan_open_ack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::core::ics04_channel::msgs::chan_open_ack::MsgChannelOpenAck;

use crate::core::ics04_channel::channel::State;
use crate::core::ics04_channel::error::ChannelError;
use crate::core::ics26_routing::context::ModuleId;
use crate::core::ics26_routing::module::ModuleId;

use crate::events::IbcEvent;

Expand All @@ -23,7 +23,7 @@ where
chan_open_ack::validate(ctx_a, &msg)?;

let module = ctx_a
.get_route(&module_id)
.get_validation_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;
module.on_chan_open_ack_validate(&msg.port_id_on_a, &msg.chan_id_on_a, &msg.version_on_b)?;

Expand All @@ -39,7 +39,7 @@ where
ExecCtx: ExecutionContext,
{
let module = ctx_a
.get_route_mut(&module_id)
.get_execution_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;
let extras =
module.on_chan_open_ack_execute(&msg.port_id_on_a, &msg.chan_id_on_a, &msg.version_on_b)?;
Expand Down Expand Up @@ -111,7 +111,7 @@ mod tests {
},
},
ics24_host::identifier::{ClientId, ConnectionId},
ics26_routing::context::ModuleId,
ics26_routing::module::ModuleId,
},
mock::context::MockContext,
test_utils::DummyTransferModule,
Expand Down Expand Up @@ -139,7 +139,7 @@ mod tests {
let mut context = MockContext::default();
let module = DummyTransferModule::new(context.ibc_store_share());
let module_id: ModuleId = MODULE_ID_STR.parse().unwrap();
context.add_route(module_id.clone(), module).unwrap();
context.add_exec_route(module_id.clone(), module).unwrap();

let client_id_on_a = ClientId::new(mock_client_type(), 45).unwrap();
let conn_id_on_a = ConnectionId::new(2);
Expand Down
10 changes: 5 additions & 5 deletions crates/ibc/src/core/context/chan_open_confirm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{core::ics04_channel::msgs::chan_open_confirm::MsgChannelOpenConfirm,

use crate::core::ics04_channel::channel::State;
use crate::core::ics04_channel::error::ChannelError;
use crate::core::ics26_routing::context::ModuleId;
use crate::core::ics26_routing::module::ModuleId;

use crate::events::IbcEvent;

Expand All @@ -22,7 +22,7 @@ where
chan_open_confirm::validate(ctx_b, &msg)?;

let module = ctx_b
.get_route(&module_id)
.get_validation_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;
module.on_chan_open_confirm_validate(&msg.port_id_on_b, &msg.chan_id_on_b)?;

Expand All @@ -38,7 +38,7 @@ where
ExecCtx: ExecutionContext,
{
let module = ctx_b
.get_route_mut(&module_id)
.get_execution_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;

let extras = module.on_chan_open_confirm_execute(&msg.port_id_on_b, &msg.chan_id_on_b)?;
Expand Down Expand Up @@ -117,7 +117,7 @@ mod tests {
},
},
ics24_host::identifier::{ChannelId, ClientId, ConnectionId},
ics26_routing::context::ModuleId,
ics26_routing::module::ModuleId,
},
mock::context::MockContext,
test_utils::DummyTransferModule,
Expand Down Expand Up @@ -145,7 +145,7 @@ mod tests {
let mut context = MockContext::default();
let module = DummyTransferModule::new(context.ibc_store_share());
let module_id: ModuleId = MODULE_ID_STR.parse().unwrap();
context.add_route(module_id.clone(), module).unwrap();
context.add_exec_route(module_id.clone(), module).unwrap();

let client_id_on_b = ClientId::new(mock_client_type(), 45).unwrap();
let conn_id_on_b = ConnectionId::new(2);
Expand Down
8 changes: 4 additions & 4 deletions crates/ibc/src/core/context/chan_open_init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::core::ics24_host::identifier::ChannelId;

use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, State};
use crate::core::ics04_channel::error::ChannelError;
use crate::core::ics26_routing::context::ModuleId;
use crate::core::ics26_routing::module::ModuleId;

use crate::events::IbcEvent;

Expand All @@ -25,7 +25,7 @@ where
let chan_id_on_a = ChannelId::new(ctx_a.channel_counter()?);

let module = ctx_a
.get_route(&module_id)
.get_validation_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;
module.on_chan_open_init_validate(
msg.ordering,
Expand All @@ -49,7 +49,7 @@ where
{
let chan_id_on_a = ChannelId::new(ctx_a.channel_counter()?);
let module = ctx_a
.get_route_mut(&module_id)
.get_execution_route(&module_id)
.ok_or(ChannelError::RouteNotFound)?;
let (extras, version) = module.on_chan_open_init_execute(
msg.ordering,
Expand Down Expand Up @@ -150,7 +150,7 @@ mod tests {
let mut context = MockContext::default();
let module_id: ModuleId = MODULE_ID_STR.parse().unwrap();
let module = DummyTransferModule::new(context.ibc_store_share());
context.add_route(module_id.clone(), module).unwrap();
context.add_exec_route(module_id.clone(), module).unwrap();

let msg_conn_init =
MsgConnectionOpenInit::try_from(get_dummy_raw_msg_conn_open_init()).unwrap();
Expand Down
Loading