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!: nuking private token #2822

Merged
merged 16 commits into from
Oct 16, 2023
Merged

feat!: nuking private token #2822

merged 16 commits into from
Oct 16, 2023

Conversation

benesjan
Copy link
Contributor

Fixes #2350

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).

@benesjan benesjan marked this pull request as draft October 12, 2023 16:13
@benesjan benesjan force-pushed the janb/no_private_token_in_docs branch from a64bbe7 to dc4482c Compare October 13, 2023 08:38
@benesjan benesjan changed the title docs: no private token in docs !chore: nuking private token Oct 13, 2023
@benesjan benesjan changed the title !chore: nuking private token docs!: nuking private token Oct 13, 2023
@AztecBot
Copy link
Collaborator

AztecBot commented Oct 13, 2023

Benchmark 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 43422805 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,900 867,788 3,448,148
l1_rollup_execution_gas 841,987 3,594,896 22,203,517
l2_block_processing_time_in_ms 1,010 (-1%) 3,912 (-1%) 15,472
note_successful_decrypting_time_in_ms 323 (+1%) 998 3,714 (+2%)
note_trial_decrypting_time_in_ms ⚠️ 26.0 (+18%) 106 (-6%) 137 (-1%)
l2_block_building_time_in_ms 8,929 35,755 149,733
l2_block_rollup_simulation_time_in_ms 6,614 26,520 105,219
l2_block_public_tx_process_time_in_ms 2,275 9,108 43,839 (+1%)

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,847 30,396
note_history_successful_decrypting_time_in_ms 2,294 (+1%) 4,669 (+1%)
note_history_trial_decrypting_time_in_ms 122 (-1%) 151 (+2%)
node_database_size_in_bytes 1,649,051 1,195,983
pxe_database_size_in_bytes 27,188 54,187

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 43.4 56,577 14,745
private-kernel-ordering 21.2 (-1%) 20,137 8,089
base-rollup 852 631,605 811
root-rollup 37.8 4,072 1,097
private-kernel-inner 35.8 72,288 14,745
public-kernel-private-input 47.2 (+1%) 37,359 14,745
public-kernel-non-first-iteration 28.1 (+1%) 37,401 14,745
merge-rollup 0.913 (+4%) 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,094

@benesjan benesjan changed the title docs!: nuking private token feat!: nuking private token Oct 13, 2023
@benesjan benesjan force-pushed the janb/no_private_token_in_docs branch from 5167219 to 3e9d2b9 Compare October 13, 2023 13:25
@benesjan benesjan marked this pull request as ready for review October 13, 2023 15:41
Copy link
Contributor

@LHerskind LHerskind left a comment

Choose a reason for hiding this comment

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

Added some nits but overall its great. If you fix these I think its good to merge.

docs/docs/dev_docs/cli/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/cli/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/cli/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/cli/main.md Outdated Show resolved Hide resolved
export class PrivateTokenContract extends ContractBase {
/** Creates a contract instance at the given address. */
public static async at(address: AztecAddress, wallet: Wallet) { ... }
export class TokenContract extends ContractBase {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this refer to some actual generated code with include code such that we don't need to change it if we are updating stuff in the contract?

client: 'pxe@0.1.0',
compatibleNargoVersion: '0.11.1-aztec.0'
client: 'pxe@#include_aztec_short_version'',
compatibleNargoVersion: '#include_noir_version'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should some of the underlying print not be altered as well? From the initial 1000000 wording seems to be the old one.

client: 'pxe@0.1.0',
compatibleNargoVersion: '0.11.1-aztec.0'
client: 'pxe@#include_aztec_short_version'',
compatibleNargoVersion: '#include_noir_version'
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar stuff here, below seems to be stale.

client: 'pxe@0.1.0',
compatibleNargoVersion: '0.11.1-aztec.0'
client: 'pxe@#include_aztec_short_version'',
compatibleNargoVersion: '#include_noir_version'
Copy link
Contributor

Choose a reason for hiding this comment

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

The below output should be reconsidered as the earlier ones. Might be out of data.

client: 'pxe@0.1.0',
compatibleNargoVersion: '0.11.1-aztec.0'
client: 'pxe@#include_aztec_short_version'',
compatibleNargoVersion: '#include_noir_version'
}
token Creating accounts using schnorr signers... +3ms
token Created Alice's account at 0x1509b252...0027 +10s
Copy link
Contributor

Choose a reason for hiding this comment

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

GH would not let me mark it, but at line 368 you also have "private token contract".

pxe_service Executed local simulation for 19bfe4fb2569be2168f01eefe5e5a4284d6c1678f17ab5e94c6ba9c811bcb214
pxe_service Sending transaction 19bfe4fb2569be2168f01eefe5e5a4284d6c1678f17ab5e94c6ba9c811bcb214
node Received tx 19bfe4fb2569be2168f01eefe5e5a4284d6c1678f17ab5e94c6ba9c811bcb214
pxe_service Registered account 0x061c94e053745973521de1826db5a1ee24af60a3c203c294570a35bd5afa3286
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we do something better than this, it pains my eyes to see how error prone this is. Don't need to be in this pr.

@benesjan benesjan force-pushed the janb/no_private_token_in_docs branch from 48e7a27 to 291ac0b Compare October 14, 2023 12:19
@benesjan benesjan merged commit 5d93a47 into master Oct 16, 2023
2 checks passed
@benesjan benesjan deleted the janb/no_private_token_in_docs branch October 16, 2023 08:02
PhilWindle pushed a commit that referenced this pull request Oct 17, 2023
🤖 I have created a release *beep* *boop*
---


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

##
[0.9.0](aztec-packages-v0.8.14...aztec-packages-v0.9.0)
(2023-10-17)


### ⚠ BREAKING CHANGES

* nuking `PublicToken` and `PrivateAirdropToken`
([#2873](#2873))
* Change blake3 to blake2 in private kernel
([#2861](#2861))
* nuking private token
([#2822](#2822))

### Features

* Contract ts interface to use only aztec.js imports
([#2876](#2876))
([6952a1a](6952a1a))
* Faucet
([#2856](#2856))
([5bad35f](5bad35f))
* Nuking `PublicToken` and `PrivateAirdropToken`
([#2873](#2873))
([c74311d](c74311d))
* Nuking private token
([#2822](#2822))
([5d93a47](5d93a47)),
closes
[#2350](#2350)


### Bug Fixes

* Aztec node to save outbox adddress to config
([#2867](#2867))
([b6418a6](b6418a6))
* Create data dir on node boot
([#2864](#2864))
([2d498b3](2d498b3))
* Don't repeatedly scan for missing messages
([#2886](#2886))
([3fe1cc8](3fe1cc8))
* Fix trailing pipe causing everything to rebuild. Sorry...
([d13ba75](d13ba75))
* Pad L1 to L2 messages upon retrieval from L1
([#2879](#2879))
([457669e](457669e))
* Sequencer aborts in-progress block
([#2883](#2883))
([b0915a8](b0915a8))


### Miscellaneous

* Change blake3 to blake2 in private kernel
([#2861](#2861))
([d629940](d629940))
* Making anvil silent again
([#2866](#2866))
([90ae5dc](90ae5dc))
* Spell check on forbidden words.
([#2887](#2887))
([06bc4f9](06bc4f9))


### Documentation

* Initial 'protocol description' toc
([#2844](#2844))
([cb18f45](cb18f45))
</details>

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

##
[0.9.0](barretenberg.js-v0.8.14...barretenberg.js-v0.9.0)
(2023-10-17)


### Miscellaneous

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

<details><summary>barretenberg: 0.9.0</summary>

##
[0.9.0](barretenberg-v0.8.14...barretenberg-v0.9.0)
(2023-10-17)


### 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).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Oct 18, 2023
🤖 I have created a release *beep* *boop*
---


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

##
[0.9.0](AztecProtocol/aztec-packages@aztec-packages-v0.8.14...aztec-packages-v0.9.0)
(2023-10-17)


### ⚠ BREAKING CHANGES

* nuking `PublicToken` and `PrivateAirdropToken`
([#2873](AztecProtocol/aztec-packages#2873))
* Change blake3 to blake2 in private kernel
([#2861](AztecProtocol/aztec-packages#2861))
* nuking private token
([#2822](AztecProtocol/aztec-packages#2822))

### Features

* Contract ts interface to use only aztec.js imports
([#2876](AztecProtocol/aztec-packages#2876))
([6952a1a](AztecProtocol/aztec-packages@6952a1a))
* Faucet
([#2856](AztecProtocol/aztec-packages#2856))
([5bad35f](AztecProtocol/aztec-packages@5bad35f))
* Nuking `PublicToken` and `PrivateAirdropToken`
([#2873](AztecProtocol/aztec-packages#2873))
([c74311d](AztecProtocol/aztec-packages@c74311d))
* Nuking private token
([#2822](AztecProtocol/aztec-packages#2822))
([5d93a47](AztecProtocol/aztec-packages@5d93a47)),
closes
[#2350](AztecProtocol/aztec-packages#2350)


### Bug Fixes

* Aztec node to save outbox adddress to config
([#2867](AztecProtocol/aztec-packages#2867))
([b6418a6](AztecProtocol/aztec-packages@b6418a6))
* Create data dir on node boot
([#2864](AztecProtocol/aztec-packages#2864))
([2d498b3](AztecProtocol/aztec-packages@2d498b3))
* Don't repeatedly scan for missing messages
([#2886](AztecProtocol/aztec-packages#2886))
([3fe1cc8](AztecProtocol/aztec-packages@3fe1cc8))
* Fix trailing pipe causing everything to rebuild. Sorry...
([d13ba75](AztecProtocol/aztec-packages@d13ba75))
* Pad L1 to L2 messages upon retrieval from L1
([#2879](AztecProtocol/aztec-packages#2879))
([457669e](AztecProtocol/aztec-packages@457669e))
* Sequencer aborts in-progress block
([#2883](AztecProtocol/aztec-packages#2883))
([b0915a8](AztecProtocol/aztec-packages@b0915a8))


### Miscellaneous

* Change blake3 to blake2 in private kernel
([#2861](AztecProtocol/aztec-packages#2861))
([d629940](AztecProtocol/aztec-packages@d629940))
* Making anvil silent again
([#2866](AztecProtocol/aztec-packages#2866))
([90ae5dc](AztecProtocol/aztec-packages@90ae5dc))
* Spell check on forbidden words.
([#2887](AztecProtocol/aztec-packages#2887))
([06bc4f9](AztecProtocol/aztec-packages@06bc4f9))


### Documentation

* Initial 'protocol description' toc
([#2844](AztecProtocol/aztec-packages#2844))
([cb18f45](AztecProtocol/aztec-packages@cb18f45))
</details>

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

##
[0.9.0](AztecProtocol/aztec-packages@barretenberg.js-v0.8.14...barretenberg.js-v0.9.0)
(2023-10-17)


### Miscellaneous

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

<details><summary>barretenberg: 0.9.0</summary>

##
[0.9.0](AztecProtocol/aztec-packages@barretenberg-v0.8.14...barretenberg-v0.9.0)
(2023-10-17)


### 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
Archived in project
Development

Successfully merging this pull request may close these issues.

Purge the remaining private_token usage and update docs
3 participants