fix: take care of the case where the requested height is above the head of latest da block height

[yarikbratashchuk]

We need to handle the case where full node goes above the max height of the da, so it's consistent with real world da implementations, and handled correctly by the full node retrieve loop.

Summary by CodeRabbit

• New Features

  • Enhanced error handling for height validation, providing clearer messages for future height requests.
  • Updated test suite to reflect changes in error handling and improve clarity in testing scenarios.

• Bug Fixes

  • Simplified error handling logic in tests to ensure robustness and accuracy in function outcomes.

[coderabbitai]

Walkthrough

The changes involve a significant update to error handling in the DummyDA implementation, specifically modifying how heights are validated. The error constant ErrNoBlobAtHeight has been replaced with ErrTooHigh, which indicates a future height request. The test suite has also been updated to reflect these changes, with a renamed test function and simplified error handling logic.

Changes

Files	Change Summary
test/dummy.go	Replaced ErrNoBlobAtHeight with ErrTooHigh; updated GetIDs method to handle future heights.
test/test_suite.go	Renamed NoBlobsAtHeightTest to HeightFromFutureTest; simplified error handling in tests.

Poem

In fields of green where bunnies play,
A change has come to brighten the day.
Heights once lost in shadows of doubt,
Now leap forward, with joy we shout!
With errors clearer, we hop with glee,
New paths await, come dance with me! 🐇✨

---

Tip

Early access features: disabled

We are currently testing the following features in early access:

• Anthropic claude-3-5-sonnet for code reviews: The new Claude model has stronger code understanding and code generation capabilities than the previous models. We're excited to hear your feedback as we evaluate its performance over the next few days.

Note:

• You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.
• Please join our Discord Community to provide feedback and report issues on the discussion post.

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

• X
• Mastodon
• Reddit
• LinkedIn

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 testing code for this file.
  • 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 generate unit testing code 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 and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @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.

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

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5e6b934 and 04d4946.

Files selected for processing (2)

• test/dummy.go (2 hunks)
• test/test_suite.go (3 hunks)

Additional context used

golangci-lint

test/dummy.go

97-97: File is not goimports-ed with -local github.com/rollkit

(goimports)

Additional comments not posted (3)

test/test_suite.go (2)

31-32: Review of HeightFromFutureTest:

The new test function HeightFromFutureTest is well-named and aligns with the PR's objective to handle future height requests. The test asserts that an error is returned for a height that is presumably in the future, which is a valid scenario to test given the changes in the DA implementation.

The changes to the test function are approved as they correctly implement the intended functionality.

Also applies to: 155-160

---

140-140: Review of ConcurrentReadWriteTest simplifications:

The simplification of error handling in ConcurrentReadWriteTest by removing specific error checks could potentially reduce the test's ability to catch subtle issues in concurrent operations. It's crucial to ensure that the test still robustly checks for errors that could occur during these operations.

Please verify that the removal of specific error checks does not impact the ability of the test to detect issues in concurrent operations. Consider adding more comprehensive error checks if necessary.

Also applies to: 147-147

test/dummy.go (1)

19-20: Review of error handling changes in DummyDA:

The introduction of ErrTooHigh is a significant improvement in error messaging, making it clear that the requested height is in the future. The modification in the GetIDs method to return this error when the height exceeds the current height is a logical change that enhances the robustness of the method.

The changes to error handling are approved as they correctly implement the intended functionality and improve the clarity of the error messages.

Also applies to: 88-97

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 04d4946 and 6e93a75.

Files selected for processing (2)

• test/dummy.go (2 hunks)
• test/test_suite.go (3 hunks)

Additional comments not posted (6)

test/test_suite.go (2)

158-162: LGTM!

The code changes are approved. The new test function HeightFromFutureTest correctly tests the scenario where the specified height is in the future and asserts that the expected error ErrTooHigh is returned.

---

140-141: Verify the Submit method usage.

The simplified error handling logic in the goroutine that calls d.GetIDs is approved. It correctly asserts that the expected error ErrTooHigh is returned.

However, ensure that the Submit method is expected to handle an empty byte slice []byte{} as its second parameter.

Run the following script to verify the Submit method usage:

Also applies to: 149-149

test/dummy.go (4)

19-20: LGTM!

The change in the error constant and message improves the clarity of the error handling related to height validation.

---

88-91: LGTM!

The new control flow prevents further processing when an invalid height is provided, enhancing the robustness of the method.

---

97-97: LGTM!

The empty line improves the readability of the code.

---

95-95: LGTM! But verify the impact on the calling code.

The change indicates that the absence of data at the requested height is no longer treated as an error condition. Ensure that the calling code handles the nil return value appropriately.

Run the following script to verify the usage of the GetIDs method:

[tzdybal]

LGTM

[tuxcanfly]

OK.