-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add ChainId to Starknet and Comet client state #173
Add ChainId to Starknet and Comet client state #173
Conversation
b8dcf58
to
ae71a7b
Compare
ae71a7b
to
546bd98
Compare
31af839
to
9904ef4
Compare
@@ -190,6 +190,7 @@ pub trait CanUseStarknetLightClientEncoding: | |||
+ CanConvertBothWays<Any, StarknetHeader> | |||
+ CanEncodeAndDecodeMut<ViaProtobuf, Timestamp> | |||
+ CanEncodeAndDecodeMut<ViaProtobuf, CommitmentRoot> | |||
// + CanEncodeAndDecodeMut<ViaProtobuf, ChainId> |
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.
Uncommenting this line fails our build.
Specifically, CanEncodeAndDecodeMut<ViaProtobuf, ChainId>
doesn't satisfy.
pub struct EncodeChainId; | ||
|
||
impl<Encoding, Strategy> MutEncoder<Encoding, Strategy, ChainId> for EncodeChainId | ||
where | ||
Encoding: CanEncodeMut<Strategy, String>, | ||
{ | ||
fn encode_mut( | ||
encoding: &Encoding, | ||
chain_id: &ChainId, | ||
buffer: &mut Encoding::EncodeBuffer, | ||
) -> Result<(), Encoding::Error> { | ||
let chain_id_str = chain_id.as_str().to_string(); | ||
encoding.encode_mut(&chain_id_str, buffer)?; | ||
Ok(()) | ||
} | ||
} | ||
|
||
impl<Encoding, Strategy> MutDecoder<Encoding, Strategy, ChainId> for EncodeChainId | ||
where | ||
Encoding: CanDecodeMut<Strategy, String> + CanRaiseError<&'static str>, | ||
{ | ||
fn decode_mut<'a>( | ||
encoding: &Encoding, | ||
buffer: &mut Encoding::DecodeBuffer<'a>, | ||
) -> Result<ChainId, Encoding::Error> { | ||
let chain_id_str = encoding.decode_mut(buffer)?; | ||
ChainId::new(&chain_id_str).map_err(|_| Encoding::raise_error("invalid chain 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.
Is this correct to satisfy CanEncodeAndDecodeMut<ViaProtobuf, ChainId>
?
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.
I have explained this to @ljoss17. This encoding strategy works for Cairo, but not for protobuf. This is because protobuf requires special field tag to be applied to field values, and "primitive" values like strings can only be field values, meaning they always require a tag.
The solution is to define a EncodeChainIdField<const TAG: u32>
encoder, which makes use of EncodeStringField<TAG>
to encode the given protobuf field.
// EncodeField< | ||
// symbol!("chain_id"), | ||
// EncodeLengthDelimitedProtoField<2, UseContext>, | ||
// >, |
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.
The commented out parts in this file are done the same way as EncodeStarknetConsensusState
and EncodeStarknetHeader
. Not sure why this doesn't work.
@ljoss17 will move the |
|
@@ -61,6 +61,7 @@ where | |||
|
|||
let new_client_state = ClientStateType { | |||
latest_height: header.height, | |||
chain_id: "dummy".parse()?, // TODO: fill with correct info |
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.
I think we need to query the current client state here, and then use the chain ID from the original client state.
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.
Done in 279deb4
Closes: #147
Status
chain_id
field toCometClientState
in relayer codechain_id
field toCometClientState
in Cairo codechain_id
field toStarknetClientState
in relayer code