rust-bitcoin / rust-bitcoin/rust-miniscript

Make `from_slice_delim` (expression parser) non-recursive

Open
#696 1 comment 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.