Do not prune blocks with Grandpa justifications#10893
Conversation
|
/cmd prdoc --audience node_dev node_operator |
skunert
changed the title
|
/cmd fmt |
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
| where | ||
| F: Fn(&Justifications) -> bool + Send + Sync, | ||
| { | ||
| fn should_preserve(&self, justifications: &Justifications) -> bool { |
There was a problem hiding this comment.
nit: how about: keep or retain ?
| let should_preserve = if let Some(justification) = | ||
| current_transaction_justifications.get(&hash) | ||
| { | ||
| let justifications = Justifications::from(justification.clone()); | ||
| self.block_pruning_filters | ||
| .iter() | ||
| .any(|f| f.should_preserve(&justifications)) | ||
| } else if let Some(justifications) = self.blockchain.justifications(hash)? { | ||
| self.block_pruning_filters | ||
| .iter() | ||
| .any(|f| f.should_preserve(&justifications)) | ||
| } else { | ||
| false | ||
| }; |
There was a problem hiding this comment.
Nit: some duplication could be avoided here, by something like this, probably this reads more smoothly:
let justifications = current_transaction_justifications
.get(&hash)
.map(|j| Justifications::from(j.clone()))
.or_else(|| self.blockchain.justifications(hash)?);
let should_preserve = justifications
.map(|j| {
self.block_pruning_filters
.iter()
.any(|f| f.should_preserve(&j))
})
.unwrap_or(false);
bkchr
left a comment
bkchr
left a comment
There was a problem hiding this comment.
Hmm, I don't like the approach :D It works right now because we keep the headers around, but in a "perfect" world we would also not need the headers anymore. Right now they are just kept to support "fast sync".
IMO it would make more sense to be able to tell the db to keep certain headers/justification around forever and that it can not prune them. So, something we say at import/maybe also setable later. But this makes it more flexible for the future.
As per quick discussion yesterday, header pruning should not be a problem. I expect it to be in line with the blocks pruning, so we can use the closure to decide an all-or-nothing pruning approach. Chose some more generic naming though. |
# Conflicts: # Cargo.lock
8 participants
Warp sync requires GRANDPA justifications at authority set change boundaries to construct proofs. When block pruning is enabled, all block bodies are removed regardless of whether they contain important justifications. The pruned nodes can then not be used to fetch warp proofs.
In this PR I add the capability to filter which blocks can be safely pruned. For parachain nodes, everything can be pruned, solochain nodes using grandpa keep blocks with justifications.
Overview:
sc-client-db
sc-consensus-grandpa
sc-service
Nodes updated
fixes #2733