rust-bitcoin / rust-miniscript

Support for Miniscript and Output Descriptors for rust-bitcoin
Creative Commons Zero v1.0 Universal
348 stars 136 forks source link

Track tree depth to avoid stack overflow #704

Closed sanket1729 closed 2 months ago

sanket1729 commented 2 months ago

We track tree depth at the time of parsing, but that is insufficient because wrappers also count towards the stack limit when creating Miniscript structure.

We could also fix this by fixing the wrappers to be limited to size 10 or something(wrappers are only for transformation of types, they don't provide any additional functionality).

Long term we want to move towards to complete non-recursive parsing, this is a good stop gap solution to address the current issues.

apoelstra commented 2 months ago

merged #697 -- can you rebase this?

apoelstra commented 2 months ago

Looks like the new test is failing.

sanket1729 commented 2 months ago

@apoelstra, the new test was not testing the correct thing :) . This is now fixed. 🤞 CI passes this time.

sanket1729 commented 2 months ago

@apoelstra ready for review.

apoelstra commented 2 months ago

Needs backport.

apoelstra commented 2 months ago

Opened https://github.com/rust-bitcoin/rust-miniscript/pull/705 to release new 12.x version.