Skip to content

Conversation

@benesjan
Copy link
Contributor

@benesjan benesjan commented Dec 18, 2025

Fixes #17775

@benesjan benesjan force-pushed the 12-15-refactor_more_robust_tagging_index_sync_as_recipient branch from d8fc670 to ce35192 Compare December 18, 2025 17:47
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from 9dfb49e to 96a2caa Compare December 18, 2025 17:47
@benesjan benesjan added ci-squash-and-merge ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure and removed ci-squash-and-merge labels Dec 18, 2025
@benesjan benesjan marked this pull request as ready for review December 18, 2025 18:02
@benesjan benesjan marked this pull request as draft December 18, 2025 18:54
@benesjan benesjan force-pushed the 12-15-refactor_more_robust_tagging_index_sync_as_recipient branch from ce35192 to 440c4f6 Compare December 18, 2025 18:55
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from 03ff16f to 4248f60 Compare December 18, 2025 18:55
@benesjan benesjan force-pushed the 12-15-refactor_more_robust_tagging_index_sync_as_recipient branch from 440c4f6 to dba8831 Compare December 18, 2025 19:42
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from 4248f60 to 68eecc3 Compare December 18, 2025 19:42
@benesjan benesjan force-pushed the 12-15-refactor_more_robust_tagging_index_sync_as_recipient branch from dba8831 to bbe743a Compare December 18, 2025 20:57
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch 3 times, most recently from 1d52d40 to 2644665 Compare December 18, 2025 22:04
@benesjan benesjan marked this pull request as ready for review December 18, 2025 22:17
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from 2644665 to 7df5f00 Compare December 19, 2025 14:18
@benesjan benesjan force-pushed the 12-15-refactor_more_robust_tagging_index_sync_as_recipient branch from 370568a to 1b335df Compare December 19, 2025 14:18
@benesjan benesjan changed the base branch from 12-15-refactor_more_robust_tagging_index_sync_as_recipient to graphite-base/19125 December 19, 2025 16:58
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from 7df5f00 to d45dade Compare December 19, 2025 16:58
@benesjan benesjan force-pushed the graphite-base/19125 branch from 1b335df to fb60766 Compare December 19, 2025 16:58
@benesjan benesjan changed the base branch from graphite-base/19125 to 12-19-feat_optimizing_aztecnode_getlogsbytags_for_new_log_sync_algo December 19, 2025 16:58
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from 54ff988 to 9c04f8f Compare December 19, 2025 19:52
@benesjan benesjan force-pushed the 12-19-refactor_dropping_pagination_from_getlogsbytags branch from 7d71804 to 822c8e6 Compare December 19, 2025 22:01
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch 2 times, most recently from 011c79a to bee9f9d Compare December 19, 2025 22:31
@benesjan benesjan force-pushed the 12-19-refactor_dropping_pagination_from_getlogsbytags branch from 822c8e6 to 88998e0 Compare December 19, 2025 22:31
@benesjan benesjan marked this pull request as ready for review December 19, 2025 22:35
AztecBot pushed a commit that referenced this pull request Dec 22, 2025
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.
github-merge-queue bot pushed a commit that referenced this pull request Dec 22, 2025
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.
@benesjan benesjan force-pushed the 12-19-refactor_dropping_pagination_from_getlogsbytags branch from 88998e0 to 99f51a4 Compare December 23, 2025 16:36
@benesjan benesjan changed the base branch from 12-19-refactor_dropping_pagination_from_getlogsbytags to graphite-base/19125 December 23, 2025 16:54
@benesjan benesjan marked this pull request as draft December 23, 2025 16:56
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from 33ab122 to fe0a7f9 Compare December 23, 2025 17:52
@benesjan benesjan force-pushed the graphite-base/19125 branch from 88998e0 to 1497662 Compare December 23, 2025 17:52
@benesjan benesjan changed the base branch from graphite-base/19125 to 12-19-refactor_dropping_pagination_from_getlogsbytags December 23, 2025 17:52
@AztecBot AztecBot force-pushed the 12-19-refactor_dropping_pagination_from_getlogsbytags branch from 1497662 to 7afc547 Compare December 23, 2025 18:10
Base automatically changed from 12-19-refactor_dropping_pagination_from_getlogsbytags to next December 23, 2025 18:42
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from fe0a7f9 to 2beecab Compare December 23, 2025 19:00
@benesjan benesjan marked this pull request as ready for review December 23, 2025 19:20
let sendersDataProvider: SendersDataProvider;
let logService: LogService;

describe('sync tagged logs', () => {
Copy link
Contributor Author

@benesjan benesjan Dec 23, 2025

Choose a reason for hiding this comment

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

These were the old algorithm tests. The new algo tests are in yarn-project/pxe/src/tagging/recipient_sync/ directory.

I didn't modify the bulkRetrieveLogs tests here - I just needed to update its test setup because it was messed up a bit and it relied on the test setup of the "sync tagged logs" tests.

@benesjan benesjan marked this pull request as draft December 23, 2025 21:35
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from 47bce0f to 6e01080 Compare December 23, 2025 21:42
@benesjan benesjan force-pushed the 12-18-feat_integrating_new_log_sync branch from 6a3088c to 7912feb Compare December 24, 2025 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement robust recipient log sync algorithm

2 participants