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

feat: merge dev into main #267

Merged
merged 8 commits into from
Mar 4, 2024
Merged

feat: merge dev into main #267

merged 8 commits into from
Mar 4, 2024

Conversation

seolaoh
Copy link
Contributor

@seolaoh seolaoh commented Mar 3, 2024

Description

This is for release v1.3.2, which bumps geth version to v0.4.4.

Summary by CodeRabbit

  • Bug Fixes

    • Adjusted deposit amount recording logic for better accuracy in metrics.
    • Updated DeltaTime value in Mainnet configuration for enhanced timing accuracy.
  • Documentation

    • Corrected a typo in the span-batches.md file for clarity in L2 legacy transactions description.

@seolaoh seolaoh requested review from a team as code owners March 3, 2024 03:51
Copy link
Contributor

coderabbitai bot commented Mar 3, 2024

Walkthrough

The recent updates involve modifications across different components, focusing on enhancing accuracy and reliability. The Challenger and L2OutputSubmitter classes now record deposit amounts earlier in their processes for better metrics tracking. The Mainnet configuration has been adjusted with a new DeltaTime value to reflect updated timing requirements. Additionally, a minor typo correction in the span-batches.md document improves the clarity of the specifications concerning legacy transactions.

Changes

Files Change Summary
kroma-validator/... Moved RecordDepositAmount call before balance comparison in HasEnoughDeposit method.
op-node/chaincfg/... Updated DeltaTime in Mainnet config to 1709107200.
specs/span-batches.md Corrected typo in protected_bits field description from "transacion" to "transaction".

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?

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>.
    • Generate unit-tests for this file.
  • 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 generate unit tests for this file.
    • @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 generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • 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/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 9a7e19d and 8bd1b36.
Files ignored due to path filters (3)
  • go.mod is excluded by: !**/*.mod
  • go.sum is excluded by: !**/*.sum
  • ops-devnet/docker-compose.yml is excluded by: !**/*.yml
Files selected for processing (5)
  • kroma-validator/challenger.go (1 hunks)
  • kroma-validator/l2_output_submitter.go (2 hunks)
  • op-node/chaincfg/chains.go (1 hunks)
  • op-node/chaincfg/chains_test.go (1 hunks)
  • specs/span-batches.md (1 hunks)
Additional comments: 5
op-node/chaincfg/chains.go (1)
  • 43-43: The update of DeltaTime to u64Ptr(1709107200) in the Mainnet configuration is noted. Ensure that this new value aligns with the intended time-based operations within the network. Given the nature of this change, it's crucial to verify that all dependent components that rely on DeltaTime have been assessed for compatibility with this new value.
op-node/chaincfg/chains_test.go (1)
  • 70-70: The update of DeltaTime to u64Ptr(1709107200) in the mainnetCfg struct for testing purposes is noted. It's important to ensure that this change is reflected in all relevant test cases that might be affected by the DeltaTime configuration. Additionally, verify that this update aligns with the DeltaTime value set in the main configuration to maintain consistency between test and production environments.
Verification successful

The review of the DeltaTime value update in chains_test.go to u64Ptr(1709107200) has been verified. The consistency of this DeltaTime value between test and main configurations is confirmed, as both chains_test.go and chains.go use this value. Additionally, another DeltaTime value (1708416000) is also consistently used across both test and main configurations, indicating intentional use for different scenarios or configurations rather than an inconsistency. Therefore, the update appears to be correctly implemented and consistent with the main configuration.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify consistency of `DeltaTime` value between test and main configurations.
rg "DeltaTime" --type go

Length of output: 8358

kroma-validator/l2_output_submitter.go (1)
  • 280-280: Moving the RecordDepositAmount call above the balance comparison logic in the HasEnoughDeposit method is a logical improvement. This ensures that the deposit amount is always recorded, regardless of whether the balance meets the required bond amount. It's a good practice to record metrics before making decisions based on those metrics to ensure accurate tracking and analysis. This change enhances the maintainability and clarity of the deposit amount tracking process.
specs/span-batches.md (1)
  • 129-129: The typo correction from "transacion" to "transaction" improves the document's accuracy and clarity.
kroma-validator/challenger.go (1)
  • 609-609: Moving the RecordDepositAmount call above the balance comparison logic is a good practice for ensuring that metrics are accurately recorded before any conditional logic is processed. This change enhances the reliability of deposit amount tracking, which is crucial for monitoring and analysis purposes.

Copy link
Contributor

@kangsorang kangsorang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@seolaoh seolaoh merged commit 0b56892 into main Mar 4, 2024
5 checks passed
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.

6 participants