Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

feat(ContractClass): add deserialization #1937

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tcoratger
Copy link

@tcoratger tcoratger commented May 30, 2024

Description

This pull request adds deserialization support for the ContractClass struct. The changes include implementing the Deserialize trait for ContractClassV1 and updating the unit tests to verify the correct deserialization of both ContractClassV0 and ContractClassV1.

Implementation of Deserialize for ContractClassV1:

  • The Deserialize trait has been implemented for ContractClassV1 to support deserialization from JSON.
  • The deserialization process involves converting the input into a JSON value, then into a JSON string, and finally using the try_from_json_string method to deserialize into a ContractClassV1 object.

This change is Reviewable

@tcoratger tcoratger changed the title Add deserialization for ContractClass feat(ContractClass): add deserialization May 30, 2024
Comment on lines +169 to +185
impl<'de> Deserialize<'de> for ContractClassV1 {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
// Deserialize into a JSON value
let json_value: serde_json::Value = Deserialize::deserialize(deserializer)?;

// Convert into a JSON string
let json_string = serde_json::to_string(&json_value)
.map_err(|err| DeserializationError::custom(err.to_string()))?;

// Use try_from_json_string to deserialize into ContractClassV1
ContractClassV1::try_from_json_string(&json_string)
.map_err(|err| DeserializationError::custom(err.to_string()))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The true issue here is that cairo_vm::program doesn't implement serde.
So we have to go through all this. It's inefficient.

I opened an issue on the cairo-vm: lambdaclass/cairo-vm#1660

I think you should push this one first, and then update in the blockifier once it is merged

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue looks lambdaclass/cairo-vm#1660 like it'll take a long time to be implemented and merged, how can we not be blocked by it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants