fix: use header height for Tendermint consensus state storage

.changelog/unreleased/bug-fixes/1080-use-header-height-for-tm-consensus-state-storage.md

@@ -0,0 +1,2 @@
+- [ibc-client-tendermint] Use header height for Tendermint consensus state storage
+  ([\#1080](https://github.com/cosmos/ibc-rs/issues/1080))

ibc-clients/ics07-tendermint/src/client_state.rs

@@ -383,8 +383,8 @@ where
             ctx.store_consensus_state(
                 ClientConsensusStatePath::new(
                     client_id.clone(),
-                    new_client_state.latest_height.revision_number(),
-                    new_client_state.latest_height.revision_height(),
+                    header_height.revision_number(),
+                    header_height.revision_height(),
                 ),
                 TmConsensusState::from(new_consensus_state).into(),
             )?;

ibc-core/ics02-client/types/src/error.rs

@@ -75,11 +75,6 @@ pub enum ClientError {
     InvalidPacketTimestamp(ibc_primitives::ParseTimestampError),
     /// mismatch between client and arguments types
     ClientArgsTypeMismatch { client_type: ClientType },
-    /// received header height (`{header_height}`) is lower than (or equal to) client latest height (`{latest_height}`)
-    LowHeaderHeight {
-        header_height: Height,
-        latest_height: Height,
-    },
     /// timestamp is invalid or missing, timestamp=`{time1}`,  now=`{time2}`
     InvalidConsensusStateTimestamp { time1: Timestamp, time2: Timestamp },
     /// the local consensus state could not be retrieved for height `{height}`

ibc-core/ics04-channel/types/src/channel.rs

@@ -313,7 +313,7 @@ impl ChannelEnd {
 
 /// Checks if the `connection_hops` has a length of `expected`.
 pub(crate) fn verify_connection_hops_length(
-    connection_hops: &Vec<ConnectionId>,
+    connection_hops: &[ConnectionId],
     expected: u64,
 ) -> Result<(), ChannelError> {
     if connection_hops.len() as u64 != expected {

ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs

@@ -226,14 +226,7 @@ where
     ) -> Result<(), ClientError> {
         match client_message.type_url.as_str() {
             MOCK_HEADER_TYPE_URL => {
-                let header = MockHeader::try_from(client_message)?;
-
-                if self.latest_height() >= header.height() {
-                    return Err(ClientError::LowHeaderHeight {
-                        header_height: header.height(),
-                        latest_height: self.latest_height(),
-                    });
-                }
+                let _header = MockHeader::try_from(client_message)?;
             }
             MOCK_MISBEHAVIOUR_TYPE_URL => {
                 let _misbehaviour = Misbehaviour::try_from(client_message)?;

ibc-testkit/tests/core/ics02_client/update_client.rs

@@ -109,6 +109,81 @@ fn test_update_client_ok(fixture: Fixture) {
     );
 }
 
+#[rstest]
+// Tests successful submission of a header with a height below the latest
+// client's height and ensures that `ConsensusState` is stored at the correct
+// path (header height).
+fn test_update_client_with_prev_header() {
+    let client_id = ClientId::default();
+    let chain_id_b = ChainId::new("mockgaiaA-0").unwrap();
+    let latest_height = Height::new(0, 42).unwrap();
+    let height_1 = Height::new(0, 43).unwrap();
+    let height_2 = Height::new(0, 44).unwrap();
+
+    let mut ctx = MockContext::default().with_client_config(
+        MockClientConfig::builder()
+            .client_chain_id(chain_id_b.clone())
+            .client_type(tm_client_type())
+            .client_id(client_id.clone())
+            .latest_height(latest_height)
+            .build(),
+    );
+    let mut router = MockRouter::new_with_transfer();
+
+    fn build_msg_from_header(
+        chain_id: ChainId,
+        client_id: ClientId,
+        target_height: Height,
+        trusted_height: Height,
+    ) -> MsgEnvelope {
+        let mut tm_block = HostBlock::generate_tm_block(
+            chain_id,
+            target_height.revision_height(),
+            Timestamp::now(),
+        );
+
+        tm_block.trusted_height = trusted_height;
+
+        let msg = MsgUpdateClient {
+            client_id,
+            client_message: TmHeader::from(tm_block).into(),
+            signer: dummy_account_id(),
+        };
+
+        MsgEnvelope::from(ClientMsg::from(msg))
+    }
+
+    let msg_1 = build_msg_from_header(
+        chain_id_b.clone(),
+        client_id.clone(),
+        height_1,
+        latest_height,
+    );
+
+    let msg_2 = build_msg_from_header(chain_id_b, client_id.clone(), height_2, latest_height);
+
+    // First, submit a header with `height_2` to set the client's latest
+    // height to `height_2`.
+    let _ = validate(&ctx, &router, msg_2.clone());
+    let _ = execute(&mut ctx, &mut router, msg_2);
+
+    // Then, submit a header with `height_1` to see if the client's latest
+    // height remains `height_2` and the consensus state is stored at the
+    // correct path (`height_1`).
+    let _ = validate(&ctx, &router, msg_1.clone());
+    let _ = execute(&mut ctx, &mut router, msg_1);
+
+    let client_state = ctx.client_state(&client_id).unwrap();
+    assert_eq!(client_state.latest_height(), height_2);
+
+    let cons_state_path = ClientConsensusStatePath::new(
+        client_id,
+        height_1.revision_number(),
+        height_1.revision_height(),
+    );
+    assert!(ctx.consensus_state(&cons_state_path).is_ok());
+}
+
 /// Tests that the Tendermint client consensus state pruning logic
 /// functions correctly.
 ///

ibc-testkit/tests/core/router.rs

@@ -222,17 +222,6 @@ fn routing_module_and_keepers() {
             want_pass: true,
             state_check: None,
         },
-        Test {
-            name: "Client update fails due to stale header".to_string(),
-            msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient {
-                client_id: client_id.clone(),
-                client_message: MockHeader::new(update_client_height).into(),
-                signer: default_signer.clone(),
-            }))
-            .into(),
-            want_pass: false,
-            state_check: None,
-        },
         Test {
             name: "Connection open init succeeds".to_string(),
             msg: MsgEnvelope::Connection(ConnectionMsg::OpenInit(