From 7ad8353eaed8a145d3eac2b1aba180a8a2f5cf75 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Tue, 10 Sep 2024 19:16:11 +0800 Subject: [PATCH 1/9] Tiny refactoring to prepare for the fast sync gap addition Primarily move the block gap handling into a separate block, zero logical changes. --- substrate/client/db/src/lib.rs | 39 ++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index 72707c306f58..105f3f440939 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -1677,7 +1677,9 @@ impl Backend { children, ); } + } + if !existing_header { if let Some(mut gap) = block_gap { match gap.gap_type { BlockGapType::MissingHeaderAndBody => @@ -1714,24 +1716,25 @@ impl Backend { unreachable!("Unsupported block gap. TODO: https://github.com/paritytech/polkadot-sdk/issues/5406") }, } - } else if operation.create_gap && - number > best_num + One::one() && - self.blockchain.header(parent_hash)?.is_none() - { - let gap = BlockGap { - start: best_num + One::one(), - end: number - One::one(), - gap_type: BlockGapType::MissingHeaderAndBody, - }; - transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode()); - transaction.set( - columns::META, - meta_keys::BLOCK_GAP_VERSION, - &BLOCK_GAP_CURRENT_VERSION.encode(), - ); - block_gap = Some(gap); - block_gap_updated = true; - debug!(target: "db", "Detected block gap {block_gap:?}"); + } else if operation.create_gap { + if number > best_num + One::one() && + self.blockchain.header(parent_hash)?.is_none() + { + let gap = BlockGap { + start: best_num + One::one(), + end: number - One::one(), + gap_type: BlockGapType::MissingHeaderAndBody, + }; + transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode()); + transaction.set( + columns::META, + meta_keys::BLOCK_GAP_VERSION, + &BLOCK_GAP_CURRENT_VERSION.encode(), + ); + block_gap = Some(gap); + block_gap_updated = true; + debug!(target: "db", "Detected block gap (warp sync) {block_gap:?}"); + } } } From 99cf12f5b0eb8517f4baccb2c2da8fe8e3957bac Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Tue, 10 Sep 2024 19:39:14 +0800 Subject: [PATCH 2/9] fast sync: create block gap when importing the first block without body --- substrate/client/db/src/lib.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index 105f3f440939..e1c1ef5818d0 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -1486,6 +1486,7 @@ impl Backend { .map(|(n, _)| n) .unwrap_or(Zero::zero()); let existing_header = number <= highest_leaf && self.blockchain.header(hash)?.is_some(); + let body_exists = pending_block.body.is_some(); // blocks are keyed by number + hash. let lookup_key = utils::number_and_hash_to_lookup_key(number, hash)?; @@ -1679,7 +1680,9 @@ impl Backend { } } - if !existing_header { + let should_check_block_gap = !existing_header || !body_exists; + + if should_check_block_gap { if let Some(mut gap) = block_gap { match gap.gap_type { BlockGapType::MissingHeaderAndBody => @@ -1734,6 +1737,24 @@ impl Backend { block_gap = Some(gap); block_gap_updated = true; debug!(target: "db", "Detected block gap (warp sync) {block_gap:?}"); + } else if number == best_num + One::one() && + self.blockchain.header(parent_hash)?.is_some() && + !body_exists + { + let gap = BlockGap { + start: number, + end: number, + gap_type: BlockGapType::MissingBody, + }; + transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode()); + transaction.set( + columns::META, + meta_keys::BLOCK_GAP_VERSION, + &BLOCK_GAP_CURRENT_VERSION.encode(), + ); + block_gap = Some(gap); + block_gap_updated = true; + debug!(target: "db", "Detected block gap (fast sync) {block_gap:?}"); } } } From 1d998e40d0248f97f8281835de166ddaa3d56c0e Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Tue, 10 Sep 2024 19:59:25 +0800 Subject: [PATCH 3/9] fast sync: increase block gap when syncing the header chain --- substrate/client/db/src/lib.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index e1c1ef5818d0..be792f2cf324 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -1716,7 +1716,25 @@ impl Backend { block_gap_updated = true; }, BlockGapType::MissingBody => { - unreachable!("Unsupported block gap. TODO: https://github.com/paritytech/polkadot-sdk/issues/5406") + // Gap increased when syncing the header chain during fast sync. + if number == gap.end + One::one() && !body_exists { + gap.end += One::one(); + utils::insert_number_to_key_mapping( + &mut transaction, + columns::KEY_LOOKUP, + number, + hash, + )?; + block_gap = Some(gap); + debug!(target: "db", "Update block gap. {block_gap:?}"); + transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode()); + transaction.set( + columns::META, + meta_keys::BLOCK_GAP_VERSION, + &BLOCK_GAP_CURRENT_VERSION.encode(), + ); + block_gap_updated = true; + } }, } } else if operation.create_gap { From a58d26fefc51795beee09b3778a9a7f87fa966db Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Tue, 10 Sep 2024 20:02:10 +0800 Subject: [PATCH 4/9] fast sync: decrease block gap when downloading the full blocks --- substrate/client/db/src/lib.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index be792f2cf324..de525c48702a 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -1734,6 +1734,29 @@ impl Backend { &BLOCK_GAP_CURRENT_VERSION.encode(), ); block_gap_updated = true; + // Gap decreased when downloading the full blocks. + } else if number == gap.start && body_exists { + gap.start += One::one(); + if gap.start > gap.end { + transaction.remove(columns::META, meta_keys::BLOCK_GAP); + transaction.remove(columns::META, meta_keys::BLOCK_GAP_VERSION); + block_gap = None; + debug!(target: "db", "Removed block gap."); + } else { + block_gap = Some(gap); + debug!(target: "db", "Update block gap. {block_gap:?}"); + transaction.set( + columns::META, + meta_keys::BLOCK_GAP, + &gap.encode(), + ); + transaction.set( + columns::META, + meta_keys::BLOCK_GAP_VERSION, + &BLOCK_GAP_CURRENT_VERSION.encode(), + ); + } + block_gap_updated = true; } }, } From e85c4dcef34ee697e1c57aa2aab1ab22ed06514a Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Thu, 12 Sep 2024 23:52:49 +0800 Subject: [PATCH 5/9] refactor: extract `insert_new_gap()` --- substrate/client/db/src/lib.rs | 53 ++++++++++------------------------ 1 file changed, 15 insertions(+), 38 deletions(-) diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index de525c48702a..b8258ce15b24 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -1683,6 +1683,16 @@ impl Backend { let should_check_block_gap = !existing_header || !body_exists; if should_check_block_gap { + let insert_new_gap = |transaction: &mut Transaction, + gap: BlockGap>| { + transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode()); + transaction.set( + columns::META, + meta_keys::BLOCK_GAP_VERSION, + &BLOCK_GAP_CURRENT_VERSION.encode(), + ); + }; + if let Some(mut gap) = block_gap { match gap.gap_type { BlockGapType::MissingHeaderAndBody => @@ -1700,18 +1710,9 @@ impl Backend { block_gap = None; debug!(target: "db", "Removed block gap."); } else { + insert_new_gap(&mut transaction, gap); block_gap = Some(gap); debug!(target: "db", "Update block gap. {block_gap:?}"); - transaction.set( - columns::META, - meta_keys::BLOCK_GAP, - &gap.encode(), - ); - transaction.set( - columns::META, - meta_keys::BLOCK_GAP_VERSION, - &BLOCK_GAP_CURRENT_VERSION.encode(), - ); } block_gap_updated = true; }, @@ -1725,14 +1726,9 @@ impl Backend { number, hash, )?; + insert_new_gap(&mut transaction, gap); block_gap = Some(gap); debug!(target: "db", "Update block gap. {block_gap:?}"); - transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode()); - transaction.set( - columns::META, - meta_keys::BLOCK_GAP_VERSION, - &BLOCK_GAP_CURRENT_VERSION.encode(), - ); block_gap_updated = true; // Gap decreased when downloading the full blocks. } else if number == gap.start && body_exists { @@ -1743,18 +1739,9 @@ impl Backend { block_gap = None; debug!(target: "db", "Removed block gap."); } else { + insert_new_gap(&mut transaction, gap); block_gap = Some(gap); debug!(target: "db", "Update block gap. {block_gap:?}"); - transaction.set( - columns::META, - meta_keys::BLOCK_GAP, - &gap.encode(), - ); - transaction.set( - columns::META, - meta_keys::BLOCK_GAP_VERSION, - &BLOCK_GAP_CURRENT_VERSION.encode(), - ); } block_gap_updated = true; } @@ -1769,12 +1756,7 @@ impl Backend { end: number - One::one(), gap_type: BlockGapType::MissingHeaderAndBody, }; - transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode()); - transaction.set( - columns::META, - meta_keys::BLOCK_GAP_VERSION, - &BLOCK_GAP_CURRENT_VERSION.encode(), - ); + insert_new_gap(&mut transaction, gap); block_gap = Some(gap); block_gap_updated = true; debug!(target: "db", "Detected block gap (warp sync) {block_gap:?}"); @@ -1787,12 +1769,7 @@ impl Backend { end: number, gap_type: BlockGapType::MissingBody, }; - transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode()); - transaction.set( - columns::META, - meta_keys::BLOCK_GAP_VERSION, - &BLOCK_GAP_CURRENT_VERSION.encode(), - ); + insert_new_gap(&mut transaction, gap); block_gap = Some(gap); block_gap_updated = true; debug!(target: "db", "Detected block gap (fast sync) {block_gap:?}"); From 6da05cbe89fcad425b627f98f32badd1b84a56ae Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 13 Sep 2024 20:33:52 +0800 Subject: [PATCH 6/9] Add prdoc --- prdoc/pr_5703.prdoc | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 prdoc/pr_5703.prdoc diff --git a/prdoc/pr_5703.prdoc b/prdoc/pr_5703.prdoc new file mode 100644 index 000000000000..03a5162de9ff --- /dev/null +++ b/prdoc/pr_5703.prdoc @@ -0,0 +1,13 @@ +title: Properly handle block gap created by fast sync + +doc: + - audience: Node Dev + description: | + Implements support for handling block gaps generated during fast sync. This includes managing the creation, + updating, and removal of block gaps. + Note that this feature is not fully activated until the `body` attribute is removed from the `LightState` + block request in chain sync, which will occur after the issue #5406 is resolved. + +crates: + - name: sc-client-db + bump: none From 917c1538a8177cabe3dac52d9886944a3d471227 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 16 Sep 2024 07:48:57 +0800 Subject: [PATCH 7/9] refactor: update `insert_new_gap()` to accept `block_gap` --- substrate/client/db/src/lib.rs | 36 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index b8258ce15b24..7ba2a362478a 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -1683,15 +1683,18 @@ impl Backend { let should_check_block_gap = !existing_header || !body_exists; if should_check_block_gap { - let insert_new_gap = |transaction: &mut Transaction, - gap: BlockGap>| { - transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode()); - transaction.set( - columns::META, - meta_keys::BLOCK_GAP_VERSION, - &BLOCK_GAP_CURRENT_VERSION.encode(), - ); - }; + let insert_new_gap = + |transaction: &mut Transaction, + new_gap: BlockGap>, + block_gap: &mut Option>>| { + transaction.set(columns::META, meta_keys::BLOCK_GAP, &new_gap.encode()); + transaction.set( + columns::META, + meta_keys::BLOCK_GAP_VERSION, + &BLOCK_GAP_CURRENT_VERSION.encode(), + ); + block_gap.replace(new_gap); + }; if let Some(mut gap) = block_gap { match gap.gap_type { @@ -1710,8 +1713,7 @@ impl Backend { block_gap = None; debug!(target: "db", "Removed block gap."); } else { - insert_new_gap(&mut transaction, gap); - block_gap = Some(gap); + insert_new_gap(&mut transaction, gap, &mut block_gap); debug!(target: "db", "Update block gap. {block_gap:?}"); } block_gap_updated = true; @@ -1726,8 +1728,7 @@ impl Backend { number, hash, )?; - insert_new_gap(&mut transaction, gap); - block_gap = Some(gap); + insert_new_gap(&mut transaction, gap, &mut block_gap); debug!(target: "db", "Update block gap. {block_gap:?}"); block_gap_updated = true; // Gap decreased when downloading the full blocks. @@ -1739,8 +1740,7 @@ impl Backend { block_gap = None; debug!(target: "db", "Removed block gap."); } else { - insert_new_gap(&mut transaction, gap); - block_gap = Some(gap); + insert_new_gap(&mut transaction, gap, &mut block_gap); debug!(target: "db", "Update block gap. {block_gap:?}"); } block_gap_updated = true; @@ -1756,8 +1756,7 @@ impl Backend { end: number - One::one(), gap_type: BlockGapType::MissingHeaderAndBody, }; - insert_new_gap(&mut transaction, gap); - block_gap = Some(gap); + insert_new_gap(&mut transaction, gap, &mut block_gap); block_gap_updated = true; debug!(target: "db", "Detected block gap (warp sync) {block_gap:?}"); } else if number == best_num + One::one() && @@ -1769,8 +1768,7 @@ impl Backend { end: number, gap_type: BlockGapType::MissingBody, }; - insert_new_gap(&mut transaction, gap); - block_gap = Some(gap); + insert_new_gap(&mut transaction, gap, &mut block_gap); block_gap_updated = true; debug!(target: "db", "Detected block gap (fast sync) {block_gap:?}"); } From 1a7c13bf34350d974952001a1d9083184c80e226 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Wed, 18 Sep 2024 21:12:48 +0800 Subject: [PATCH 8/9] Rename `body_exists` to `existing_body` for consistency --- substrate/client/db/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index 7ba2a362478a..970dcc7d97a5 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -1486,7 +1486,7 @@ impl Backend { .map(|(n, _)| n) .unwrap_or(Zero::zero()); let existing_header = number <= highest_leaf && self.blockchain.header(hash)?.is_some(); - let body_exists = pending_block.body.is_some(); + let existing_body = pending_block.body.is_some(); // blocks are keyed by number + hash. let lookup_key = utils::number_and_hash_to_lookup_key(number, hash)?; @@ -1680,7 +1680,7 @@ impl Backend { } } - let should_check_block_gap = !existing_header || !body_exists; + let should_check_block_gap = !existing_header || !existing_body; if should_check_block_gap { let insert_new_gap = @@ -1720,7 +1720,7 @@ impl Backend { }, BlockGapType::MissingBody => { // Gap increased when syncing the header chain during fast sync. - if number == gap.end + One::one() && !body_exists { + if number == gap.end + One::one() && !existing_body { gap.end += One::one(); utils::insert_number_to_key_mapping( &mut transaction, @@ -1732,7 +1732,7 @@ impl Backend { debug!(target: "db", "Update block gap. {block_gap:?}"); block_gap_updated = true; // Gap decreased when downloading the full blocks. - } else if number == gap.start && body_exists { + } else if number == gap.start && existing_body { gap.start += One::one(); if gap.start > gap.end { transaction.remove(columns::META, meta_keys::BLOCK_GAP); @@ -1761,7 +1761,7 @@ impl Backend { debug!(target: "db", "Detected block gap (warp sync) {block_gap:?}"); } else if number == best_num + One::one() && self.blockchain.header(parent_hash)?.is_some() && - !body_exists + !existing_body { let gap = BlockGap { start: number, From a1f158cb6e9f419b065a286219d28ff35d393f08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 19 Nov 2024 23:10:57 +0100 Subject: [PATCH 9/9] Update prdoc/pr_5703.prdoc --- prdoc/pr_5703.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_5703.prdoc b/prdoc/pr_5703.prdoc index 03a5162de9ff..3cef4468a87d 100644 --- a/prdoc/pr_5703.prdoc +++ b/prdoc/pr_5703.prdoc @@ -10,4 +10,4 @@ doc: crates: - name: sc-client-db - bump: none + bump: patch