fix: deriving serde on more public structs#17380
Merged
Conversation
This was referenced Sep 30, 2025
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
benesjan
commented
Sep 30, 2025
| @@ -1,14 +0,0 @@ | |||
| use crate::{hash::compute_secret_hash, oracle::random::random}; | |||
|
|
|||
| pub struct SecretAndHash { | |||
benesjan
commented
Sep 30, 2025
| } | ||
|
|
||
| #[test] | ||
| fn serialization_of_empty() { |
Contributor
Author
There was a problem hiding this comment.
Dropped this test as now, that serde is derived, it seems out of place.
benesjan
commented
Sep 30, 2025
|
|
||
| // TODO: Replace this with the derived version. Not done yet as this uncovered a bug in the AVM. See the discussion | ||
| // below for more details: | ||
| // https://aztecprotocol.slack.com/archives/C02M7VC7TN0/p1754463522334449 |
Contributor
Author
There was a problem hiding this comment.
CI passed so this is no longer an issue. What a nice surprise.
nventuro
approved these changes
Sep 30, 2025
Contributor
nventuro
left a comment
nventuro
left a comment
There was a problem hiding this comment.
Thanks! How did you find these and check there are no remnants?
AztecBot
force-pushed
the
09-26-refactor_dropping_getnotes_from_testwallet
branch
2 times, most recently
from
c18f5d1 to
b4fea5d
Compare
benesjan
force-pushed
the
09-30-fix_deriving_serde_on_more_public_structs
branch
from
aa83f67 to
e226e02
Compare
benesjan
force-pushed
the
09-26-refactor_dropping_getnotes_from_testwallet
branch
from
b4fea5d to
a185f09
Compare
benesjan
force-pushed
the
09-30-fix_deriving_serde_on_more_public_structs
branch
from
e226e02 to
ceb46ab
Compare
AztecBot
force-pushed
the
09-26-refactor_dropping_getnotes_from_testwallet
branch
from
5467aab to
079f99d
Compare
Contributor
Author
@nventuro I just looked through the occurrences of "pub struct" string in noir-projects/aztec-nr and then guessed if it makes sense to ever return from a function. I didn't apply it to a lot of those structs as I couldn't foresee those to ever be returned (e.g. state variables). |
Base automatically changed from
09-26-refactor_dropping_getnotes_from_testwallet
to
next
September 30, 2025 14:50
benesjan
force-pushed
the
09-30-fix_deriving_serde_on_more_public_structs
branch
from
ceb46ab to
ee8ab33
Compare
In a PR down the stack I needed to slap `#[derive(Deserialize)]` on `RetrievedNote` because I returned it from a function. Nico proposed that we derive serde on more public structs because external won't be able to do so. In this PR I went through the public structs in Aztec.nr and applied serde where I thought it could make sense to return from a contract function - in most of the cases it didn't make sense.
AztecBot
force-pushed
the
09-30-fix_deriving_serde_on_more_public_structs
branch
from
ee8ab33 to
4369e5e
Compare
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 30, 2025
In a PR down the stack I needed to slap `#[derive(Deserialize)]` on `RetrievedNote` because I returned it from a function. Nico proposed that we derive serde on more public structs because external won't be able to do so. In this PR I went through the public structs in Aztec.nr and applied serde where I thought it could make sense to return from a contract function - in most of the cases it didn't make sense.
auto-merge was automatically disabled
September 30, 2025 18:19
Pull Request is not mergeable
benesjan
deleted the
09-30-fix_deriving_serde_on_more_public_structs
branch
ludamad
pushed a commit
that referenced
this pull request
Dec 16, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In a PR down the stack I needed to slap
#[derive(Deserialize)]onRetrievedNotebecause I returned it from a function. Nico proposed that we derive serde on more public structs because external won't be able to do so.In this PR I went through the public structs in Aztec.nr and applied serde where I thought it could make sense to return from a contract function - in most of the cases it didn't make sense.