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
Cleanup
  • Loading branch information
davxy committed Aug 12, 2022
commit 0ddaedc200d04c0d2183e08ad4745807d474ca48
4 changes: 1 addition & 3 deletions client/consensus/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@ polkadot-primitives = { git = "https://github.com/paritytech/polkadot", branch =
# Cumulus
cumulus-relay-chain-interface = { path = "../../relay-chain-interface" }

# Temporary
# TEMPORARY (REMOVE THIS)
log = "0.4.17"
lazy_static = "1.4.0"
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }

[dev-dependencies]
futures-timer = "3.0.2"
Expand Down
11 changes: 1 addition & 10 deletions client/consensus/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,6 @@ where
params.origin == sp_consensus::BlockOrigin::NetworkInitialSync,
));

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

let res = self.inner.import_block(params, cache).await;
if let Err(err) = &res {
dbg!(&err);
log::error!("RAW ERRROR: {:?}", err);
let err_str = err.to_string();
log::error!("ERROR IMPORTING: {}", err_str);
}
res
self.inner.import_block(params, cache).await
}
}
123 changes: 19 additions & 104 deletions client/consensus/common/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use cumulus_test_client::{
use futures::{channel::mpsc, executor::block_on, select, FutureExt, Stream, StreamExt};
use futures_timer::Delay;
use polkadot_primitives::v2::Id as ParaId;
use sc_client_api::UsageProvider;
use sc_client_api::{Backend as _, UsageProvider};
use sc_consensus::{BlockImport, BlockImportParams, ForkChoiceStrategy};
use sp_blockchain::Error as ClientError;
use sp_consensus::BlockOrigin;
Expand Down Expand Up @@ -380,24 +380,19 @@ fn prune_blocks_on_leaves_overflow() {
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 client = Arc::new(TestClientBuilder::with_backend(backend.clone()).build());

// TODO: maybe we have to pass a weak ref
let mut para_import = ParachainBlockImport::new(
client.clone(),
backend.clone(),
LeavesLevelLimit::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 root_id = BlockId::Number(*root_block.header.number());

// Here we are using the timestamp value to generate blocks with different hashes.
let blocks = (0..LEVEL_LIMIT)
.into_iter()
.map(|i| {
Expand All @@ -411,103 +406,23 @@ fn prune_blocks_on_leaves_overflow() {
})
.collect::<Vec<_>>();

let _ = dbg!(&blocks.iter().map(|b| b.header.hash()).collect::<Vec<_>>());

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

{
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 leaves = backend.blockchain().leaves().unwrap();
let mut expected = blocks.iter().map(|b| b.header.hash()).collect::<Vec<_>>();

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);
assert_eq!(leaves.len(), LEVEL_LIMIT);
assert_eq!(leaves, expected);

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

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

// ========= START TEMPORARY HACK =========

use std::sync::Mutex as StdMutex;

lazy_static::lazy_static! {
static ref BACKEND: StdMutex<Option<Arc<Backend>>> = StdMutex::new(None);
}
use sc_client_api::{
backend::Backend as _,
blockchain::{Backend as _, HeaderBackend as _},
};

#[no_mangle]
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();

log::debug!(target: "parachain", ">>>>>>>>>>>>>>>>>>>>> BLOCK Number: {}", number);

// Interface contract: results are ordered best (longest, highest) chain first.
let mut leaves = blockchain.leaves().expect("Error fetching leaves from backend");
log::debug!(target: "parachain", ">>>>>>>>>>>>>>>>>>>>> Leaves Number: {}", leaves.len());

// TODO: Just debugging
for leaf in leaves.iter() {
let number = match blockchain.number(*leaf).ok().flatten() {
Some(n) => n,
None => {
let msg = format!("Unexpected missing number for leaf {}", leaf);
panic!("{}", msg);
},
};
log::debug!(target: "parachain", ">>> (@{}) : {}", number, leaf);
}

// 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() < level_limit {
return
}

// 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.
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
}
remove_count -= 1;
if remove_count == 0 {
break
}
}
let block = build_and_import_block_ext(
&*client,
false,
&mut para_import,
Some(root_id),
Some(LEVEL_LIMIT as u64 * TIMESTAMP_MULTIPLIER),
);

let leaves = blockchain.leaves().expect("Error fetching leaves from backend");
log::debug!(target: "parachain", ">>>>>>>>>>>>>>>>>>>>> Leaves Number (post-del): {}", leaves.len());
}
let leaves = backend.blockchain().leaves().unwrap();
expected.remove(0);
expected.push(block.header.hash());
assert_eq!(leaves.len(), LEVEL_LIMIT);
assert_eq!(leaves, expected);
}

// ========= END TEMPORARY HACK =========
3 changes: 0 additions & 3 deletions polkadot-parachain/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,6 @@ cumulus-relay-chain-interface = { path = "../client/relay-chain-interface" }
cumulus-relay-chain-inprocess-interface = { path = "../client/relay-chain-inprocess-interface" }
cumulus-relay-chain-rpc-interface = { path = "../client/relay-chain-rpc-interface" }

# Temporary
lazy_static = "1.4.0"

[build-dependencies]
substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }

Expand Down