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

Note object refactoring #645

Closed
bobbinth opened this issue Apr 30, 2024 · 3 comments · Fixed by #664
Closed

Note object refactoring #645

bobbinth opened this issue Apr 30, 2024 · 3 comments · Fixed by #664
Assignees
Labels
refactoring Code clean-ups, improvements, and refactoring
Milestone

Comments

@bobbinth
Copy link
Contributor

This is a subset to #464 that is limited to the Note and a few related objects. One of the objects of this is to provide an "intuitive" way to think about note objects but also to separate core note contents from note metadata. The objects involved in this refactoring are Note and NoteEnvelope which currently look like so:

pub struct Note {
    assets: NoteAssets,
    metadata: NoteMetadata,
    recipient: NoteRecipient,

    id: NoteId,
    nullifier: Nullifier,
}

pub struct NoteEnvelope {
    note_id: NoteId,
    note_metadata: NoteMetadata,
}

I am thinking of refactoring of renaming NoteEnvelope into NoteHeader and refactoring these as follows:

pub struct NoteHeader {
    id: NoteId,
    metadata: NoteMetadata,
}

pub struct NoteDetails {
    assets: NoteAssets,
    recipient: NoteRecipient,
}

pub struct Note {
    header: NoteHeader,
    details: NoteDetails,

    nullifier: Nullifier,
}

This has a couple of benefits:

  1. It roughly follows the structure the Block struct of having the objects consist of header and contents.
  2. I think NoteHeader is a bit more intuitive name as compared to NoteEnvelope.
  3. It more closely matches how things work on the node and the client (e.g., sometimes we manipulate note details separately from note metadata).
  4. It also should match more closely some of the protobuf messages we use in the node.

Once implemented, this should unblock #412.

@hackaugusto, @polydez, @igamigo - would appreciate any feedback on this.

@bobbinth bobbinth added the refactoring Code clean-ups, improvements, and refactoring label Apr 30, 2024
@hackaugusto
Copy link
Contributor

related: #350

@hackaugusto
Copy link
Contributor

I left my comment on #350, but basically the idea would be to have a structure for each hash operation.

@bobbinth bobbinth added this to the v0.3 milestone May 4, 2024
@bobbinth bobbinth self-assigned this May 6, 2024
@bobbinth bobbinth linked a pull request May 6, 2024 that will close this issue
@bobbinth
Copy link
Contributor Author

bobbinth commented May 6, 2024

Closed by #664.

@bobbinth bobbinth closed this as completed May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code clean-ups, improvements, and refactoring
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants