From 1d430c9f86168816b55cd9185c6c632c4190118d Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 18 Mar 2022 01:54:48 +0530 Subject: [PATCH] Callbacks return ModuleOutput --- modules/src/core/ics04_channel/handler.rs | 73 ++++++++++++----------- modules/src/core/ics26_routing/context.rs | 47 ++++++--------- modules/src/core/ics26_routing/handler.rs | 20 ++----- modules/src/mock/context.rs | 23 +++---- 4 files changed, 72 insertions(+), 91 deletions(-) diff --git a/modules/src/core/ics04_channel/handler.rs b/modules/src/core/ics04_channel/handler.rs index 372ae7de3e..cae8ddf6b6 100644 --- a/modules/src/core/ics04_channel/handler.rs +++ b/modules/src/core/ics04_channel/handler.rs @@ -8,7 +8,7 @@ use crate::core::ics04_channel::{msgs::PacketMsg, packet::PacketResult}; use crate::core::ics05_port::capabilities::ChannelCapability; use crate::core::ics24_host::identifier::{ChannelId, PortId}; use crate::core::ics26_routing::context::{Ics26Context, ModuleId, ModuleOutput, Router}; -use crate::handler::HandlerOutput; +use crate::handler::{HandlerOutput, HandlerOutputBuilder}; pub mod acknowledgement; pub mod chan_close_confirm; @@ -108,8 +108,7 @@ pub fn channel_callback( module_id: &ModuleId, msg: &ChannelMsg, result: &mut ChannelResult, - module_output: &mut ModuleOutput, -) -> Result<(), Error> +) -> Result, Error> where Ctx: Ics26Context, { @@ -118,9 +117,8 @@ where .get_route_mut(module_id) .ok_or_else(Error::route_not_found)?; - match msg { + let output = match msg { ChannelMsg::ChannelOpenInit(msg) => cb.on_chan_open_init( - module_output, msg.channel.ordering, &msg.channel.connection_hops, &msg.port_id, @@ -130,8 +128,7 @@ where &msg.channel.version, )?, ChannelMsg::ChannelOpenTry(msg) => { - let version = cb.on_chan_open_try( - module_output, + let output = cb.on_chan_open_try( msg.channel.ordering, &msg.channel.connection_hops, &msg.port_id, @@ -140,25 +137,24 @@ where msg.channel.counterparty(), &msg.counterparty_version, )?; + let (version, output) = destructure_output(output); result.channel_end.version = version; + output + } + ChannelMsg::ChannelOpenAck(msg) => { + cb.on_chan_open_ack(&msg.port_id, &result.channel_id, &msg.counterparty_version)? } - ChannelMsg::ChannelOpenAck(msg) => cb.on_chan_open_ack( - module_output, - &msg.port_id, - &result.channel_id, - &msg.counterparty_version, - )?, ChannelMsg::ChannelOpenConfirm(msg) => { - cb.on_chan_open_confirm(module_output, &msg.port_id, &result.channel_id)? + cb.on_chan_open_confirm(&msg.port_id, &result.channel_id)? } ChannelMsg::ChannelCloseInit(msg) => { - cb.on_chan_close_init(module_output, &msg.port_id, &result.channel_id)? + cb.on_chan_close_init(&msg.port_id, &result.channel_id)? } ChannelMsg::ChannelCloseConfirm(msg) => { - cb.on_chan_close_confirm(module_output, &msg.port_id, &result.channel_id)? + cb.on_chan_close_confirm(&msg.port_id, &result.channel_id)? } - } - Ok(()) + }; + Ok(output) } pub fn packet_validate(ctx: &Ctx, msg: &PacketMsg) -> Result @@ -214,8 +210,7 @@ pub fn packet_callback( ctx: &mut Ctx, module_id: &ModuleId, msg: &PacketMsg, - module_output: &mut ModuleOutput, -) -> Result<(), Error> +) -> Result, Error> where Ctx: Ics26Context, { @@ -224,9 +219,10 @@ where .get_route_mut(module_id) .ok_or_else(Error::route_not_found)?; - match msg { + let output = match msg { PacketMsg::RecvPacket(msg) => { - let (ack, write_fn) = cb.on_recv_packet(module_output, &msg.packet, &msg.signer); + let output = cb.on_recv_packet(&msg.packet, &msg.signer); + let ((ack, write_fn), output) = destructure_output(output); match ack { None => {} Some(ack) => { @@ -239,19 +235,26 @@ where // TODO(hu55a1n1): write ack } } + output } - PacketMsg::AckPacket(msg) => cb.on_acknowledgement_packet( - module_output, - &msg.packet, - &msg.acknowledgement, - &msg.signer, - )?, - PacketMsg::ToPacket(msg) => { - cb.on_timeout_packet(module_output, &msg.packet, &msg.signer)? - } - PacketMsg::ToClosePacket(msg) => { - cb.on_timeout_packet(module_output, &msg.packet, &msg.signer)? + PacketMsg::AckPacket(msg) => { + cb.on_acknowledgement_packet(&msg.packet, &msg.acknowledgement, &msg.signer)? } - } - Ok(()) + PacketMsg::ToPacket(msg) => cb.on_timeout_packet(&msg.packet, &msg.signer)?, + PacketMsg::ToClosePacket(msg) => cb.on_timeout_packet(&msg.packet, &msg.signer)?, + }; + Ok(output) +} + +fn destructure_output(output: ModuleOutput) -> (T, ModuleOutput<()>) { + let HandlerOutput { + result, + log, + events, + } = output; + let output = HandlerOutputBuilder::new() + .with_log(log) + .with_events(events) + .with_result(()); + (result, output) } diff --git a/modules/src/core/ics26_routing/context.rs b/modules/src/core/ics26_routing/context.rs index 8f8563d25e..f20cd331d7 100644 --- a/modules/src/core/ics26_routing/context.rs +++ b/modules/src/core/ics26_routing/context.rs @@ -18,7 +18,7 @@ use crate::core::ics05_port::capabilities::ChannelCapability; use crate::core::ics05_port::context::PortReader; use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId}; use crate::events::IbcEvent; -use crate::handler::HandlerOutput; +use crate::handler::{HandlerOutput, HandlerOutputBuilder}; use crate::signer::Signer; /// This trait captures all the functional dependencies (i.e., context) which the ICS26 module @@ -88,13 +88,12 @@ pub type DeferredWriteResult = (Option>, Option>); // FIXME(hu55a1n1): Define concrete type that implements `Into`? pub type ModuleEvent = IbcEvent; -pub type ModuleOutput = HandlerOutput<(), ModuleEvent>; +pub type ModuleOutput = HandlerOutput; pub trait Module: Debug + Send + Sync + AsAnyMut + 'static { #[allow(clippy::too_many_arguments)] fn on_chan_open_init( &mut self, - _output: &mut ModuleOutput, _order: Order, _connection_hops: &[ConnectionId], _port_id: &PortId, @@ -102,14 +101,13 @@ pub trait Module: Debug + Send + Sync + AsAnyMut + 'static { _channel_cap: &ChannelCapability, _counterparty: &Counterparty, _version: &Version, - ) -> Result<(), Error> { - Ok(()) + ) -> Result, Error> { + Ok(HandlerOutputBuilder::new().with_result(())) } #[allow(clippy::too_many_arguments)] fn on_chan_open_try( &mut self, - _output: &mut ModuleOutput, _order: Order, _connection_hops: &[ConnectionId], _port_id: &PortId, @@ -117,71 +115,64 @@ pub trait Module: Debug + Send + Sync + AsAnyMut + 'static { _channel_cap: &ChannelCapability, _counterparty: &Counterparty, _counterparty_version: &Version, - ) -> Result; + ) -> Result, Error>; fn on_chan_open_ack( &mut self, - _output: &mut ModuleOutput, _port_id: &PortId, _channel_id: &ChannelId, _counterparty_version: &Version, - ) -> Result<(), Error> { - Ok(()) + ) -> Result, Error> { + Ok(HandlerOutputBuilder::new().with_result(())) } fn on_chan_open_confirm( &mut self, - _output: &mut ModuleOutput, _port_id: &PortId, _channel_id: &ChannelId, - ) -> Result<(), Error> { - Ok(()) + ) -> Result, Error> { + Ok(HandlerOutputBuilder::new().with_result(())) } fn on_chan_close_init( &mut self, - _output: &mut ModuleOutput, _port_id: &PortId, _channel_id: &ChannelId, - ) -> Result<(), Error> { - Ok(()) + ) -> Result, Error> { + Ok(HandlerOutputBuilder::new().with_result(())) } fn on_chan_close_confirm( &mut self, - _output: &mut ModuleOutput, _port_id: &PortId, _channel_id: &ChannelId, - ) -> Result<(), Error> { - Ok(()) + ) -> Result, Error> { + Ok(HandlerOutputBuilder::new().with_result(())) } fn on_recv_packet( &self, - _output: &mut ModuleOutput, _packet: &Packet, _relayer: &Signer, - ) -> DeferredWriteResult { - (None, None) + ) -> ModuleOutput> { + HandlerOutputBuilder::new().with_result((None, None)) } fn on_acknowledgement_packet( &mut self, - _output: &mut ModuleOutput, _packet: &Packet, _acknowledgement: &GenericAcknowledgement, _relayer: &Signer, - ) -> Result<(), Error> { - Ok(()) + ) -> Result, Error> { + Ok(HandlerOutputBuilder::new().with_result(())) } fn on_timeout_packet( &mut self, - _output: &mut ModuleOutput, _packet: &Packet, _relayer: &Signer, - ) -> Result<(), Error> { - Ok(()) + ) -> Result, Error> { + Ok(HandlerOutputBuilder::new().with_result(())) } } diff --git a/modules/src/core/ics26_routing/handler.rs b/modules/src/core/ics26_routing/handler.rs index fd8f2bf0e0..5131d76ffc 100644 --- a/modules/src/core/ics26_routing/handler.rs +++ b/modules/src/core/ics26_routing/handler.rs @@ -83,16 +83,9 @@ where let mut handler_output = ics4_msg_dispatcher(ctx, &msg).map_err(Error::ics04_channel)?; - let mut module_output = HandlerOutput::builder().with_result(()); - let cb_result = ics4_callback( - ctx, - &module_id, - &msg, - &mut handler_output.result, - &mut module_output, - ); - handler_output.merge(module_output); - cb_result.map_err(Error::ics04_channel)?; + let cb_result = ics4_callback(ctx, &module_id, &msg, &mut handler_output.result) + .map_err(Error::ics04_channel)?; + handler_output.merge(cb_result); // Apply any results to the host chain store. ctx.store_channel_result(handler_output.result) @@ -133,10 +126,9 @@ where .with_result(())); } - let mut module_output = HandlerOutput::builder().with_result(()); - let cb_result = ics4_packet_callback(ctx, &module_id, &msg, &mut module_output); - handler_output.merge(module_output); - cb_result.map_err(Error::ics04_channel)?; + let cb_result = + ics4_packet_callback(ctx, &module_id, &msg).map_err(Error::ics04_channel)?; + handler_output.merge(cb_result); // Apply any results to the host chain store. ctx.store_packet_result(handler_output.result) diff --git a/modules/src/mock/context.rs b/modules/src/mock/context.rs index c887396c10..50bd8d91a1 100644 --- a/modules/src/mock/context.rs +++ b/modules/src/mock/context.rs @@ -1250,6 +1250,7 @@ mod tests { use crate::core::ics26_routing::context::{ Acknowledgement, DeferredWriteResult, Module, ModuleId, ModuleOutput, Router, RouterBuilder, }; + use crate::handler::HandlerOutputBuilder; use crate::mock::context::MockContext; use crate::mock::context::MockRouterBuilder; use crate::mock::host::HostType; @@ -1422,7 +1423,6 @@ mod tests { impl Module for FooModule { fn on_chan_open_try( &mut self, - _output: &mut ModuleOutput, _order: Order, _connection_hops: &[ConnectionId], _port_id: &PortId, @@ -1430,23 +1430,23 @@ mod tests { _channel_cap: &ChannelCapability, _counterparty: &Counterparty, counterparty_version: &Version, - ) -> Result { - Ok(counterparty_version.clone()) + ) -> Result, Error> { + Ok(HandlerOutputBuilder::new().with_result(counterparty_version.clone())) } fn on_recv_packet( &self, - _output: &mut ModuleOutput, _packet: &Packet, _relayer: &Signer, - ) -> DeferredWriteResult { - ( + ) -> ModuleOutput> { + let result = ( Some(Box::new(MockAck::default())), Some(Box::new(|module| { let module = module.downcast_mut::().unwrap(); module.counter += 1; })), - ) + ); + HandlerOutputBuilder::new().with_result(result) } } @@ -1456,7 +1456,6 @@ mod tests { impl Module for BarModule { fn on_chan_open_try( &mut self, - _output: &mut ModuleOutput, _order: Order, _connection_hops: &[ConnectionId], _port_id: &PortId, @@ -1465,7 +1464,7 @@ mod tests { _counterparty: &Counterparty, counterparty_version: &Version, ) -> Result { - Ok(counterparty_version.clone()) + Ok(HandlerOutputBuilder::new().with_result(counterparty_version.clone())) } } @@ -1487,11 +1486,7 @@ mod tests { let mut on_recv_packet_result = |module_id: &'static str| { let module_id = ModuleId::from_str(module_id).unwrap(); let m = ctx.router.get_route_mut(&module_id).unwrap(); - let result = m.on_recv_packet( - &mut ModuleOutput::builder().with_result(()), - &Packet::default(), - &Signer::new(""), - ); + let result = m.on_recv_packet(&Packet::default(), &Signer::new("")); (module_id, result) };