Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fixing boxes wip #4183

Closed
wants to merge 19 commits into from
Closed

fix: fixing boxes wip #4183

wants to merge 19 commits into from

Conversation

signorecello
Copy link
Contributor

@signorecello signorecello commented Jan 23, 2024

Refactoring boxes.

This PR does the following:

  • Fixes the frontend side of boxes, which was broken and not tested in CI (only jest tests were tested in CI).
  • Fixes the README which was quite outdated
  • It also refactors the boxes to be little bit easier to work with.

This is prob not the last contribution to boxes. We will probably:

  • Add a frontend test to run on CI so we can catch when the frontend breaks too.
  • Remove A LOT of frontend code which was nice but not really necessary for a minimal, barebones boilerplate, such as form validation and other features.

Summary

  • Move artifacts and tests outside of the src folder.
  • Makes the README much more straightforward. Less.
  • Makes package.json scripts follow a pattern where dev will run webpack in dev mode (with hot reload, etc), build runs webpack in prod, and serve serves it.
  • For some reason, there were two scripts setting up the PXE. Unified them in one and provided a helper class instead.

Closes AztecProtocol/dev-rel#134

@AztecBot
Copy link
Collaborator

AztecBot commented Jan 23, 2024

Benchmark results

Metrics with a significant change:

  • note_history_trial_decrypting_time_in_ms (5): 156 (+90%)
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 92fb2b09 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,412 179,524 (-1%) 716,068 (-1%)
l1_rollup_calldata_gas 221,632 866,884 3,447,928
l1_rollup_execution_gas 314,119 984,624 3,667,585
l2_block_processing_time_in_ms 1,159 4,362 17,271
note_successful_decrypting_time_in_ms 304 973 3,616 (-2%)
note_trial_decrypting_time_in_ms 13.3 (-8%) 106 142 (-3%)
l2_block_building_time_in_ms 18,173 (+1%) 72,038 (+2%) 288,899 (+1%)
l2_block_rollup_simulation_time_in_ms 13,043 51,644 207,838
l2_block_public_tx_process_time_in_ms 5,102 (+4%) 20,245 (+6%) 80,821 (+6%)

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 13,823 (+1%) 26,359
note_history_successful_decrypting_time_in_ms 2,573 (+3%) 4,692 (+3%)
note_history_trial_decrypting_time_in_ms ⚠️ 156 (+90%) 254 (+20%)
node_database_size_in_bytes 17,350,736 32,989,264
pxe_database_size_in_bytes 29,923 59,478

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 309 44,481 26,193
private-kernel-ordering 193 (-3%) 44,513 14,929 (-10%)
base-rollup 1,404 128,970 881
root-rollup 82.4 4,088 677
private-kernel-inner 411 (-8%) 71,640 26,193
public-kernel-private-input 242 (-2%) 32,615 26,193
public-kernel-non-first-iteration 242 (-2%) 32,657 26,193
merge-rollup 7.63 2,608 881

Tree insertion stats

The duration to insert a fixed batch of leaves into each tree type.

Metric 1 leaves 2 leaves 8 leaves 16 leaves 32 leaves 128 leaves 64 leaves 512 leaves 1024 leaves 2048 leaves 8192 leaves
batch_insert_into_append_only_tree_16_depth_ms 10.2 11.3 (+1%) 12.7 (-1%) 16.7 (-3%) 22.9 (+1%) 63.9 (-2%) N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_16_depth_hash_count 16.9 17.5 23.0 31.6 47.0 143 N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_16_depth_hash_ms 0.593 0.630 (+1%) 0.540 (-1%) 0.518 (-3%) 0.479 (+1%) 0.441 (-1%) N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_32_depth_ms N/A N/A N/A N/A N/A 75.2 (+1%) 47.6 240 461 895 (-1%) 3,557
batch_insert_into_append_only_tree_32_depth_hash_count N/A N/A N/A N/A N/A 159 96.0 543 1,055 2,079 8,223
batch_insert_into_append_only_tree_32_depth_hash_ms N/A N/A N/A N/A N/A 0.465 (+1%) 0.488 0.437 0.431 0.427 (-1%) 0.427
batch_insert_into_indexed_tree_20_depth_ms N/A N/A N/A N/A N/A 103 56.7 349 (-1%) 688 1,365 5,417
batch_insert_into_indexed_tree_20_depth_hash_count N/A N/A N/A N/A N/A 197 104 691 1,363 2,707 10,771
batch_insert_into_indexed_tree_20_depth_hash_ms N/A N/A N/A N/A N/A 0.491 0.496 0.476 0.472 0.474 0.472
batch_insert_into_indexed_tree_40_depth_ms N/A N/A N/A 56.5 N/A N/A N/A N/A N/A N/A N/A
batch_insert_into_indexed_tree_40_depth_hash_count N/A N/A N/A 94.1 N/A N/A N/A N/A N/A N/A N/A
batch_insert_into_indexed_tree_40_depth_hash_ms N/A N/A N/A 0.577 N/A N/A N/A N/A N/A N/A N/A

Miscellaneous

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

Metric 0 deployed contracts 1 deployed contracts
tx_size_in_bytes 15,575 (-9%) 38,272 (-4%)

Transaction processing duration by data writes.

Metric 0 new commitments 1 new commitments
tx_pxe_processing_time_ms 547 (-2%) 1,371 (+4%)
Metric 0 public data writes 1 public data writes
tx_sequencer_processing_time_ms 0.560 (+6%) 630 (+7%)

@signorecello signorecello marked this pull request as ready for review January 23, 2024 19:01
@signorecello
Copy link
Contributor Author

closes AztecProtocol/dev-rel#146

@signorecello
Copy link
Contributor Author

signorecello commented Jan 24, 2024

gonna stack this up to Resolve AztecProtocol/dev-rel#147 too

barretenberg/cpp/.clangd Outdated Show resolved Hide resolved
Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@inquirer/[email protected] Transitive: environment +9 4.07 MB sboudrias
npm/@inquirer/[email protected] Transitive: environment +8 4.06 MB sboudrias
npm/@inquirer/[email protected] Transitive: environment +9 4.08 MB sboudrias
npm/@inquirer/[email protected] Transitive: environment +9 4.09 MB sboudrias
npm/@inquirer/[email protected] None 0 4.62 kB sboudrias
npm/@types/[email protected] None 0 3.07 kB types
npm/[email protected] environment 0 26.1 kB jonschlinkert
npm/[email protected] environment 0 4.77 kB knownasilya
npm/[email protected] None 0 11.8 kB jorgebucaran
npm/[email protected] None 0 181 kB abetomo, shadowspawn, somekittens, ...1 more
npm/[email protected] environment +2 248 kB jonschlinkert
npm/[email protected] None 0 12.1 kB sindresorhus
npm/[email protected] None 0 5.48 kB bevacqua
npm/[email protected] None 0 11.4 kB terkelg
npm/[email protected] None 0 14.2 kB terkelg
npm/[email protected] network 0 26.2 kB tootallnate
npm/[email protected] None 0 12.6 kB lukeed
npm/[email protected] None 0 6.43 kB lukekarrys
npm/[email protected] environment, filesystem, network, shell +1 1.57 MB tyriar
npm/[email protected] None +1 415 kB sniphpet
npm/[email protected] None 0 7.71 kB sboudrias
npm/[email protected] environment, filesystem, network, shell +10 373 kB nake89
npm/[email protected] filesystem +2 37.9 kB terkelg

View full report↗︎

Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSource
Native code npm/[email protected]
Install scripts npm/[email protected]
  • Install script: postinstall
  • Source: node scripts/post-install.js

View full report↗︎

Next steps

What's wrong with native code?

Contains native code which could be a vector to obscure malicious code, and generally decrease the likelihood of reproducible or reliable installs.

Ensure that native code bindings are expected. Consumers may consider pure JS and functionally similar alternatives to avoid the challenges and risks associated with native code bindings.

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@catmcgee catmcgee force-pushed the zpedro/boxes_fix branch 3 times, most recently from 23769f5 to 71e1344 Compare February 6, 2024 14:57
@signorecello
Copy link
Contributor Author

Closing as github refuses to acknowledge there's no diff with master except on boxes. Will open a new PR

@signorecello signorecello deleted the zpedro/boxes_fix branch February 12, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Review state of Aztec Boxes
3 participants