Skip to content

chore(backport): fix from master that asks the block body from the header source first#1154

Merged
JesseTheRobot merged 2 commits into
release/testnet/0.2.2from
fix-ask-the-sender-3
Feb 18, 2026
Merged

chore(backport): fix from master that asks the block body from the header source first#1154
JesseTheRobot merged 2 commits into
release/testnet/0.2.2from
fix-ask-the-sender-3

Conversation

@antouhou

@antouhou antouhou commented Feb 18, 2026

Copy link
Copy Markdown
Contributor

Describe the changes

Related Issue(s)

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

Summary by CodeRabbit

  • New Features

    • Protocol-version aware peer health checks and endpoint routing
    • Added block body retrieval directly from specific peers
  • Bug Fixes

    • Improved block validation error reporting with block hash and failure reason context
  • Refactor

    • Peer identity management moved from database to file-based storage for better initialization flow
  • Tests

    • Enhanced block synchronization verification before validation
    • Increased timeout durations for restart stability

@antouhou antouhou changed the base branch from master to release/testnet/0.2.2 February 18, 2026 14:23
@coderabbitai

coderabbitai Bot commented Feb 18, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

This pull request migrates peer ID storage from the database to the filesystem, enhances P2P block-fetching with protocol-version awareness, reworks error propagation in block pre-validation, and adjusts test timing. The changes span multiple crates and introduce several public API modifications.

Changes

Cohort / File(s) Summary
Block Validation Error Handling
crates/actors/src/block_tree_service.rs, crates/actors/src/block_validation.rs
Added AddBlockFailed { block_hash: H256, reason: String } variant to PreValidationError enum. Reworked on_block_prevalidated to propagate detailed error payloads and separated scheduling logic from the add-block success path.
Peer ID Storage Migration
crates/chain/src/chain.rs, crates/database/src/database.rs, crates/database/src/metadata.rs, crates/types/src/config/node.rs
Shifted peer ID storage from database to filesystem. Updated get_or_create_peer_id signature from DatabaseProvider to NodeConfig parameter; now reads/writes peer keys from/to disk. Removed database functions get_peer_id and set_peer_id; removed MetadataKey::PeerId variant. Added peer_info_dir() accessor to NodeConfig.
P2P Block Fetching Protocol Improvements
crates/p2p/src/gossip_client.rs, crates/p2p/src/gossip_data_handler.rs, crates/p2p/src/peer_network_service.rs
Enhanced health checks and data pulling with protocol-version awareness; added protocol_version parameter to GossipClient::check_health. Added new pull_block_body_from_peer function. Updated pull_block_body signature to accept source_peer_id and attempt source-peer fetching first. Switched logging/messaging from peer addresses to peer IDs; hardened V1 peer handling for block body requests.
Test Adjustments
crates/chain/tests/multi_node/reth_restart.rs, crates/chain/tests/validation/poa_cases.rs
Increased WAIT_SECS constant from 2 to 30 in reth restart test. Added synchronization wait in PoA validation test to ensure mined blocks reach block index before proceeding.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • feat: changed peer id location #1138: Implements identical peer-ID storage migration—moving get_or_create_peer_id to file-based key management with NodeConfig, removing database peer-ID accessors, and adding NodeConfig::peer_info_dir.
  • fix: peer id and health check naming consistency #1146: Modifies the same P2P modules, updating GossipClient::check_health to accept ProtocolVersion and refactoring peer identification across gossip_client, gossip_data_handler, and peer_network_service.
  • feat: better sync crash heuristics #1132: Alters block pre-validation and validation scheduling in block_tree_service.rs, overlapping with the error propagation and scheduling-flow changes in this PR.

Suggested reviewers

  • roberts-pumpurs
  • DanMacDonald
🚥 Pre-merge checks | ✅ 3
✅ 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 describes the main change: asking the block body from the header source first, which aligns with the primary objective and the majority of the changes across multiple files in the gossip and peer network handling.
Docstring Coverage ✅ Passed Docstring coverage is 88.57% 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 fix-ask-the-sender-3

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 JesseTheRobot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, wait for CI before merging

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.

2 participants