refactor: staged writes in tagging stores #19476

mverzilli · 2026-01-09T21:45:55Z

Third part of the series started with #19445.

This makes the stores related to tagging synchronization work based on staged writes.

nventuro

It looks like the capsule changes got here. Can you fix the branch?

benesjan

Most of it are nits and would approve if it were not for the sender_tagging_store test. Those seems to be quite messy.

All the naming suggestions are up for a debate. Just felt like plenty of it could be cuter.

BTW really like the whole staging approach you came up with. It's very elegant. Great job!

yarn-project/pxe/src/tagging/sender_sync/utils/load_and_store_new_tagging_indexes.ts

yarn-project/pxe/src/tagging/sender_sync/sync_sender_tagging_indexes.ts

yarn-project/pxe/src/storage/tagging_store/recipient_tagging_store.ts

yarn-project/pxe/src/storage/tagging_store/sender_tagging_store.test.ts

mverzilli · 2026-01-13T10:08:46Z

It looks like the capsule changes got here. Can you fix the branch?

Probably a glitch while the capsule changes were squashed by CI, should be fine now

AztecBot · 2026-01-13T10:35:01Z

Flakey Tests

🤖 says: This CI run detected 3 tests that failed, but were tolerated due to a .test_patterns.yml entry.

\033FLAKED\033 (8;;http://ci.aztec-labs.com/2b3e965790613a8f�2b3e965790613a8f8;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_epochs/epochs_invalidate_block.parallel.test.ts "committee member invalidates a block if proposer does not come through" (95s) (code: 1) group:e2e-p2p-epoch-flakes (\033Martin Verzilli\033: refactor: staged writes in tagging stores (#19476))
\033FLAKED\033 (8;;http://ci.aztec-labs.com/4e90f968a13b7d76�4e90f968a13b7d768;;�): yarn-project/end-to-end/scripts/run_test.sh web3signer src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts (36s) (code: 1) (\033Martin Verzilli\033: refactor: staged writes in tagging stores (#19476))
\033FLAKED\033 (8;;http://ci.aztec-labs.com/3b53493300c95615�3b53493300c956158;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_p2p/gossip_network.test.ts (441s) (code: 1) group:e2e-p2p-epoch-flakes (\033Martin Verzilli\033: refactor: staged writes in tagging stores (#19476))

benesjan

It's all very cute now. Thanks

I think we might just be missing a few test cases but that's not a sufficient reason to block this so LGTM

benesjan · 2026-01-13T16:27:38Z

yarn-project/pxe/src/storage/tagging_store/recipient_tagging_store.test.ts

+      // Updating again with a higher value in the same job should work
+      // (if staged data wasn't cleared, it would still have the old value cached)


We are not using the same job below though so this seems out of place?

benesjan · 2026-01-13T16:29:21Z

yarn-project/pxe/src/storage/tagging_store/recipient_tagging_store.test.ts

+      expect(await taggingStore.getHighestAgedIndex(secret1, 'job2')).toBe(10);
+      await taggingStore.commit('job2');
+
+      expect(await taggingStore.getHighestAgedIndex(secret1, 'job1')).toBe(10);


Suggested change

expect(await taggingStore.getHighestAgedIndex(secret1, 'job1')).toBe(10);

// Now we verify that we don't receive the data originally staged in job1

expect(await taggingStore.getHighestAgedIndex(secret1, 'job1')).toBe(10);

Took me a bit to figure out how exactly this is testing the staged gets cleared.

benesjan · 2026-01-13T16:31:32Z

yarn-project/pxe/src/storage/tagging_store/recipient_tagging_store.test.ts

+      expect(await taggingStore.getHighestFinalizedIndex(secret2, 'job2')).toBe(6);
+    });
+
+    it('clears staged data after commit', async () => {


I know it's annoying but given that aged and finalized indexes are stored completely separately we should have this test case (and some of the following) duplicated for both type of indexes.

benesjan · 2026-01-13T16:32:55Z

yarn-project/pxe/src/storage/tagging_store/recipient_tagging_store.test.ts

+      expect(await taggingStore.getHighestAgedIndex(secret1, 'job1')).toBe(10);
+    });
+
+    it('does not affect other jobs when committing', async () => {


This one also should have the finalized index counterpart.

benesjan · 2026-01-13T17:00:39Z

yarn-project/pxe/src/storage/tagging_store/sender_tagging_store.test.ts

+    });
+  });
+
+  describe('staged writes', () => {


We have 3 write operations on the public interface of the store:

storePendingIndexes

dropPendingIndexes

finalizePendingIndexes

I think for each of these we want to test that:

they are isolated between jobs,

discarding works as expected,

committing works as expected

So we seem to miss some test cases here. Or would you say it's unnecessery?

I mean we use the same private functions under the hood (e.g. #writePendingIndexes) so IDK it might be unnecessary.

Maybe the private functions should be on a separate class such that the higher level functionality could access only the functions that already handle the staging etc.?

That way we could test the staging directly and we would not deduce it works correctly from a higher level behavior.

But anyway think it's fine to merge this and do that in a followup PR if you agree.

Discussed in Slack, to sum up:

we might end up refactoring and sharing some code between staged stores, but don't want to rush those abstractions

I'll merge this as is so I can propagate the latest changes in next to the branches that are downstream of this (in particular I want to get the NoteStore in soon because it's by far the trickiest). Then I'll do another round with specifics from code review, and I'll include adding the tests you mention here

Third part of the series started with #19445. This makes the stores related to tagging synchronization work based on staged writes.

mverzilli added the ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure label Jan 9, 2026

mverzilli changed the base branch from next to martin/capsule-store-with-staged-writes January 9, 2026 21:46

mverzilli changed the title ~~refactor: staged writes in tagging stores (wip)~~ refactor: staged writes in tagging stores Jan 9, 2026

mverzilli requested review from Thunkar, benesjan and nventuro January 9, 2026 23:08

mverzilli mentioned this pull request Jan 12, 2026

refactor: crash tolerant PXE #19293

Closed

AztecBot force-pushed the martin/capsule-store-with-staged-writes branch from 1de3908 to b30c796 Compare January 12, 2026 18:35

Base automatically changed from martin/capsule-store-with-staged-writes to next January 12, 2026 19:18

nventuro reviewed Jan 12, 2026

View reviewed changes

benesjan reviewed Jan 12, 2026

View reviewed changes

mverzilli requested review from benesjan and nventuro January 13, 2026 12:19

benesjan approved these changes Jan 13, 2026

View reviewed changes

mverzilli enabled auto-merge January 14, 2026 08:46

mverzilli added ci-squash-and-merge and removed ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure labels Jan 14, 2026

refactor: staged writes in tagging stores

9619e78

Third part of the series started with #19445. This makes the stores related to tagging synchronization work based on staged writes.

AztecBot removed the ci-squash-and-merge label Jan 14, 2026

AztecBot force-pushed the martin/staged-writes-in-tagging-stores branch from e3c729d to 9619e78 Compare January 14, 2026 09:05

mverzilli added this pull request to the merge queue Jan 14, 2026

Merged via the queue into next with commit d41cfbf Jan 14, 2026
16 checks passed

mverzilli deleted the martin/staged-writes-in-tagging-stores branch January 14, 2026 09:39

		// Updating again with a higher value in the same job should work
		// (if staged data wasn't cleared, it would still have the old value cached)

	expect(await taggingStore.getHighestAgedIndex(secret1, 'job1')).toBe(10);
	// Now we verify that we don't receive the data originally staged in job1
	expect(await taggingStore.getHighestAgedIndex(secret1, 'job1')).toBe(10);

refactor: staged writes in tagging stores #19476

refactor: staged writes in tagging stores #19476

Uh oh!

Conversation

mverzilli commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nventuro left a comment

Choose a reason for hiding this comment

Uh oh!

benesjan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mverzilli commented Jan 13, 2026

Uh oh!

AztecBot commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Flakey Tests

Uh oh!

benesjan left a comment

Choose a reason for hiding this comment

Uh oh!

benesjan Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

benesjan Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

benesjan Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

benesjan Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

benesjan Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

mverzilli Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mverzilli commented Jan 9, 2026 •

edited

Loading

AztecBot commented Jan 13, 2026 •

edited

Loading