-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
fjord: Fastlz GasPriceOracle #10534
fjord: Fastlz GasPriceOracle #10534
Conversation
49c7f7f
to
ab64193
Compare
ab64193
to
5219399
Compare
WalkthroughWalkthroughThis update introduces the "Fjord" allocation mode alongside the existing "Ecotone" mode across various components. Changes include configuration file updates, initialization script modifications, smart contract enhancements, and test additions to incorporate the new allocation logic, time conditions, and fee calculations associated with the Fjord mode. Changes
Possibly Related Issues
Recent Review DetailsConfiguration used: .coderabbit.yml Files ignored due to path filters (1)
Files selected for processing (2)
Additional comments not posted (11)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 2
Out of diff range and nitpick comments (5)
bedrock-devnet/devnet/__init__.py (5)
Line range hint
7-7
: Remove unused importcalendar
.The
calendar
module is imported but not used in the code. Removing it will clean up the code.-import calendar
Line range hint
12-12
: Remove unused importgzip
.The
gzip
module is imported but not used in the code. Removing it will clean up the code.-import gzip
Line range hint
18-18
: Remove unused importdevnet.log_setup
.The
devnet.log_setup
module is imported but not used in the code. Removing it will clean up the code.-import devnet.log_setup
Line range hint
221-221
: Avoid equality comparisons toFalse
.Use
if not os.path.exists(l2_allocs_path):
instead ofif os.path.exists(l2_allocs_path) == False:
for better readability.- if os.path.exists(l2_allocs_path) == False or DEVNET_FPAC == True: + if not os.path.exists(l2_allocs_path) or DEVNET_FPAC:
Line range hint
310-310
: Remove unused variablee
.The variable
e
is assigned but never used. Removing it will clean up the code.except Exception as e: - raise RuntimeError(f"Error executing '{' '.join(command.args)}': {e}") + raise RuntimeError(f"Error executing '{' '.join(command.args)}'")
Just small nits, generally looks good to me |
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.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
packages/contracts-bedrock/src/L2/GasPriceOracle.sol (1)
29-31
: Update the version constant to reflect the new version.Ensure that the version number is updated consistently across all relevant documentation and deployment scripts.
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.
Still LGTM!
52abfb5
Description
This PR adds support for improving the estimation of L1 fees by compressing the transaction with fastlz, then using a linear regression to estimate the final Brotli 10 compressed size.
Tests