Skip to content

chore(breaking): sync transactions follow ups#1357

Merged
Mirko-von-Leipzig merged 3 commits intonextfrom
santiagopittella-sync-transactions-followups
Nov 17, 2025
Merged

chore(breaking): sync transactions follow ups#1357
Mirko-von-Leipzig merged 3 commits intonextfrom
santiagopittella-sync-transactions-followups

Conversation

@SantiagoPittella
Copy link
Collaborator

closes #1258

This PR:

  • Fixes inconsistencies in the naming of the input note nullifiers for transactions. Renames input_notes to nullifiers in the transactions table.
  • Removes N+1 queries to retrieve associated notes.
  • Collects size assumptions for paginated queries in a single file, and set them all to 4MB.
  • Renames in the proto messages: transaction_records -> transactions, transaction_header -> header.

@SantiagoPittella SantiagoPittella force-pushed the santiagopittella-sync-transactions-followups branch from 7360fa5 to a94b6d7 Compare November 11, 2025 17:25
@SantiagoPittella SantiagoPittella requested review from Mirko-von-Leipzig, bobbinth and igamigo and removed request for bobbinth November 11, 2025 17:26
Copy link
Collaborator

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

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

Code looks good; just the question about whether to inline the note retrieval as part of the original database query.


// Nullifiers of the input notes of the transaction.
repeated primitives.Digest input_notes = 4;
repeated primitives.Digest nullifiers = 4;
Copy link
Contributor

@drahnr drahnr Nov 16, 2025

Choose a reason for hiding this comment

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

Thank you ❤️ There's nothing like getting confused by a good old misnomer.

@Mirko-von-Leipzig Mirko-von-Leipzig merged commit 2b00852 into next Nov 17, 2025
6 checks passed
@Mirko-von-Leipzig Mirko-von-Leipzig deleted the santiagopittella-sync-transactions-followups branch November 17, 2025 06:44
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.

SyncTransactions follow-ups

3 participants