[Taproot API project] move several files and tighten up iterator API #807
+268
−181
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
All this does is wrap the existing (depth, script) return value in a structure that has a pile of accessors on it. The accessors include a couple of expensive computations which are currently done manually. A later commit/PR will introduce a `SpendInfo` structure which will cache these computations and amortize much of the hashing needed to produce control blocks. But these new accessors simplify some things.
apoelstra
force-pushed
the
2025-04--taproot-2
branch
from
9479891 to
dd17df5
Compare
|
Drafting for a bit -- I need to test all feature combinations. |
We are going to move TapTree into the module (hence the name), and before we can do this we need to move the iterator.
This completes the move of all the taptree-related stuff into its own module, where it will be easier to mess around with this stuff.
Moving this separately from the rest of the TapTree stuff because I changed the Policy import to Semantic and the Error import to just crate::Error (since this is the only place that type is used in the taptree module, and I don't really like this type). So this is a move-only diff but it might not look like it, so I figured I should at least make the diff small.
The name `iter_scripts` is dumb and undiscoverable. More importantly, it's misleading -- this iterator does not yield scripts. It yields Miniscripts, which can be converted to scripts if you have ToPublicKey and are willing to pay a cost.
apoelstra
force-pushed
the
2025-04--taproot-2
branch
from
dd17df5 to
8e5d809
Compare
sanket1729
approved these changes
Apr 13, 2025
There was a problem hiding this comment.
code review ACK 8e5d809.
Thanks for the helpful commit messages.
| /// | ||
| /// This function is potentially expensive. | ||
| #[inline] | ||
| pub fn compute_script(&self) -> bitcoin::ScriptBuf { self.node.encode() } |
There was a problem hiding this comment.
I like this idea of potentially naming all functions that could be expensive with compute prefix.
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.
This PR mostly just moves files around, but it includes two nontrivial API changes:
iter_scriptsiterator toleavesonTrandTaptree(u8, Script)tupleIn the next PRs we will completely rewrite this iterator, and
Taptreeand these preparatory commits help to reduce the diff throughout the rest of the codebase.