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

Feat: Add PositionPath and PositionPathIter to calculate the path and side positions from root to leaf #55

Merged
merged 10 commits into from
Jan 10, 2022

Conversation

bvrooman
Copy link
Contributor

@bvrooman bvrooman commented Jan 3, 2022

Related issues:

This PR introduces the PositionPath, a concrete representation of the path formed by a start position (root) and an end position (leaf). A path can be generated by calling path(...) on a start position, supplying the end position and leaves count as arguments. A path can be iterated upon by calling iter(), returning a PositionPathIter. Each iteration yields the next path node, as well as the corresponding side node.

The PositionPathIter wraps the existing PathIter, providing additional logic specific to the Position struct. The underlying path supplies the PositionPathIter with the specified leaves_count argument, allowing the iterator to handle imbalanced trees (Merkle Mountain Ranges).

This PR also modifies the existing common::Node trait to require a height member method on implementing structs, so that the PathIter can identify the correct starting index (distance from the MSB) when reading the next "left or right" instruction bit. This is necessary for dense (non-sparse) trees that do not fill their leaf index keys.

@bvrooman bvrooman marked this pull request as draft January 3, 2022 02:41
@bvrooman bvrooman changed the title Feat: Add Position::path_set to return the path and side positions from root to leaf Feat: Add PositionPath and PositionPathIter to calculate the path and side positions from root to leaf Jan 4, 2022
@bvrooman bvrooman marked this pull request as ready for review January 4, 2022 01:28
@bvrooman bvrooman force-pushed the bvrooman-feat-position-root-leaf-path branch from 21d6fbb to d132470 Compare January 8, 2022 20:22
@bvrooman bvrooman added the enhancement New feature or request label Jan 8, 2022
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

Generally looks fine to me! Will let the Rustaceans make sure the Rust is okay.

Dentosal
Dentosal previously approved these changes Jan 9, 2022
Copy link
Member

@Dentosal Dentosal left a comment

Choose a reason for hiding this comment

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

A couple of nits, but generally LGTM

src/sparse/node.rs Outdated Show resolved Hide resolved
src/common/position_path.rs Outdated Show resolved Hide resolved
Copy link
Member

@Dentosal Dentosal left a comment

Choose a reason for hiding this comment

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

LGTM

@bvrooman bvrooman merged commit 316e074 into master Jan 10, 2022
@bvrooman bvrooman deleted the bvrooman-feat-position-root-leaf-path branch January 10, 2022 21:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants