feat!: PXE.getNotes(...) + refactor of note types

[benesjan]

Fixes #2908
Fixes #2909
Fixes #2910
Fixes #3061

Note 1: This PR became huge because it made sense to refactor PXE.addNotes here as well since I now use the ExtendedNote type there.
Note 2: Since this PR is already too big I decided to finish renaming preimage as serialized_note in Noir in a separate PR.
Note 3: Note processor now stores a CompleteAddress instead of just a public key because I need the address from CompleteAddress to construct the ExtendedNote.
Note 4: This is a breaking change because of the PXE.getNotes change and the PXE.getPrivateStorageAt removal.

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

• If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
• I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
• Every change is related to the PR description.
• I have linked this pull request to relevant issues (if any exist).

[AztecBot]

Benchmark results

Metrics with a significant change:

• note_trial_decrypting_time_in_ms (8): 15.0 (-85%)
• note_trial_decrypting_time_in_ms (32): 11.0 (-81%)

Detailed results

All benchmarks are run on txs on the Benchmarking contract on the repository. Each tx consists of a batch call to create_note and increment_balance, which guarantees that each tx has a private call, a nested private call, a public call, and a nested public call, as well as an emitted private note, an unencrypted log, and public storage read and write.

This benchmark source data is available in JSON format on S3 here.

Values are compared against data from master at commit a6786ae6 and shown if the difference exceeds 1%.

L2 block published to L1

Each column represents the number of txs on an L2 block published to L1.

Metric	8 txs	32 txs	128 txs
l1_rollup_calldata_size_in_bytes	45,444	179,588	716,132
l1_rollup_calldata_gas	222,960	868,244	3,448,172
l1_rollup_execution_gas	842,047	3,595,352	22,203,541
l2_block_processing_time_in_ms	2,038 (+3%)	7,646 (+2%)	30,059 (+1%)
note_successful_decrypting_time_in_ms	313 (+6%)	902 (+6%)	3,310 (+3%)
note_trial_decrypting_time_in_ms	⚠️ 15.0 (-85%)	⚠️ 11.0 (-81%)	133 (-1%)
l2_block_building_time_in_ms	13,375 (+2%)	52,656 (+1%)	211,580 (+1%)
l2_block_rollup_simulation_time_in_ms	12,039 (+2%)	47,384 (+1%)	190,072 (+1%)
l2_block_public_tx_process_time_in_ms	1,297 (+2%)	5,142 (+1%)	20,999 (+2%)

L2 chain processing

Each column represents the number of blocks on the L2 chain where each block has 16 txs.

Metric	5 blocks	10 blocks
node_history_sync_time_in_ms	22,007 (+3%)	42,968 (+3%)
note_history_successful_decrypting_time_in_ms	2,053 (+4%)	4,027 (+4%)
note_history_trial_decrypting_time_in_ms	120 (-1%)	143 (+1%)
node_database_size_in_bytes	1,627,773	1,097,815
pxe_database_size_in_bytes	29,748 (+9%)	59,307 (+9%)

Circuits stats

Stats on running time and I/O sizes collected for every circuit run across all benchmarks.

Circuit	circuit_simulation_time_in_ms	circuit_input_size_in_bytes	circuit_output_size_in_bytes
private-kernel-init	73.0 (+1%)	61,697	18,841
private-kernel-ordering	44.0 (+2%)	24,233	8,089
base-rollup	1,782 (+1%)	656,309	811
root-rollup	78.9 (+1%)	4,072	1,097
private-kernel-inner	54.0 (+2%)	81,504	18,841
public-kernel-private-input	42.2 (+2%)	41,455	18,841
public-kernel-non-first-iteration	27.2 (+3%)	41,497	18,841
merge-rollup	0.949 (+7%)	2,592	873

Miscellaneous

Transaction sizes based on how many contracts are deployed in the tx.

Metric	0 deployed contracts	1 deployed contracts
tx_size_in_bytes	8,723	27,240

[spalladino]

LGTM! Left a few comments, but nothing to block merging. There's one discussion around naming notes in the receipt worth expanding though.

[spalladino]

Now that I see it, I'm worried this getNotes could end up being confusing to users. From its name, it seems like it returns all notes created in the tx, whereas it's returning all notes visible by the pxe in the tx. So if I make a token transfer to a recipient, the note to the recipient will only be visible if the recipient happens to be in the same pxe (which will usually be the case in tests but not in apps).

But I don't really have a suggestion on how to fix this. Maybe we should scope the notes returned by this method to be only the ones owned by the sender wallet? And rename this method to getNotesToSelf (or something nicer)?

We could also take a step back and rethink: what's the use case for this method? What are we trying to simplify by exposing it?

[benesjan]

You have a point. One option would be renaming that to getVisibleNotes but what I don't like about the name is that it's not consistent with that we have in Noir.

Maybe the best approach would be to teach devs that PXE always works just with info of registered accoutns.

[spalladino]

Agree in that the name is not very good, but I wouldn't dismiss this just on the basis of "teaching devs" since it's an easy mistake to do. At the very least we should clarify it on the jsdoc.

[benesjan]

Addressed in #3137

[spalladino]

Again, this is slightly misleading, since these are only the notes created in this tx that are visible by the current pxe.

[benesjan]

You are right. WDYT about calling it visibleNotes and documenting it properly? I think that should be sufficient because I would say the term "visible" will spark curiosity in devs.