starknet-rs

starknet-rs copied to clipboard

This PR aims at bringing support for ByteArray type.

I think I've JSON rpc related errors, if you have the opportunity to manually trigger the workflow it could be great @xJonathanLEI. Thanks a lot!

I'm thinking that we should introduce a Bytes31 type to ensure that the data is actually with the correct format. WDYT @xJonathanLEI?

EDIT: useless question sorry, will implement it in the PR. :+1:

@xJonathanLEI here's the updated version of the PR. Few months of blank here, sorry for that.

I've added the Bytes31 type to be consistent with Cairo.

Happy to address any comment!

@xJonathanLEI rebased on master, happy to dive in any update as necessary.

Hey @glihm thanks for the PR. I see you're modelling the types here directly after their Cairo implementations, with the internals exposed.

It kinda makes me wonder though: do we really expect users to care about what a particular array's data, pending_word, and pending_word_len really are?

I'm not exactly convinced that it's the case. The exact composition should only matter when crossing the Rust <-> Cairo boundary (i.e. encoding/decoding), which should be mostly opaque to users. Users most likely just want to deal with bytes.

So IMO a better design would be a fully encapsulated ByteArray type that's internally backed by Vec<u8>. We could provide getters for viewing the components like data, pending_word, and pending_word_len, but I also don't think that's actually needed.

Then we should have this type implement Encode and Decode to make crossing the Cairo boundary seamless.

Superseded by #682.

No way! I was just looking for a ByteArray implementation and turns out it was implemented 5 days ago! Man, we really are on the bleeding edge haha 😅. Thanks for the new feature guys!