Add height to tap tree #588
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tcharding
force-pushed
the
08-01-tap-tree
branch
from
0ae4b40 to
c960798
Compare
|
In 37fea86 you rename two methods -- we should add a deprecated version under the old name. (One gets renamed again in the last commit :) we only need to keep the original name.). Otherwise c960798 looks good, I think. I'm a little uncomfortable adding a public field which can be constructed in an incorrect way, but I think it's okay for now. In general I'd like to go through this library and make things non-constructable, but that's a much bigger change and I don't have a timeline for it. |
|
Re deprecation, can do. Re renaming twice, sorry about that :) I'll put a bit more effort in and re-spin. |
This is a private function on the `TapTree` struct, we can make the name more terse with no loss of meaning.
We use `TapTree` therefore the all lowercase version should be `tap_tree` not `taptree`.
tcharding
force-pushed
the
08-01-tap-tree
branch
from
c960798 to
88f96d3
Compare
Use the descriptor string currently in the unit tests and add a basic test of `tap_tree_height`. Done in preparation for adding a height field to the tap tree.
Currently calculating the height of a `TapTree::Tree` uses a recursive call, we can optimize this out by storing the height along with the tree nodes. Convert the `TapTree::Tree` tuple struct to a struct with named fields. Include a `height` field in the new struct.
tcharding
force-pushed
the
08-01-tap-tree
branch
from
88f96d3 to
476e358
Compare
|
Changes in force push:
|
apoelstra
approved these changes
Aug 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I stumbled across a
todoin the code last week and thought I'd hack it up.tap_tree_heightFrom commit log of patch 3