refactor: more robust tagging index sync as recipient #19030
Merged
+1,019
−11
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Dec 15, 2025
Contributor
Author
This was referenced Dec 15, 2025
benesjan
force-pushed
the
10-09-feat_more_robust_unconstrained_tagging
branch
from
11a7226 to
bcf2da9
Compare
benesjan
force-pushed
the
12-15-refactor_more_robust_tagging_index_sync_as_recipient
branch
2 times, most recently
from
4097236 to
46b912e
Compare
benesjan
force-pushed
the
10-09-feat_more_robust_unconstrained_tagging
branch
2 times, most recently
from
a78d1a8 to
bb588b3
Compare
benesjan
force-pushed
the
12-15-refactor_more_robust_tagging_index_sync_as_recipient
branch
from
b03272a to
6d590c5
Compare
benesjan
changed the base branch from
10-09-feat_more_robust_unconstrained_tagging
to
graphite-base/19030
benesjan
force-pushed
the
12-15-refactor_more_robust_tagging_index_sync_as_recipient
branch
from
6d590c5 to
bfd7013
Compare
benesjan
changed the base branch from
graphite-base/19030
to
10-09-feat_more_robust_unconstrained_tagging
AztecBot
force-pushed
the
10-09-feat_more_robust_unconstrained_tagging
branch
from
e52830e to
eb5f940
Compare
Base automatically changed from
10-09-feat_more_robust_unconstrained_tagging
to
next
December 16, 2025 21:55
benesjan
force-pushed
the
12-15-refactor_more_robust_tagging_index_sync_as_recipient
branch
5 times, most recently
from
71a3b8a to
d8fc670
Compare
benesjan
force-pushed
the
12-15-refactor_more_robust_tagging_index_sync_as_recipient
branch
5 times, most recently
from
370568a to
1b335df
Compare
benesjan
commented
Dec 19, 2025
yarn-project/pxe/src/tagging/recipient_sync/utils/load_logs_for_range.ts
Show resolved
Hide resolved
benesjan
commented
Dec 19, 2025
yarn-project/pxe/src/tagging/recipient_sync/utils/load_logs_for_range.ts
Show resolved
Hide resolved
benesjan
commented
Dec 19, 2025
yarn-project/pxe/src/tagging/recipient_sync/utils/load_logs_for_range.ts
Show resolved
Hide resolved
benesjan
force-pushed
the
12-15-refactor_more_robust_tagging_index_sync_as_recipient
branch
from
1b335df to
fc54bb0
Compare
benesjan
force-pushed
the
12-15-refactor_more_robust_tagging_index_sync_as_recipient
branch
from
fc54bb0 to
32cf7bd
Compare
This was referenced Dec 19, 2025
mverzilli
requested changes
Dec 22, 2025
Contributor
mverzilli
left a comment
mverzilli
left a comment
There was a problem hiding this comment.
Looks awesome, just want to sync particularly regarding data storage
yarn-project/pxe/src/tagging/recipient_sync/utils/load_logs_for_range.ts
Show resolved
Hide resolved
yarn-project/pxe/src/tagging/recipient_sync/utils/load_logs_for_range.ts
Show resolved
Hide resolved
yarn-project/pxe/src/tagging/recipient_sync/utils/load_logs_for_range.ts
Outdated
Show resolved
Hide resolved
yarn-project/pxe/src/tagging/recipient_sync/utils/load_logs_for_range.test.ts
Show resolved
Hide resolved
yarn-project/pxe/src/tagging/recipient_sync/utils/load_logs_for_range.ts
Show resolved
Hide resolved
yarn-project/pxe/src/tagging/recipient_sync/new_recipient_tagging_data_provider.ts
Show resolved
Hide resolved
yarn-project/pxe/src/tagging/recipient_sync/load_private_logs_for_sender_recipient_pair.ts
Outdated
Show resolved
Hide resolved
yarn-project/pxe/src/tagging/recipient_sync/load_private_logs_for_sender_recipient_pair.ts
Show resolved
Hide resolved
yarn-project/pxe/src/tagging/recipient_sync/load_private_logs_for_sender_recipient_pair.ts
Show resolved
Hide resolved
yarn-project/pxe/src/tagging/recipient_sync/load_private_logs_for_sender_recipient_pair.ts
Show resolved
Hide resolved
Fixes #17775 In this PR I implement a new log sync algorithm we've come up with in Buenos Aires that should be fully robust against any kind of private log losses. The algorithm is explained in the body of `loadPrivateLogsForSenderRecipientPair` function. That function should be the entrypoint when reviewing this PR (if checking the utility functions first you would not have enough context). Unfortunately this PR introduces a performance regression that is tracked by [this issue](https://linear.app/aztec-labs/issue/F-229/improve-log-sync-performance). I am fairly certain that the regression is caused by an unreasonable number of requests performed by the `loadLogsForRange` function - that will be tackled in a followup PR. In this PR the algorithm is not yet integrated into the system. That is done in a [PR up the stack](#19125). The directory structure is not yet final - I wanted to keep this PR contained in one place to not have conflicts with Martin's PR. So please ignore that for now (will move stuff around in a final pass). My plan is as follows: 1. Merge this PR, 2. fix the regression by modifying the `getLogsByTag` API such that it gives me all the info I need (now it doesn't give me block timestamp), 3. once that PR is merged most likely wait for Martin's refactor to be merged and then rebase and polish my integrating new log sync PR, 4. move some files around to make it all cuter. ## Update on regression of time in e2e tests Unfortunately realized that this has caused a regression and log sync is now significantly slower. Did 2 runs of the `e2e_l1_with_wall_time` test and on `next` the results are 337 and 334 seconds but with the new code it's 661 and 658 seconds. Will work on dropping that down before merging that "integrating new log sync" PR.
AztecBot
force-pushed
the
12-15-refactor_more_robust_tagging_index_sync_as_recipient
branch
from
6ac2b16 to
262640c
Compare
Collaborator
Flakey Tests🤖 says: This CI run detected 2 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
benesjan
deleted the
12-15-refactor_more_robust_tagging_index_sync_as_recipient
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #17775
In this PR I implement a new log sync algorithm we've come up with in Buenos Aires that should be fully robust against any kind of private log losses. The algorithm is explained in the body of
loadPrivateLogsForSenderRecipientPairfunction. That function should be the entrypoint when reviewing this PR (if checking the utility functions first you would not have enough context).Unfortunately this PR introduces a performance regression that is tracked by this issue. I am fairly certain that the regression is caused by an unreasonable number of requests performed by the
loadLogsForRangefunction - that will be tackled in a followup PR.In this PR the algorithm is not yet integrated into the system. That is done in a PR up the stack.
The directory structure is not yet final - I wanted to keep this PR contained in one place to not have conflicts with Martin's PR. So please ignore that for now (will move stuff around in a final pass).
My plan is as follows:
getLogsByTagAPI such that it gives me all the info I need (now it doesn't give me block timestamp),Update on regression of time in e2e tests
Unfortunately realized that this has caused a regression and log sync is now significantly slower.
Did 2 runs of the
e2e_l1_with_wall_timetest and onnextthe results are 337 and 334 seconds but with the new code it's 661 and 658 seconds.Will work on dropping that down before merging that "integrating new log sync" PR.