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

fix: exclude L1 message from block payload size validation #476

Merged
merged 5 commits into from
Aug 23, 2023

Conversation

Thegaram
Copy link

1. Purpose or design rationale of this PR

Currently, both worker and block_validator consider L1 messages when enforcing the block payload size limit. This is wrong (an unnecessary over-estimation), since L1 messages are not part of the batch payload.

It is risky to merge this fix in this form. It has the risk that we produce a block that's valid according to the updated size limit, but is still rejected by old-version nodes. This risk is limited by the fact that we currently only allow 10 L1 messages per block. An alternative, safer roll-out is:

  1. First merge only the block_validator part.
  2. Once most nodes have upgraded, merge the worker part.

2. PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

  • fix: A bug fix

3. Deployment tag versioning

Has the version in params/version.go been updated?

  • This PR doesn't involve a new deployment, git tag, docker image tag, and it doesn't affect traces
  • Yes

4. Breaking change label

Does this PR have the breaking-change label?

  • This PR is not a breaking change
  • Yes

@Thegaram Thegaram marked this pull request as ready for review August 23, 2023 11:25
HAOYUatHZ
HAOYUatHZ previously approved these changes Aug 23, 2023
mask-pp and others added 2 commits August 23, 2023 19:48
* fix bug when calculate l2 tx count

* Update version
@Thegaram Thegaram merged commit 727f88b into develop Aug 23, 2023
5 checks passed
@Thegaram Thegaram deleted the exclude-l1-msg-from-block-size-limit branch August 23, 2023 12:06
Thegaram added a commit that referenced this pull request Aug 25, 2023
* fix: exclude L1 message from block payload size validation

* fix the bug when calculating l2TxCount. (#479)

* fix bug when calculate l2 tx count

* Update version

* bump version

---------

Co-authored-by: maskpp <maskpp266@gmail.com>
Thegaram added a commit that referenced this pull request Aug 25, 2023
* feat: store skipped txs in local db

* bump version

* bump version

* fix test

* include L2 txs in skipped index

* goimports

* rename more

* bump version

* fix missed renames

* fix the bug when calculating l2TxCount. (#479)

* fix bug when calculate l2 tx count

* Update version

* fix: exclude L1 message from block payload size validation (#476)

* fix: exclude L1 message from block payload size validation

* fix the bug when calculating l2TxCount. (#479)

* fix bug when calculate l2 tx count

* Update version

* bump version

---------

Co-authored-by: maskpp <maskpp266@gmail.com>

* fix: update row estimation with scroll-prover `v0.7.2` (#475)

* Fix row estimation.

* Update libzkp.

* more

* prepare

* finish

* upgrade

* bump version

* fix tests

* Update to scroll-prover `v0.7.2`.

* fix tests

* Update miner/worker.go

Co-authored-by: Péter Garamvölgyi <peter@scroll.io>

* Update miner/worker.go

Co-authored-by: Péter Garamvölgyi <peter@scroll.io>

* Reset ccc when skips first tx.

* do not unnecessarily skip L1 message

* fix ccc reset and improve code readability

* seal block on circuitcapacitychecker.ErrUnknown

---------

Co-authored-by: HAOYUatHZ <haoyu@protonmail.com>
Co-authored-by: Péter Garamvölgyi <peter@scroll.io>

* refactor: simplify ccc revert to snapshot (#480)

* simplify ccc revert to snapshot

* Update version.go

---------

Co-authored-by: HAOYUatHZ <37070449+HAOYUatHZ@users.noreply.github.com>

* feat: use --gcmode=archive and --cache.noprefetch=true by default (#482)

* feat: use --gcmode=archive and --cache.noprefetch=true by default

* refuse to start with invalid config

* lint

* lint

* feat(sdk): support compressed response (#469)

* enable use compression algorithm

* fix ci

* Just enable decode compressed content at ethclient

* fix comments

---------

Co-authored-by: Haichen Shen <shenhaichen@gmail.com>

* fix: disable pruning and prefetch if not flags are provided (#483)

Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com>

* fix: update libzkp to use scroll-prover `v0.7.5` (#484)

* Update libzkp to use scroll-prover `v0.7.5`.

* Update version.

* address comments

* bump version

* nit

---------

Co-authored-by: maskpp <maskpp266@gmail.com>
Co-authored-by: Steven <asongala@163.com>
Co-authored-by: HAOYUatHZ <haoyu@protonmail.com>
Co-authored-by: iczc <iczcalan@gmail.com>
Co-authored-by: HAOYUatHZ <37070449+HAOYUatHZ@users.noreply.github.com>
Co-authored-by: Haichen Shen <shenhaichen@gmail.com>
Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants