Skip to content
This repository has been archived by the owner on Jun 9, 2024. It is now read-only.

fix(miner): Do not use pending block for now #1255

Merged
merged 3 commits into from
Oct 30, 2023
Merged

Conversation

itsdevbear
Copy link

@itsdevbear itsdevbear commented Oct 27, 2023

Summary by CodeRabbit

  • Refactor: Updated the method of retrieving blocks and their states from the blockchain. This change enhances the accuracy of data by fetching the current block instead of the miner's pending block.
  • User Impact: This update ensures users receive the most up-to-date and accurate information from the blockchain, improving the reliability of the data they interact with.

@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2023

Walkthrough

The changes primarily focus on modifying the retrieval of blocks and their states in the Ethereum backend. The logic now uses the current block header from the blockchain instead of the miner's pending block, ensuring more accurate and up-to-date data retrieval.

Changes

File Path Summary
.../api_backend.go The BlockByNumber and StateAndHeaderByNumber functions have been updated to retrieve the block and its state based on the current block header instead of the miner's pending block.

🐇

"In the world of code, where blocks align,

Changes come, as stars do shine.

No more pending, we're up-to-date,

With current headers, we navigate. 🌟"


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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 help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

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.json

Copy link

@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 d34a732 and 065e21e.
Files selected for processing (1)
  • eth/polar/api_backend.go (2 hunks)
Additional comments: 4
eth/polar/api_backend.go (4)
  • 246-247: The new code fetches the current block from the blockchain instead of the miner's pending block when the block number corresponds to the pending block. This change seems to be aimed at avoiding potential inconsistencies or errors that might arise from using a pending block. However, it's important to ensure that this change doesn't break any functionality that relies on the pending block in this context.

  • 312-320: Similar to the previous hunk, the new code fetches the current block header from the blockchain instead of the miner's pending state and header when the block number corresponds to the pending block. Again, verify that this change doesn't break any functionality that relies on the pending state and header in this context.

  • 324-327: The error handling here is good, as it ensures that any error encountered while resolving the block number and retrieving the header is properly returned. This allows the caller to handle the error appropriately.

  • 329-331: The check for a nil header and the corresponding error return is a good practice. It ensures that the function doesn't proceed with a nil header, which could lead to a runtime panic. The use of a predefined error (core.ErrBlockNotFound) also helps with consistency and potential error handling by the caller.

Copy link

@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 065e21e and c267b46.
Files selected for processing (1)
  • eth/polar/api_backend.go (2 hunks)
Additional comments: 2
eth/polar/api_backend.go (2)
  • 246-247: The new code retrieves the current block from the blockchain instead of the pending block from the miner. This change could potentially affect the behavior of any code that relies on the pending block. Ensure that this change does not introduce any unintended side effects.
- block := b.polar.miner.PendingBlock()
- return block, nil
+ header := b.polar.blockchain.CurrentBlock()
+ return b.polar.blockchain.GetBlock(header.Hash(), header.Number.Uint64()), nil
  • 312-319: Similar to the previous comment, the new code retrieves the current block from the blockchain instead of the pending state from the miner. This change could potentially affect the behavior of any code that relies on the pending state. Ensure that this change does not introduce any unintended side effects.
- block, state := b.polar.miner.Pending()
- return state, block.Header(), nil
+ header = b.polar.blockchain.CurrentBlock()

@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Merging #1255 (94f4206) into main (7dd6df9) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1255   +/-   ##
=======================================
  Coverage   49.22%   49.22%           
=======================================
  Files          77       77           
  Lines        4642     4642           
=======================================
  Hits         2285     2285           
  Misses       2195     2195           
  Partials      162      162           

@itsdevbear itsdevbear merged commit 9c25a75 into main Oct 30, 2023
14 checks passed
@itsdevbear itsdevbear deleted the pending-block-rm branch October 30, 2023 16:11
itsdevbear pushed a commit that referenced this pull request Oct 31, 2023
itsdevbear pushed a commit that referenced this pull request Oct 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant