Skip to content
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

[Feature]: decode a slice of FieldElement into a list of Token #318

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

haroune-mohammedi
Copy link

@haroune-mohammedi haroune-mohammedi commented Mar 2, 2023

This PR addresses issue #286

We implemented a decode fucntion

fn decode(types: &[ParamType], data: &[FieldElement]) -> Result<Vec<Token>, DecoderError>

Where:

pub enum ParamType {
    FieldElement,
    Array,
    Tuple(usize),
}

#[derive(PartialEq, Eq, Debug)]
pub enum Token {
    FieldElement(FieldElement),
    Array(Vec<FieldElement>),
    Tuple(Vec<FieldElement>),
}

@haroune-mohammedi haroune-mohammedi marked this pull request as ready for review March 3, 2023 09:27
@xJonathanLEI xJonathanLEI self-assigned this Mar 3, 2023
@fracek
Copy link
Contributor

fracek commented Mar 10, 2023

I like where you're going with this PR. My suggestion is to add a new trait for encoding and decoding Starknet types.

pub trait AbiEncode {
  /// Encode type to a token.
  // TODO: should it consume the type or should take a reference?
  fn encode(self) -> Token;
}

pub trait AbiDecode {
  type Error;
  
  fn decode(tokens: impl AsRef<[Token]>) -> Result<Self, Self::Error>;
}

Then it's possible to implement the encoder and decoder for all rust builtin types like numbers (u64, u32), and structs (vectors, tuples, and arrays).

@haroune-mohammedi
Copy link
Author

@xJonathanLEI can you please take a look here and tell us what do you think ?

I added a macro that auto generates a Decode implementation for structs from a slice of Token, check https://github.com/xJonathanLEI/starknet-rs/blob/59f42eaddab7651e94474cd70f8872b27eadebea/examples/decoder_example.rs

@haroune-mohammedi
Copy link
Author

I like where you're going with this PR. My suggestion is to add a new trait for encoding and decoding Starknet types.

pub trait AbiEncode {
  /// Encode type to a token.
  // TODO: should it consume the type or should take a reference?
  fn encode(self) -> Token;
}

pub trait AbiDecode {
  type Error;
  
  fn decode(tokens: impl AsRef<[Token]>) -> Result<Self, Self::Error>;
}

Then it's possible to implement the encoder and decoder for all rust builtin types like numbers (u64, u32), and structs (vectors, tuples, and arrays).

I wen with TryFrom for converting Token to rust types because that's what is used for FieldElement

Now, after I introduced Decode to convert a Vec<Token> to a struct, naming the trait that convert Token to a "scalar" type AbiDecode will be confusing, maybe using TryFrom is the best option after all, what do you guys think ?

@glihm
Copy link
Contributor

glihm commented Sep 27, 2023

@haroune-mohammedi @fracek it could be awesome to have your feedbacks on #475 related to this! As this one may be closed due to the overlap. 👍

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

Successfully merging this pull request may close these issues.

4 participants