Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Limit number of blocks per level (2nd attempt) #1559

Merged
merged 68 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
b1db89b
Temporary hack to fetch leaves from parachain block import
davxy Jul 28, 2022
c211faa
Merge branch 'master' into davxy/remove-leaves
davxy Aug 1, 2022
e9bd438
Working temporary PoC implementation
davxy Aug 1, 2022
8aaf2e8
Merge branch 'master' into davxy/remove-leaves
davxy Aug 10, 2022
e0a7da1
TEMPORARY COMMIT
davxy Aug 10, 2022
48367c6
Test excercising leaves pruning feature
davxy Aug 11, 2022
0613b3b
Removed global backend hack and introduced configurable leaves level …
davxy Aug 12, 2022
0ddaedc
Cleanup
davxy Aug 12, 2022
e811887
Nitpicks
davxy Aug 12, 2022
be75d7b
Removed debug logs
davxy Aug 12, 2022
bd8852b
Adjust leaves per level default value to be equal to the substrate ba…
davxy Aug 12, 2022
5b299dd
Better docs
davxy Aug 12, 2022
6e3cef9
Don't rely on the backend implementation order for the leaves
davxy Aug 19, 2022
7437611
Merge branch 'master' into davxy/remove-leaves-portable
davxy Aug 19, 2022
7453fe0
Leaves monitoring code encapsulation
davxy Aug 19, 2022
45fc0d1
Fix panic for unsigned subtraction underflow
davxy Aug 19, 2022
12427d5
Cargo fmt
davxy Aug 19, 2022
b2c197f
Improval leaves cache insertion/deletion strategy
davxy Aug 21, 2022
0c81d6b
Apply suggestions from code review
davxy Aug 22, 2022
aabc69a
Fix to default level limit
davxy Aug 22, 2022
55e598e
Remove redundant type
davxy Aug 23, 2022
d939757
Allow removal of non-leaves blocks and restore from backend
davxy Aug 30, 2022
ca4b383
Encapsulate limit monitor in its own module
davxy Aug 30, 2022
9c13d60
Proper cleanup for descendants
davxy Aug 31, 2022
765b59a
Apply suggestions from code review
davxy Sep 1, 2022
ffeeb22
Optimization
davxy Sep 1, 2022
7e6ed47
Fix
davxy Sep 1, 2022
5dbfe14
Fix typo
davxy Sep 1, 2022
2d9242c
Nit. Rename some fresher to freshest
davxy Sep 1, 2022
eff06c5
Dedicated ParachainBlockImport constructor with explicit LevelLimit
davxy Sep 8, 2022
7a8d663
Apply suggestions from code review
davxy Sep 14, 2022
4f15278
Fmt
davxy Sep 14, 2022
cc60027
Fix typo
davxy Sep 14, 2022
48e0cc9
Apply suggestions from code review
davxy Sep 14, 2022
7b51bb0
Use block number instead of u64 for import counter
davxy Sep 14, 2022
a9d0b1e
Separate target find and removal logic
davxy Sep 14, 2022
27e8f04
Code adjustments
davxy Sep 14, 2022
b675d48
Refactory to reduce cognitive complexity
davxy Sep 15, 2022
5b5f1d0
Merge branch 'master' into davxy/remove-leaves-portable
davxy Sep 15, 2022
1029cff
Apply suggestions from code review
davxy Sep 28, 2022
1bc4e61
Fix type
davxy Sep 28, 2022
d8f1c84
Restore monitor within the constructor
davxy Sep 28, 2022
9766f86
Rename Monitor 'update' to 'block_imported'
davxy Sep 28, 2022
19695b4
Keep track of invalidated leaves
davxy Oct 1, 2022
77e2549
Check import before add block to the monitor
davxy Oct 1, 2022
8ecb772
Update test
davxy Oct 1, 2022
3d86b2e
Adjust log levels
davxy Oct 1, 2022
59fcb7a
[ci] Apply cargo-fmt
Oct 1, 2022
283d33b
Extra debugging logs
davxy Oct 5, 2022
fd939e9
Added target to debug logs
davxy Oct 5, 2022
bbde030
Report errors as such
davxy Oct 7, 2022
7dd4ebe
Added extra log to the monitor restore method
davxy Oct 13, 2022
c89b2c2
Merge branch 'master' into davxy/remove-leaves-portable
davxy Oct 13, 2022
f08c6e4
Fix after master merge
davxy Oct 13, 2022
3cd5d31
Bugfix: level monitor cache should use block 'post-hash'
davxy Oct 13, 2022
f0c18c1
TEMPORARY
davxy Oct 18, 2022
d945832
Merge branch 'master' into davxy/remove-leaves-portable
davxy Oct 21, 2022
01c37b7
Minor tweaks
davxy Oct 21, 2022
70c7c7e
Merge branch 'master' into davxy/remove-leaves-portable
davxy Nov 10, 2022
61304ec
Small code refactory
davxy Nov 11, 2022
e92d651
Invalidate leaf only on removal
davxy Nov 11, 2022
bc1643d
Removed temporary logs
davxy Nov 16, 2022
408580e
Explicit recovery for announced candidate blocks (#1891)
davxy Dec 9, 2022
f396157
Merge branch 'master' into davxy/remove-leaves-portable
davxy Dec 9, 2022
92bb6ac
Fix after master merge
davxy Dec 9, 2022
819ade6
Merge branch 'master' into davxy/remove-leaves-portable
davxy Dec 20, 2022
8886f67
Moved block recovery related types in pov-recovery crate
davxy Dec 20, 2022
0eff586
Fix after refactory
davxy Dec 20, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Test excercising leaves pruning feature
  • Loading branch information
davxy committed Aug 11, 2022
commit 48367c6a4d534d8dc58fd93561ada9e4fcc77eab
23 changes: 14 additions & 9 deletions client/consensus/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,20 @@ impl<B: BlockT> ParachainConsensus<B> for Box<dyn ParachainConsensus<B> + Send +
/// This is used to set `block_import_params.fork_choice` to `false` as long as the block origin is
/// not `NetworkInitialSync`. The best block for parachains is determined by the relay chain. Meaning
/// we will update the best block, as it is included by the relay-chain.
pub struct ParachainBlockImport<I>(I);
pub struct ParachainBlockImport<I> {
inner: I,
level_limit: Option<usize>,
}

impl<I> ParachainBlockImport<I> {
/// Create a new instance.
pub fn new(inner: I) -> Self {
Self(inner)
pub fn new(inner: I, level_limit: Option<usize>) -> Self {
Self { inner, level_limit }
}
}

extern "Rust" {
fn check_leaves(number: u32);
fn check_leaves(number: u32, level_limit: usize);
}

#[async_trait::async_trait]
Expand All @@ -100,7 +103,7 @@ where
&mut self,
block: sc_consensus::BlockCheckParams<Block>,
) -> Result<sc_consensus::ImportResult, Self::Error> {
self.0.check_block(block).await
self.inner.check_block(block).await
}

async fn import_block(
Expand All @@ -112,9 +115,11 @@ where
let hash = params.header.hash();
log::debug!(target: "parachain", ">>>>>>>>>>>>>> Importing block {:?} @ {}", hash, number);

unsafe {
let number = number as *const _ as *const u32;
check_leaves(*number);
if let Some(limit) = self.level_limit {
unsafe {
let number = number as *const _ as *const u32;
check_leaves(*number, limit);
}
}

// Best block is determined by the relay chain, or if we are doing the initial sync
Expand All @@ -125,7 +130,7 @@ where

// Check if we require to prune a leaf before trying to import block

let res = self.0.import_block(params, cache).await;
let res = self.inner.import_block(params, cache).await;
if let Err(err) = &res {
dbg!(&err);
log::error!("RAW ERRROR: {:?}", err);
Expand Down
113 changes: 76 additions & 37 deletions client/consensus/common/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,26 @@ impl crate::parachain_consensus::RelaychainClient for Relaychain {
}
}

#[inline]
davxy marked this conversation as resolved.
Show resolved Hide resolved
fn build_and_import_block(mut client: Arc<Client>, import_as_best: bool) -> Block {
let builder = client.init_block_builder(None, Default::default());
build_and_import_block_ext(&*client.clone(), import_as_best, &mut client, None, None)
}

fn build_and_import_block_ext<B: InitBlockBuilder, I: BlockImport<Block>>(
builder: &B,
import_as_best: bool,
importer: &mut I,
at: Option<BlockId<Block>>,
timestamp: Option<u64>,
) -> Block {
let builder = match at {
Some(at) => match timestamp {
Some(ts) =>
builder.init_block_builder_with_timestamp(&at, None, Default::default(), ts),
None => builder.init_block_builder_at(&at, None, Default::default()),
},
None => builder.init_block_builder(None, Default::default()),
};

let block = builder.build().unwrap().block;
let (header, body) = block.clone().deconstruct();
Expand All @@ -113,7 +131,7 @@ fn build_and_import_block(mut client: Arc<Client>, import_as_best: bool) -> Bloc
block_import_params.fork_choice = Some(ForkChoiceStrategy::Custom(import_as_best));
block_import_params.body = Some(body);

block_on(client.import_block(block_import_params, Default::default())).unwrap();
block_on(importer.import_block(block_import_params, Default::default())).unwrap();

block
}
Expand Down Expand Up @@ -358,37 +376,61 @@ fn do_not_set_best_block_to_older_block() {

#[test]
fn prune_blocks_on_leaves_overflow() {
const NUM_BLOCKS: usize = 4;
const LEVEL_LIMIT: usize = 3;
const TIMESTAMP_MULTIPLIER: u64 = 60000;

let backend = Arc::new(Backend::new_test(1000, 1));
{
let mut mtx = BACKEND.lock().unwrap();
*mtx = Some(backend.clone());
}
{
let mut mtx = BACKEND.lock().unwrap();
*mtx = Some(backend.clone());
}

let client = Arc::new(TestClientBuilder::with_backend(backend).build());
let client = Arc::new(TestClientBuilder::with_backend(backend.clone()).build());

let blocks = (0..NUM_BLOCKS)
let mut para_import = ParachainBlockImport::new(client.clone(), Some(LEVEL_LIMIT));

let root_block = build_and_import_block(client.clone(), true);
let _num = root_block.header.number();
let root_id = BlockId::Hash(root_block.header.hash());

let blocks = (0..LEVEL_LIMIT)
.into_iter()
.map(|_| build_and_import_block(client.clone(), true))
.map(|i| {
build_and_import_block_ext(
&*client,
false,
&mut para_import,
Some(root_id),
Some(i as u64 * TIMESTAMP_MULTIPLIER),
)
})
.collect::<Vec<_>>();

{
let builder = client.init_block_builder(None, Default::default());
let _ = dbg!(&blocks.iter().map(|b| b.header.hash()).collect::<Vec<_>>());

let block = builder.build().unwrap().block;
let (header, body) = block.clone().deconstruct();
let leaves = backend.blockchain().leaves();
let _ = dbg!(&leaves);

let mut block_import_params = BlockImportParams::new(BlockOrigin::Own, header);
block_import_params.fork_choice = Some(ForkChoiceStrategy::Custom(false));
block_import_params.body = Some(body);
{
let builder = client.init_block_builder_with_timestamp(
&root_id,
None,
Default::default(),
LEVEL_LIMIT as u64 * TIMESTAMP_MULTIPLIER,
);

let block = builder.build().unwrap().block;
let (header, body) = block.clone().deconstruct();

let mut para_import = ParachainBlockImport::new(client);
block_on(para_import.import_block(block_import_params, Default::default())).unwrap();
let mut block_import_params = BlockImportParams::new(BlockOrigin::Own, header);
block_import_params.fork_choice = Some(ForkChoiceStrategy::Custom(false));
block_import_params.body = Some(body);

}
block_on(para_import.import_block(block_import_params, Default::default())).unwrap();
}

let leaves = backend.blockchain().leaves();
let _ = dbg!(&leaves);
}

// ========= START TEMPORARY HACK =========
Expand All @@ -404,7 +446,7 @@ use sc_client_api::{
};

#[no_mangle]
pub fn check_leaves(number: u32) {
pub fn check_leaves(number: u32, level_limit: usize) {
let lock = BACKEND.lock().unwrap();
let backend = lock.as_ref().expect("Backend should be already initialized");
let blockchain = backend.blockchain();
Expand All @@ -415,7 +457,7 @@ pub fn check_leaves(number: u32) {
let mut leaves = blockchain.leaves().expect("Error fetching leaves from backend");
log::debug!(target: "parachain", ">>>>>>>>>>>>>>>>>>>>> Leaves Number: {}", leaves.len());

// TODO: Just debugging
// TODO: Just debugging
for leaf in leaves.iter() {
let number = match blockchain.number(*leaf).ok().flatten() {
Some(n) => n,
Expand All @@ -427,33 +469,30 @@ pub fn check_leaves(number: u32) {
log::debug!(target: "parachain", ">>> (@{}) : {}", number, leaf);
}

// Magic number
const MAX_LEAVES_PER_LEVEL: usize = 3;

// First cheap check: the number of leaves at level `number` is always less than the total.
if leaves.len() >= MAX_LEAVES_PER_LEVEL {
// Now focus on the leaves at the given height.
leaves.retain(|hash|
// First cheap check: the number of leaves at level `number` is always less than the total.
if leaves.len() >= level_limit {
// Now focus on the leaves at the given height.
leaves.retain(|hash| {
blockchain.number(*hash).ok().flatten().map(|n| n == number).unwrap_or_default()
);
if leaves.len() < MAX_LEAVES_PER_LEVEL {
});
if leaves.len() < level_limit {
return
}

// TODO: double check
let mut remove_count = (leaves.len() + 1) - MAX_LEAVES_PER_LEVEL;
// TODO: double check
let mut remove_count = (leaves.len() + 1) - level_limit;

// TODO: Better strategy
let best = blockchain.info().best_hash;

// TODO: here we strongly assume that the leaves returned by the backend are returned
// by "age". This is actually true in our backend implementation...
// We can add a constraint to the leaves() method signature.
// TODO: here we strongly assume that the leaves returned by the backend are returned
// by "age". This is actually true in our backend implementation...
// We can add a constraint to the leaves() method signature.
for hash in leaves.into_iter().filter(|hash| *hash != best) {
log::debug!(target: "parachain", ">>>>>>>>>>>>>>>>>>>>> Removing block: {}", hash);
if backend.remove_leaf_block(&hash).is_err() {
log::warn!(target: "parachain", "Unable to remove block {}", hash);
continue;
continue
}
remove_count -= 1;
if remove_count == 0 {
Expand Down
2 changes: 1 addition & 1 deletion client/consensus/relay-chain/src/import_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ where

Ok(BasicQueue::new(
verifier,
Box::new(cumulus_client_consensus_common::ParachainBlockImport::new(block_import)),
Box::new(cumulus_client_consensus_common::ParachainBlockImport::new(block_import, None)),
None,
spawner,
registry,
Expand Down
1 change: 1 addition & 0 deletions client/consensus/relay-chain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ where
create_inherent_data_providers: Arc::new(create_inherent_data_providers),
block_import: Arc::new(futures::lock::Mutex::new(ParachainBlockImport::new(
block_import,
None,
))),
relay_chain_interface,
_phantom: PhantomData,
Expand Down
22 changes: 11 additions & 11 deletions polkadot-parachain/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ pub fn check_leaves(number: u32) {
let mut leaves = blockchain.leaves().expect("Error fetching leaves from backend");
log::debug!(target: "parachain", ">>>>>>>>>>>>>>>>>>>>> Leaves Number: {}", leaves.len());

// TODO: Just debugging
// TODO: Just debugging
for leaf in leaves.iter() {
let number = match blockchain.number(*leaf).ok().flatten() {
Some(n) => n,
Expand All @@ -544,33 +544,33 @@ pub fn check_leaves(number: u32) {
log::debug!(target: "parachain", ">>> (@{}) : {}", number, leaf);
}

// Magic number
// Magic number
const MAX_LEAVES_PER_LEVEL: usize = 3;

// First cheap check: the number of leaves at level `number` is always less than the total.
// First cheap check: the number of leaves at level `number` is always less than the total.
if leaves.len() >= MAX_LEAVES_PER_LEVEL {
// Now focus on the leaves at the given height.
leaves.retain(|hash|
// Now focus on the leaves at the given height.
leaves.retain(|hash| {
blockchain.number(*hash).ok().flatten().map(|n| n == number).unwrap_or_default()
);
});
if leaves.len() < MAX_LEAVES_PER_LEVEL {
return
}

// TODO: double check
// TODO: double check
let mut remove_count = (leaves.len() + 1) - MAX_LEAVES_PER_LEVEL;

// TODO: Better strategy
let best = blockchain.info().best_hash;

// TODO: here we strongly assume that the leaves returned by the backend are returned
// by "age". This is actually true in our backend implementation...
// We can add a constraint to the leaves() method signature.
// TODO: here we strongly assume that the leaves returned by the backend are returned
// by "age". This is actually true in our backend implementation...
// We can add a constraint to the leaves() method signature.
for hash in leaves.into_iter().filter(|hash| *hash != best) {
log::debug!(target: "parachain", ">>>>>>>>>>>>>>>>>>>>> Removing block: {}", hash);
if backend.remove_leaf_block(&hash).is_err() {
log::warn!(target: "parachain", "Unable to remove block {}", hash);
continue;
continue
}
remove_count -= 1;
if remove_count == 0 {
Expand Down