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: requiring only 1 sig from user #8146

Merged
merged 3 commits into from
Aug 22, 2024

Conversation

benesjan
Copy link
Contributor

Fixes #8098

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @benesjan and the rest of your teammates on Graphite Graphite

* @param app_payload The payload that contains the calls to be executed in the app phase.
* @param fee_payload The payload that contains the calls to be executed in the setup phase.
*/
// docs:start:entrypoint
pub fn entrypoint(self, app_payload: AppPayload, fee_payload: FeePayload) {
let valid_fn = self.is_valid_impl;

let fee_hash = fee_payload.hash();
assert(valid_fn(self.context, fee_hash));
let combined_payload_hash = poseidon2_hash_with_separator(
Copy link
Contributor Author

@benesjan benesjan Aug 22, 2024

Choose a reason for hiding this comment

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

i decided to go with the combined payload hash because it seems good enough and it required much less time than somehow merging AppPayload and FeePayload into 1 struct. Given that this is not enshrined in protocol I think we don't need to chase perfection here (that would require monster refactor).

On an unrelated note. I think we could optimize this a bit more and just add the app payload and fee payload hashes instead of hashing them because as far as I know we don't really need the robust crypto properties on the outer hash here. But seems to make sense to do that later if it turns out we need to hyper optimize this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we could optimize this a bit more and just add the app payload and fee payload hashes

Thinking about this more this could theoretically make it possible to swap the order in which fee payload and app payload are executed so it doesn't seem like a good idea.

@benesjan benesjan marked this pull request as ready for review August 22, 2024 14:39
@benesjan benesjan requested review from alexghr and Thunkar August 22, 2024 14:40
@AztecBot
Copy link
Collaborator

AztecBot commented Aug 22, 2024

Benchmark results

Metrics with a significant change:

  • protocol_circuit_simulation_time_in_ms (private-kernel-tail-to-public): 777 (-48%)
  • avm_simulation_time_ms (Token:transfer_public): 34.9 (+73%)
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,778 1,566 707 (-2%) 746 (-3%) 778
proof_construction_time_sha256_30_ms 11,567 3,099 1,381 1,424 (-1%) 1,466
proof_construction_time_sha256_100_ms 44,183 11,931 (+1%) 5,449 5,404 5,776 (-1%)
proof_construction_time_poseidon_hash_ms 79.0 (+1%) 34.0 34.0 58.0 88.0
proof_construction_time_poseidon_hash_30_ms 1,538 423 201 229 (+1%) 271 (-1%)
proof_construction_time_poseidon_hash_100_ms 5,676 1,519 677 746 (+3%) 754 (+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,660 92,482 177,716
l1_rollup_execution_gas 1,373,688 2,107,406 3,892,696
l2_block_processing_time_in_ms 264 (+9%) 446 (+2%) 814 (+4%)
l2_block_building_time_in_ms 9,078 (+1%) 17,529 34,972 (+1%)
l2_block_rollup_simulation_time_in_ms 9,078 (+1%) 17,528 34,972 (+1%)
l2_block_public_tx_process_time_in_ms 7,649 (+1%) 16,006 33,437 (+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 3,033 (+2%) 3,859 (+1%)
node_database_size_in_bytes 12,595,280 16,666,704
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 85.8 (-5%) 377 (-5%) 21,755 44,860 N/A
private-kernel-inner 171 (+3%) 673 (-6%) 72,566 45,007 N/A
private-kernel-reset-tiny 458 835 (-4%) 65,675 44,846 N/A
private-kernel-tail 195 154 (-3%) 50,686 52,257 N/A
base-parity 5.54 N/A 160 96.0 N/A
root-parity 35.0 (-2%) N/A 73,948 96.0 N/A
base-rollup 2,755 N/A 189,136 664 N/A
root-rollup 39.6 (-1%) N/A 58,173 716 N/A
public-kernel-setup 82.6 N/A 105,085 71,222 N/A
public-kernel-app-logic 95.3 N/A 104,911 71,222 N/A
public-kernel-tail 553 (+1%) N/A 410,534 16,414 N/A
private-kernel-reset-small 451 (-2%) N/A 66,341 45,629 N/A
private-kernel-tail-to-public ⚠️ 777 (-48%) 623 (-4%) 460,796 1,825 N/A
public-kernel-teardown 82.2 (-2%) N/A 105,349 71,222 N/A
merge-rollup 20.0 N/A 38,174 664 N/A
undefined N/A N/A N/A N/A 81,945 (+1%)

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 342
ContractInstanceDeployer:deploy 1,408 11,731 18.1
MultiCallEntrypoint:entrypoint 1,920 11,731 404
FeeJuice:deploy 1,376 11,731 390 (+1%)
SchnorrAccount:constructor 1,312 11,731 73.4 (-1%)
SchnorrAccount:entrypoint 2,304 11,731 396 (-3%)
Token:privately_mint_private_note 1,280 11,731 100 (+1%)
FPC:fee_entrypoint_public 1,344 11,731 28.6 (+5%)
Token:transfer 1,312 11,731 224 (-2%)
Benchmarking:create_note 1,344 11,731 86.4 (-1%)
SchnorrAccount:verify_private_authwit 1,280 11,731 27.4 (-1%)
Token:unshield 1,376 11,731 520 (-1%)
FPC:fee_entrypoint_private 1,376 11,731 704 (+1%)

AVM Simulation

Time to simulate various public functions in the AVM.

Function time_ms bytecode_size_in_bytes
FeeJuice:_increase_public_balance 54.3 (-5%) 7,739
FeeJuice:set_portal 9.26 (-2%) 2,354
Token:constructor 82.6 (-13%) 26,051
FPC:constructor 53.4 (-2%) 18,001
FeeJuice:mint_public 38.9 (-2%) 5,877
Token:mint_public 45.2 (+3%) 10,917
Token:assert_minter_and_mint 65.5 7,512
AuthRegistry:set_authorized 46.8 4,391
FPC:prepare_fee 239 (+2%) 7,043
Token:transfer_public ⚠️ 34.9 (+73%) 39,426
FPC:pay_refund 54.2 (-9%) 10,234
Benchmarking:increment_balance 938 6,563
Token:_increase_public_balance 39.3 (-3%) 8,433
FPC:pay_refund_with_shielded_rebate 62.2 10,783

Public DB Access

Time to access various public DBs.

Function time_ms
get-nullifier-index 0.168 (+4%)

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.18 3.86 (+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 0.109 (+1%) 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 (+1%) 18.4 (+5%) 31.2 (+1%) 60.5 (+3%) 118 (+7%)
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 0.107 (+4%) 0.101 (+1%) 0.104 (+3%) 0.105 (+5%)
batch_insert_into_indexed_tree_20_depth_ms N/A N/A 14.6 (+1%) 25.9 (+2%) 43.9 82.7 (+1%) 163 (+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.111 (+1%) 0.104 (+1%) 0.106 (+1%) 0.102 (+1%) 0.100
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 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 | |
| - | |

@benesjan benesjan removed the request for review from Thunkar August 22, 2024 16:43
@benesjan benesjan force-pushed the 08-22-refactor_requiring_only_1_sig_from_user branch from 57c0507 to 4a423f2 Compare August 22, 2024 17:09
@benesjan benesjan enabled auto-merge (squash) August 22, 2024 17:10
@benesjan benesjan merged commit f0b564b into master Aug 22, 2024
96 checks passed
@benesjan benesjan deleted the 08-22-refactor_requiring_only_1_sig_from_user branch August 22, 2024 17:45
spypsy pushed a commit that referenced this pull request Aug 23, 2024
🤖 I have created a release *beep* *boop*
---


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

##
[0.50.1](aztec-package-v0.50.0...aztec-package-v0.50.1)
(2024-08-23)


### Miscellaneous

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

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

##
[0.50.1](barretenberg.js-v0.50.0...barretenberg.js-v0.50.1)
(2024-08-23)


### Miscellaneous

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

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

##
[0.50.1](aztec-packages-v0.50.0...aztec-packages-v0.50.1)
(2024-08-23)


### Features

* Free instances and circuits earlier to reduce max memory usage
([#8118](#8118))
([32a04c1](32a04c1))
* Prepare protocol circuits for batch rollup
([#7727](#7727))
([a126e22](a126e22))
* Share the commitment key between instances to reduce mem
([#8154](#8154))
([c3dddf8](c3dddf8))


### Bug Fixes

* Cli-wallet manifest
([#8156](#8156))
([2ffcda3](2ffcda3))


### Miscellaneous

* Replace relative paths to noir-protocol-circuits
([5372ac4](5372ac4))
* Requiring only 1 sig from user
([#8146](#8146))
([f0b564b](f0b564b))
</details>

<details><summary>barretenberg: 0.50.1</summary>

##
[0.50.1](barretenberg-v0.50.0...barretenberg-v0.50.1)
(2024-08-23)


### Features

* Free instances and circuits earlier to reduce max memory usage
([#8118](#8118))
([32a04c1](32a04c1))
* Share the commitment key between instances to reduce mem
([#8154](#8154))
([c3dddf8](c3dddf8))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Aug 24, 2024
🤖 I have created a release *beep* *boop*
---


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

##
[0.50.1](AztecProtocol/aztec-packages@aztec-package-v0.50.0...aztec-package-v0.50.1)
(2024-08-23)


### Miscellaneous

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

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

##
[0.50.1](AztecProtocol/aztec-packages@barretenberg.js-v0.50.0...barretenberg.js-v0.50.1)
(2024-08-23)


### Miscellaneous

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

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

##
[0.50.1](AztecProtocol/aztec-packages@aztec-packages-v0.50.0...aztec-packages-v0.50.1)
(2024-08-23)


### Features

* Free instances and circuits earlier to reduce max memory usage
([#8118](AztecProtocol/aztec-packages#8118))
([32a04c1](AztecProtocol/aztec-packages@32a04c1))
* Prepare protocol circuits for batch rollup
([#7727](AztecProtocol/aztec-packages#7727))
([a126e22](AztecProtocol/aztec-packages@a126e22))
* Share the commitment key between instances to reduce mem
([#8154](AztecProtocol/aztec-packages#8154))
([c3dddf8](AztecProtocol/aztec-packages@c3dddf8))


### Bug Fixes

* Cli-wallet manifest
([#8156](AztecProtocol/aztec-packages#8156))
([2ffcda3](AztecProtocol/aztec-packages@2ffcda3))


### Miscellaneous

* Replace relative paths to noir-protocol-circuits
([5372ac4](AztecProtocol/aztec-packages@5372ac4))
* Requiring only 1 sig from user
([#8146](AztecProtocol/aztec-packages#8146))
([f0b564b](AztecProtocol/aztec-packages@f0b564b))
</details>

<details><summary>barretenberg: 0.50.1</summary>

##
[0.50.1](AztecProtocol/aztec-packages@barretenberg-v0.50.0...barretenberg-v0.50.1)
(2024-08-23)


### Features

* Free instances and circuits earlier to reduce max memory usage
([#8118](AztecProtocol/aztec-packages#8118))
([32a04c1](AztecProtocol/aztec-packages@32a04c1))
* Share the commitment key between instances to reduce mem
([#8154](AztecProtocol/aztec-packages#8154))
([c3dddf8](AztecProtocol/aztec-packages@c3dddf8))
</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.

Check 1 combined signature in account entrypoints
3 participants