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

Add a derive attribute with support for (de)serialize macros on existing complex types #3379

Closed
1 task
Tracked by #4594
Savio-Sou opened this issue Oct 31, 2023 · 8 comments
Closed
1 task
Tracked by #4594
Labels
aztec.nr Helpful for development of Aztec.nr the smart contract framework enhancement New feature or request
Milestone

Comments

@Savio-Sou
Copy link
Collaborator

Savio-Sou commented Oct 31, 2023

Blocked by

Preview Give feedback
  1. 10 of 11

Problem

@rahul-kothari brought up that Noir does not currently have a canonical way of (de)serializing all types (including custom structs), hence users have to write their own custom methods to achieve so.

An example of such custom method in Aztec.nr.

Happy Case

@kevaundray suggested we could add a derive attribute similar to Rust with support of derive macros.

For this GitHub Issue, we can aim at:

  1. Add #[derive(serialize)] and #[derive(deserialize)]
  2. For all complex data types that currently exist natively in Noir (i.e. arrays, tuples, slices, vectors, structs, traits)

P.S. Pardon my lack of knowledge in Rust. If the scope does not make sense, definitely comment below.

Support for e.g. additional derive macros, custom derive macros definable by users can be added later with separate issues.

Alternatives Considered

No response

Additional Context

@kevaundray noted that his might not necessarily be the best solution for the problem if the serialization needs to be duplicated in another language like TypeScript.

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@Savio-Sou Savio-Sou added enhancement New feature or request aztec.nr Helpful for development of Aztec.nr the smart contract framework labels Oct 31, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Oct 31, 2023
@Savio-Sou
Copy link
Collaborator Author

Savio-Sou commented Oct 31, 2023

Assigning P-HIGH as @rahul-kothari suggested this is helpful for both Aztec.nr development and most if not all its users.

Reassigning to P-MEDIUM as this is not strictly a blocker to Aztec.nr development and its users, albeit a very nice to have.

@Savio-Sou Savio-Sou added P-MEDIUM and removed P-HIGH labels Oct 31, 2023
@jfecher
Copy link
Contributor

jfecher commented Oct 31, 2023

I think we should wait until after we have traits for this. We'll likely want the derive macro to be able to derive certain traits like it can in rust, with Serialize and Deserialize being two of those traits.

@Savio-Sou
Copy link
Collaborator Author

Thanks @jfecher, added Traits as a blocker of this Issue.

@Savio-Sou
Copy link
Collaborator Author

Savio-Sou commented Oct 31, 2023

@kevaundray @jfecher would you anticipate this to be considerably more complex to implement versus #2240? (Otherwise seems like this could supersede that.)

@rahul-kothari
Copy link

@jfecher when will Noir start using traits? Aren't they already at least experimental? I remember Kev saying we could use them in sandbox land

@jfecher
Copy link
Contributor

jfecher commented Nov 1, 2023

@rahul-kothari no official time on them yet since they're still experimental. They're in the compiler already for the most part but you'll receive warnings if you try to use them as there are still some missing features and edge cases. Most notably I'm working now to add support for generics to traits which is important since generics are required for even something as simple as equality on arrays.

You may be fine using them now if you're alright with the warnings and limit yourself to only non-generic types on non-generic traits. ETA for generics is... perhaps roughly 1 month's time.

@jfecher
Copy link
Contributor

jfecher commented Nov 1, 2023

@Savio-Sou I suppose adding a derive macro version to derive Serialize/Deserialize would be a bit more complex than writing a built in function to do so. It's just that we know we want a derive macro solution already for traits anyway, so tacking this onto that is essentially little extra work. Compared to the builtin function approach which we may want to remove once we have a trait version of (De)Serialize.

@jfecher
Copy link
Contributor

jfecher commented Aug 5, 2024

We have derive in Noir's stdlib now so it should be possible to define your own deriver function to derive Serialize and Deserialize for any type following the derive_eq and derive_default examples in the stdlib.

We can't implement this deriver function in Noir's stdlib since we don't define Serialize or Deserialize there either - that will be work for the authors of those libraries. Since it should be possible now, I'm closing this issue.

@jfecher jfecher closed this as completed Aug 5, 2024
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aztec.nr Helpful for development of Aztec.nr the smart contract framework enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

3 participants