Skip to content

feat(rust-client): screen NTL notes - #2474

Open
ricomateo wants to merge 9 commits into
ricomateo-concurrent-ntl-chain-syncfrom
ricomateo-screen-ntl-notes
Open

feat(rust-client): screen NTL notes#2474
ricomateo wants to merge 9 commits into
ricomateo-concurrent-ntl-chain-syncfrom
ricomateo-screen-ntl-notes

Conversation

@ricomateo

@ricomateo ricomateo commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Warning

Do not merge before #2453

Motivation
When the client fetches notes from the NTL using its tracked tags, the NTL can return notes that match a tag but are addressed to a different account (since an account's note tag covers only the 14 most significant bits of its ID prefix, so it is not unique to that account). This can lead to storing notes that none of the tracked accounts can consume.

Description
Updates the sync_state so that the notes fetched from the NTL are now screened when their tag matches a tracked account's tag, discarding the ones that none oft he tracked account can consume.

Closes #2448

@ricomateo ricomateo changed the title Ricomateo screen ntl notes feat(rust-client): screen NTL notes Aug 31, 2026
@ricomateo
ricomateo marked this pull request as ready for review September 1, 2026 15:39
client.sync_state().await.unwrap();
let notes = client.get_input_notes(NoteFilter::All).await.unwrap();
assert!(notes.is_empty(), "a note no tracked account can consume must not be stored");
}

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.

we can add a sanity check here, to verify that the note was registered correctly:

assert_eq!(mock_node.read().get_notes(&[tag], NoteTransportCursor::init()).0.len(), 1);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done here 8e562c9

Comment on lines +440 to +456
async fn screen_transport_notes(
&self,
notes: &mut Vec<(Note, Option<BlockNumber>)>,
) -> Result<(), ClientError> {
let account_tags = self.tracked_account_tags().await?;

let notes_to_screen: Vec<Note> = notes
.iter()
.filter(|(note, _)| account_tags.contains(&note.metadata().tag()))
.map(|(note, _)| note.clone())
.collect();
let consumable = self.note_screener().get_batch_consumability(&notes_to_screen).await?;

// Discard the notes whose tag match the tracked accounts but are not consumable.
notes.retain(|(note, _)| {
!account_tags.contains(&note.metadata().tag()) || consumable.contains_key(&note.id())
});

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.

We are not using the block number at all, we can just change the parameter to Vec<Note>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The thing is that the caller function fetch_note_transport_updates requires to have the note binded to the block number, since it uses it as a hint when fetching the notes from the node, so I think is simpler to keep it that way.

Comment thread crates/rust-client/src/note_transport/mod.rs Outdated
/// The screening filter must not discard a note the client can actually consume: same delivery
/// path as the test above, with the note targeting the account that tracks the tag.
#[tokio::test]
async fn note_delivered_by_tag_match_is_kept_when_a_tracked_account_can_consume_it() {

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.

maybe we can just merge the tests into one

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done here 2fdf309

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.

3 participants