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

Provide an implementation of a note with a single field from aztec.nr #2948

Closed
spalladino opened this issue Oct 20, 2023 · 10 comments · Fixed by #3037
Closed

Provide an implementation of a note with a single field from aztec.nr #2948

spalladino opened this issue Oct 20, 2023 · 10 comments · Fixed by #3037
Assignees
Labels
C-aztec.nr Component: Aztec smart contract framework T-feedback Type: recording user feedback

Comments

@spalladino
Copy link
Collaborator

Storing a single field value in a private note in a singleton requires the user to write the following boilerplate code. Until we have something to autogenerate this code (noir macros or code generators), we should provide an implementation for it from aztec.nr, similar to how we do with the value note.

use dep::aztec::note::{
    note_header::NoteHeader,
    note_interface::NoteInterface,
    utils::compute_note_hash_for_read_or_nullify,
};
use dep::aztec::oracle::{
    get_secret_key::get_secret_key,
    get_public_key::get_public_key,
};
use dep::aztec::types::address::AztecAddress;

struct FieldNote {
    value: Field,
    header: NoteHeader,
}

global FIELD_NOTE_LEN: Field = 1;

impl FieldNote {
    fn new(_value: Field) -> Self {
        FieldNote {
            value: _value,
            header: NoteHeader::empty(),
        }
    }

    fn serialize(self) -> [Field; FIELD_NOTE_LEN] {
        [self.value]
    }

    fn deserialize(preimage: [Field; FIELD_NOTE_LEN]) -> Self {
        FieldNote {
            value: preimage[0],
            header: NoteHeader::empty(),
        }
    }

    fn compute_note_hash(self) -> Field {
        dep::std::hash::pedersen([
            self.value
        ])[0]
    }

    fn compute_nullifier(self) -> Field {
         0
    }

    fn set_header(&mut self, header: NoteHeader) {
        self.header = header;
    }
}

fn deserialize(preimage: [Field; FIELD_NOTE_LEN]) -> FieldNote {
    FieldNote::deserialize(preimage)
}

fn serialize(note: FieldNote) -> [Field; FIELD_NOTE_LEN] {
    note.serialize()
}

fn compute_note_hash(note: FieldNote) -> Field {
    note.compute_note_hash()
}

fn compute_nullifier(note: FieldNote) -> Field {
    note.compute_nullifier()
}

fn get_header(note: FieldNote) -> NoteHeader {
    note.header
}

fn set_header(note: &mut FieldNote, header: NoteHeader) {
    note.set_header(header)
}

global FieldNoteMethods = NoteInterface {
    deserialize,
    serialize,
    compute_note_hash,
    compute_nullifier,
    get_header,
    set_header,
};
@spalladino spalladino added C-aztec.nr Component: Aztec smart contract framework T-feedback Type: recording user feedback labels Oct 20, 2023
@critesjosh
Copy link
Contributor

If the contents of this note are intended to be private, it should also include some randomness, correct?

@spalladino
Copy link
Collaborator Author

spalladino commented Oct 20, 2023

Isn't the randomness injected as part of the NoteHeader, which is shared across all note types? cc @LeilaWang

@critesjosh
Copy link
Contributor

If so than we can close this one #2921

@rahul-kothari
Copy link
Contributor

@spalladino
Copy link
Collaborator Author

Well, damn. Seems like this is an easy footgun. How can we alert users if they've forgotten to include randomness generation in their notes..?

@rahul-kothari
Copy link
Contributor

this came up a couple of times. The consensus from the team was we need to do minimal behind-the-scenes magic for devs so they know exactly what is happening and learn how to handle privacy leakage

@rahul-kothari
Copy link
Contributor

Because I never said it explicitly, I believe this should have randomness added to it! cc @benesjan since looks like Jan will pick it up :)

@benesjan
Copy link
Contributor

If we add randomness to the note then the only difference between a FieldNote and a ValueNote is the owner struct member. Given that now the notes have to have a broadcast method I effectively would need to store the owner as well to be able to fetch the pub key in broadcast. This would make a FieldNote and a ValueNote equivalent. If that's the case then there is nothing to do and we can probably close this.

@spalladino Or am I missing something here? Why the grantees didn't use ValueNote?

@spalladino
Copy link
Collaborator Author

@benesjan because the note doesn't have an owner. They are using it to store a constant value, set at construction time, that is immutable and needs to be accessed from private state. The note is not broadcasted, but manually added to the pxe by the dapp.

This means that most likely they would not need randomness for the note, unless they wanted to keep it private across a set of users, in which case they could distribute that randomness from a gated frontend. More info on their scenario here and here.

@benesjan
Copy link
Contributor

benesjan commented Oct 25, 2023

@spalladino That makes sense. Thank you for the context. I will just put assert false in the broadcast method.

@benesjan benesjan mentioned this issue Oct 25, 2023
4 tasks
@benesjan benesjan moved this from Todo to In Progress in A3 Oct 26, 2023
benesjan added a commit that referenced this issue Oct 31, 2023
1. Fixes #2948
2. Removed redundant dependency from `TokenContract`.
3. No camel case in `TestContract`
@github-project-automation github-project-automation bot moved this from In Progress to Done in A3 Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-aztec.nr Component: Aztec smart contract framework T-feedback Type: recording user feedback
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants