Estimate compression ratio at rollupFee for Feynman #1197

ranchalp · 2025-06-09T10:28:42Z

1. Purpose or design rationale of this PR

See #1196 for more.

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

coderabbitai · 2025-06-09T10:28:49Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

Thegaram · 2025-06-10T09:52:56Z

rollup/fees/rollup_fee.go

+	// Default compression ratio is 1.0 (no compression)
+	compressionRatioInt := big.NewInt(rcfg.Precision.Int64())
+
+	if len(data) != 0 {


Is this zero check needed? This should never happen, plus this error case is covered inside CompressScrollBatchBytes.

True, it is already in CompressScrollBatchBytes. Removed.

rollup/fees/rollup_fee.go

Thegaram · 2025-06-10T09:55:09Z

rollup/fees/rollup_fee.go

+	compressionRatioInt := big.NewInt(rcfg.Precision.Int64())
+
+	if len(data) != 0 {
+		compressedBytes, err := zstd.CompressScrollBatchBytes(data)


Fyi this will add 128 byte overhead (compressBufferOverhead).

should we just subtract these 128 bytes from the result then and continue using our own zstd implementation (probably in a wrapper function)?

That could work, though it leaks implementation details of da-codec.

Thegaram · 2025-06-10T09:55:50Z

rollup/fees/rollup_fee.go

+	compressionRatioInt := big.NewInt(rcfg.Precision.Int64())
+
+	if len(data) != 0 {
+		compressedBytes, err := zstd.CompressScrollBatchBytes(data)


I think we can just use any go zstd lib, don't need to use our da-codec, what do you think? @colinlyguo

What are the motivations to not use our own zstd lib? the 128 bytes overhead?

Please sync with Colin, this is something we want to do in other codebases too. Circuits can now support the whole zstd spec so we don't need to hardcode using a specific implementation (true with some caveats).

Yeah. We can use any Go Zstd lib. And a reminder is that the spec can also be used in Reth, for constructing historical blocks from DA.

https://github.com/DataDog/zstd, https://github.com/valyala/gozstd. These libraries look good (listed in https://facebook.github.io/zstd/), the implementation is the official zstd C implementation.

A historically symbol conflict issue: https://scrollco.slack.com/archives/C02PKAQ2430/p1720694878378479

Pure Go implementation, such as https://github.com/klauspost/compress/tree/master/zstd#zstd won't have any issues with C's global symbol conflicts when loading multiple zstd libraries. But I'm afraid Rust cannot have an identical implementation to reproduce the same compression ratio.

Also, pls double-check if a version of zstd running on different operating systems will produce the same results.

zstd is deterministic given same input, same set of parameters and same version.

Got it and thanks!

Thegaram · 2025-06-10T09:57:56Z

rollup/fees/rollup_fee.go

+			compressionRatioInt.Mul(compressedSize, rcfg.Precision)
+			compressionRatioInt.Div(compressionRatioInt, txSize)


While I believe it's always true that compressedSize <= size, I'm not 100% convinced this is the case. I suggest you add a check to cover this

good catch, also not 100% sure. Done

Thegaram · 2025-06-10T09:58:11Z

rollup/fees/rollup_fee.go

-	// compression_ratio(tx) = 1 (placeholder, scaled to match scalars precision)
-	compressionRatio := big.NewInt(rcfg.Precision.Int64())
+	// Default compression ratio is 1.0 (no compression)
+	compressionRatioInt := big.NewInt(rcfg.Precision.Int64())


Why rename to compressionRatioInt?

Fair, removed.

Co-authored-by: Péter Garamvölgyi <[email protected]>

ranchalp · 2025-06-10T20:25:31Z

Waiting for a decision on which zstd library to use (I am not sure I understand why not just going with our own?), otherwise comments resolved.

ranchalp · 2025-06-12T13:39:36Z

Following the discussion here, I reverted the use of a standard go zstd library in favour for the da-codec's version.

jonastheis

Please revert all the whitespace changes. Next to being a pain when reviewing it also increases the diff.

Good common practice is to check files and their content before committing to avoid such whitespace changes from eg linter and to make sure what you're committing is actually sensible.

jonastheis · 2025-06-19T09:15:19Z

rollup/fees/rollup_fee.go

 	compressionRatio := big.NewInt(rcfg.Precision.Int64())

+	compressedBytes, err := zstd.CompressScrollBatchBytes(data)
+	if err != nil {
+		log.Error("Batch compression failed, using 1.0 compression ratio", "err", err)


adjust the log message and would be good to also add the tx hash to the log message here to make it more meaningful.

jonastheis · 2025-06-19T09:17:59Z

rollup/fees/rollup_fee.go

 	compressionRatio := big.NewInt(rcfg.Precision.Int64())

+	compressedBytes, err := zstd.CompressScrollBatchBytes(data)


might still be good to use the zstd version that comes with da-codec. maybe we need to introduce a new function there to get rid of the overhead

ranchalp added the breaking-change label Jun 9, 2025

Estimate compression ratio

4c1dc4d

ranchalp force-pushed the rollupfee-estimate-compression branch from 9d4f5bc to 4c1dc4d Compare June 9, 2025 10:31

ranchalp requested review from Thegaram and jonastheis June 9, 2025 13:18

Thegaram reviewed Jun 10, 2025

View reviewed changes

ranchalp and others added 3 commits June 10, 2025 22:18

Merge branch 'l1data-to-rollupfee' into rollupfee-estimate-compression

1a3db6c

Update rollup/fees/rollup_fee.go

459ba38

Co-authored-by: Péter Garamvölgyi <[email protected]>

Check compressed size smaller and remove unnecessary checks/changes

e3c5bd7

ranchalp marked this pull request as ready for review June 10, 2025 20:27

ranchalp force-pushed the l1data-to-rollupfee branch from d0e730b to ce2498e Compare June 11, 2025 16:00

ranchalp force-pushed the rollupfee-estimate-compression branch from b83f7dc to e3c5bd7 Compare June 12, 2025 13:38

ranchalp added 2 commits June 12, 2025 16:07

Merge branch 'l1data-to-rollupfee' into rollupfee-estimate-compression

89ee137

goimports locally run

375ab14

jonastheis reviewed Jun 19, 2025

View reviewed changes

		compressionRatioInt.Mul(compressedSize, rcfg.Precision)
		compressionRatioInt.Div(compressionRatioInt, txSize)

		compressionRatio := big.NewInt(rcfg.Precision.Int64())

		compressedBytes, err := zstd.CompressScrollBatchBytes(data)

Estimate compression ratio at rollupFee for Feynman #1197

Are you sure you want to change the base?

Estimate compression ratio at rollupFee for Feynman #1197

Uh oh!

Conversation

ranchalp commented Jun 9, 2025

1. Purpose or design rationale of this PR

2. PR title

3. Deployment tag versioning

4. Breaking change label

Uh oh!

coderabbitai bot commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranchalp Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

colinlyguo Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranchalp Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranchalp commented Jun 10, 2025

Uh oh!

ranchalp commented Jun 12, 2025

Uh oh!

jonastheis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonastheis Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot commented Jun 9, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

ranchalp Jun 10, 2025 •

edited

Loading

colinlyguo Jun 11, 2025 •

edited

Loading

ranchalp Jun 10, 2025 •

edited

Loading

jonastheis Jun 19, 2025 •

edited

Loading