Can we eliminate the Ctx parameter?

[apoelstra]

It has been a long-term goal to eliminate the Ctx parameter and move it to a dyn ScriptContext or an enum or some sort of runtime-checked object. The reasoning is that we only need the context object "at the edges" of the API and having to thread it through pretty-much the entire codebase is exhausting and probably partly responsible for how large the codebase is.

However, I'm unsure how feasible it is to actually eliminate it. The context object is used:

• During string parsing
• During decoding (to decide whether multi or multi_a is active and the byte length of keys
• During encoding (to decide whether DescriptorPublicKey et al should be encoded as normal keys or xonly ones)
• During lifting (to check for resource limits and timelock mixing before allowing semantic analysis)

It's really the first point where we have trouble, because we want to be able to use the core::str::FromStr trait. However, this trait does not have any way to provide a context object other than as a type parameter. So if you try to parse a Miniscript that looks like c:pk_k(<xonly key>) it's really not clear how the user should indicate whether this is a Taproot script (okay) or a legacy one (not okay).

One thought is that we could split the Miniscript<Pk, Ctx> type into a Miniscript<Pk> type that does no validation and a Validated<Miniscript<Pk>, Ctx> type which does validation on construction. Then our Descriptor enum would contain Validated objects rather than directly containing Miniscript objects.

Then the unvalidated Miniscript<Pk> object could be parsed from a string or a script, re-serialized as a string, and typechecked (which includes a few useful things like checking for safeness, timelock mixing, malleability, etc), but not lifted or converted to a script. You would need to validate it to put it into a descriptor, encode it to script, lift it. In principle this would be a big breaking change, but I think that in practice very few people are using the Miniscript type directly. Looking at the docs for the type it seems to have a limited and ad-hoc API, and ideally everybody would be using Descriptor instead, which wouldn't substantially change. Especially if the Validated type implemented Deref to the underlying Miniscript.

[apoelstra]

Ok, after sleeping on this I have a much better idea. I propose that we effectively merge ScriptContext and ExtParams into a new data structure which describes all the limits/constraints on a Miniscript. It would look something like

#[non_exhaustive]
pub struct ValidationParams {
    pub allow_duplicated_pubkeys: bool,
    pub allow_xonly_pubkeys: bool,
    pub allow_standard_pubkeys: bool,
    pub allow_uncompressed_pubkeys; bool,
    pub max_script_size: usize,
    ...
}

This structure would represent a monotonic set of constraints. Unlike ScriptContext this structure would exist at runtime and unlike ExtParams it would be carried with Miniscript objects. (I am tempted to extend the existing ExtParams struct but I don't like the names of the fields and I need to break the API anyway, so I think a better strategy would be to leave the existing struct alone, deprecate it, and add a conversion function.)

Then we would have a set of constants representing the current script contexts. So LEGACY (legacy policy constraints + sanity), LEGACY_NONSTANDARD (legacy consensus constraints + sanity), LEGACY_INSANE (legacy consensus constraints), TAPROOT, TAPROOT_NONSTANDARD, TAPROOT_INSANE, etc. We would also have MAX (everything allowed) and SANE_MAX (only sanity rules enforced, no context-dependent limits).

The rules would be:

• By default when parsing a Miniscript from a string you use ValidationParams::SANE_MAX. Ditto for constructing Miniscript leaf nodes.
• We would have a method analogous to from_str_ext that lets you override this. (Or maybe we can tweak the existing from_str_ext to take an I: Into<ValidationParams>. I think that's technically breaking but only in obscure cases.)
• We would also have a fallible with_validation_params method which would apply the intersection of the existing parameters with new ones that the user provides.
• We would have the same method on Descriptor.
• When constructing Miniscript combinators, the parameters on the combination are the intersection of the parameters of the children. Construction is fallible, as we ensure the parameters are obeyed by the new Miniscript. Every Miniscript object will have a set of validation parameters and be valid with respect to its parameters. You will no longer be able to construct invalid Miniscript objects.
• When constructing a descriptor from a Miniscript, we add the LEGACY/SEGWIT_V0/TAPROOT constraints as appropriate.
• ...but have an alternate constructor which instead uses LEGACY_INSANE/SEGWIT_V0_INSANE/TAPROOT_INSANE (i.e. applies the bare minimum constraints for the descriptor to be consensus-viable) and expects the user to apply their own constraints.
• When encoding a Miniscript as Script, you have to specify whether to use the Taproot encoding or the non-Taproot encoding. (The difference is whether keys are xonly and signatures are Schnorr; our existing API uses the compile-time Ctx object to determine this and it would be too invasive to change that, e.g. by making MiniscriptKey types declare upfront whether they are xonly or not.)

I believe we can implement this in an easy-to-write/review way by starting with a copy of ExtParams then one-by-one moving the context-dependent rules from Ctx into the new struct, until Ctx is empty and can be dropped.

The result will be a cleaner, easier to understand, and more flexible API (e.g. it will let users set script size limits lower than any of the consensus limits, if they want). And it should have much smaller codegen since we aren't parameterizing every little thing by Ctx. It will also eliminate the possibility of constructing invalid Miniscript objects, which is currently easy to do because we don't keep track of what rules are supposed to be applied to what scripts. (#238 is an instance of this but there are many others.)

[sanket1729]

@apoelstra, just read through this.

Unlike ScriptContext this structure would exist at runtime and unlike ExtParams it would be carried with Miniscript objects.

Similar to how we store type information and ExtData (Structure representing the extra type properties of a fragment; like cost of satisfaction etc.). It feels a bit strange to carry around effectively constants and save them repeatedly in intermediate nodes of the tree. But it is not large amount of data, so not a big deal.

The result will be a cleaner, easier to understand, and more flexible API (e.g. it will let users set script size limits lower than any of the consensus limits, if they want).

100% agreed with the above approach.

[apoelstra]

Awesome, glad you are in agreement.

I will try to complete this before I get tired of this project, though once I dug into it I found that a lot of prepatory changes need to be made. (Well, the change would be much nicer with these changes anyway.)

My game plan is:

• Change policy::Semantic and policy::Concrete to be "real" structs with invariants and private fields, much like Miniscript is. Rather than just having a bare recursive enum { Child(Arc<Self>, Arc<Self>) } which users can construct willy-nilly
• Introduce the new ValidationParams structure and embed that in both new structs as well as Miniscript.
• Replace all the existing validation logic with methods on ValidationParams.

But in starting step 1, I found that I wanted to clean up the error-return logic of things like parsing and construction, add iterators in place of Vec<T> accessors, and eliminate recursion. This has led to the series of PRs that I've been opening recently.