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

fork-tree: add support for find_node_mut_where#4784

Merged
rphmeier merged 8 commits intomasterfrom
sp-fork-tree
Feb 1, 2020
Merged

fork-tree: add support for find_node_mut_where#4784
rphmeier merged 8 commits intomasterfrom
sp-fork-tree

Conversation

@sorpaas
Copy link
Copy Markdown
Contributor

@sorpaas sorpaas commented Jan 31, 2020

(Split out of #4600, for easier review/merge.)

This adds support to get mutable references of Node in a ForkTree, by using a helper method find_node_index_where that returns the indexes to access the node. Mutable reference is needed for consensus such as Sassafras, because we need to push VRF proofs after the epoch data is created.

@sorpaas sorpaas added the A0-please_review Pull request needs code review. label Jan 31, 2020
Comment thread utils/fork-tree/src/lib.rs Outdated
Comment thread utils/fork-tree/src/lib.rs Outdated
Comment thread utils/fork-tree/src/lib.rs Outdated
sorpaas and others added 4 commits January 31, 2020 10:37
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Comment thread utils/fork-tree/src/lib.rs Outdated
// an iterative definition instead.
pub fn find_node_where<F, P, E>(
///
/// The returned indexes are from last to first, meaning the last is the least significant
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// The returned indexes are from last to first, meaning the last is the least significant
/// The returned indices are from last to first, meaning the last is the least significant

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's not clear what "least significant" means in this comment, but from reading the code i take it to be the earlier index in the traversal path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I updated the docs just calling it the "earliest" and "final" index.

@rphmeier rphmeier added A8-looksgood and removed A0-please_review Pull request needs code review. labels Feb 1, 2020
@rphmeier
Copy link
Copy Markdown
Contributor

rphmeier commented Feb 1, 2020

Thank you Wei!

@rphmeier rphmeier merged commit a52588d into master Feb 1, 2020
@rphmeier rphmeier deleted the sp-fork-tree branch February 1, 2020 15:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants