Skip to content

feat!: check rlc when downloading fibre rows#7041

Merged
mcrakhman merged 15 commits intomainfrom
mcrakhman/fibre-download-rlc-check
Apr 15, 2026
Merged

feat!: check rlc when downloading fibre rows#7041
mcrakhman merged 15 commits intomainfrom
mcrakhman/fibre-download-rlc-check

Conversation

@mcrakhman
Copy link
Copy Markdown
Member

@mcrakhman mcrakhman commented Apr 10, 2026

Overview

Now if the download failed we check rlc for the downloaded rows to filter rows with incorrect encoding.

Important: check the full description (https://linear.app/celestia/issue/PROTOCO-1330/check-encoding-correctness-rlc-when-downloading-rows) of the change and why it was made here before attempting to review.


Open with Devin

@mcrakhman mcrakhman requested a review from a team as a code owner April 10, 2026 14:48
@mcrakhman mcrakhman requested review from ninabarbakadze and removed request for a team April 10, 2026 14:48
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 6 additional findings.

Open in Devin Review

@rootulp rootulp self-requested a review April 10, 2026 14:59
@mcrakhman
Copy link
Copy Markdown
Member Author

Overview

Now if the download failed we check rlc for the downloaded rows to filter rows with incorrect encoding.

Important: check the full description (https://linear.app/celestia/issue/PROTOCO-1330/check-encoding-correctness-rlc-when-downloading-rows) of the change and why it was made here before attempting to review.

Open with Devin

Protobuf check fails, but it is fine, because we didn't release fibre, so I think we can change the definitions.

Copy link
Copy Markdown
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

Review comments below.

Comment thread fibre/blob.go Outdated
Comment thread fibre/client_download.go
Comment thread fibre/client_download.go
@rootulp
Copy link
Copy Markdown
Collaborator

rootulp commented Apr 10, 2026

Update: those were Claude generated review comments. I'm still manually reviewing, so far LGTM but haven't finished yet.

rootulp
rootulp previously approved these changes Apr 10, 2026
Copy link
Copy Markdown
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

LGTM. I don't have any other feedback besides the Claude generated comments above

@mcrakhman mcrakhman requested a review from rootulp April 13, 2026 14:22
rootulp
rootulp previously approved these changes Apr 13, 2026
walldiss
walldiss previously approved these changes Apr 14, 2026
# Conflicts:
#	fibre/store_bench_test.go
@mcrakhman mcrakhman dismissed stale reviews from walldiss and rootulp via e439cbb April 14, 2026 20:54
@mcrakhman mcrakhman requested a review from rootulp April 14, 2026 20:55
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

rootulp
rootulp previously approved these changes Apr 14, 2026
@mcrakhman mcrakhman requested a review from walldiss April 15, 2026 09:18
@mcrakhman mcrakhman requested a review from rootulp April 15, 2026 10:07
@mcrakhman mcrakhman added this pull request to the merge queue Apr 15, 2026
Merged via the queue into main with commit c13efbe Apr 15, 2026
32 of 34 checks passed
@mcrakhman mcrakhman deleted the mcrakhman/fibre-download-rlc-check branch April 15, 2026 14:17
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.

3 participants