refactor: propagate an error if block tree rejects a block by roberts-pumpurs · Pull Request #1137 · Irys-xyz/irys

roberts-pumpurs · 2026-02-13T15:24:15Z

Describe the changes

remove silently dropping errors when we fail adding a block to the tree

Related Issue(s)
Please link to the issue(s) that will be closed with this PR.

Checklist

Tests have been added/updated for the changes.
Documentation has been updated for the changes (if applicable).
The code follows Rust's style guidelines.

Additional Context
Add any other context about the pull request here.

Summary by CodeRabbit

Improvements
- Enhanced error reporting and structured logging for block-add failures, now including the block identifier and a clear failure reason to aid diagnostics.
Tests
- Increased wait durations in a multi-node restart test to improve timing stability.
- Added an explicit synchronization step after mining in a PoA validation test to ensure correct block availability before validation.

coderabbitai · 2026-02-13T15:24:25Z

📝 Walkthrough

Walkthrough

Adds a new PreValidationError::AddBlockFailed { block_hash: H256, reason: String }, updates block-tree add_block error handling to return that variant with structured logging and preserved success-path scheduling logic, and increases a test wait constant and inserts a wait-for-confirmation step in PoA tests.

Changes

Cohort / File(s)	Summary
Error Enum `crates/actors/src/block_validation.rs`	Added `PreValidationError::AddBlockFailed { block_hash: H256, reason: String }` with message describing failed add_block and reason.
Block-tree add_block handling `crates/actors/src/block_tree_service.rs`	Replaced prior inline add_block failure path with a `map_err` producing `AddBlockFailed { block_hash, reason }`; preserved success-path actions (mark scheduled, record diagnostics, send validation request); updated structured logging on failure.
Tests — timing & sync `crates/chain/tests/multi_node/reth_restart.rs`, `crates/chain/tests/validation/poa_cases.rs`	Increased `WAIT_SECS` from `2` to `30` in reth_restart.rs; added a wait_until_height_confirmed(...) step after mining in poa_cases.rs to ensure block index/migration before PoA checks. Pay attention to test runtime and potential flakiness.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

refactor: only staked nodes produce ingress proofs #1118 — Also modifies crates/actors/src/block_validation.rs (adds a PreValidationError variant); likely related to error-variant additions and handling.

Suggested reviewers

DanMacDonald
antouhou
JesseTheRobot

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (14 files): ⚔️ `Cargo.lock` (content) ⚔️ `crates/actors/src/block_tree_service.rs` (content) ⚔️ `crates/actors/src/block_validation.rs` (content) ⚔️ `crates/chain/tests/multi_node/reth_restart.rs` (content) ⚔️ `crates/chain/tests/validation/poa_cases.rs` (content) ⚔️ `crates/p2p/Cargo.toml` (content) ⚔️ `crates/p2p/src/chain_sync.rs` (content) ⚔️ `crates/p2p/src/gossip_client.rs` (content) ⚔️ `crates/p2p/src/gossip_data_handler.rs` (content) ⚔️ `crates/p2p/src/peer_network_service.rs` (content) ⚔️ `crates/p2p/src/tests/util.rs` (content) ⚔️ `crates/p2p/src/types.rs` (content) ⚔️ `crates/utils/utils/Cargo.toml` (content) ⚔️ `crates/utils/utils/src/lib.rs` (content) These conflicts must be resolved before merging into `master`.	Resolve conflicts locally and push changes to this branch.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: converting silent error handling into explicit error propagation when the block tree rejects a block, which aligns with the core refactoring across multiple files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch rob/add-logs

⚔️ Resolve merge conflicts (beta)

Auto-commit resolved conflicts to branch rob/add-logs
Create stacked PR with resolved conflicts
Post resolved changes as copyable diffs in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

JesseTheRobot

LGTM - I'll put this in with the peer ID change into the RC

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@crates/chain/tests/validation/poa_cases.rs`:
- Around line 93-97: Replace the hard-coded literal 20 passed into
genesis_node.wait_until_height_confirmed with the existing seconds_to_wait
variable to avoid divergence if the timeout value changes; update the call using
genesis_node.wait_until_height_confirmed(new_block.height,
seconds_to_wait).await? so it uses seconds_to_wait consistently.

coderabbitai · 2026-02-16T11:51:00Z

+    // Wait for the data block to be confirmed and migrated to the block index,
+    // so that poa_is_valid can look up block bounds for chunk offsets.
+    genesis_node
+        .wait_until_height_confirmed(new_block.height, 20)
+        .await?;


🧹 Nitpick | 🔵 Trivial

Use the seconds_to_wait variable instead of the magic number 20.

Line 21 defines seconds_to_wait = 20 for exactly this purpose. Using the literal 20 here is inconsistent and will silently diverge if the constant is updated.

Proposed fix

genesis_node - .wait_until_height_confirmed(new_block.height, 20) + .wait_until_height_confirmed(new_block.height, seconds_to_wait) .await?;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

// Wait for the data block to be confirmed and migrated to the block index,

// so that poa_is_valid can look up block bounds for chunk offsets.

genesis_node

.wait_until_height_confirmed(new_block.height, 20)

.await?;

genesis_node

.wait_until_height_confirmed(new_block.height, seconds_to_wait)

.await?;

🤖 Prompt for AI Agents

In `@crates/chain/tests/validation/poa_cases.rs` around lines 93 - 97, Replace the hard-coded literal 20 passed into genesis_node.wait_until_height_confirmed with the existing seconds_to_wait variable to avoid divergence if the timeout value changes; update the call using genesis_node.wait_until_height_confirmed(new_block.height, seconds_to_wait).await? so it uses seconds_to_wait consistently.

* propagate errors from the block tree when it cannot add a block

roberts-pumpurs added 2 commits February 13, 2026 15:54

propagate errors from the block tree when cannot add a block

518b317

Merge remote-tracking branch 'origin/master' into rob/add-logs

5544335

roberts-pumpurs changed the title ~~Rob/add logs~~ refactor: propagate an error if block tree rejects a block Feb 13, 2026

roberts-pumpurs self-assigned this Feb 13, 2026

roberts-pumpurs marked this pull request as ready for review February 13, 2026 18:02

roberts-pumpurs requested review from DanMacDonald, JesseTheRobot and antouhou February 13, 2026 18:36

Merge branch 'master' into rob/add-logs

6c81ad9

JesseTheRobot approved these changes Feb 13, 2026

View reviewed changes

roberts-pumpurs and others added 3 commits February 14, 2026 11:28

Merge branch 'master' into rob/add-logs

6e2e78d

Merge branch 'master' into rob/add-logs

df46ce2

test fix

bae7128

roberts-pumpurs force-pushed the rob/add-logs branch from 6a101f3 to bae7128 Compare February 16, 2026 11:48

coderabbitai Bot reviewed Feb 16, 2026

View reviewed changes

Merge branch 'master' into rob/add-logs

89788f8

roberts-pumpurs merged commit 49d8a6f into master Feb 16, 2026
18 checks passed

roberts-pumpurs deleted the rob/add-logs branch February 16, 2026 12:22

JesseTheRobot pushed a commit that referenced this pull request Feb 16, 2026

PICK: refactor: propagate an error if block tree rejects a block (#1137)

27a9310

* propagate errors from the block tree when it cannot add a block

coderabbitai Bot mentioned this pull request Feb 23, 2026

feat: add cross-node request lifecycle tracing and observability improvements #1156

Merged

3 tasks

This was referenced May 12, 2026

feat: preval perf #1414

Merged

fix: validation error disambiguation #1416

Merged

fix: cached data roots block set #1421

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: propagate an error if block tree rejects a block#1137

refactor: propagate an error if block tree rejects a block#1137
roberts-pumpurs merged 7 commits into
masterfrom
rob/add-logs

roberts-pumpurs commented Feb 13, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Feb 13, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

JesseTheRobot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Feb 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

roberts-pumpurs commented Feb 13, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

JesseTheRobot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

roberts-pumpurs commented Feb 13, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Feb 13, 2026 •

edited

Loading