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

fix(prover): Decouple core/ prover database management #1029

Merged

EmilLuta merged 15 commits into main from evl-separate-core-from-prover-migrations

Feb 13, 2024

Contributor

EmilLuta commented Feb 7, 2024

This is PR 1 out of many PRs to come. In first step, prover database
needs to be managed by code. The changes adds a new DAL/ migration for
prover that initializes the database with all the needed tables (as
observed in production).

Additional changes are made to zk to not break local development. Note
that from this point forward, development will be done on 2 databases
(as opposed to 1 database model). Alongside these changes, stage env
will be moved to dual database model.

This has been long overdue (8+ months) after core and prover has been
split from infra side during an outage. This is the continuation (and
with following PRs, finalization) of said effort.

Note that whilst there's no automated proper testing of the typescript code modified, I've reran and tested working condition locally manually multiple times. I expect no breakage, but can't guarantee it.

EmilLuta added 3 commits

February 7, 2024 15:46


          fix(prover): Decouple core/ prover database management

e36498f

This is PR #1 out of many PRs to come. In first step, prover database
needs to be managed by code. The changes adds a new DAL/ migration for
prover that initializes the database with all the needed tables (as
observed in production).

Additional changes are made to `zk` to not break local development. Note
that from this point forward, development will be done on 2 databases
(as opposed to 1 database model). Alongside these changes, stage env
will be moved to dual database model.

This has been long overdue (8+ months) after core and prover has been
split from infra side during an outage. This is the continuation (and
with following PRs, finalization) of said effort.


          Missed formatting for typescript

f1d18c9


          Add EoL for migration

b025c29

EmilLuta requested review from popzxc, Deniallugo, RomanBrodetski and AnastasiiaVashchuk

February 7, 2024 14:58

EmilLuta added 5 commits

February 7, 2024 16:05


          typo

00a7358


          fix sqlx data check

58e543a


          TS lints

11f557c


          Fix cargo sqlx prepare

8ca862e


          zk fmt for typescript

5817f6f

Deniallugo reviewed

View reviewed changes

infrastructure/zk/src/database.ts Outdated Show resolved Hide resolved

infrastructure/zk/src/database.ts Show resolved Hide resolved

infrastructure/zk/src/database.ts Outdated Show resolved Hide resolved

RomanBrodetski previously approved these changes

View reviewed changes

Collaborator

RomanBrodetski left a comment

Looks really good!

infrastructure/zk/src/database.ts Outdated Show resolved Hide resolved

infrastructure/zk/src/database.ts Show resolved Hide resolved

prover/prover_dal/migrations/20240131134938_initial-prover-migration.up.sql Show resolved Hide resolved

AnastasiiaVashchuk reviewed

View reviewed changes

infrastructure/zk/src/database.ts Outdated Show resolved Hide resolved

AnastasiiaVashchuk reviewed

View reviewed changes

infrastructure/zk/src/database.ts Outdated Show resolved Hide resolved

popzxc reviewed

View reviewed changes

infrastructure/zk/src/init.ts Outdated Show resolved Hide resolved

EmilLuta added 2 commits

February 9, 2024 14:37


          Address PR comments

a98385e

In no specific order
- enforce you can add a migration to only 1 database at a time
- undo functionality breakage for external-nodes
- rename dalDb to dbUrl
- add enum for dalPath
- add comments in postgresql for prover tables
- remove unnecessary comments
- explicitly setup which databases are manipulated from other files (function calls)


          Satisfy zk fmt for TS

c7ba09a

EmilLuta dismissed RomanBrodetski’s stale review via

c7ba09a

February 9, 2024 13:44

EmilLuta added 2 commits

February 9, 2024 14:49


          Remove unnecessary 'main' from logging


          Restore template functionality

58adb5a

EmilLuta commented

View reviewed changes

infrastructure/zk/src/database.ts Show resolved Hide resolved

EmilLuta commented

View reviewed changes

infrastructure/zk/src/hyperchain_wizard.ts Show resolved Hide resolved

EmilLuta commented

View reviewed changes

infrastructure/zk/src/server.ts Show resolved Hide resolved

AnastasiiaVashchuk reviewed

View reviewed changes

infrastructure/zk/src/database.ts Outdated Show resolved Hide resolved

AnastasiiaVashchuk previously approved these changes

View reviewed changes


          Replace hardcoded strings with purpose built enum

bbd0a64

EmilLuta dismissed AnastasiiaVashchuk’s stale review via

bbd0a64

February 9, 2024 15:21

popzxc reviewed

View reviewed changes

infrastructure/zk/src/database.ts Outdated Show resolved Hide resolved

infrastructure/zk/src/database.ts Outdated Show resolved Hide resolved

infrastructure/zk/src/database.ts Outdated Show resolved Hide resolved

EmilLuta added 2 commits

February 12, 2024 22:24


          Strongly type CLI usage

9b91cc0


          Zk fmt

5c40335

RomanBrodetski approved these changes

View reviewed changes

popzxc approved these changes

View reviewed changes

EmilLuta added this pull request to the merge queue

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

EmilLuta added this pull request to the merge queue

github-merge-queue bot pushed a commit that referenced this pull request


          fix(prover): Decouple core/ prover database management (#1029)

b7912a1

This is PR 1 out of many PRs to come. In first step, prover database
needs to be managed by code. The changes adds a new DAL/ migration for
prover that initializes the database with all the needed tables (as
observed in production).

Additional changes are made to `zk` to not break local development. Note
that from this point forward, development will be done on 2 databases
(as opposed to 1 database model). Alongside these changes, stage env
will be moved to dual database model.

This has been long overdue (8+ months) after core and prover has been
split from infra side during an outage. This is the continuation (and
with following PRs, finalization) of said effort.

Note that whilst there's no automated proper testing of the typescript
code modified, I've reran and tested working condition locally manually
multiple times. I expect no breakage, but can't guarantee it.

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

EmilLuta added this pull request to the merge queue

Merged via the queue into main with commit 37674fd

36 checks passed

EmilLuta deleted the evl-separate-core-from-prover-migrations branch

February 13, 2024 13:31

zksync-era-bot mentioned this pull request

chore(main): release prover 12.0.0 #970

Merged

EmilLuta added a commit that referenced this pull request


          fix: Clean prover tables from core database

45e92cd

This is PR 2 in the Prover/ Core DAL split (#1029).

This PR cleans unused/ unnnecessary tables that currently exist in
core database. All these tables have been officially moved to prover
database. All environemnts have been migrated to using two databases
(core db for core components, prover db for prover components).

This has long been a tech debt and source of confusion for anyone trying
to understand the system. The cleanup gets rid of "witness_inputs_fri
from which database, again?" or "I don't see it in the tables;
Did you connect to the right database?" kind of questions.

EmilLuta mentioned this pull request

fix: Clean prover tables from core database #1099

Closed

github-merge-queue bot pushed a commit that referenced this pull request


          chore(main): release prover 12.0.0 (#970)

7b54689

🤖 I have created a release *beep* *boop*
---


##
[12.0.0](prover-v11.0.0...prover-v12.0.0)
(2024-03-04)


### ⚠ BREAKING CHANGES

* **prover:** Add EIP4844 support for provers subsystem
([#1200](#1200))
* Set 21 as latest protocol version
([#1262](#1262))

### Features

* Adding ability to generate 4844 setup key and refactor
([#1143](#1143))
([975f54b](975f54b))
* **api:** Remove unused and obsolete token info
([#1071](#1071))
([e920897](e920897))
* **dal:** `zksync_types::Transaction` to use protobuf for wire encoding
(BFT-407)
([#1047](#1047))
([ee94bee](ee94bee))
* **db:** Soft-remove `storage` table
([#982](#982))
([601f893](601f893))
* **en:** Integrate snapshots recovery into EN
([#1032](#1032))
([c7cfaf9](c7cfaf9))
* **healthcheck:** Various healthcheck improvements
([#1166](#1166))
([1e34148](1e34148))
* improving verification key generation
([#1050](#1050))
([6f715c8](6f715c8))
* Prover interface and L1 interface crates
([#959](#959))
([4f7e107](4f7e107))
* **prover:** Add EIP4844 support for provers subsystem
([#1200](#1200))
([6953e89](6953e89))
* **prover:** Added --recompute-if-missing option to key generator
([#1151](#1151))
([cad7278](cad7278))
* **prover:** Added 4844 circuit to verification keys
([#1141](#1141))
([8b0cc4a](8b0cc4a))
* **prover:** Adding first support for 4844 circuit
([#1155](#1155))
([6f63c53](6f63c53))
* **prover:** adding keystore object to handle reading and writing of
prover keys
([#1132](#1132))
([1471615](1471615))
* **prover:** merging key generation into a single binary
([#1101](#1101))
([6de8b84](6de8b84))
* **prover:** Moved setup key generation logic to test harness
([#1113](#1113))
([469ab06](469ab06))
* **prover:** Use new shivini function for 4844 circuits
([#1205](#1205))
([376c09e](376c09e))
* Set 21 as latest protocol version
([#1262](#1262))
([30579ef](30579ef))
* **vlog:** Remove env getters from vlog
([#1077](#1077))
([00d3429](00d3429))


### Bug Fixes

* fix link
([#1007](#1007))
([f1424ce](f1424ce))
* make `zk status prover` use the new prover table
([#1044](#1044))
([9b21d7f](9b21d7f))
* **prover:** Decouple core/ prover database management
([#1029](#1029))
([37674fd](37674fd))
* **prover:** Fix initial prover migration
([#1083](#1083))
([6d54010](6d54010))
* **prover:** QoL socket utilization
([#1020](#1020))
([13a6816](13a6816))
* update harness to include fix to new boojum OOM
([#1053](#1053))
([4976941](4976941))


### Performance Improvements

* bump harness version
([#1003](#1003))
([1cbb4c9](1cbb4c9))
* reduce memory consumption of witness generation
([#696](#696))
([dea6768](dea6768))
* upgrade harness version to improve witness generation memory spike
([#1034](#1034))
([09bbb84](09bbb84))
* use jemalloc in witness generator
([#1014](#1014))
([917b2dc](917b2dc))

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

Reviewers

Deniallugo Deniallugo left review comments

githubdoramon githubdoramon left review comments

RomanBrodetski RomanBrodetski approved these changes

popzxc popzxc approved these changes

AnastasiiaVashchuk AnastasiiaVashchuk left review comments