Skip to content

Top level checks are only used for miniscript APIs and not descriptor APIs. #788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sanket1729 opened this issue Dec 31, 2024 · 4 comments

Comments

@sanket1729
Copy link
Member

    #[test]
    fn test_2() {
        let desc = Descriptor::<String>::from_str("wsh(thresh(1,pk(A),s:pk(B)))").unwrap();
        let desc = Descriptor::<String>::from_str("wsh(thresh(1,pk(A),s:pk(A)))").unwrap(); // This should also ERR because duplicate keys.
        
        // But miniscript APIs are fine.
        let ms = Miniscript::<String, Segwitv0>::from_str("thresh(1,pk(A),s:pk(B))").unwrap();
        let ms = Miniscript::<String, Segwitv0>::from_str("thresh(1,pk(A),s:pk(A))").expect_err("This is not a valid miniscript");
    }

bitcoinfuzz/bitcoinfuzz#73 (comment)

We should have those in descriptor APIs too. In the above example, we have duplicate keys being allowed in descriptors.

@apoelstra
Copy link
Member

I plan to address this in 2025 by overhauling our sanity check system.

@erickcestari
Copy link

@apoelstra how's it going?

@apoelstra
Copy link
Member

@erickcestari I have a private 56-commit branch in which I have successfully implemented a new ValidationParams type which contains the size limits, the sanity rules, the key-related rules, etc. And I use this as the exclusive set of checks on the Miniscript type. Along the way I also introduced strong error types for string parsing and script decoding. I would like to add position/span information to these errors but that's not done yet.

I would like also to use it on the two Policy types but I haven't done that yet. I would also like to get rid of the Ctx parameter on the Miniscript type but even after 50+ commits of refactoring there's still more hacking to do.

Anyway once @sanket1729 merges #815 I'll start PRing the stuff that I've got in bits and pieces.

@erickcestari
Copy link

@erickcestari I have a private 56-commit branch in which I have successfully implemented a new ValidationParams type which contains the size limits, the sanity rules, the key-related rules, etc. And I use this as the exclusive set of checks on the Miniscript type. Along the way I also introduced strong error types for string parsing and script decoding. I would like to add position/span information to these errors but that's not done yet.

I would like also to use it on the two Policy types but I haven't done that yet. I would also like to get rid of the Ctx parameter on the Miniscript type but even after 50+ commits of refactoring there's still more hacking to do.

Anyway once @sanket1729 merges #815 I'll start PRing the stuff that I've got in bits and pieces.

Cool, thanks for the overview! Looking forward to seeing your PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants