Skip to content

Celestia: re-add block verification#2764

Open
citizen-stig wants to merge 4 commits intodevfrom
nikolai/bring-back-celestia-verification
Open

Celestia: re-add block verification#2764
citizen-stig wants to merge 4 commits intodevfrom
nikolai/bring-back-celestia-verification

Conversation

@citizen-stig
Copy link
Copy Markdown
Member

@citizen-stig citizen-stig commented Apr 20, 2026

Description

Re-adds the previously-reverted boolean block verification on fetch (PR #2489 / reverted in PR #2520). It is disabled by default to match current behaviour, but can be enabled via config.

Possible panic.

If it is enabled with log error, in some rare cases it can cause panic, which happens internally in the verifier; This will be addressed in follow up: #2404

  • I have updated CHANGELOG.md with a new entry if my PR makes any breaking changes or fixes a bug. If my PR removes a feature or changes its behavior, I provide help for users on how to migrate to the new behavior.
  • I have carefully reviewed all my Cargo.toml changes before opening the PRs. (Are all new dependencies necessary? Is any module dependency leaked into the full-node (hint: it shouldn't)?)

Testing

All existing tests are passing

Docs

No updates

@citizen-stig citizen-stig marked this pull request as ready for review April 20, 2026 14:59
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 3 additional findings.

Open in Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

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 found 1 new potential issue.

View 7 additional findings in Devin Review.

Open in Devin Review

Comment thread CHANGELOG.md
Comment on lines +379 to +385
VerifyOnFetchMode::ReturnError => {
self.verify_block_integrity(&block).map_err(|error| {
anyhow::anyhow!(
"Celestia block integrity verification failed at height {height}: {error}"
)
})?;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📝 Info: ReturnError mode double-wraps the error, losing the anyhow chain

At crates/adapters/celestia/src/da_service/mod.rs:380-384, the ReturnError branch calls .map_err(|error| anyhow::anyhow!("...{error}")) on the result of verify_block_integrity, which already returns anyhow::Result. This creates a new anyhow::Error using the Display of the inner error, discarding the original error's backtrace and cause chain. The user-visible error message is preserved (since {error} uses Display), but anyhow's .context() or direct ? propagation would retain the full chain. This is a minor style concern — not a functional bug — but could make debugging slightly harder in production when investigating verification failures.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

1 participant