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

Fix obj-caps facility #2018

Closed
wants to merge 45 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
d7e4ff3
Add ChannelCapabilityPath to ICS24
hu55a1n1 Mar 27, 2022
abd9ae7
Simplify channel handler
hu55a1n1 Mar 27, 2022
5576ca6
Fix tests
hu55a1n1 Mar 27, 2022
8865166
Provide handlers with assoc chan/port capabilities
hu55a1n1 Mar 27, 2022
bb06bd2
Remove packet_validate and pass caps to packet handlers
hu55a1n1 Mar 28, 2022
263d09f
Improve PortReader and ChannelReader's capabilities API
hu55a1n1 Mar 28, 2022
973873e
Update packet handler API to use new API
hu55a1n1 Mar 28, 2022
ea35d86
Fix mock caps and tests
hu55a1n1 Mar 28, 2022
7fe7da1
Major refactoring
hu55a1n1 Mar 28, 2022
8ad0e3f
Merge remote-tracking branch 'origin/master' into hu55a1n1/2013-impro…
hu55a1n1 Apr 8, 2022
7096036
Extract Port/Channel CapabilityReader/Keeper into separate traits
hu55a1n1 Apr 8, 2022
a93f689
Store channel capability after handler dispatch
hu55a1n1 Apr 8, 2022
a7b3b48
Adjust handlers to add {Channel,Port}Capability bound to context
hu55a1n1 Apr 8, 2022
ec20b5a
Introduce ScopedCapability{Keeper,Reader}
hu55a1n1 Apr 8, 2022
0f44595
Constrain Capability{Keeper,Reader} scope to core module for Ics26Con…
hu55a1n1 Apr 8, 2022
17f4436
WIP: Update Mock impl to use scoped capability readers/keepers
hu55a1n1 Apr 8, 2022
6d2cf00
Capability Readers/Keepers are now scoped by definition
hu55a1n1 Apr 11, 2022
1d0135c
Impl Default and Into<ModuleId> for CoreModuleId
hu55a1n1 Apr 11, 2022
c92fa84
Simplify Ics26Context bounds based on new design
hu55a1n1 Apr 11, 2022
77ad8da
Update all channel handlers to use CapabilityReaders/Keepers scoped t…
hu55a1n1 Apr 11, 2022
4708f05
CoreModuleId ToString now returns 'ibc'
hu55a1n1 Apr 11, 2022
779a5b9
Update tests to use new API
hu55a1n1 Apr 11, 2022
4c9e28e
Implement dummy MockOCap
hu55a1n1 Apr 11, 2022
8cdb2f8
Update Mock tests
hu55a1n1 Apr 11, 2022
0659fbb
Improve Capability ctor to accept index as input
hu55a1n1 Apr 12, 2022
fb8e80d
Make claim_capability() and release_capability() are fallible
hu55a1n1 Apr 12, 2022
19e92e3
Don't recreate capability in store_channel_capability()
hu55a1n1 Apr 12, 2022
aa6da61
Polish MockCap impl and remove legacy ocap facility
hu55a1n1 Apr 12, 2022
81e1e17
Add capability related errors
hu55a1n1 Apr 12, 2022
d993a3b
Impl on_chan_open_init for DummyModule
hu55a1n1 Apr 12, 2022
bfe717d
Manually implement Clone for MockContext
hu55a1n1 Apr 12, 2022
fea6038
MockContext now initializes a router and ocaps by default
hu55a1n1 Apr 12, 2022
2f55b74
Fix missing cap auth in write ack handler
hu55a1n1 Apr 12, 2022
a8b96d4
Fix all handler tests
hu55a1n1 Apr 12, 2022
2d4a0c2
Make write ack handler public
hu55a1n1 Apr 12, 2022
74131a0
Fix tests
hu55a1n1 Apr 12, 2022
287f7df
Fix clippy errors
hu55a1n1 Apr 12, 2022
c21ce3e
Remove TypedCapabilites
hu55a1n1 Apr 12, 2022
1a71f48
Capability as trait
hu55a1n1 Apr 12, 2022
32d8733
Add .changelog entry
hu55a1n1 Apr 12, 2022
d27b5bb
Fix clippy warnings
hu55a1n1 Apr 12, 2022
8d49315
Resurrect channel/packet_validate()
hu55a1n1 Apr 12, 2022
a931d4a
Update tests for API changes
hu55a1n1 Apr 12, 2022
ac365f8
Minor refactoring
hu55a1n1 Apr 12, 2022
9374f2c
Use is_none() instead of matches!(_, None)
hu55a1n1 Apr 12, 2022
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 @@
- Improve the object capability system.
([#2013](https://github.com/informalsystems/ibc-rs/issues/2013))
87 changes: 72 additions & 15 deletions modules/src/core/ics04_channel/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ use crate::core::ics02_client::client_state::AnyClientState;
use crate::core::ics03_connection::connection::ConnectionEnd;
use crate::core::ics04_channel::channel::ChannelEnd;
use crate::core::ics04_channel::handler::recv_packet::RecvPacketResult;
use crate::core::ics04_channel::handler::{ChannelIdState, ChannelResult};
use crate::core::ics04_channel::handler::{ChannelIdState, ChannelResultSansCap};
use crate::core::ics04_channel::{error::Error, packet::Receipt};
use crate::core::ics05_port::capabilities::ChannelCapability;
use crate::core::ics05_port::context::CapabilityReader;
use crate::core::ics05_port::capabilities::CapabilityName;
use crate::core::ics05_port::context::{CapabilityKeeper, CapabilityReader};
use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
use crate::core::ics24_host::path::ChannelCapabilityPath;
use crate::core::ics26_routing::context::ModuleId;
use crate::prelude::*;
use crate::timestamp::Timestamp;
Expand All @@ -22,7 +23,7 @@ use crate::Height;
use super::packet::{PacketResult, Sequence};

/// A context supplying all the necessary read-only dependencies for processing any `ChannelMsg`.
pub trait ChannelReader: CapabilityReader {
pub trait ChannelReader {
/// Returns the ChannelEnd for the given `port_id` and `chan_id`.
fn channel_end(&self, port_channel_id: &(PortId, ChannelId)) -> Result<ChannelEnd, Error>;

Expand All @@ -41,8 +42,6 @@ pub trait ChannelReader: CapabilityReader {
height: Height,
) -> Result<AnyConsensusState, Error>;

fn authenticated_capability(&self, port_id: &PortId) -> Result<ChannelCapability, Error>;

fn get_next_sequence_send(
&self,
port_channel_id: &(PortId, ChannelId),
Expand Down Expand Up @@ -106,19 +105,12 @@ pub trait ChannelReader: CapabilityReader {
fn block_delay(&self, delay_period_time: Duration) -> u64 {
calculate_block_delay(delay_period_time, self.max_expected_time_per_block())
}

/// Return the module_id along with the capability associated with a given (channel-id, port_id)
fn lookup_module_by_channel(
&self,
channel_id: &ChannelId,
port_id: &PortId,
) -> Result<(ModuleId, ChannelCapability), Error>;
}

/// A context supplying all the necessary write-only dependencies (i.e., storage writing facility)
/// for processing any `ChannelMsg`.
pub trait ChannelKeeper {
fn store_channel_result(&mut self, result: ChannelResult) -> Result<(), Error> {
fn store_channel_result(&mut self, result: ChannelResultSansCap) -> Result<(), Error> {
// The handler processed this channel & some modifications occurred, store the new end.
self.store_channel(
(result.port_id.clone(), result.channel_id),
Expand All @@ -139,7 +131,7 @@ pub trait ChannelKeeper {
// Initialize send, recv, and ack sequence numbers.
self.store_next_sequence_send((result.port_id.clone(), result.channel_id), 1.into())?;
self.store_next_sequence_recv((result.port_id.clone(), result.channel_id), 1.into())?;
self.store_next_sequence_ack((result.port_id, result.channel_id), 1.into())?;
self.store_next_sequence_ack((result.port_id.clone(), result.channel_id), 1.into())?;
}

Ok(())
Expand Down Expand Up @@ -280,6 +272,71 @@ pub trait ChannelKeeper {
fn increase_channel_counter(&mut self);
}

pub trait ChannelCapabilityReader<M: Into<ModuleId>>: CapabilityReader<M> {
/// Return the `ModuleId` along with the `Capability` associated with a given
/// `PortId`-`ChannelId`
fn lookup_module_by_channel(
&self,
channel_id: ChannelId,
port_id: PortId,
) -> Result<(ModuleId, Self::Capability), Error> {
self.lookup_module(&channel_capability_name(port_id, channel_id))
.map_err(Error::ics05_port)
}

/// Get the `Capability` associated with the specified `PortId`-`ChannelId`
fn get_channel_capability(
&self,
port_id: PortId,
channel_id: ChannelId,
) -> Result<Self::Capability, Error> {
self.get_capability(&channel_capability_name(port_id, channel_id))
.map_err(Error::ics05_port)
}

/// Authenticate a `Capability` against the specified `PortId`-`ChannelId` by checking if
/// the capability was previously generated and bound to the specified port/channel
fn authenticate_channel_capability(
&self,
port_id: PortId,
channel_id: ChannelId,
capability: &Self::Capability,
) -> Result<(), Error> {
self.authenticate_capability(&channel_capability_name(port_id, channel_id), capability)
.map_err(Error::ics05_port)
}

fn create_channel_capability(
Copy link
Member Author

@hu55a1n1 hu55a1n1 Apr 13, 2022

Choose a reason for hiding this comment

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

This is an important deviation from the SDK's capability design. We require implementations to be able to create capabilities without mutating the state. The capability returned from this call is later stored to the state by calling ChannelCapabilityKeeper::store_channel_capability(). This is required because our handlers do not mutate state, but still need to be able to create capabilities.

&self,
port_id: PortId,
channel_id: ChannelId,
) -> Result<Self::Capability, Error> {
self.create_capability(channel_capability_name(port_id, channel_id))
.map_err(Error::ics05_port)
}
}

pub trait ChannelCapabilityKeeper<M: Into<ModuleId>>: CapabilityKeeper<M> {
/// Create a new capability associated with the given `PortId`-`ChannelId`
fn store_channel_capability(
&mut self,
port_id: PortId,
channel_id: ChannelId,
channel_cap: Self::Capability,
) -> Result<(), Error> {
self.claim_capability(channel_capability_name(port_id, channel_id), channel_cap)
.map(Into::into)
.map_err(Error::ics05_port)
}
}

pub(crate) fn channel_capability_name(port_id: PortId, channel_id: ChannelId) -> CapabilityName {
ChannelCapabilityPath(port_id, channel_id)
.to_string()
.parse()
.expect("ChannelEndsPath cannot be empty string")
}

pub fn calculate_block_delay(
delay_period_time: Duration,
max_expected_time_per_block: Duration,
Expand Down
134 changes: 78 additions & 56 deletions modules/src/core/ics04_channel/handler.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
//! This module implements the processing logic for ICS4 (channel) messages.

use crate::core::ics04_channel::channel::ChannelEnd;
use crate::core::ics04_channel::context::ChannelReader;
use crate::core::ics04_channel::error::Error;
use crate::core::ics04_channel::msgs::ChannelMsg;
use crate::core::ics04_channel::{msgs::PacketMsg, packet::PacketResult};
use crate::core::ics05_port::capabilities::ChannelCapability;
use crate::core::ics05_port::capabilities::Capability;
use crate::core::ics24_host::identifier::{ChannelId, PortId};
use crate::core::ics26_routing::context::{
Ics26Context, ModuleId, ModuleOutput, OnRecvPacketAck, Router,
Expand Down Expand Up @@ -38,61 +37,102 @@ pub enum ChannelIdState {
}

#[derive(Clone, Debug)]
pub struct ChannelResult {
pub struct ChannelResult<C: Capability> {
pub port_id: PortId,
pub channel_id: ChannelId,
pub channel_id_state: ChannelIdState,
pub channel_cap: ChannelCapability,
pub channel_cap: C,
pub channel_end: ChannelEnd,
}

pub fn channel_validate<Ctx>(ctx: &Ctx, msg: &ChannelMsg) -> Result<ModuleId, Error>
pub struct ChannelResultSansCap {
pub port_id: PortId,
pub channel_id: ChannelId,
pub channel_id_state: ChannelIdState,
pub channel_end: ChannelEnd,
}

impl<Cap: Capability> ChannelResult<Cap> {
pub(crate) fn destructure(self) -> (ChannelResultSansCap, Cap) {
let ChannelResult {
port_id,
channel_id,
channel_id_state,
channel_cap,
channel_end,
} = self;
(
ChannelResultSansCap {
port_id,
channel_id,
channel_id_state,
channel_end,
},
channel_cap,
)
}
}

pub struct DispatchResult<R> {
pub output: HandlerOutputBuilder<()>,
pub result: R,
}

pub type ChannelDispatchResult<C> = DispatchResult<ChannelResult<C>>;
pub type PacketDispatchResult = DispatchResult<PacketResult>;

pub fn channel_validate<Ctx, Cap>(ctx: &Ctx, msg: &ChannelMsg) -> Result<(ModuleId, Cap), Error>
where
Ctx: Ics26Context,
Ctx: Ics26Context<Capability = Cap>,
Cap: Capability,
{
let module_id = msg.lookup_module(ctx)?;
let (module_id, cap) = msg.lookup_module(ctx)?;
if ctx.router().has_route(&module_id) {
Ok(module_id)
Ok((module_id, cap))
} else {
Err(Error::route_not_found())
}
}

/// General entry point for processing any type of message related to the ICS4 channel open and
/// channel close handshake protocols.
pub fn channel_dispatch<Ctx>(
pub fn channel_dispatch<Ctx, Cap>(
ctx: &Ctx,
msg: &ChannelMsg,
) -> Result<(HandlerOutputBuilder<()>, ChannelResult), Error>
cap: Cap,
) -> Result<ChannelDispatchResult<Cap>, Error>
where
Ctx: ChannelReader,
Ctx: Ics26Context<Capability = Cap>,
Cap: Capability,
{
let output = match msg {
ChannelMsg::ChannelOpenInit(msg) => chan_open_init::process(ctx, msg),
ChannelMsg::ChannelOpenTry(msg) => chan_open_try::process(ctx, msg),
ChannelMsg::ChannelOpenAck(msg) => chan_open_ack::process(ctx, msg),
ChannelMsg::ChannelOpenConfirm(msg) => chan_open_confirm::process(ctx, msg),
ChannelMsg::ChannelCloseInit(msg) => chan_close_init::process(ctx, msg),
ChannelMsg::ChannelCloseConfirm(msg) => chan_close_confirm::process(ctx, msg),
ChannelMsg::ChannelOpenInit(msg) => chan_open_init::process(ctx, msg, cap),
ChannelMsg::ChannelOpenTry(msg) => chan_open_try::process(ctx, msg, cap),
ChannelMsg::ChannelOpenAck(msg) => chan_open_ack::process(ctx, msg, cap),
ChannelMsg::ChannelOpenConfirm(msg) => chan_open_confirm::process(ctx, msg, cap),
ChannelMsg::ChannelCloseInit(msg) => chan_close_init::process(ctx, msg, cap),
ChannelMsg::ChannelCloseConfirm(msg) => chan_close_confirm::process(ctx, msg, cap),
}?;

let HandlerOutput {
result,
log,
events,
} = output;
let builder = HandlerOutput::builder().with_log(log).with_events(events);
Ok((builder, result))
let output = HandlerOutput::builder().with_log(log).with_events(events);
Ok(ChannelDispatchResult { output, result })
}

pub fn channel_callback<Ctx>(
pub fn channel_callback<Ctx, Cap>(
ctx: &mut Ctx,
module_id: &ModuleId,
msg: &ChannelMsg,
mut result: ChannelResult,
mut result: ChannelResult<Cap>,
module_output: &mut ModuleOutput,
) -> Result<ChannelResult, Error>
) -> Result<ChannelResult<Cap>, Error>
where
Ctx: Ics26Context,
Cap: Capability,
{
let cb = ctx
.router_mut()
Expand Down Expand Up @@ -142,60 +182,42 @@ where
Ok(result)
}

pub fn packet_validate<Ctx>(ctx: &Ctx, msg: &PacketMsg) -> Result<ModuleId, Error>
pub fn packet_validate<Ctx, Cap>(ctx: &Ctx, msg: &PacketMsg) -> Result<(ModuleId, Cap), Error>
where
Ctx: Ics26Context,
Ctx: Ics26Context<Capability = Cap>,
Cap: Capability,
{
let module_id = match msg {
PacketMsg::RecvPacket(msg) => {
ctx.lookup_module_by_channel(
&msg.packet.destination_channel,
&msg.packet.destination_port,
)?
.0
}
PacketMsg::AckPacket(msg) => {
ctx.lookup_module_by_channel(&msg.packet.source_channel, &msg.packet.source_port)?
.0
}
PacketMsg::ToPacket(msg) => {
ctx.lookup_module_by_channel(&msg.packet.source_channel, &msg.packet.source_port)?
.0
}
PacketMsg::ToClosePacket(msg) => {
ctx.lookup_module_by_channel(&msg.packet.source_channel, &msg.packet.source_port)?
.0
}
};

let (module_id, cap) = msg.lookup_module(ctx)?;
if ctx.router().has_route(&module_id) {
Ok(module_id)
Ok((module_id, cap))
} else {
Err(Error::route_not_found())
}
}

/// Dispatcher for processing any type of message related to the ICS4 packet protocols.
pub fn packet_dispatch<Ctx>(
pub fn packet_dispatch<Ctx, Cap>(
ctx: &Ctx,
msg: &PacketMsg,
) -> Result<(HandlerOutputBuilder<()>, PacketResult), Error>
cap: Cap,
) -> Result<PacketDispatchResult, Error>
where
Ctx: ChannelReader,
Ctx: Ics26Context<Capability = Cap>,
Cap: Capability,
{
let output = match msg {
PacketMsg::RecvPacket(msg) => recv_packet::process(ctx, msg),
PacketMsg::AckPacket(msg) => acknowledgement::process(ctx, msg),
PacketMsg::ToPacket(msg) => timeout::process(ctx, msg),
PacketMsg::ToClosePacket(msg) => timeout_on_close::process(ctx, msg),
PacketMsg::RecvPacket(msg) => recv_packet::process(ctx, msg, cap),
PacketMsg::AckPacket(msg) => acknowledgement::process(ctx, msg, cap),
PacketMsg::ToPacket(msg) => timeout::process(ctx, msg, cap),
PacketMsg::ToClosePacket(msg) => timeout_on_close::process(ctx, msg, cap),
}?;
let HandlerOutput {
result,
log,
events,
} = output;
let builder = HandlerOutput::builder().with_log(log).with_events(events);
Ok((builder, result))
let output = HandlerOutput::builder().with_log(log).with_events(events);
Ok(PacketDispatchResult { output, result })
}

pub fn packet_callback<Ctx>(
Expand Down
Loading