Skip to content

chore: address followup comments for node update & re-export NetworkNotePricer - #2475

Open
juan518munoz wants to merge 7 commits into
nextfrom
jmunoz-node-update-followup
Open

chore: address followup comments for node update & re-export NetworkNotePricer#2475
juan518munoz wants to merge 7 commits into
nextfrom
jmunoz-node-update-followup

Conversation

@juan518munoz

@juan518munoz juan518munoz commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Closes #2473

  1. CHANGELOG fix: The five entries that pointed at issue Update node to 0.16 #2360/chore: update node to 0.16 #2431 now cite PR chore: update node to 0.16 #2431. The GetAccount behavior entry was dropped per your earlier decision.
  2. Pass SyncedNote to observer: NoteObserver::observe now takes &SyncedNote.
  3. Flatten SyncedNote: flattened to note_id, metadata, inclusion_proof, details, attachments.
  4. "Is the first check covered by the second?": the validation now verifies set equality: every requested key must appear in the response, and the response must not contain unrequested keys.
  5. Attachment parsing hard to follow: simplified using the ReportedAttachment enum and ReportedAttachments::from_reports.
  6. Reintroduce ResolvedNoteContent: superseded by the flatten of point 3.
  7. Hardcode insecure key: the runtime keygen in scripts/start-test-node.sh was replaced by three hardcoded insecure test-only keys.

Closes #2517

  1. Re-export NetworkNotePricer

@juan518munoz
juan518munoz force-pushed the jmunoz-node-update-followup branch from 19dfecf to 7910877 Compare September 1, 2026 13:03
@juan518munoz
juan518munoz marked this pull request as ready for review September 1, 2026 15:29
@juan518munoz juan518munoz changed the title chore: address followup comments for node 0.16 update chore: address followup comments for node update Sep 8, 2026
@juan518munoz juan518munoz changed the title chore: address followup comments for node update chore: address followup comments for node update & re-export NetworkNotePricer Sep 8, 2026

@igamigo igamigo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but I think there might be a regression here. In any case I think this warrants a bigger refactor (#2118) so maybe we fix the issue, merge this anyway and improve the whole setup afterward. I believe there are a couple of structs we could get rid of.

Comment thread CHANGELOG.md

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There does not seem to be any entry here for PR 2475

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added entries in 20dc251

Comment on lines +1222 to +1224
// The screener and the note tracker work on the sync record alone, with attachments
// supplied separately, so hand them the record without its resolved content.
let committed = CommittedNote::new(note_id, metadata, inclusion_proof);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this effectively removes the attachments which means the user does not have that information during the callback

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The screener's record is now built with SyncedNote::to_committed_note() instead of CommittedNote::new(), so it carries the resolved attachments (an empty set for a note without any) rather than None.

20dc251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Re-export NetworkNotePricer update node to 0.16 - pending followups

2 participants