rust-bitcoin / rust-bitcoin/rust-miniscript
Make `from_slice_delim` (expression parser) non-recursive
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 426
- Forks
- 200
- Avg merge
- 7d 17h
- Merged PRs (30d)
- 8
Description
Sorry if it's a dumb question, as far as I understand there is a recursion depth limit on from_slice_delim to avoid running out of stack space (based on https://github.com/sipa/miniscript/pull/5) when parsing an expression with round brackets. However, the parse function from sipa/miniscript is not recursive anymore (https://github.com/sipa/miniscript/pull/68). Worths make it non-recursive here as well? I noticed that some miniscripts that can be parsed on Core fails here because of this limit.
// https://github.com/sipa/miniscript/pull/5 for discussion on this number
const MAX_RECURSION_DEPTH: u32 = 402;
impl<'a> Tree<'a> {
/// Parse an expression with round brackets
pub fn from_slice(sl: &'a str) -> Result<(Tree<'a>, &'a str), Error> {
// Parsing TapTree or just miniscript
Self::from_slice_delim(sl, 0u32, '(')
}
pub(crate) fn from_slice_delim(
mut sl: &'a str,
depth: u32,
delim: char,
) -> Result<(Tree<'a>, &'a str), Error> {
if depth >= MAX_RECURSION_DEPTH {
return Err(Error::MaxRecursiveDepthExceeded);
}
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at Tree::from_slice_delim and review the linked sipa/miniscript parser change for how recursion was removed. The work is done when deeply nested miniscripts that Core accepts can be parsed here without the current recursion-depth failure, while existing parsing behavior remains intact.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- compilers
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100