Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

fix: Propagate JSONL import parse and verify failures#5242

Open
Officeyutong wants to merge 1 commit into
nervosnetwork:developnervosnetwork/ckb:developfrom
Officeyutong:fix-ckb-importOfficeyutong/ckb:fix-ckb-importCopy head branch name to clipboard
Open

fix: Propagate JSONL import parse and verify failures#5242
Officeyutong wants to merge 1 commit into
nervosnetwork:developnervosnetwork/ckb:developfrom
Officeyutong:fix-ckb-importOfficeyutong/ckb:fix-ckb-importCopy head branch name to clipboard

Conversation

@Officeyutong

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:
ckb import parsed JSONL blocks in a background thread, but several failure paths were not propagated to the import result.

Malformed JSON on the first line used expect and could panic. Later malformed lines were parsed inside a spawned thread whose JoinHandle was dropped, so parser panics or errors were not reported to the main import path. Line read errors were also discarded with filter_map(Result::ok). After submitting blocks, the import loop only waited for the largest block number to appear in the database, while verification errors were only printed from the callback. This could lead to panic, silent partial import, or an indefinite wait when a submitted block failed verification.

What is changed and how it works?

Proposal: xxx

What's Changed:

Return structured errors for first-line parsing, make the parser thread return Result, and join it from the main import path. JSONL read, parse, and send failures are now propagated instead of being ignored or hidden in a detached thread.

Verification callbacks now report their result through a channel. The import command waits for callbacks from all submitted non-genesis blocks and returns an error for verification failure or Ok(false). This replaces the previous largest-height polling loop, so failed imports terminate with an explicit error instead of hanging.

Related changes

  • PR to update owner/repo:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression
  • Breaking backward compatibility

@Officeyutong
Officeyutong requested a review from a team as a code owner June 16, 2026 11:36
@Officeyutong
Officeyutong requested review from quake and removed request for a team June 16, 2026 11:36
@eval-exec

Copy link
Copy Markdown
Collaborator

@codex Review.

Copilot AI left a comment

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.

Pull request overview

This PR hardens ckb import of JSONL/JSONL-gz block data by ensuring JSON parsing, line-read failures, and background parsing/verification outcomes are properly propagated back to the import result instead of being hidden, ignored, or causing hangs/panics.

Changes:

  • Replace first-line expect parsing with proper error propagation for empty/malformed input.
  • Make the JSONL parser thread return Result and join it from the main import path to surface parse/read/send failures.
  • Replace “poll largest height” waiting with a channel-based mechanism that waits for verification callbacks from all submitted blocks.

Comment on lines +206 to 220
Box::new(move |verify_result: VerifyResult| match verify_result {
Ok(true) => {
progress_bar.inc(block_size as u64);
let _ = verify_tx.send(Ok(block_number));
}
Ok(false) => {
let _ = verify_tx
.send(Err(format!("block {block_number} was not verified")));
}
Err(err) => {
eprintln!("Error verifying block: {:?}", err);
let _ = verify_tx
.send(Err(format!("verify block {block_number} failed: {err:?}")));
}
})
Comment on lines +227 to 240
Box::new(move |verify_result: VerifyResult| match verify_result {
Ok(true) => {
let _ = verify_tx.send(Ok(block_number));
}
Ok(false) => {
let _ = verify_tx
.send(Err(format!("block {block_number} was not verified")));
}
Err(err) => {
eprintln!("Error verifying block: {:?}", err);
let _ = verify_tx
.send(Err(format!("verify block {block_number} failed: {err:?}")));
}
})
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.