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

chore(libzkp): upgrade to v0.9.10, optimize ccc for follower&chunking #676

Merged
merged 2 commits into from
Mar 22, 2024

Conversation

lispc
Copy link

@lispc lispc commented Mar 21, 2024

1. Purpose or design rationale of this PR

The poseidon circuit row usage is difficult to estimate fast. So previously we used some conservative estimation. Now we caliberated this estimation method using mainnet real metics. After upgrading this, total gas per chunk can be improved so we need less chunk provers.

Original commit: scroll-tech/zkevm-circuits@c625e05.

change a coefficient from 12 to 6, with the avg/mean is below 1:

mpt-ratio

(btw v0.9.9 is "add verification after chunk proof generated", so not related to l2geth.)

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:

  • build: Changes that affect the build system or external dependencies (example scopes: yarn, eslint, typescript)
  • ci: Changes to our CI configuration files and scripts (example scopes: vercel, github, cypress)
  • docs: Documentation-only changes
  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that doesn't fix a bug, or add a feature, or improves performance
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • test: Adding missing tests or correcting existing tests

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

So for this change, we'll need to update follower nodes before the signer node, right?

What do we need to do/test/check before we can merge this?

@lispc
Copy link
Author

lispc commented Mar 22, 2024

upgrade follower node (whose ccc results are used for chunk proposer). Whether or not to upgrade signer will be ok.

I think along with current follower node, we can launch another follower with this new version, and the expected behavior is, for same block we fetch ccc results using the get_rowConsumption API, and we will have two map[string]int. For the 2 row consumptions, "poseidon" row usage should be smaller for new version, and row usages should remain same.

@lispc lispc marked this pull request as ready for review March 22, 2024 07:52
@Thegaram
Copy link

Whether or not to upgrade signer will be ok.

Follower nodes who enable CCC will use it during block validation. So in this case, we need to update all follower nodes before the signer, otherwise signer could create a block that's not accepted by the (more conservative) follower ccc.

we can launch another follower with this new version...

As discussed on Slack, we will upgrade the follower node to the new version directly.

@Thegaram Thegaram merged commit e9246f1 into develop Mar 22, 2024
6 checks passed
@Thegaram Thegaram deleted the feat/opt-ccc-for-follower branch March 22, 2024 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants