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

Implement PrivateSet.pop_notes #4989

Closed
Tracked by #2783
nventuro opened this issue Mar 6, 2024 · 2 comments · Fixed by #7834
Closed
Tracked by #2783

Implement PrivateSet.pop_notes #4989

nventuro opened this issue Mar 6, 2024 · 2 comments · Fixed by #7834
Assignees

Comments

@nventuro
Copy link
Contributor

nventuro commented Mar 6, 2024

Once #4940 is merged we should perform this rename. There's multiple use cases in which we only care about removal and not retrieval, so we get things like:

let notes = set.get_notes(opts);
assert(notes[0].is_some());

which are how we'd make sure we deleted at least one note, though the intent is highly cryptic. #4988 will also help in removing the assert statement, so let _ = set.pop_notes(opts) will mean that at least some notes were deleted.

@github-project-automation github-project-automation bot moved this to Todo in A3 Mar 6, 2024
@LHerskind LHerskind changed the title Rename set.get_notes to pop_notes refactor(Notes): Rename set.get_notes to pop_notes Mar 8, 2024
@LHerskind LHerskind changed the title refactor(Notes): Rename set.get_notes to pop_notes refactor(Notes): Rename set get_notes to pop_notes Mar 9, 2024
@nventuro
Copy link
Contributor Author

With #4940 closed as wont-merge, this is no longer 'just' a rename. We may wish to implement pop_notes and/or pop_note for PrivateSet, but then it'd be separate functionality from get_notes. Perhaps the existence of both functions would help the reader understand that get_notes does not nullify them, and why nullifying is important.

@nventuro nventuro changed the title refactor(Notes): Rename set get_notes to pop_notes Implement PrivateSet.pop_notes Jul 25, 2024
@nventuro
Copy link
Contributor Author

nventuro commented Aug 7, 2024

Once we have #7831, we can add pop_notes to take care of the read + destroy unsafe cycle, also skipping the check inside remove that prevents removal of unread notes (since we can guarantee that we have read it, and we save more hashing and checks that way).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants