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

refactor: replace usage of GrumpkinPrivateKey with EmbeddedCurveScalar #7384

Conversation

benesjan
Copy link
Contributor

@benesjan benesjan commented Jul 8, 2024

Replacing use of GrumpkinPrivateKey since I did so for GrumpkinPoint as well in the previous PR.

Copy link
Contributor Author

benesjan commented Jul 8, 2024

@benesjan benesjan changed the title refactor: replace usage of GrumpkinPrivateKey with EmbeddedCurveScalar refactor: replace usage of GrumpkinPrivateKey with EmbeddedCurveScalar Jul 8, 2024
@benesjan benesjan force-pushed the 07-08-fix_replace_usage_of_grumpkinpoint_with_embeddedcurvepoint branch 3 times, most recently from a8d9bbc to 1b2aa60 Compare July 9, 2024 11:46
@benesjan benesjan force-pushed the 07-08-refactor_replace_usage_of_grumpkinprivatekey_with_embeddedcurvescalar branch from ee59bfb to b4d06d0 Compare July 9, 2024 12:24
Copy link
Contributor

github-actions bot commented Jul 9, 2024

Changes to circuit sizes

Generated at commit: 2f5c7367b1a4fe7930323fbeb146850231b345f6, compared to commit: 52796958738f8f1eb90d9691ff489d189f9bce90

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_reset_small -160 ✅ -0.25% -208 ✅ -0.02%
private_kernel_reset_medium -320 ✅ -0.43% -416 ✅ -0.04%
private_kernel_reset_big -640 ✅ -0.68% -832 ✅ -0.07%
private_kernel_reset -1,280 ✅ -0.95% -1,664 ✅ -0.10%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_reset_small 63,618 (-160) -0.25% 839,990 (-208) -0.02%
private_kernel_reset_medium 73,653 (-320) -0.43% 963,647 (-416) -0.04%
private_kernel_reset_big 93,722 (-640) -0.68% 1,211,168 (-832) -0.07%
private_kernel_reset 133,858 (-1,280) -0.95% 1,706,208 (-1,664) -0.10%

@benesjan benesjan force-pushed the 07-08-fix_replace_usage_of_grumpkinpoint_with_embeddedcurvepoint branch 2 times, most recently from 8d04851 to df950c6 Compare July 9, 2024 13:01
@AztecBot
Copy link
Collaborator

AztecBot commented Jul 9, 2024

Benchmark results

Metrics with a significant change:

  • avm_simulation_time_ms (Token:mint_public): 50.4 (-85%)
  • avm_simulation_time_ms (Token:assert_minter_and_mint): 77.4 (+85%)
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.

Proof generation

Each column represents the number of threads used in proof generation.

Metric 1 threads 4 threads 16 threads 32 threads 64 threads
proof_construction_time_sha256_ms 5,756 (+1%) 1,550 (+1%) 704 747 (-3%) 770
proof_construction_time_sha256_30_ms 11,812 3,167 (+1%) 1,403 1,454 (+2%) 1,472 (+1%)
proof_construction_time_sha256_100_ms 44,045 (+1%) 11,838 5,408 (-1%) 5,423 5,340
proof_construction_time_poseidon_hash_ms 78.0 34.0 34.0 58.0 (-2%) 87.0 (-1%)
proof_construction_time_poseidon_hash_30_ms 1,528 (+1%) 418 200 228 (+1%) 275 (+3%)
proof_construction_time_poseidon_hash_100_ms 5,654 (+1%) 1,526 (+1%) 678 725 (-1%) 751

L2 block published to L1

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

Metric 4 txs 8 txs 16 txs
l1_rollup_calldata_size_in_bytes 1,412 1,412 1,412
l1_rollup_calldata_gas 9,476 9,462 9,476
l1_rollup_execution_gas 613,504 613,641 613,806
l2_block_processing_time_in_ms 763 (+2%) 1,411 (+1%) 2,714 (+3%)
l2_block_building_time_in_ms 13,117 (+2%) 25,814 (+1%) 50,590 (+1%)
l2_block_rollup_simulation_time_in_ms 13,117 (+2%) 25,814 (+1%) 50,590 (+1%)
l2_block_public_tx_process_time_in_ms 11,057 (+2%) 23,569 (+1%) 48,349 (+1%)

L2 chain processing

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

Metric 3 blocks 5 blocks
node_history_sync_time_in_ms 6,950 (-1%) 9,905 (+1%)
node_database_size_in_bytes 12,431,440 16,121,936 (-1%)
pxe_database_size_in_bytes 16,254 26,813

Circuits stats

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

Circuit simulation_time_in_ms witness_generation_time_in_ms proving_time_in_ms input_size_in_bytes output_size_in_bytes proof_size_in_bytes num_public_inputs size_in_gates
private-kernel-init 110 (+1%) 427 14,100 (-1%) 20,002 55,022 76,256 2,316 524,288
private-kernel-inner 227 768 47,190 82,134 55,022 76,256 2,316 2,097,152
private-kernel-tail 1,077 (+2%) 2,524 44,418 (-1%) 62,409 62,089 14,944 400 2,097,152
base-parity 6.37 (+1%) 945 2,442 (-1%) 160 96.0 2,240 3.00 131,072
root-parity 70.5 66.0 (+1%) 34,171 (+1%) 27,868 96.0 2,752 19.0 2,097,152
base-rollup 4,171 (+1%) 5,154 (+1%) 78,798 (+1%) 172,028 632 3,552 44.0 4,194,304
root-rollup 129 (+1%) 74.3 (-1%) 19,580 25,053 652 3,488 42.0 1,048,576
public-kernel-setup 191 (-1%) 2,334 36,474 103,911 80,310 108,992 3,339 2,097,152
public-kernel-app-logic 149 (+1%) 3,396 (+1%) 36,825 (-1%) 103,911 80,310 108,992 3,339 2,097,152
public-kernel-tail 903 (+1%) 24,110 (+6%) 157,791 (-1%) 400,808 10,046 14,944 400 8,388,608
private-kernel-reset-small 294 (+1%) 1,292 (+2%) 27,809 (-3%) 79,273 55,022 76,256 2,316 1,048,576
public-kernel-teardown 136 (-1%) 3,353 36,914 103,911 80,310 108,992 3,339 2,097,152
merge-rollup 40.2 (+1%) N/A N/A 16,094 632 N/A N/A N/A
private-kernel-tail-to-public N/A 8,679 (-1%) 48,704 N/A N/A 108,992 3,339 2,097,152

Stats on running time collected for app circuits

Function input_size_in_bytes output_size_in_bytes witness_generation_time_in_ms proof_size_in_bytes proving_time_in_ms size_in_gates num_public_inputs
ContractClassRegisterer:register 1,312 9,344 398 N/A N/A N/A N/A
ContractInstanceDeployer:deploy 1,376 9,344 25.5 N/A N/A N/A N/A
MultiCallEntrypoint:entrypoint 1,888 9,344 669 N/A N/A N/A N/A
GasToken:deploy 1,344 9,344 572 (+1%) N/A N/A N/A N/A
SchnorrAccount:constructor 1,280 9,344 512 N/A N/A N/A N/A
SchnorrAccount:entrypoint 2,272 9,344 860 (+1%) 16,352 5,010 (+5%) 131,072 444
Token:privately_mint_private_note 1,248 9,344 588 (-2%) N/A N/A N/A N/A
FPC:fee_entrypoint_public 1,312 9,344 105 (-2%) 16,352 1,866 (-1%) 65,536 444
Token:transfer 1,280 9,344 1,763 (+1%) 16,352 10,472 (-2%) 524,288 444
AuthRegistry:set_authorized (avm) 19,222 N/A N/A 91,232 1,563 (-6%) N/A N/A
FPC:prepare_fee (avm) 26,664 N/A N/A 91,296 2,885 (-1%) N/A N/A
Token:transfer_public (avm) 42,914 N/A N/A 91,296 3,625 (+2%) N/A N/A
AuthRegistry:consume (avm) 33,100 N/A N/A 91,232 2,905 (+3%) N/A N/A
FPC:pay_refund (avm) 36,829 N/A N/A 91,264 18,396 (+4%) N/A N/A
Benchmarking:create_note 1,312 9,344 490 (+1%) N/A N/A N/A N/A
SchnorrAccount:verify_private_authwit 1,248 9,344 41.5 N/A N/A N/A N/A
Token:unshield 1,344 9,344 1,399 (+1%) N/A N/A N/A N/A
FPC:fee_entrypoint_private 1,344 9,344 1,811 N/A N/A N/A N/A

AVM Simulation

Time to simulate various public functions in the AVM.

Function time_ms bytecode_size_in_bytes
GasToken:_increase_public_balance 80.6 (+4%) 13,790
GasToken:set_portal 13.4 (+11%) 3,339
Token:constructor 103 23,692
FPC:constructor 71.9 (-2%) 13,592
GasToken:mint_public 61.6 (-4%) 10,158
Token:mint_public ⚠️ 50.4 (-85%) 19,034
Token:assert_minter_and_mint ⚠️ 77.4 (+85%) 12,925
AuthRegistry:set_authorized 33.6 (-6%) 7,812
FPC:prepare_fee 110 (+3%) 15,062
Token:transfer_public 35.4 (+20%) 31,218
FPC:pay_refund 132 (-4%) 25,260
Benchmarking:increment_balance 1,332 (+1%) 15,267
Token:_increase_public_balance 15.5 15,006
FPC:pay_refund_with_shielded_rebate 163 (-3%) 26,347

Public DB Access

Time to access various public DBs.

Function time_ms
get-nullifier-index 0.151 (+1%)

Tree insertion stats

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

Metric 1 leaves 16 leaves 64 leaves 128 leaves 256 leaves 512 leaves 1024 leaves
batch_insert_into_append_only_tree_16_depth_ms 10.4 16.7 N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_16_depth_hash_count 16.8 31.7 N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_16_depth_hash_ms 0.601 0.515 N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_32_depth_ms N/A N/A 48.2 75.4 131 (+1%) 244 471 (+1%)
batch_insert_into_append_only_tree_32_depth_hash_count N/A N/A 95.9 159 287 543 1,055
batch_insert_into_append_only_tree_32_depth_hash_ms N/A N/A 0.493 0.464 0.451 (+1%) 0.443 0.440 (+1%)
batch_insert_into_indexed_tree_20_depth_ms N/A N/A 59.6 (+1%) 111 (+1%) 184 (+2%) 352 695 (+3%)
batch_insert_into_indexed_tree_20_depth_hash_count N/A N/A 109 207 355 691 1,363
batch_insert_into_indexed_tree_20_depth_hash_ms N/A N/A 0.503 0.498 (+1%) 0.486 (+2%) 0.477 0.476 (+3%)
batch_insert_into_indexed_tree_40_depth_ms N/A N/A 72.9 (+1%) N/A N/A N/A N/A
batch_insert_into_indexed_tree_40_depth_hash_count N/A N/A 133 N/A N/A N/A N/A
batch_insert_into_indexed_tree_40_depth_hash_ms N/A N/A 0.518 N/A N/A N/A N/A

Miscellaneous

Transaction sizes based on how many contract classes are registered in the tx.

Metric 0 registered classes 1 registered classes
tx_size_in_bytes 74,105 667,868

Transaction size based on fee payment method

| Metric | |
| - | |

@benesjan benesjan force-pushed the 07-08-refactor_replace_usage_of_grumpkinprivatekey_with_embeddedcurvescalar branch from b4d06d0 to 0b73463 Compare July 9, 2024 13:03
@benesjan benesjan force-pushed the 07-08-fix_replace_usage_of_grumpkinpoint_with_embeddedcurvepoint branch from df950c6 to 700a8d4 Compare July 9, 2024 13:27
@benesjan benesjan force-pushed the 07-08-refactor_replace_usage_of_grumpkinprivatekey_with_embeddedcurvescalar branch from 0b73463 to d4b98d8 Compare July 9, 2024 13:28
@benesjan benesjan force-pushed the 07-08-fix_replace_usage_of_grumpkinpoint_with_embeddedcurvepoint branch 2 times, most recently from 5c59713 to 4978dae Compare July 9, 2024 15:01
@benesjan benesjan force-pushed the 07-08-refactor_replace_usage_of_grumpkinprivatekey_with_embeddedcurvescalar branch from d4b98d8 to 7bc1885 Compare July 9, 2024 15:02
@benesjan benesjan force-pushed the 07-08-fix_replace_usage_of_grumpkinpoint_with_embeddedcurvepoint branch from 4978dae to 79bd1a2 Compare July 9, 2024 17:41
Base automatically changed from 07-08-fix_replace_usage_of_grumpkinpoint_with_embeddedcurvepoint to master July 9, 2024 18:10
@benesjan benesjan force-pushed the 07-08-refactor_replace_usage_of_grumpkinprivatekey_with_embeddedcurvescalar branch from 7bc1885 to eaa2761 Compare July 9, 2024 18:11
@benesjan benesjan marked this pull request as ready for review July 9, 2024 19:05
@benesjan benesjan requested a review from sklppy88 July 9, 2024 19:05
Copy link
Contributor

@sklppy88 sklppy88 left a comment

Choose a reason for hiding this comment

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

👍

@benesjan benesjan enabled auto-merge (squash) July 9, 2024 19:57
@benesjan benesjan merged commit a917198 into master Jul 9, 2024
89 checks passed
@benesjan benesjan deleted the 07-08-refactor_replace_usage_of_grumpkinprivatekey_with_embeddedcurvescalar branch July 9, 2024 20:22
Thunkar pushed a commit that referenced this pull request Jul 10, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.46.1</summary>

##
[0.46.1](aztec-package-v0.46.0...aztec-package-v0.46.1)
(2024-07-10)


### Miscellaneous

* **aztec-package:** Synchronize aztec-packages versions
</details>

<details><summary>barretenberg.js: 0.46.1</summary>

##
[0.46.1](barretenberg.js-v0.46.0...barretenberg.js-v0.46.1)
(2024-07-10)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>aztec-packages: 0.46.1</summary>

##
[0.46.1](aztec-packages-v0.46.0...aztec-packages-v0.46.1)
(2024-07-10)


### Features

* Apply `no_predicates` in stdlib
(noir-lang/noir#5454)
([6237d96](6237d96))
* Lsp rename/find-all-references for globals
(noir-lang/noir#5415)
([6237d96](6237d96))
* Lsp rename/find-all-references for local variables
(noir-lang/noir#5439)
([6237d96](6237d96))
* Remove duplicated array reads at constant indices
(noir-lang/noir#5445)
([6237d96](6237d96))
* Remove redundant `EnableSideEffects` instructions
(noir-lang/noir#5440)
([6237d96](6237d96))


### Bug Fixes

* Account for the expected kind when resolving turbofish generics
(noir-lang/noir#5448)
([6237d96](6237d96))
* Added bb to noir-projects deps
([#7412](#7412))
([6d3ed3a](6d3ed3a))
* Fix issue with unresolved results
(noir-lang/noir#5453)
([6237d96](6237d96))
* Prevent `no_predicates` from removing predicates in calling function
(noir-lang/noir#5452)
([6237d96](6237d96))


### Miscellaneous

* Replace relative paths to noir-protocol-circuits
([db45302](db45302))
* Replace usage of `GrumpkinPoint` with `EmbeddedCurvePoint`
([#7382](#7382))
([5279695](5279695))
* Replace usage of `GrumpkinPrivateKey` with `EmbeddedCurveScalar`
([#7384](#7384))
([a917198](a917198))
</details>

<details><summary>barretenberg: 0.46.1</summary>

##
[0.46.1](barretenberg-v0.46.0...barretenberg-v0.46.1)
(2024-07-10)


### Miscellaneous

* **barretenberg:** Synchronize aztec-packages versions
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
github-merge-queue bot pushed a commit to noir-lang/noir that referenced this pull request Jul 10, 2024
Automated pull of Noir development from
[aztec-packages](https://github.com/AztecProtocol/aztec-packages).
BEGIN_COMMIT_OVERRIDE
feat: Sync from noir
(AztecProtocol/aztec-packages#7400)
refactor: replace usage of `GrumpkinPrivateKey` with
`EmbeddedCurveScalar`
(AztecProtocol/aztec-packages#7384)
refactor: replace usage of `GrumpkinPoint` with `EmbeddedCurvePoint`
(AztecProtocol/aztec-packages#7382)
feat: Sync from noir
(AztecProtocol/aztec-packages#7392)
feat!: constant inputs for blackbox
(AztecProtocol/aztec-packages#7222)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <[email protected]>
Co-authored-by: Tom French <[email protected]>
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Jul 11, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.46.1</summary>

##
[0.46.1](AztecProtocol/aztec-packages@aztec-package-v0.46.0...aztec-package-v0.46.1)
(2024-07-10)


### Miscellaneous

* **aztec-package:** Synchronize aztec-packages versions
</details>

<details><summary>barretenberg.js: 0.46.1</summary>

##
[0.46.1](AztecProtocol/aztec-packages@barretenberg.js-v0.46.0...barretenberg.js-v0.46.1)
(2024-07-10)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>aztec-packages: 0.46.1</summary>

##
[0.46.1](AztecProtocol/aztec-packages@aztec-packages-v0.46.0...aztec-packages-v0.46.1)
(2024-07-10)


### Features

* Apply `no_predicates` in stdlib
(noir-lang/noir#5454)
([6237d96](AztecProtocol/aztec-packages@6237d96))
* Lsp rename/find-all-references for globals
(noir-lang/noir#5415)
([6237d96](AztecProtocol/aztec-packages@6237d96))
* Lsp rename/find-all-references for local variables
(noir-lang/noir#5439)
([6237d96](AztecProtocol/aztec-packages@6237d96))
* Remove duplicated array reads at constant indices
(noir-lang/noir#5445)
([6237d96](AztecProtocol/aztec-packages@6237d96))
* Remove redundant `EnableSideEffects` instructions
(noir-lang/noir#5440)
([6237d96](AztecProtocol/aztec-packages@6237d96))


### Bug Fixes

* Account for the expected kind when resolving turbofish generics
(noir-lang/noir#5448)
([6237d96](AztecProtocol/aztec-packages@6237d96))
* Added bb to noir-projects deps
([#7412](AztecProtocol/aztec-packages#7412))
([6d3ed3a](AztecProtocol/aztec-packages@6d3ed3a))
* Fix issue with unresolved results
(noir-lang/noir#5453)
([6237d96](AztecProtocol/aztec-packages@6237d96))
* Prevent `no_predicates` from removing predicates in calling function
(noir-lang/noir#5452)
([6237d96](AztecProtocol/aztec-packages@6237d96))


### Miscellaneous

* Replace relative paths to noir-protocol-circuits
([db45302](AztecProtocol/aztec-packages@db45302))
* Replace usage of `GrumpkinPoint` with `EmbeddedCurvePoint`
([#7382](AztecProtocol/aztec-packages#7382))
([5279695](AztecProtocol/aztec-packages@5279695))
* Replace usage of `GrumpkinPrivateKey` with `EmbeddedCurveScalar`
([#7384](AztecProtocol/aztec-packages#7384))
([a917198](AztecProtocol/aztec-packages@a917198))
</details>

<details><summary>barretenberg: 0.46.1</summary>

##
[0.46.1](AztecProtocol/aztec-packages@barretenberg-v0.46.0...barretenberg-v0.46.1)
(2024-07-10)


### Miscellaneous

* **barretenberg:** Synchronize aztec-packages versions
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants