-
Notifications
You must be signed in to change notification settings - Fork 305
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: persistent archiver store #3410
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
import { TokenContractArtifact } from '@aztec/noir-contracts/artifacts'; | ||
|
||
import { readFileSync } from 'fs'; | ||
|
||
// docs:start:get-tokens | ||
export async function getToken(client) { | ||
const addresses = JSON.parse(readFileSync('addresses.json')); | ||
return Contract.at(addresses.token, TokenContractArtifact, client); | ||
return Contract.at(AztecAddress.fromString(addresses.token), TokenContractArtifact, client); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out we were passing a string when Contract.at
wants an instance of AztecAddress
and the only reason this worked was because the old MemoryArchiverStore
was using address.toString()
to find the contract. The new store actually really needs an address instance so this was crashing.
Might be worth going through all of the JS tests files and adding jsdoc typings so we at least spot issues in the editor.
const archiverStore = rootDB | ||
? new LMDBArchiverStore(rootDB, config.maxLogs ?? 1000) | ||
: new MemoryArchiverStore(config.maxLogs ?? 1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left the old MemoryArchiverStore in as a reference for the code review and in case we might need it for other tests but otherwise all code paths should pass a rootDB
now and use a temporary LMDB instance if persistance isn't needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if this function should just accept the ArchiverStore interface and the concrete implementation is created at node setup. Otherwise, if I wanted to add a different DB implementation I would need to pollute the arguments.
Benchmark resultsMetrics with a significant change:
Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Values are compared against data from master at commit L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 16 txs.
Circuits statsStats on running time and I/O sizes collected for every circuit run across all benchmarks.
MiscellaneousTransaction sizes based on how many contracts are deployed in the tx.
|
Tracked down issue to how the trees restored themselves from the LevelDB instance. Fix in these two commits https://github.com/AztecProtocol/aztec-packages/pull/3410/files/8fc557ed4966a16d46672c1b4984e1763ed8d6bb..a4235f93bc8c31fdd198f7b8bbb7857d28e3948e |
const archiverStore = rootDB | ||
? new LMDBArchiverStore(rootDB, config.maxLogs ?? 1000) | ||
: new MemoryArchiverStore(config.maxLogs ?? 1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if this function should just accept the ArchiverStore interface and the concrete implementation is created at node setup. Otherwise, if I wanted to add a different DB implementation I would need to pollute the arguments.
* @param testName - The name of the test suite. | ||
* @param getStore - Returns an instance of a store that's already been initialized. | ||
*/ | ||
export function describeArchiverDataStore(testName: string, getStore: () => ArchiverDataStore) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great!
…nt-archiver-store
186e413
to
801fb6f
Compare
🤖 I have created a release *beep* *boop* --- <details><summary>aztec-packages: 0.16.1</summary> ## [0.16.1](aztec-packages-v0.16.0...aztec-packages-v0.16.1) (2023-11-28) ### Features * Added poseidon2 hash function to barretenberg/crypto ([#3118](#3118)) ([d47782b](d47782b)) * Aztec CI files in Noir ([#3430](#3430)) ([1621f3a](1621f3a)) * Persistent archiver store ([#3410](#3410)) ([4735bde](4735bde)), closes [#3361](#3361) ### Bug Fixes * **ci:** Don't leave DRY_DEPLOY unset ([#3449](#3449)) ([454e316](454e316)) * **ci:** Publishing dockerhub manifests ([#3451](#3451)) ([a59e7f0](a59e7f0)) * Hotfix noir sync ([#3436](#3436)) ([c4e4745](c4e4745)) ### Miscellaneous * **docs:** Core concepts page in getting-started ([#3401](#3401)) ([1a62f73](1a62f73)) * Point acir tests at noir master branch ([#3440](#3440)) ([106e690](106e690)) ### Documentation * Further updates to the gas and fees whitepaper ([#3448](#3448)) ([4152ba6](4152ba6)) * Updates to gas and fees yellow paper ([#3438](#3438)) ([5f0e1ca](5f0e1ca)) </details> <details><summary>barretenberg.js: 0.16.1</summary> ## [0.16.1](barretenberg.js-v0.16.0...barretenberg.js-v0.16.1) (2023-11-28) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>barretenberg: 0.16.1</summary> ## [0.16.1](barretenberg-v0.16.0...barretenberg-v0.16.1) (2023-11-28) ### Features * Added poseidon2 hash function to barretenberg/crypto ([#3118](#3118)) ([d47782b](d47782b)) ### Miscellaneous * Point acir tests at noir master branch ([#3440](#3440)) ([106e690](106e690)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- <details><summary>aztec-packages: 0.16.1</summary> ## [0.16.1](AztecProtocol/aztec-packages@aztec-packages-v0.16.0...aztec-packages-v0.16.1) (2023-11-28) ### Features * Added poseidon2 hash function to barretenberg/crypto ([#3118](AztecProtocol/aztec-packages#3118)) ([d47782b](AztecProtocol/aztec-packages@d47782b)) * Aztec CI files in Noir ([#3430](AztecProtocol/aztec-packages#3430)) ([1621f3a](AztecProtocol/aztec-packages@1621f3a)) * Persistent archiver store ([#3410](AztecProtocol/aztec-packages#3410)) ([4735bde](AztecProtocol/aztec-packages@4735bde)), closes [#3361](AztecProtocol/aztec-packages#3361) ### Bug Fixes * **ci:** Don't leave DRY_DEPLOY unset ([#3449](AztecProtocol/aztec-packages#3449)) ([454e316](AztecProtocol/aztec-packages@454e316)) * **ci:** Publishing dockerhub manifests ([#3451](AztecProtocol/aztec-packages#3451)) ([a59e7f0](AztecProtocol/aztec-packages@a59e7f0)) * Hotfix noir sync ([#3436](AztecProtocol/aztec-packages#3436)) ([c4e4745](AztecProtocol/aztec-packages@c4e4745)) ### Miscellaneous * **docs:** Core concepts page in getting-started ([#3401](AztecProtocol/aztec-packages#3401)) ([1a62f73](AztecProtocol/aztec-packages@1a62f73)) * Point acir tests at noir master branch ([#3440](AztecProtocol/aztec-packages#3440)) ([106e690](AztecProtocol/aztec-packages@106e690)) ### Documentation * Further updates to the gas and fees whitepaper ([#3448](AztecProtocol/aztec-packages#3448)) ([4152ba6](AztecProtocol/aztec-packages@4152ba6)) * Updates to gas and fees yellow paper ([#3438](AztecProtocol/aztec-packages#3438)) ([5f0e1ca](AztecProtocol/aztec-packages@5f0e1ca)) </details> <details><summary>barretenberg.js: 0.16.1</summary> ## [0.16.1](AztecProtocol/aztec-packages@barretenberg.js-v0.16.0...barretenberg.js-v0.16.1) (2023-11-28) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>barretenberg: 0.16.1</summary> ## [0.16.1](AztecProtocol/aztec-packages@barretenberg-v0.16.0...barretenberg-v0.16.1) (2023-11-28) ### Features * Added poseidon2 hash function to barretenberg/crypto ([#3118](AztecProtocol/aztec-packages#3118)) ([d47782b](AztecProtocol/aztec-packages@d47782b)) ### Miscellaneous * Point acir tests at noir master branch ([#3440](AztecProtocol/aztec-packages#3440)) ([106e690](AztecProtocol/aztec-packages@106e690)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This PR adds a new implementation for an archiver store that's backed by LMDB. See #3361 for details.
Fix #3361