Chain Selection Subsystem Logic #3277
Conversation
rphmeier
added
A3-in_progress
github-actions
bot
added
A0-please_review
|
I've tested this to a satisfactory degree. I think the tests are very comprehensive, at least for the features we support now. The follow-up PR will add stagnation and with that we'll be able to test that approvals make chains viable again. I found a few bugs but did not find any bugs in |
|
|
||
| // Propagate viability update to descendants of the given block. This writes | ||
| // the `base` entry as well as all descendants. If the parent of the block | ||
| // entry is not viable, this wlil not affect any descendants. |
There was a problem hiding this comment.
| // entry is not viable, this wlil not affect any descendants. | |
| // entry is not viable, this will not affect any descendants. |
| } | ||
|
|
||
| #[derive(Debug, thiserror::Error)] | ||
| #[allow(missing_docs)] |
| // a leaf. this is fine according to the expected usage of the | ||
| // function. `None` responses should just `unwrap_or(required)`, | ||
| // so if the required block is the finalized block, then voilá. | ||
|
|
There was a problem hiding this comment.
Is this worth a tracing::debug! message if the best leaf is equiv the finalized required - it seems that should happen rather rarely and indicates potential issues in block production or overzealous finalization.
EDIT: this actually similar to what you mentioned in the guide.
| /// return true if `ancestor` is in `head`'s chain. | ||
| /// | ||
| /// If the ancestor is an older finalized block, this will return `false`. | ||
| fn contains_ancestor( |
There was a problem hiding this comment.
This could be part of the Backend.
There was a problem hiding this comment.
Yes, I can see that, but I believe it's simpler right now to expose a minimal Backend API and build on top of that. We're going to be disk-bound by this anyway.
There was a problem hiding this comment.
This function is \Omega(block_height) in the worst case, so if someone were to pass in the genesis hash we would basically read the entire chain, right?
I wonder whether it makes sense to put a limit on the depth of this loop...
There was a problem hiding this comment.
@Lldenaurois It's actually O(block_height - finalized_height) because we only store unfinalized subtrees here. Pretty much all the algorithms here have the same complexity but as long as we don't have thousands of unfinalized blocks they'll work fine.
There was a problem hiding this comment.
Right now it's more inefficient than it needs to be but we could easily keep a HashSet<Hash> of block-entries we've visited already and bail early when we encounter a parent hash we've already seen. Most chains will have some common ancestor before the last finalized block.
Co-authored-by: Bernhard Schuster <[email protected]>
…itytech/polkadot into rh-chain-selection-subsystem
|
bot merge |
|
Waiting for commit status. |
ghost
merged commit 
Closes #3235
Closes #3103
This implements the logic of the chain selection subsystem as described in #3262 . I took a slightly different approach to DB handling than what is done in other subsystems that wrap a DB, by using a
Backendtrait and definingBackendWriteOps, along with a helper structBackendOverlayfor maintaining a consistent state when querying changes that haven't been committed. I think this simplifies the code and will make it more difficult to write logic errors in the class of #3311The primary code here is located within the
treesubmodule which is used to maintain a coherent view of all unfinalized blocks and implements the core operations of import, finalization, reversion, approval, and stagnation.Unlike #3235 which describes separate fields for stagnation and reversion, I realized that I could combine these fields into a single field for tracking the earliest unviable ancestor of each block. This allows for further code reduction as we don't need to handle these in separate paths. The helper function for
propagate_viability_updatedoes most of the heavy lifting and the algorithm functions the same both when propagating a viable->unviable change and unviable->viable changes.TODO:
Deferred to later: