feat(lightclient): add CommitClientStatus message to persist client s… #4859

0xumutkk · 2025-07-19T14:16:41Z

Adds a new 'CommitClientStatus' message to explicitly commit the client's status (Active, Expired, or Frozen) into state storage as a hashed commitment using 'store_commit'.

Closes #4842

vercel · 2025-07-19T14:16:47Z

Someone is attempting to deploy a commit to the unionbuild Team on Vercel.

A member of the Team first needs to authorize it.

benluelo

nice work! this also needs to be implemented in the solidity ibc stack for feature parity.

lib/ibc-union-spec/src/path.rs

cosmwasm/ibc-union/core/msg/src/lightclient.rs

cosmwasm/ibc-union/core/msg/src/msg.rs

benluelo · 2025-07-20T10:09:33Z

also please verify your commits as per our contributing guide.

cosmwasm/ibc-union/core/src/contract.rs

lib/ibc-union-spec/Cargo.toml

lib/ibc-union-spec/src/path.rs

benluelo · 2025-07-21T09:37:27Z

voyager/modules/state/ethereum/src/main.rs

+    ) -> RpcResult<Option<u8>> {
+        let status = self
+            .query_smart::<_, u8>(
+                &ibc_union_msg::query::QueryMsg::GetStatus { client_id },


this is the wrong query; you need to use GetCommittedStatus

aeryz

Let's do the rename in the following PR.

cosmwasm/ibc-union/core/msg/src/query.rs

lib/ibc-union-spec/src/path.rs

benluelo · 2025-07-24T08:58:24Z

cosmwasm/ibc-union/core/src/contract.rs

+                    client_id: msg.client_id,
+                }
+                .key(),
+                &commit(status_bytes),


store the bytes directly, as a U256 value

benluelo · 2025-07-24T08:59:34Z

voyager/modules/state/ethereum/src/main.rs

+        &self,
+        height: Height,
+        client_id: ClientId,
+    ) -> RpcResult<Option<u8>> {


this should return Option<Status>

benluelo · 2025-07-24T09:00:06Z

voyager/modules/state/cosmos-sdk-union/src/main.rs

+        &self,
+        height: Height,
+        client_id: ClientId,
+    ) -> RpcResult<Option<u8>> {


Option<Status>

cosmwasm/ibc-union/lightclient/berachain/src/client.rs

cosmwasm/ibc-union/lightclient/base/src/client.rs

cosmwasm/ibc-union/lightclient/base/Cargo.toml

cosmwasm/ibc-union/lightclient/arbitrum/src/client.rs

cosmwasm/ibc-union/lightclient/arbitrum/Cargo.toml

benluelo · 2025-07-24T09:07:58Z

voyager/modules/state/cosmos-sdk-union/src/main.rs

+            %client_id
+        )
+    )]
+    async fn query_client_status(


rename this to query_committed_client_status (in the evm impl as well)

benluelo

please remove your merge commit and rebase off of main

benluelo · 2025-07-28T08:31:20Z

cosmwasm/ibc-union/core/src/contract.rs

+                    client_id: msg.client_id,
+                }
+                .key(),
+                &commit(U256::from(status as u32).to_le_bytes()),


don't store the commit, but the u256 value directly

benluelo · 2025-07-28T08:34:30Z

cosmwasm/ibc-union/core/msg/src/query.rs

    GetStatus { client_id: ClientId },
+    #[cfg_attr(feature = "cw-orch-interface", returns(Option<H256>))]


return status here

benluelo · 2025-08-03T13:17:50Z

cosmwasm/ibc-union/core/src/contract.rs

+                },
+            )?;
+            let encoded = U256::from(status as u32);
+            deps.storage


this needs to be written to the commitment store, not the regular storage. it needs to be committed as the un-hashed value. use store_commit.

benluelo · 2025-08-03T13:18:24Z

cosmwasm/ibc-union/core/src/contract.rs

@@ -2186,6 +2200,13 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> Result<Binary, ContractErr
            )?;
            Ok(to_json_binary(&status)?)
        }
+        QueryMsg::GetCommittedStatus { client_id } => {
+            let val = deps.storage.read::<ClientStatuses>(&client_id)?;


same comment as above, use read_commit here.

benluelo · 2025-08-03T13:18:44Z

cosmwasm/ibc-union/core/src/state.rs

+pub enum ClientStatuses {}
+impl Store for ClientStatuses {
+    const PREFIX: Prefix = Prefix::new(b"client_status");
+
+    type Key = ClientId;
+    type Value = U256;
+}
+id_key!(ClientStatuses);
+impl ValueCodec<U256> for ClientStatuses {
+    fn encode_value(value: &U256) -> Bytes {
+        value.to_le_bytes().into()
+    }
+
+    fn decode_value(raw: &Bytes) -> StdResult<U256> {
+        read_fixed_bytes(raw).map(U256::from_le_bytes)
+    }
+}
+


remove this store

benluelo · 2025-08-03T13:20:11Z

lib/ibc-union-spec/src/types.rs

+impl TryFrom<u8> for Status {
+    type Error = &'static str;
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            1 => Ok(Status::Active),
+            2 => Ok(Status::Expired),
+            3 => Ok(Status::Frozen),
+            _ => Err("Invalid status value"),
+        }
+    }
+}


use UnknownEnumVariant<u8> here

benluelo · 2025-08-03T13:20:27Z

lib/ibc-union-spec/src/types.rs

@@ -111,3 +112,16 @@ impl fmt::Display for Status {
        })
    }
 }
+
+impl TryFrom<u8> for Status {


also add a TryFrom<U256> impl

benluelo · 2025-08-03T13:20:52Z

cosmwasm/ibc-union/core/src/lib.rs

+        "{} invalid committed status value: {value}",
+        ContractErrorKind::from(self)
+    )]
+    InvalidClientStatusValue { value: u32 },


why use u32?

benluelo · 2025-08-03T13:22:02Z

cosmwasm/ibc-union/core/src/contract.rs

+            let val = deps.storage.read::<ClientStatuses>(&client_id)?;
+            let raw = val.to_le_bytes()[0];
+            let status = Status::try_from(raw)
+                .map_err(|_| ContractError::InvalidClientStatusValue { value: raw as u32 })?;


don't arbitrarily cast to an integer width. you need to consider all possible values this may take. additionally, since we control this error type, we don't need to cast to a specific integer width - we can just change the field type (as i mentioned in another comment).

benluelo · 2025-08-06T13:39:28Z

lib/ibc-union-spec/src/types.rs

+    type Error = UnknownEnumVariant;
+
+    fn try_from(value: U256) -> Result<Self, Self::Error> {
+        let byte = value.to_le_bytes()[0];


this drops the majority of the u256. implement this correctly

benluelo · 2025-08-06T13:43:44Z

lib/ibc-union-spec/src/types.rs

+#[derive(Debug, Clone, PartialEq)]
+pub struct UnknownEnumVariant {
+    pub value: u8,
+}


don't redefine this, reuse it from unionlabs

benluelo reviewed Jul 20, 2025

View reviewed changes

lib/ibc-union-spec/src/path.rs Outdated Show resolved Hide resolved

cosmwasm/ibc-union/core/msg/src/lightclient.rs Outdated Show resolved Hide resolved

benluelo reviewed Jul 20, 2025

View reviewed changes

cosmwasm/ibc-union/core/msg/src/msg.rs Outdated Show resolved Hide resolved

0xumutkk force-pushed the feat/lightclient-commit-status branch from 55a5626 to 8e70297 Compare July 21, 2025 07:52

benluelo requested changes Jul 21, 2025

View reviewed changes

0xumutkk mentioned this pull request Jul 23, 2025

Implement committed client status query in the solidity ibc stack for feature parity #4888

Open

aeryz previously approved these changes Jul 24, 2025

View reviewed changes

cosmwasm/ibc-union/core/msg/src/query.rs Show resolved Hide resolved

benluelo requested changes Jul 24, 2025

View reviewed changes

0xumutkk dismissed aeryz’s stale review via 4cac861 July 25, 2025 08:45

benluelo requested changes Jul 28, 2025

View reviewed changes

feat: add committed status query/store for lightclients

b3cb34c

0xumutkk force-pushed the feat/lightclient-commit-status branch from 5641ae7 to b3cb34c Compare July 31, 2025 08:43

benluelo requested changes Aug 3, 2025

View reviewed changes

feat: store client status in commitment mapping

dc6a8d9

0xumutkk force-pushed the feat/lightclient-commit-status branch from cff7db0 to dc6a8d9 Compare August 5, 2025 16:23

0xumutkk requested a review from benluelo August 6, 2025 09:25

benluelo requested changes Aug 6, 2025

View reviewed changes

cor closed this Aug 6, 2025

unionlabs locked and limited conversation to collaborators Aug 6, 2025

		GetStatus { client_id: ClientId },
		#[cfg_attr(feature = "cw-orch-interface", returns(Option<H256>))]

feat(lightclient): add CommitClientStatus message to persist client s… #4859

feat(lightclient): add CommitClientStatus message to persist client s… #4859

Uh oh!

Conversation

0xumutkk commented Jul 19, 2025

Uh oh!

vercel bot commented Jul 19, 2025

Uh oh!

benluelo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benluelo commented Jul 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeryz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benluelo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!