Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to disable gap creation during block import #5343

Merged
17 changes: 17 additions & 0 deletions prdoc/pr_5343.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
title: Allow to disable gap creation during block import

doc:
- audience: Node Dev
description: |
New API `Backend::no_gap()` allows to modify `Backend::BlockImportOperation` to not create block gap in case block
nazar-pc marked this conversation as resolved.
Show resolved Hide resolved
has no parent, which is helpful for sync protocols that do not need to sync the gap after this happens.

crates:
- name: sc-client-api
bump: major
- name: sc-consensus
bump: minor
- name: sc-client-db
bump: minor
- name: sc-service
bump: minor
3 changes: 3 additions & 0 deletions substrate/client/api/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,9 @@ pub trait Backend<Block: BlockT>: AuxStore + Send + Sync {

/// Tells whether the backend requires full-sync mode.
fn requires_full_sync(&self) -> bool;

/// Do not create gap if newly imported block is missing parent
fn no_gap(&self, operation: &mut Self::BlockImportOperation);
}

/// Mark for all Backend implementations, that are making use of state data, stored locally.
Expand Down
2 changes: 2 additions & 0 deletions substrate/client/api/src/in_mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,8 @@ impl<Block: BlockT> backend::Backend<Block> for Backend<Block> {
let mut blocks = self.pinned_blocks.write();
blocks.entry(hash).and_modify(|counter| *counter -= 1).or_insert(-1);
}

fn no_gap(&self, _operation: &mut Self::BlockImportOperation) {}
}

impl<Block: BlockT> backend::LocalBackend<Block> for Backend<Block> {}
Expand Down
3 changes: 3 additions & 0 deletions substrate/client/consensus/common/src/block_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ pub struct BlockImportParams<Block: BlockT> {
pub fork_choice: Option<ForkChoiceStrategy>,
/// Re-validate existing block.
pub import_existing: bool,
/// Whether to create "block gap" in case this block doesn't have parent.
pub create_gap: bool,
/// Cached full header hash (with post-digests applied).
pub post_hash: Option<Block::Hash>,
}
Expand All @@ -234,6 +236,7 @@ impl<Block: BlockT> BlockImportParams<Block> {
auxiliary: Vec::new(),
fork_choice: None,
import_existing: false,
create_gap: true,
post_hash: None,
}
}
Expand Down
11 changes: 9 additions & 2 deletions substrate/client/db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,7 @@ pub struct BlockImportOperation<Block: BlockT> {
finalized_blocks: Vec<(Block::Hash, Option<Justification>)>,
set_head: Option<Block::Hash>,
commit_state: bool,
create_gap: bool,
index_ops: Vec<IndexOperation>,
}

Expand Down Expand Up @@ -1707,8 +1708,9 @@ impl<Block: BlockT> Backend<Block> {
&(start, end).encode(),
);
}
} else if number > best_num + One::one() &&
number > One::one() && self.blockchain.header(parent_hash)?.is_none()
} else if operation.create_gap &&
Copy link
Contributor

Choose a reason for hiding this comment

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

cc #4984

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly sure how it is related necessarily, it still had to pass through consensus successfully before ending up here IIUC.

I do see those "block has an unknown parent" in some cases though and subscribed to linked issue a while ago.

number > best_num + One::one() &&
nazar-pc marked this conversation as resolved.
Show resolved Hide resolved
self.blockchain.header(parent_hash)?.is_none()
{
let gap = (best_num + One::one(), number - One::one());
transaction.set(columns::META, meta_keys::BLOCK_GAP, &gap.encode());
Expand Down Expand Up @@ -2060,6 +2062,7 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
finalized_blocks: Vec::new(),
set_head: None,
commit_state: false,
create_gap: true,
index_ops: Default::default(),
})
}
Expand Down Expand Up @@ -2538,6 +2541,10 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
self.blockchain.unpin(hash);
}
}

fn no_gap(&self, operation: &mut Self::BlockImportOperation) {
nazar-pc marked this conversation as resolved.
Show resolved Hide resolved
operation.create_gap = false;
}
}

impl<Block: BlockT> sc_client_api::backend::LocalBackend<Block> for Backend<Block> {}
Expand Down
5 changes: 5 additions & 0 deletions substrate/client/service/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ where
fork_choice,
intermediates,
import_existing,
create_gap,
..
} = import_block;

Expand All @@ -537,6 +538,10 @@ where

*self.importing_block.write() = Some(hash);

if !create_gap {
self.backend.no_gap(&mut operation.op);
}

let result = self.execute_and_import_block(
operation,
origin,
Expand Down
Loading