When are sanity_checks supposed to be run?

On the Miniscript type, which users are not really supposed to use directly, we run sanity checks (e.g. all branches are "safe") on from_str. You have to use from_str_ext or from_str_insane to override this.

However, we don't do the same for Descriptor::from_str, which simply parses a tree then calls Miniscript::from_tree, bypassing all sanity checks. So this seems backward -- by default for the users-shouldn't-use-this type we have sanity checks, while for descriptors, you have to manually call the sanity_check method.

However^2, for Taproot descriptors we do run the sanity checks on parsing, because we have bizarre special-purpose parsing logic which actually calls Miniscript::from_str for individual tapbranches rather than calling Miniscript::from_tree.

You can see this with the following unit test

commit 966524264e948b3e68f68a10eba513692f2839e6
Author: Andrew Poelstra <apoelstra@wpsoftware.net>
Date:   Sat Aug 31 20:36:29 2024 +0000

    f unit test

diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs
index 0b0bd02b..72f533f7 100644
--- a/src/descriptor/mod.rs
+++ b/src/descriptor/mod.rs
@@ -2003,6 +2003,12 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
         Descriptor::<DescriptorPublicKey>::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1;2;3>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/<0;1;2>/*)))").unwrap_err();
     }

+    #[test]
+    fn taproot_s_check() {
+        Descriptor::<DescriptorPublicKey>::from_str("sh(or_i(pk(0202baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a66a),1))").unwrap();
+        Descriptor::<DescriptorPublicKey>::from_str("tr(02baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a66a,1)").unwrap_err();
+    }
+
     #[test]
     fn test_context_pks() {
         let comp_key = bitcoin::PublicKey::from_str(

We have the identical policy expressed as a sh (can parse no problem) or as a tr (will not parse, complains about the sigless `1( branch).

Start in src/descriptor/mod.rs, comparing Descriptor::from_str with Miniscript::from_str, Miniscript::from_tree, and the taproot parsing path. Run or extend the taproot_s_check test and inspect the existing sanity_check behavior; done means the parsing policy is consistent for sh and tr descriptors.