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

feat: reduce the amount of key hashing required to fetch public keys #7997

Closed
wants to merge 3 commits into from

Conversation

nventuro
Copy link
Contributor

Closes #7825

We now hash each set of public keys once instead of twice by having a smarter oracle function that always returns the correct value, instead of two that we need to call conditionally.

This reduces the gate count of a token transfer by ~3k, though this reduction is expected to be lower and closer to ~1k once #7954 is closed, since we'll then be performing fewer total key read operations.

@nventuro nventuro requested review from benesjan and sklppy88 August 14, 2024 20:11
@AztecBot
Copy link
Collaborator

Benchmark results

Metrics with a significant change:

  • protocol_circuit_simulation_time_in_ms (private-kernel-tail-to-public): 776 (-41%)
  • avm_simulation_time_ms (Token:mint_public): 43.4 (-39%)
  • avm_simulation_time_ms (Token:assert_minter_and_mint): 67.8 (+70%)
  • avm_simulation_time_ms (Token:transfer_public): 20.4 (-37%)
  • avm_simulation_time_ms (Token:_increase_public_balance): 81.2 (+106%)
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,783 1,559 706 (-1%) 754 (+1%) 764 (-1%)
proof_construction_time_sha256_30_ms 11,846 3,167 (-1%) 1,413 1,441 (-1%) 1,471 (-1%)
proof_construction_time_sha256_100_ms 45,423 (-1%) 12,039 5,412 (-2%) 5,386 (-2%) 5,362 (-1%)
proof_construction_time_poseidon_hash_ms 78.0 34.0 34.0 (-3%) 59.0 (+2%) 87.0 (-1%)
proof_construction_time_poseidon_hash_30_ms 1,533 423 202 (-1%) 228 (-2%) 267 (-2%)
proof_construction_time_poseidon_hash_100_ms 5,643 1,516 676 721 (+1%) 747 (-1%)

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 4,324 7,844 14,852
l1_rollup_calldata_gas 49,648 92,496 177,500
l1_rollup_execution_gas 1,294,311 2,042,127 3,869,170
l2_block_processing_time_in_ms 253 (-1%) 446 (+2%) 788 (-2%)
l2_block_building_time_in_ms 9,263 (-2%) 18,192 36,040 (-1%)
l2_block_rollup_simulation_time_in_ms 9,262 (-2%) 18,192 36,040 (-1%)
l2_block_public_tx_process_time_in_ms 7,822 (-2%) 16,627 34,470 (-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 2,983 (+1%) 3,842 (+3%)
node_database_size_in_bytes 12,669,008 16,711,760
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 input_size_in_bytes output_size_in_bytes proving_time_in_ms
private-kernel-init 95.8 (+1%) 390 (-1%) 21,846 44,858 N/A
private-kernel-inner 173 (+1%) 694 (-1%) 72,545 45,005 N/A
private-kernel-reset-tiny 502 (+1%) 1,214 69,614 44,844 N/A
private-kernel-tail 270 193 (-2%) 50,760 52,256 N/A
base-parity 5.54 (-1%) N/A 160 96.0 N/A
root-parity 33.1 (-1%) N/A 69,084 96.0 N/A
base-rollup 2,854 (-1%) N/A 187,817 664 N/A
root-rollup 38.5 N/A 54,525 716 N/A
public-kernel-setup 99.6 N/A 103,760 71,222 N/A
public-kernel-app-logic 106 N/A 103,599 71,222 N/A
public-kernel-tail 570 N/A 409,190 16,414 N/A
private-kernel-reset-small 496 (+3%) N/A 66,533 45,629 N/A
private-kernel-tail-to-public ⚠️ 776 (-41%) 702 (-1%) 507,554 1,697 N/A
public-kernel-teardown 94.4 N/A 104,005 71,222 N/A
merge-rollup 19.2 (+1%) N/A 35,742 664 N/A
undefined N/A N/A N/A N/A 67,795 (-2%)

Stats on running time collected for app circuits

Function input_size_in_bytes output_size_in_bytes witness_generation_time_in_ms
ContractClassRegisterer:register 1,344 11,731 346
ContractInstanceDeployer:deploy 1,408 11,731 18.1
MultiCallEntrypoint:entrypoint 1,920 11,731 428
FeeJuice:deploy 1,376 11,731 400 (+2%)
SchnorrAccount:constructor 1,312 11,731 116 (+10%)
SchnorrAccount:entrypoint 2,304 11,731 442 (+3%)
Token:privately_mint_private_note 1,280 11,731 146 (+5%)
FPC:fee_entrypoint_public 1,344 11,731 21.5 (-23%)
Token:transfer 1,312 11,731 299 (+5%)
Benchmarking:create_note 1,344 11,731 112 (+12%)
SchnorrAccount:verify_private_authwit 1,280 11,731 27.7
Token:unshield 1,376 11,731 575 (+2%)
FPC:fee_entrypoint_private 1,376 11,731 764

AVM Simulation

Time to simulate various public functions in the AVM.

Function time_ms bytecode_size_in_bytes
FeeJuice:_increase_public_balance 54.2 (-1%) 8,139
FeeJuice:set_portal 9.48 (-10%) 2,362
Token:constructor 80.5 (-1%) 31,107
FPC:constructor 54.8 (+2%) 22,380
FeeJuice:mint_public 38.0 (+1%) 6,150
Token:mint_public ⚠️ 43.4 (-39%) 11,720
Token:assert_minter_and_mint ⚠️ 67.8 (+70%) 8,028
AuthRegistry:set_authorized 37.5 (-17%) 4,537
FPC:prepare_fee 243 (+6%) 8,812
Token:transfer_public ⚠️ 20.4 (-37%) 47,374
FPC:pay_refund 56.4 (-3%) 12,114
Benchmarking:increment_balance 972 7,450
Token:_increase_public_balance ⚠️ 81.2 (+106%) 8,960
FPC:pay_refund_with_shielded_rebate 68.1 (+3%) 12,663

Public DB Access

Time to access various public DBs.

Function time_ms
get-nullifier-index 0.164 (-2%)

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 2.16 3.82 (+1%) 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.112 (+1%) 0.108 N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_32_depth_ms N/A N/A 11.6 (+3%) 18.0 (+1%) 31.1 59.6 (+2%) 111 (-3%)
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.111 (+3%) 0.104 0.101 0.103 (+2%) 0.101 (-2%)
batch_insert_into_indexed_tree_20_depth_ms N/A N/A 14.5 (+1%) 25.8 44.1 (+1%) 83.1 (+1%) 160 (-1%)
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.110 0.104 0.107 (+1%) 0.103 (+1%) 0.103
batch_insert_into_indexed_tree_40_depth_ms N/A N/A 16.4 (+1%) N/A N/A N/A N/A
batch_insert_into_indexed_tree_40_depth_hash_count N/A N/A 132 N/A N/A N/A N/A
batch_insert_into_indexed_tree_40_depth_hash_ms N/A N/A 0.105 (+1%) 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 64,779 668,997

Transaction size based on fee payment method

| Metric | |
| - | |

Copy link
Contributor

@benesjan benesjan left a comment

Choose a reason for hiding this comment

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

Cute 🐣💖😊🌸✨

@nventuro nventuro changed the title feat: reduce the number of key hashing required to fetch public keys feat: reduce the amount of key hashing required to fetch public keys Aug 19, 2024
@nventuro
Copy link
Contributor Author

nventuro commented Sep 5, 2024

This is failing because my new unconstrained helper creates an UnconstrainedContext, which requires calling some oracles (namely block number and timestamp), and some tests are running in a heavily mocked environment in which these are unavailable.

Fixing this is not terribly hard but also not trivial, so I'll leave this open since we're unsure about whether we'll keep key rotation at all.

@nventuro
Copy link
Contributor Author

Closing this as we're changing how addresses and keys are related.

@nventuro nventuro closed this Sep 10, 2024
@nventuro nventuro deleted the nv/single-hash-hint branch September 10, 2024 22:15
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.

Perform single key hash in key registry read
3 participants