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.

rollup/fees/rollup_fee.go

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.

rollup/fees/rollup_fee.go

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

rollup/fees/rollup_fee.go

+	// Default compression ratio is 1.0 (no compression)
 	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

Updated the da-codec version to codecv8's zstd compression: f31326d.

might still be good to use the zstd version that comes with da-codec

The call stack would be a bit counter-intuitive here. calculateEncodedL1DataFeeFeynman is called based on IsCurie and IsFeynman checks.

I can think of two refactoring ways:

Passing blockNumber and blockTime as parameters of calculateEncodedL1DataFeeFeynman, or determining it outside and passing the compression function as a parameter. This will require remembering to update this util function in da-codec repo, which is the same as updating here in future upgrades.

Or another solution would be to expose a new interface based on the da-codec version, dedicated to compression ratio estimation here (determining the fees).

Do either of these approaches align with your thoughts, or do you have a different method in mind?

I don't understand your point about the counter-intuitive call stack.

But I agree we should:

Pass compressionRatio as parameter for easier testing, and

Expose an estimate tx compression ratio function in da-codec

Replied to the comment here: #1196 (comment).

accounts/abi/reflect.go

rollup/fees/rollup_fee.go

* Change l1DataFee to rollupFee for Feynman * Update CommitScalarSlot to ExecScalarSlot * Rename l1DataFee to rollupFee * Scale compression ratio to match scalars precision * Use l1DataFee and commitScalar instead of rollupFee and execScalar * Use blocktime not blocknumber * Add unit test * Clarify test comment * feat(feynman): upgrade gas oracle predeploy * address comments * revert renaming CalculateL1DataFee to CalculateRollupFee * simplify EstimateL1DataFeeForMessage * address comments * add a comment based on pr reviews * update formula * tweaks * tweak comments * Estimate compression ratio at rollupFee for Feynman (#1197) * Estimate compression ratio * Update rollup/fees/rollup_fee.go Co-authored-by: Péter Garamvölgyi <[email protected]> * Check compressed size smaller and remove unnecessary checks/changes * goimports locally run * rollback format changes * update da-codec's zstd function * apply zstd * tweaks * tweak unit test * make sure compression ratio >= 1 * nit --------- Co-authored-by: Péter Garamvölgyi <[email protected]> Co-authored-by: colinlyguo <[email protected]> Co-authored-by: colin <[email protected]> * refactoring * remove incorrect merge artifact * error handling * update da-codec commit * update da-codec commit --------- Co-authored-by: Péter Garamvölgyi <[email protected]> Co-authored-by: colin <[email protected]> Co-authored-by: colinlyguo <[email protected]>

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

georgehao reviewed Jun 24, 2025

View reviewed changes

accounts/abi/reflect.go Outdated Show resolved Hide resolved

colinlyguo force-pushed the rollupfee-estimate-compression branch from 434dbf6 to 185d45e Compare June 24, 2025 19:23

rollback format changes

bfcbb4b

colinlyguo force-pushed the rollupfee-estimate-compression branch from 185d45e to bfcbb4b Compare June 24, 2025 19:24

colinlyguo and others added 6 commits June 25, 2025 04:14

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

ef9f04e

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

c79cf76

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

b0f3ab2

update da-codec's zstd function

f31326d

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

dbd2275

apply zstd

0346191

colinlyguo force-pushed the rollupfee-estimate-compression branch from b6e7b53 to 0346191 Compare June 24, 2025 21:59

tweaks

11dd73e

colinlyguo and others added 2 commits June 25, 2025 06:09

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

6c97803

tweak unit test

d754ddd

georgehao approved these changes Jun 25, 2025

View reviewed changes

Thegaram reviewed Jun 25, 2025

View reviewed changes

rollup/fees/rollup_fee.go Show resolved Hide resolved

rollup/fees/rollup_fee.go Show resolved Hide resolved

colinlyguo and others added 3 commits June 25, 2025 16:55

make sure compression ratio >= 1

e96378c

nit

bc2cd28

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

02b6972

Thegaram approved these changes Jun 25, 2025

View reviewed changes

Thegaram merged commit 872ccf5 into l1data-to-rollupfee Jun 25, 2025
10 checks passed

Thegaram deleted the rollupfee-estimate-compression branch June 25, 2025 10:17

colinlyguo mentioned this pull request Jun 25, 2025

feat(feynman): add new compression-aware rollup fee formula #1196

Merged

13 tasks

Estimate compression ratio at rollupFee for Feynman #1197

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

colinlyguo Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

colinlyguo Jun 24, 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!

Thegaram Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

colinlyguo Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

coderabbitai bot commented Jun 9, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

jonastheis Jun 19, 2025 •

edited

Loading

colinlyguo Jun 24, 2025 •

edited

Loading