Skip to content

Commit

Permalink
Merge pull request #104 from datachainlab/specify-client-id-externally
Browse files Browse the repository at this point in the history
Allow client ID to be specified externally in `init_client`

Signed-off-by: Jun Kimura <[email protected]>
bluele authored Jun 2, 2024
2 parents a680eab + 162ad24 commit 16fa999
Showing 16 changed files with 168 additions and 77 deletions.
8 changes: 8 additions & 0 deletions enclave-modules/ecall-handler/src/light_client/errors.rs
Original file line number Diff line number Diff line change
@@ -15,6 +15,14 @@ define_error! {
SealedEnclaveKeyNotFound
|_| { "Sealed EnclaveKey not found" },

ClientAlreadyExists
{
client_id: String
}
|e| {
format_args!("client already exists: client_id={}", e.client_id)
},

LightClient
[light_client::Error]
|_| { "LightClient error" },
12 changes: 5 additions & 7 deletions enclave-modules/ecall-handler/src/light_client/init_client.rs
Original file line number Diff line number Diff line change
@@ -21,24 +21,22 @@ pub fn init_client<R: LightClientResolver, S: KVStore, K: Signer>(
let ek = ctx.get_enclave_key();
let res = lc.create_client(ctx, any_client_state.clone(), any_consensus_state.clone())?;
let client_type = lc.client_type();
let client_id = gen_client_id(client_type.clone(), ctx.client_counter()?)?;
let client_id = ClientId::from_str(&input.client_id)?;
client_id.validate(&client_type)?;

if ctx.client_exists(&client_id) {
return Err(Error::client_already_exists(client_id.to_string()));
}
ctx.store_client_type(client_id.clone(), client_type)?;
ctx.store_any_client_state(client_id.clone(), any_client_state)?;
ctx.store_any_consensus_state(client_id.clone(), res.height, any_consensus_state)?;
ctx.increase_client_counter();

let proof = if res.prove {
prove_commitment(ek, input.signer, res.message)?
} else {
CommitmentProof::new_with_no_signature(res.message.to_bytes())
};
Ok(LightClientResponse::InitClient(InitClientResponse {
client_id,
proof,
}))
}

fn gen_client_id(client_type: String, counter: u64) -> Result<ClientId, Error> {
Ok(ClientId::from_str(&format!("{}-{}", client_type, counter))?)
}
26 changes: 17 additions & 9 deletions enclave-modules/ecall-handler/src/light_client/query.rs
Original file line number Diff line number Diff line change
@@ -10,13 +10,21 @@ pub fn query_client<R: LightClientResolver, S: KVStore, K: Signer>(
ctx: &mut Context<R, S, K>,
input: QueryClientInput,
) -> Result<LightClientResponse, Error> {
let lc = get_light_client_by_client_id(ctx, &input.client_id)?;
let any_client_state = ctx.client_state(&input.client_id)?;
let any_consensus_state =
ctx.consensus_state(&input.client_id, &lc.latest_height(ctx, &input.client_id)?)?;

Ok(LightClientResponse::QueryClient(QueryClientResponse {
any_client_state,
any_consensus_state,
}))
if ctx.client_exists(&input.client_id) {
let lc = get_light_client_by_client_id(ctx, &input.client_id)?;
let any_client_state = ctx.client_state(&input.client_id)?;
let any_consensus_state =
ctx.consensus_state(&input.client_id, &lc.latest_height(ctx, &input.client_id)?)?;
Ok(LightClientResponse::QueryClient(QueryClientResponse {
found: true,
any_client_state: Some(any_client_state),
any_consensus_state: Some(any_consensus_state),
}))
} else {
Ok(LightClientResponse::QueryClient(QueryClientResponse {
found: false,
any_client_state: None,
any_consensus_state: None,
}))
}
}
7 changes: 4 additions & 3 deletions modules/ecall-commands/src/light_client.rs
Original file line number Diff line number Diff line change
@@ -41,6 +41,7 @@ impl EnclaveKeySelector for LightClientCommand {

#[derive(Serialize, Deserialize, Debug)]
pub struct InitClientInput {
pub client_id: String,
pub any_client_state: Any,
pub any_consensus_state: Any,
pub current_timestamp: Time,
@@ -105,7 +106,6 @@ pub enum LightClientResponse {

#[derive(Serialize, Deserialize, Debug)]
pub struct InitClientResponse {
pub client_id: ClientId,
pub proof: CommitmentProof,
}

@@ -123,6 +123,7 @@ pub struct VerifyNonMembershipResponse(pub CommitmentProof);

#[derive(Serialize, Deserialize, Debug)]
pub struct QueryClientResponse {
pub any_client_state: Any,
pub any_consensus_state: Any,
pub found: bool,
pub any_client_state: Option<Any>,
pub any_consensus_state: Option<Any>,
}
7 changes: 4 additions & 3 deletions modules/ecall-commands/src/msgs.rs
Original file line number Diff line number Diff line change
@@ -23,6 +23,7 @@ impl TryFrom<MsgCreateClient> for InitClientInput {
.ok_or_else(|| Error::invalid_argument("consensus_state must be non-nil".into()))?
.into();
Ok(Self {
client_id: msg.client_id,
any_client_state,
any_consensus_state,
current_timestamp: Time::now(),
@@ -116,7 +117,6 @@ impl TryFrom<MsgQueryClientRequest> for QueryClientInput {
impl From<InitClientResponse> for MsgCreateClientResponse {
fn from(res: InitClientResponse) -> Self {
Self {
client_id: res.client_id.to_string(),
message: res.proof.message,
signer: res.proof.signer.into(),
signature: res.proof.signature,
@@ -167,8 +167,9 @@ impl From<VerifyNonMembershipResponse> for MsgVerifyNonMembershipResponse {
impl From<QueryClientResponse> for MsgQueryClientResponse {
fn from(res: QueryClientResponse) -> Self {
Self {
client_state: Some(res.any_client_state.into()),
consensus_state: Some(res.any_consensus_state.into()),
found: res.found,
client_state: res.any_client_state.map(Into::into),
consensus_state: res.any_consensus_state.map(Into::into),
}
}
}
3 changes: 2 additions & 1 deletion modules/enclave-api/src/api/proto.rs
Original file line number Diff line number Diff line change
@@ -11,10 +11,11 @@ use store::transaction::CommitStore;

pub trait EnclaveProtoAPI<S: CommitStore>: EnclaveCommandAPI<S> {
fn proto_create_client(&self, msg: MsgCreateClient) -> Result<MsgCreateClientResponse> {
let client_id = msg.client_id.clone();
let res = self.init_client(msg.try_into()?)?;
info!(
"create_client: client_id={} message={{{}}}",
res.client_id,
client_id,
res.proof.message()?
);
Ok(res.into())
32 changes: 7 additions & 25 deletions modules/light-client/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::types::{Any, ClientId, Height, Time};
use crate::{
errors::Error,
path::{ClientConsensusStatePath, ClientStatePath, ClientTypePath, NEXT_CLIENT_SEQUENCE},
path::{ClientConsensusStatePath, ClientStatePath, ClientTypePath},
prelude::*,
};
use store::KVStore;
@@ -12,6 +12,12 @@ pub trait HostContext {
}

pub trait ClientReader: KVStore {
/// Returns `true` if the client exists in the store.
fn client_exists(&self, client_id: &ClientId) -> bool {
self.get(format!("{}", ClientTypePath::new(client_id)).as_bytes())
.is_some()
}

/// Returns the ClientType for the given identifier `client_id`.
fn client_type(&self, client_id: &ClientId) -> Result<String, Error> {
let value = self.get(format!("{}", ClientTypePath::new(client_id)).as_bytes());
@@ -54,19 +60,6 @@ pub trait ClientReader: KVStore {
.0,
)
}

/// Returns a natural number, counting how many clients have been created thus far.
/// The value of this counter should increase only via method `ClientKeeper::increase_client_counter`.
fn client_counter(&self) -> Result<u64, Error> {
match self.get(NEXT_CLIENT_SEQUENCE.as_bytes()) {
Some(bz) => {
let mut b: [u8; 8] = Default::default();
b.copy_from_slice(&bz);
Ok(u64::from_be_bytes(b))
}
None => Ok(0),
}
}
}

pub trait ClientKeeper: ClientReader {
@@ -106,17 +99,6 @@ pub trait ClientKeeper: ClientReader {
self.set(format!("{}", path).into_bytes(), bz);
Ok(())
}

/// 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) {
let next_counter = <Self as ClientReader>::client_counter(self).unwrap() + 1;
self.set(
NEXT_CLIENT_SEQUENCE.as_bytes().to_vec(),
next_counter.to_be_bytes().to_vec(),
);
}
}

pub trait HostClientReader: HostContext + ClientReader {}
6 changes: 1 addition & 5 deletions modules/light-client/src/ibc.rs
Original file line number Diff line number Diff line change
@@ -138,11 +138,7 @@ impl<'a, ClientState: Ics02ClientState, ConsensusState: Ics02ConsensusState> Val
}

fn client_counter(&self) -> Result<u64, ibc::core::ContextError> {
self.parent.client_counter().map_err(|e| {
ContextError::ClientError(ClientError::ClientSpecific {
description: e.to_string(),
})
})
unimplemented!()
}

fn connection_end(
2 changes: 0 additions & 2 deletions modules/light-client/src/path.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use crate::types::{ClientId, Height};
use derive_more::Display;

pub static NEXT_CLIENT_SEQUENCE: &str = "nextClientSequence";

#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
#[display(fmt = "clients/{_0}/clientType")]
pub struct ClientTypePath(pub ClientId);
20 changes: 20 additions & 0 deletions modules/types/src/errors.rs
Original file line number Diff line number Diff line change
@@ -46,6 +46,26 @@ define_error! {
|_| {
"identifier cannot be empty"
},
ClientIdInvalidFormat
{ id: String }
|e| {
format_args!("identifier `{}` must be in the format `{{client_type}}-{{counter}}`", e.id)
},
ClientIdInvalidClientType
{ id: String, client_type: String }
|e| {
format_args!("identifier `{}` must have client type `{}`", e.id, e.client_type)
},
ClientIdInvalidCounter
{ id: String }
|e| {
format_args!("identifier `{}` must have a valid counter", e.id)
},
ClientIdInvalidCounterParseIntError
{ id: String, e: core::num::ParseIntError }
|e| {
format_args!("identifier `{}` counter parse error: {}", e.id, e.e)
},
MrenclaveBytesConversion
{
bz: Vec<u8>,
72 changes: 72 additions & 0 deletions modules/types/src/host.rs
Original file line number Diff line number Diff line change
@@ -29,6 +29,35 @@ impl ClientId {
pub fn as_bytes(&self) -> &[u8] {
self.0.as_bytes()
}

/// Validate the client identifier
pub fn validate(&self, client_type: &str) -> Result<(), TypeError> {
validate_client_identifier(self.0.as_str())?;
self.0.rfind('-').map_or(
Err(TypeError::client_id_invalid_format(self.0.clone())),
|pos| {
let (client_type_, prefixed_counter) = self.0.split_at(pos);
if client_type_ != client_type {
return Err(TypeError::client_id_invalid_client_type(
self.0.clone(),
client_type.to_string(),
));
}
match prefixed_counter.strip_prefix('-') {
None => Err(TypeError::client_id_invalid_counter(self.0.clone())),
Some(counter) if counter.starts_with('0') && counter.len() > 1 => {
Err(TypeError::client_id_invalid_counter(self.0.clone()))
}
Some(counter) => {
counter.parse::<u64>().map_err(|e| {
TypeError::client_id_invalid_counter_parse_int_error(self.0.clone(), e)
})?;
Ok(())
}
}
},
)
}
}

/// This implementation provides a `to_string` method.
@@ -116,3 +145,46 @@ fn validate_identifier(id: &str, min: usize, max: usize) -> Result<(), TypeError
// All good!
Ok(())
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_valid_client_id() {
let cases = vec![
("testclient", "testclient-0"),
("testclient", "testclient-1"),
("testclient", "testclient-10"),
("07-tendermint", "07-tendermint-0"),
("07-tendermint", "07-tendermint-1"),
("07-tendermint", "07-tendermint-10"),
("testclient", "testclient-12345678901234567890"),
];
for (i, (client_type, c)) in cases.iter().enumerate() {
let cl = ClientId::from_str(c).unwrap();
let res = cl.validate(client_type);
assert!(res.is_ok(), "case: {}, error: {:?}", i, res);
}
}

#[test]
fn test_invalid_client_id() {
let cases = vec![
("testclient", "testclient"),
("testclient", "testclient1"),
("07-tendermint", "07-tendermint"),
("07-tendermint", "07-tendermint0"),
("07-tendermint", "07-tendermint1"),
("07-tendermint", "07-tendermint-01"),
("client", "client-0"),
("", ""),
("", "07-tendermint"),
("testclient", "testclient-123456789012345678901"),
];
for (i, (client_type, c)) in cases.iter().enumerate() {
let res = ClientId::from_str(c).and_then(|cl| cl.validate(client_type));
assert!(res.is_err(), "case: {}, error: {:?}", i, res);
}
}
}
6 changes: 4 additions & 2 deletions proto/definitions/lcp/service/elc/v1/query.proto
Original file line number Diff line number Diff line change
@@ -24,9 +24,11 @@ message QueryClientResponse {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

// if false, the client_state and consensus_state fields will be empty
bool found = 1;
// light client state
google.protobuf.Any client_state = 1 [(gogoproto.moretags) = "yaml:\"client_state\""];
google.protobuf.Any client_state = 2 [(gogoproto.moretags) = "yaml:\"client_state\""];
// consensus state associated with the client that corresponds to a given
// height.
google.protobuf.Any consensus_state = 2 [(gogoproto.moretags) = "yaml:\"consensus_state\""];
google.protobuf.Any consensus_state = 3 [(gogoproto.moretags) = "yaml:\"consensus_state\""];
}
14 changes: 7 additions & 7 deletions proto/definitions/lcp/service/elc/v1/tx.proto
Original file line number Diff line number Diff line change
@@ -31,24 +31,24 @@ message MsgCreateClient {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

string client_id = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
// light client state
google.protobuf.Any client_state = 1 [(gogoproto.moretags) = "yaml:\"client_state\""];
google.protobuf.Any client_state = 2 [(gogoproto.moretags) = "yaml:\"client_state\""];
// consensus state associated with the client that corresponds to a given
// height.
google.protobuf.Any consensus_state = 2 [(gogoproto.moretags) = "yaml:\"consensus_state\""];
google.protobuf.Any consensus_state = 3 [(gogoproto.moretags) = "yaml:\"consensus_state\""];
// enclave key for signing
bytes signer = 3;
bytes signer = 4;
}

// MsgCreateClientResponse defines the Msg/CreateClient response type.
message MsgCreateClientResponse {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

string client_id = 1;
bytes message = 2;
bytes signer = 3;
bytes signature = 4;
bytes message = 1;
bytes signer = 2;
bytes signature = 3;
}

// MsgUpdateClient defines an sdk.Msg to update a IBC client state using
Binary file modified proto/src/descriptor.bin
Binary file not shown.
Loading

0 comments on commit 16fa999

Please sign in to comment.