-
Notifications
You must be signed in to change notification settings - Fork 3.6k
feat(lightclient): add CommitClientStatus message to persist client s… #4859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Someone is attempting to deploy a commit to the unionbuild Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work! this also needs to be implemented in the solidity ibc stack for feature parity.
lib/ibc-union-spec/src/path.rs
Outdated
|
||
impl IbcStorePathKey for ClientStatusPath { | ||
type Spec = IbcUnion; | ||
type Value = u8; // repr(u8) status stored as raw byte |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should move the status enum into this crate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Status
here
Active, | ||
Expired, | ||
Frozen, | ||
Active = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be non-zero (i.e. starting at 1) such that it can be reliably differentiated from an empty storage value
#[serde(deny_unknown_fields)] | ||
pub struct MsgCommitClientStatus { | ||
pub client_id: ClientId, | ||
pub height: Option<u64>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this field is unused?
also please verify your commits as per our contributing guide. |
55a5626
to
8e70297
Compare
@@ -2171,6 +2194,10 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> Result<Binary, ContractErr | |||
)?; | |||
Ok(to_json_binary(&status)?) | |||
} | |||
QueryMsg::GetCommittedStatus { client_id, .. } => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the field is unused, remove it from the msg
lib/ibc-union-spec/Cargo.toml
Outdated
@@ -28,7 +28,7 @@ voyager-primitives = { workspace = true } | |||
ibc-union-spec = { workspace = true, features = ["ethabi", "schemars", "serde"] } | |||
|
|||
[features] | |||
default = [] | |||
default = ["serde"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove, do not add default features
lib/ibc-union-spec/src/path.rs
Outdated
Frozen = 3, | ||
} | ||
|
||
/// Represents the path to a client's committed status (Active, Expired, etc.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Represents the path to a client's committed status (Active, Expired, etc.) | |
/// Represents the path to a client's committed [`Status`]. |
lib/ibc-union-spec/src/path.rs
Outdated
|
||
impl IbcStorePathKey for ClientStatusPath { | ||
type Spec = IbcUnion; | ||
type Value = u8; // repr(u8) status stored as raw byte |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Status
here
) -> RpcResult<Option<u8>> { | ||
let status = self | ||
.query_smart::<_, u8>( | ||
&ibc_union_msg::query::QueryMsg::GetStatus { client_id }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the wrong query; you need to use GetCommittedStatus
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