Remove panics from IBC packet handling code

penumbra-zone · Mar 4, 2024 · 9e22cf2 · 9e22cf2
1 parent 66e6c7e
commit 9e22cf2
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 11 deletions.
diff --git a/crates/core/component/ibc/src/component/app_handler.rs b/crates/core/component/ibc/src/component/app_handler.rs
@@ -53,8 +53,8 @@ pub trait AppHandlerExecute: Send + Sync {
     async fn chan_close_confirm_execute<S: StateWrite>(state: S, msg: &MsgChannelCloseConfirm);
     async fn chan_close_init_execute<S: StateWrite>(state: S, msg: &MsgChannelCloseInit);
 
-    async fn recv_packet_execute<S: StateWrite>(state: S, msg: &MsgRecvPacket);
-    async fn timeout_packet_execute<S: StateWrite>(state: S, msg: &MsgTimeout);
+    async fn recv_packet_execute<S: StateWrite>(state: S, msg: &MsgRecvPacket) -> Result<()>;
+    async fn timeout_packet_execute<S: StateWrite>(state: S, msg: &MsgTimeout) -> Result<()>;
     async fn acknowledge_packet_execute<S: StateWrite>(state: S, msg: &MsgAcknowledgement);
 }
 

diff --git a/crates/core/component/ibc/src/component/client.rs b/crates/core/component/ibc/src/component/client.rs
@@ -472,8 +472,12 @@ mod tests {
         ) {
         }
         async fn chan_close_init_execute<S: StateWrite>(_state: S, _msg: &MsgChannelCloseInit) {}
-        async fn recv_packet_execute<S: StateWrite>(_state: S, _msg: &MsgRecvPacket) {}
-        async fn timeout_packet_execute<S: StateWrite>(_state: S, _msg: &MsgTimeout) {}
+        async fn recv_packet_execute<S: StateWrite>(_state: S, _msg: &MsgRecvPacket) -> Result<()> {
+            Ok(())
+        }
+        async fn timeout_packet_execute<S: StateWrite>(_state: S, _msg: &MsgTimeout) -> Result<()> {
+            Ok(())
+        }
         async fn acknowledge_packet_execute<S: StateWrite>(_state: S, _msg: &MsgAcknowledgement) {}
     }
 

diff --git a/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs b/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs
@@ -152,7 +152,7 @@ impl MsgHandler for MsgRecvPacket {
         let transfer = PortId::transfer();
         // todo: should this be part of the app handler logic?
         if self.packet.port_on_b == transfer {
-            AH::recv_packet_execute(state, self).await;
+            AH::recv_packet_execute(state, self).await?;
         } else {
             anyhow::bail!("invalid port id");
         }

diff --git a/crates/core/component/ibc/src/component/msg_handler/timeout.rs b/crates/core/component/ibc/src/component/msg_handler/timeout.rs
@@ -143,7 +143,7 @@ impl MsgHandler for MsgTimeout {
 
         let transfer = PortId::transfer();
         if self.packet.port_on_b == transfer {
-            H::timeout_packet_execute(state, self).await;
+            H::timeout_packet_execute(state, self).await?;
         } else {
             anyhow::bail!("invalid port id");
         }

diff --git a/crates/core/component/shielded-pool/src/component/transfer.rs b/crates/core/component/shielded-pool/src/component/transfer.rs
@@ -518,7 +518,7 @@ impl AppHandlerExecute for Ics20Transfer {
     async fn chan_open_confirm_execute<S: StateWrite>(_state: S, _msg: &MsgChannelOpenConfirm) {}
     async fn chan_close_confirm_execute<S: StateWrite>(_state: S, _msg: &MsgChannelCloseConfirm) {}
     async fn chan_close_init_execute<S: StateWrite>(_state: S, _msg: &MsgChannelCloseInit) {}
-    async fn recv_packet_execute<S: StateWrite>(mut state: S, msg: &MsgRecvPacket) {
+    async fn recv_packet_execute<S: StateWrite>(mut state: S, msg: &MsgRecvPacket) -> Result<()> {
         // recv packet should never fail a transaction, but it should record a failure acknowledgement.
         let ack: Vec<u8> = match recv_transfer_packet_inner(&mut state, msg).await {
             Ok(_) => {
@@ -535,14 +535,18 @@ impl AppHandlerExecute for Ics20Transfer {
         state
             .write_acknowledgement(&msg.packet, &ack)
             .await
-            .expect("able to write acknowledgement");
+            .context("able to write acknowledgement")?;
+
+        Ok(())
     }
 
-    async fn timeout_packet_execute<S: StateWrite>(mut state: S, msg: &MsgTimeout) {
-        // timeouts should never fail
+    async fn timeout_packet_execute<S: StateWrite>(mut state: S, msg: &MsgTimeout) -> Result<()> {
+        // timeouts may fail due to counterparty chains sending transfers of u128-1
         timeout_packet_inner(&mut state, msg)
             .await
-            .expect("able to timeout packet");
+            .context("able to timeout packet")?;
+
+        Ok(())
     }
 
     async fn acknowledge_packet_execute<S: StateWrite>(_state: S, _msg: &MsgAcknowledgement) {}