-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(v30): Remove custom DA #3970
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
Conversation
…r-labs/zksync-era into vg/v29-upgrade-process
…r-labs/zksync-era into vg/v29-upgrade-process
…into vg/v29-upgrade-process
## What ❔ Merges `BatchRootProcessor` and `BatchRootProcessorInterop` into a single one <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- The `Why` has to be clear to non-Matter Labs entities running their own ZK Chain --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Is this a breaking change? - [ ] Yes - [ ] No ## Operational changes <!-- Any config changes? Any new flags? Any changes to any scripts? --> <!-- Please add anything that non-Matter Labs entities running their own ZK Chain may need to know --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev lint`.
dimazhornyk
left a comment
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.
didn't have a chance to review everything, but here is a fix for the issue raised earlier
6de3fc5 to
f34e690
Compare
Signed-off-by: Danil <[email protected]>
f34e690 to
3ddddd8
Compare
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
7dc493d to
8bd46bc
Compare
Signed-off-by: Danil <[email protected]>
8bd46bc to
0c65984
Compare
## What ❔ If received batch details contain None/Null chain_id (commit_chain_id, prove_chain_id, execute_chain_id) set it to config provided chain id for L1. This happens for old batches. ## Why ❔ If you try syning EN from scratch with a "old" chain you will encounter following error: ``` Error: One or more tasks failed: ["Task transaction_finality_updater failed: No batch transactions to process for chain id 1 while there are some for None chain_id.Error is thrown so node can restart and reload SL data. If node doesn't make any progress after restart, then it's bug, please contact developers."] ``` Logic for fetching batch transactions to process their finality status assumes chain_ids are set properly and no transaction should have chain_id = None. However this is the case for old chains. When fetching they return *_chain_id: null in zks_getL1BatchDetails for some old batches. ## Is this a breaking change? - [ ] Yes - [ ] No ## Operational changes <!-- Any config changes? Any new flags? Any changes to any scripts? --> <!-- Please add anything that non-Matter Labs entities running their own ZK Chain may need to know --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev lint`.
## What ❔ During the reorg detection take into account not only root hash, but the whole commitment ## Why ❔ We have at least one example, where during wrong execution we have correct root hash, but wrong commitment ## Is this a breaking change? - [ ] Yes - [ ] No ## Operational changes <!-- Any config changes? Any new flags? Any changes to any scripts? --> <!-- Please add anything that non-Matter Labs entities running their own ZK Chain may need to know --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev lint`. --------- Signed-off-by: Danil <[email protected]>
dimazhornyk
left a comment
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.
looks good! usually the most problematic part with DA is ENs, but everything seems to be fine here
| @@ -236,7 +236,7 @@ pub async fn deploy_l2_da_validator( | |||
| shell: &Shell, | |||
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.
can we completely remove this function? it is not needed in v30 if I understand correctly
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.
No, we still need to deploy it...
## What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- The `Why` has to be clear to non-Matter Labs entities running their own ZK Chain --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Is this a breaking change? - [ ] Yes - [ ] No ## Operational changes <!-- Any config changes? Any new flags? Any changes to any scripts? --> <!-- Please add anything that non-Matter Labs entities running their own ZK Chain may need to know --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev lint`. Signed-off-by: Danil <[email protected]>
## What ❔ Only apply seal criteria checks through PanicSealer on >=v29 protocol ## Why ❔ Some older protocol versions had higher circuit (and maybe other) limits that are no longer avilable in code constants ## Is this a breaking change? - [ ] Yes - [ ] No ## Operational changes <!-- Any config changes? Any new flags? Any changes to any scripts? --> <!-- Please add anything that non-Matter Labs entities running their own ZK Chain may need to know --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev lint`.
🤖 I have created a release *beep* *boop* --- ## [29.3.0](core-v29.2.0...core-v29.3.0) (2025-09-18) ### Features * **en:** Check commitment allonside with roothashes ([#4492](#4492)) ([6e8f526](6e8f526)) * **en:** Improved en commitment generation error handling ([#4483](#4483)) ([1bb82d6](1bb82d6)) ### Bug Fixes * **en:** Add check for commitment in diverged batch ([#4493](#4493)) ([e693fd0](e693fd0)) * **en:** allow to disable sanity checks for commitment generation ([#4487](#4487)) ([7692f31](7692f31)) * **EN:** properly set chain_id for old batch transactions ([#4488](#4488)) ([634b6dd](634b6dd)) * **eth-proof-manager:** don't fail component if proof was incorrect ([#4444](#4444)) ([daa1161](daa1161)) * **eth-proof-manager:** verify protocol version and proving mode alignment ([#4473](#4473)) ([99e0423](99e0423)) * **eth-sender:** Fix selector for old batches ([#4489](#4489)) ([bb42dcf](bb42dcf)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: zksync-era-bot <[email protected]>
⚠ Detected differing instruction counts
Changes in number of opcodes executed indicate that the gas price of the benchmark has changed, which causes it to run out of gas at a different time. |
What ❔
Instead of using custom DA validators, now we have a specific set of possible DA validators and we use them instead.
Why ❔
It's a rare occasion to introduce new DA validator
Is this a breaking change?
Operational changes
Checklist
zkstack dev fmtandzkstack dev lint.