fix(api)!: increase_*_counters methods return Result type

.changelog/unreleased/breaking-change/857-use-result-for-safe-counters-increment.md

@@ -0,0 +1,3 @@
+- Allow hosts to handle overflow cases in `increase_*_counter` methods by
+  returning `Result<(),ContextError>` type.
+  ([#857](https://github.com/cosmos/ibc-rs/issues/857))

crates/ibc/src/core/context.rs

@@ -251,7 +251,7 @@ pub trait ExecutionContext: ValidationContext {
     /// Called upon client creation.
     /// Increases the counter which keeps track of how many clients have been created.
     /// Should never fail.
-    fn increase_client_counter(&mut self);
+    fn increase_client_counter(&mut self) -> Result<(), ContextError>;
 
     /// Called upon successful client update.
     /// Implementations are expected to use this to record the specified time as the time at which
@@ -290,7 +290,7 @@ pub trait ExecutionContext: ValidationContext {
     /// Called upon connection identifier creation (Init or Try process).
     /// Increases the counter which keeps track of how many connections have been created.
     /// Should never fail.
-    fn increase_connection_counter(&mut self);
+    fn increase_connection_counter(&mut self) -> Result<(), ContextError>;
 
     /// Stores the given packet commitment at the given store path
     fn store_packet_commitment(
@@ -353,7 +353,7 @@ pub trait ExecutionContext: ValidationContext {
     /// Called upon channel identifier creation (Init or Try message processing).
     /// Increases the counter which keeps track of how many channels have been created.
     /// Should never fail.
-    fn increase_channel_counter(&mut self);
+    fn increase_channel_counter(&mut self) -> Result<(), ContextError>;
 
     /// Emit the given IBC event
     fn emit_ibc_event(&mut self, event: IbcEvent);

crates/ibc/src/core/ics02_client/error.rs

@@ -103,6 +103,8 @@ pub enum ClientError {
     MisbehaviourHandlingFailure { reason: String },
     /// client specific error: `{description}`
     ClientSpecific { description: String },
+    /// client counter overflow error
+    CounterOverflow,
     /// other error: `{description}`
     Other { description: String },
 }

crates/ibc/src/core/ics02_client/handler/create_client.rs

@@ -81,7 +81,7 @@ where
 
     ctx.store_update_time(client_id.clone(), latest_height, ctx.host_timestamp()?)?;
     ctx.store_update_height(client_id.clone(), latest_height, ctx.host_height()?)?;
-    ctx.increase_client_counter();
+    ctx.increase_client_counter()?;
 
     let event = IbcEvent::CreateClient(CreateClient::new(
         client_id.clone(),
@@ -144,8 +144,12 @@ mod tests {
 
         assert!(res.is_ok(), "execution happy path");
 
+        assert_eq!(ctx.client_counter().unwrap(), 1);
+
         let expected_client_state = ctx.decode_client_state(msg.client_state).unwrap();
+
         assert_eq!(expected_client_state.client_type(), client_type);
+
         assert_eq!(ctx.client_state(&client_id).unwrap(), expected_client_state);
     }
 
@@ -173,13 +177,19 @@ mod tests {
         );
 
         let res = validate(&ctx, msg.clone());
+
         assert!(res.is_ok(), "tendermint client validation happy path");
 
         let res = execute(&mut ctx, msg.clone());
+
         assert!(res.is_ok(), "tendermint client execution happy path");
 
+        assert_eq!(ctx.client_counter().unwrap(), 1);
+
         let expected_client_state = ctx.decode_client_state(msg.client_state).unwrap();
+
         assert_eq!(expected_client_state.client_type(), client_type);
+
         assert_eq!(ctx.client_state(&client_id).unwrap(), expected_client_state);
     }
 }

crates/ibc/src/core/ics03_connection/error.rs

@@ -85,6 +85,8 @@ pub enum ConnectionError {
     },
     /// timestamp overflowed error: `{0}`
     TimestampOverflow(TimestampOverflowError),
+    /// connection counter overflow error
+    CounterOverflow,
     /// other error: `{description}`
     Other { description: String },
 }

crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs

@@ -77,7 +77,7 @@ where
         ctx_a.emit_ibc_event(event);
     }
 
-    ctx_a.increase_connection_counter();
+    ctx_a.increase_connection_counter()?;
     ctx_a.store_connection_to_client(
         &ClientConnectionPath::new(&msg.client_id_on_a),
         conn_id_on_a.clone(),
@@ -161,6 +161,9 @@ mod tests {
             }
             Expect::Success => {
                 assert!(res.is_ok(), "{err_msg}");
+
+                assert_eq!(fxt.ctx.connection_counter().unwrap(), 1);
+
                 assert_eq!(fxt.ctx.events.len(), 2);
 
                 assert!(matches!(

crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs

@@ -160,7 +160,7 @@ where
     ctx_b.emit_ibc_event(event);
     ctx_b.log_message("success: conn_open_try verification passed".to_string());
 
-    ctx_b.increase_connection_counter();
+    ctx_b.increase_connection_counter()?;
     ctx_b.store_connection_to_client(
         &ClientConnectionPath::new(&msg.client_id_on_b),
         vars.conn_id_on_b.clone(),
@@ -294,6 +294,9 @@ mod tests {
             }
             Expect::Success => {
                 assert!(res.is_ok(), "{err_msg}");
+
+                assert_eq!(fxt.ctx.connection_counter().unwrap(), 1);
+
                 assert_eq!(fxt.ctx.events.len(), 2);
 
                 assert!(matches!(

crates/ibc/src/core/ics04_channel/error.rs

@@ -68,14 +68,16 @@ pub enum ChannelError {
     ProcessedHeightNotFound { client_id: ClientId, height: Height },
     /// application module error: `{description}`
     AppModule { description: String },
-    /// other error: `{description}`
-    Other { description: String },
     /// Undefined counterparty connection for `{connection_id}`
     UndefinedConnectionCounterparty { connection_id: ConnectionId },
     /// invalid proof: empty proof
     InvalidProof,
     /// identifier error: `{0}`
     InvalidIdentifier(IdentifierError),
+    /// channel counter overflow error
+    CounterOverflow,
+    /// other error: `{description}`
+    Other { description: String },
 }
 
 #[derive(Debug, Display)]

crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs

@@ -67,7 +67,7 @@ where
         let chan_end_path_on_a = ChannelEndPath::new(&msg.port_id_on_a, &chan_id_on_a);
         ctx_a.store_channel(&chan_end_path_on_a, chan_end_on_a)?;
 
-        ctx_a.increase_channel_counter();
+        ctx_a.increase_channel_counter()?;
 
         // Initialize send, recv, and ack sequence numbers.
         let seq_send_path = SeqSendPath::new(&msg.port_id_on_a, &chan_id_on_a);
@@ -251,7 +251,10 @@ mod tests {
 
         assert!(res.is_ok(), "Execution succeeds; good parameters");
 
+        assert_eq!(ctx.channel_counter().unwrap(), 1);
+
         assert_eq!(ctx.events.len(), 2);
+
         assert!(matches!(
             ctx.events[0],
             IbcEvent::Message(MessageEvent::Channel)

crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs

@@ -75,7 +75,7 @@ where
 
         let chan_end_path_on_b = ChannelEndPath::new(&msg.port_id_on_b, &chan_id_on_b);
         ctx_b.store_channel(&chan_end_path_on_b, chan_end_on_b)?;
-        ctx_b.increase_channel_counter();
+        ctx_b.increase_channel_counter()?;
 
         // Initialize send, recv, and ack sequence numbers.
         let seq_send_path = SeqSendPath::new(&msg.port_id_on_b, &chan_id_on_b);
@@ -337,7 +337,10 @@ mod tests {
 
         assert!(res.is_ok(), "Execution success: happy path");
 
+        assert_eq!(ctx.channel_counter().unwrap(), 1);
+
         assert_eq!(ctx.events.len(), 2);
+
         assert!(matches!(
             ctx.events[0],
             IbcEvent::Message(MessageEvent::Channel)

crates/ibc/src/mock/context.rs

@@ -1090,8 +1090,15 @@ impl ExecutionContext for MockContext {
         self
     }
 
-    fn increase_client_counter(&mut self) {
-        self.ibc_store.lock().client_ids_counter += 1
+    fn increase_client_counter(&mut self) -> Result<(), ContextError> {
+        let mut ibc_store = self.ibc_store.lock();
+
+        ibc_store.client_ids_counter = ibc_store
+            .client_ids_counter
+            .checked_add(1)
+            .ok_or(ClientError::CounterOverflow)?;
+
+        Ok(())
     }
 
     fn store_update_time(
@@ -1148,8 +1155,15 @@ impl ExecutionContext for MockContext {
         Ok(())
     }
 
-    fn increase_connection_counter(&mut self) {
-        self.ibc_store.lock().connection_ids_counter += 1;
+    fn increase_connection_counter(&mut self) -> Result<(), ContextError> {
+        let mut ibc_store = self.ibc_store.lock();
+
+        ibc_store.connection_ids_counter = ibc_store
+            .connection_ids_counter
+            .checked_add(1)
+            .ok_or(ClientError::CounterOverflow)?;
+
+        Ok(())
     }
 
     fn store_packet_commitment(
@@ -1299,8 +1313,15 @@ impl ExecutionContext for MockContext {
         Ok(())
     }
 
-    fn increase_channel_counter(&mut self) {
-        self.ibc_store.lock().channel_ids_counter += 1;
+    fn increase_channel_counter(&mut self) -> Result<(), ContextError> {
+        let mut ibc_store = self.ibc_store.lock();
+
+        ibc_store.channel_ids_counter = ibc_store
+            .channel_ids_counter
+            .checked_add(1)
+            .ok_or(ClientError::CounterOverflow)?;
+
+        Ok(())
     }
 
     fn emit_ibc_event(&mut self, event: IbcEvent) {