ElementsProject / elements-miniscript

Creative Commons Zero v1.0 Universal
11 stars 14 forks source link

Possible to hit assertion failure in malleability check #29

Open apoelstra opened 1 year ago

apoelstra commented 1 year ago

Adding this to miniscript/mod.rs results in a panic.

    #[test]
    fn test_regression_29() {
        let _ = Descriptor::<String>::from_str("eltr(,thresh(1,spk_eq(,00)))");
    }
assertion failed: self.mall.non_malleable || self.corr.input != Input::Zero', /store/home/apoelstra/code/ElementsProject/elements-miniscript/fuzzing/src/miniscript/types/mod.rs:556:9
apoelstra commented 1 year ago

I think the issue is that our extensions are all Dissat::Unknown and also Input::zero. These combine in the threshold malleability logic from upstream which says that any threshold where not every sub-fragment is Dissat::Unique must be malleable.

(This logic seems wrong to me, btw, at least in the case that k == n, but we'll accept it for now.)

Anyway I think Input::Zero and Dissat::Unknown are logically incompatible ... I think that if we have Input::Zero we should always have Dissat::Unique, and changing this will fix the bug.

Trying to find a smaller test case..

sanket1729 commented 1 year ago

Converting this issue into revisiting all the correctness/malleability properties for extensions.