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.

Sign up for GitHub

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

Jump to bottom

feat: new proving broker #10174

Merged
merged 9 commits into from
Nov 28, 2024
Merged

feat: new proving broker #10174

merged 9 commits into from
Nov 28, 2024
+2,330 −1,092

Conversation

alexghr
Copy link
Contributor

@alexghr alexghr commented Nov 25, 2024 •
edited
Loading

This PR integrates the new proving broker implementation added in #8495 into the orchestrator and adds a new caching layer between the orchestrator and the job broker able to resolve jobs that have already been finished (in order to provide crash recovery for the prover-node).

@alexghr alexghr changed the title feat: new proving broker feat: new proving broker [WIP] Nov 25, 2024
@alexghr alexghr added the e2e-all CI: Enables this CI job. label Nov 25, 2024
This was linked to issues Nov 26, 2024
[Prover] Orchestrator persists partial proving state as proofs are generated #9528
Closed
[Prover] add k8s files for ProvingBroker and small/big ProvingAgent #10108
Closed
@alexghr alexghr changed the title feat: new proving broker [WIP] feat: new proving broker Nov 26, 2024
spalladino
spalladino reviewed Nov 26, 2024
View reviewed changes
Copy link
Collaborator

@spalladino spalladino left a comment

Choose a reason for hiding this comment

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

Quite a big one! Looks great, there's just one blocker at the very beginning caused by one of the most fearsome concepts that a developer must deal with: the boolean.

yarn-project/aztec/src/cli/cmds/start_prover_agent.ts Outdated Show resolved Hide resolved
yarn-project/circuit-types/src/interfaces/proving-job.ts Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Love seeing all that cleanup

yarn-project/end-to-end/webpack.config.js Outdated Show resolved Hide resolved
yarn-project/prover-client/src/prover-agent/memory-proving-queue.ts
Comment on lines +257 to +258

if (!this.queue.put(item as ProvingJobWithResolvers<any>)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey, that's cheating, you just wrapped the any in another type! :-P

yarn-project/prover-client/src/proving_broker/caching_broker_facade.ts
Comment on lines +136 to +147
getAvmProof(
inputs: AvmCircuitInputs,
signal?: AbortSignal,
_blockNumber?: number,
): Promise<ProofAndVerificationKey<typeof AVM_PROOF_LENGTH_IN_FIELDS>> {
return this.enqueueAndWaitForJob(
this.generateId(ProvingRequestType.PUBLIC_VM, inputs),
ProvingRequestType.PUBLIC_VM,
inputs,
signal,
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

At some point I'd like to delete all these methods from the Prover interface, and just have a getProof with a type discriminator. It would remove so much boilerplate.

yarn-project/prover-client/src/proving_broker/proving_broker_database/persisted.ts Outdated Show resolved Hide resolved
yarn-project/prover-client/src/proving_broker/rpc.ts Outdated Show resolved Hide resolved
yarn-project/prover-client/src/tx-prover/tx-prover.ts Outdated Show resolved Hide resolved
yarn-project/prover-node/src/prover-cache/cache_manager.ts Outdated Show resolved Hide resolved
yarn-project/prover-node/src/prover-node.ts Outdated Show resolved Hide resolved
alexghr
alexghr commented Nov 27, 2024
View reviewed changes
yarn-project/prover-client/src/proving_broker/proof_store.ts
Comment on lines +94 to +95
const encoded = encodeURIComponent(jsonStringify(obj));
return (PREFIX + SEPARATOR + encoded) as ProofUri;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spalladino proofs are now string-encoded into the data-uri to avoid double b64 encoding

@spalladino spalladino mentioned this pull request Nov 27, 2024
Merged
@PhilWindle PhilWindle merged commit 6fd5fc1 into master Nov 28, 2024
102 checks passed
@PhilWindle PhilWindle deleted the ag/proving-changes branch November 28, 2024 13:19
@AztecBot AztecBot mentioned this pull request Nov 28, 2024
Merged
ludamad pushed a commit that referenced this pull request Nov 28, 2024
@AztecBot
chore(master): Release 0.65.2 (#10258)
10754db 
🤖 I have created a release *beep* *boop*
---


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

##
[0.65.2](aztec-package-v0.65.1...aztec-package-v0.65.2)
(2024-11-28)


### Features

* New proving broker
([#10174](#10174))
([6fd5fc1](6fd5fc1))
</details>

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

##
[0.65.2](barretenberg.js-v0.65.1...barretenberg.js-v0.65.2)
(2024-11-28)


### Miscellaneous

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

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

##
[0.65.2](aztec-packages-v0.65.1...aztec-packages-v0.65.2)
(2024-11-28)


### Features

* Fee foresight support
([#10262](#10262))
([9e19244](9e19244))
* New proving broker
([#10174](#10174))
([6fd5fc1](6fd5fc1))
* Sequential insertion in indexed trees
([#10111](#10111))
([bfd9fa6](bfd9fa6))
* Swap polys to facilitate dynamic trace overflow
([#9976](#9976))
([b7b282c](b7b282c))


### Bug Fixes

* Don't store indices of zero leaves.
([#10270](#10270))
([c22be8b](c22be8b))
* Expect proper duplicate nullifier error patterns in e2e tests
([#10256](#10256))
([4ee8344](4ee8344))


### Miscellaneous

* Check artifact consistency
([#10271](#10271))
([6a49405](6a49405))
* Dont import things that themselves import jest in imported functions
([#10260](#10260))
([9440c1c](9440c1c))
* Fix bad merge in integration l1 publisher
([#10272](#10272))
([b5a6aa4](b5a6aa4))
* Fixing sol warnings
([#10276](#10276))
([3d113b2](3d113b2))
* Pull out sync changes
([#10274](#10274))
([391a6b7](391a6b7))
* Pull value merger code from sync
([#10080](#10080))
([3392629](3392629))
* Remove default gas settings
([#10163](#10163))
([c9a4d88](c9a4d88))
* Replace relative paths to noir-protocol-circuits
([654d801](654d801))
* Teardown context in prover coordination test
([#10257](#10257))
([7ea3888](7ea3888))
</details>

<details><summary>barretenberg: 0.65.2</summary>

##
[0.65.2](barretenberg-v0.65.1...barretenberg-v0.65.2)
(2024-11-28)


### Features

* Sequential insertion in indexed trees
([#10111](#10111))
([bfd9fa6](bfd9fa6))
* Swap polys to facilitate dynamic trace overflow
([#9976](#9976))
([b7b282c](b7b282c))


### Bug Fixes

* Don't store indices of zero leaves.
([#10270](#10270))
([c22be8b](c22be8b))


### Miscellaneous

* Pull value merger code from sync
([#10080](#10080))
([3392629](3392629))
</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 Nov 29, 2024
@AztecBot
chore(master): Release 0.65.2 (#10258)
bef9e48 
🤖 I have created a release *beep* *boop*
---


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

##
[0.65.2](AztecProtocol/aztec-packages@aztec-package-v0.65.1...aztec-package-v0.65.2)
(2024-11-28)


### Features

* New proving broker
([#10174](AztecProtocol/aztec-packages#10174))
([6fd5fc1](AztecProtocol/aztec-packages@6fd5fc1))
</details>

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

##
[0.65.2](AztecProtocol/aztec-packages@barretenberg.js-v0.65.1...barretenberg.js-v0.65.2)
(2024-11-28)


### Miscellaneous

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

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

##
[0.65.2](AztecProtocol/aztec-packages@aztec-packages-v0.65.1...aztec-packages-v0.65.2)
(2024-11-28)


### Features

* Fee foresight support
([#10262](AztecProtocol/aztec-packages#10262))
([9e19244](AztecProtocol/aztec-packages@9e19244))
* New proving broker
([#10174](AztecProtocol/aztec-packages#10174))
([6fd5fc1](AztecProtocol/aztec-packages@6fd5fc1))
* Sequential insertion in indexed trees
([#10111](AztecProtocol/aztec-packages#10111))
([bfd9fa6](AztecProtocol/aztec-packages@bfd9fa6))
* Swap polys to facilitate dynamic trace overflow
([#9976](AztecProtocol/aztec-packages#9976))
([b7b282c](AztecProtocol/aztec-packages@b7b282c))


### Bug Fixes

* Don't store indices of zero leaves.
([#10270](AztecProtocol/aztec-packages#10270))
([c22be8b](AztecProtocol/aztec-packages@c22be8b))
* Expect proper duplicate nullifier error patterns in e2e tests
([#10256](AztecProtocol/aztec-packages#10256))
([4ee8344](AztecProtocol/aztec-packages@4ee8344))


### Miscellaneous

* Check artifact consistency
([#10271](AztecProtocol/aztec-packages#10271))
([6a49405](AztecProtocol/aztec-packages@6a49405))
* Dont import things that themselves import jest in imported functions
([#10260](AztecProtocol/aztec-packages#10260))
([9440c1c](AztecProtocol/aztec-packages@9440c1c))
* Fix bad merge in integration l1 publisher
([#10272](AztecProtocol/aztec-packages#10272))
([b5a6aa4](AztecProtocol/aztec-packages@b5a6aa4))
* Fixing sol warnings
([#10276](AztecProtocol/aztec-packages#10276))
([3d113b2](AztecProtocol/aztec-packages@3d113b2))
* Pull out sync changes
([#10274](AztecProtocol/aztec-packages#10274))
([391a6b7](AztecProtocol/aztec-packages@391a6b7))
* Pull value merger code from sync
([#10080](AztecProtocol/aztec-packages#10080))
([3392629](AztecProtocol/aztec-packages@3392629))
* Remove default gas settings
([#10163](AztecProtocol/aztec-packages#10163))
([c9a4d88](AztecProtocol/aztec-packages@c9a4d88))
* Replace relative paths to noir-protocol-circuits
([654d801](AztecProtocol/aztec-packages@654d801))
* Teardown context in prover coordination test
([#10257](AztecProtocol/aztec-packages#10257))
([7ea3888](AztecProtocol/aztec-packages@7ea3888))
</details>

<details><summary>barretenberg: 0.65.2</summary>

##
[0.65.2](AztecProtocol/aztec-packages@barretenberg-v0.65.1...barretenberg-v0.65.2)
(2024-11-28)


### Features

* Sequential insertion in indexed trees
([#10111](AztecProtocol/aztec-packages#10111))
([bfd9fa6](AztecProtocol/aztec-packages@bfd9fa6))
* Swap polys to facilitate dynamic trace overflow
([#9976](AztecProtocol/aztec-packages#9976))
([b7b282c](AztecProtocol/aztec-packages@b7b282c))


### Bug Fixes

* Don't store indices of zero leaves.
([#10270](AztecProtocol/aztec-packages#10270))
([c22be8b](AztecProtocol/aztec-packages@c22be8b))


### Miscellaneous

* Pull value merger code from sync
([#10080](AztecProtocol/aztec-packages#10080))
([3392629](AztecProtocol/aztec-packages@3392629))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
spalladino added a commit that referenced this pull request Dec 2, 2024
@spalladino
feat: Process blocks in parallel during epoch proving (#10263)
a9d418c 
Instead of processing blocks in sequencer during epoch proving and
trigger proving jobs as each block is processed, we fetch the state
prior to each block by forking off a world state following the pending
(not proven) chain and process each block (ie execute public functions)
in parallel. This means tx execution is less of a bottleneck for
proving.

Main change is that the epoch orchestrator now requires not a db, but
something that can return forks at given block numbers of the db. It
also means that the orchestrator accepts out-of-order operations for
block building, so multiple blocks can be started, and their txs added
in any order (though following the order within each block).

Builds on #10174

Fixes #10265

Pending:
- Ensuring the state after processing each block matches what we had
used from world-state
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spalladino spalladino spalladino approved these changes

Assignees
No one assigned
Labels
e2e-all CI: Enables this CI job.
Projects
None yet
Milestone
No milestone
3 participants
@alexghr @spalladino @PhilWindle