We shouldn't implement `AssetProvider` for `T: Satisfier`

[apoelstra]

If you have a satisfier, you should just call descriptor.satisfy() with it. There's no reason to call .plan() except if you have some extra knowledge about which keys are available (or not available) and want to save calls to your satisfier.

If you have a T: Satisfier and call descriptor.plan().satisfy() with it, you get the same result as if you'd called descriptor.satisfy() except that you lookup all your signatures twice.

cc @afilini

[apoelstra]

Oh, actually I think this is fine (it's a bit silly but if you call .plan().satisfy() then it's visible that you're making two calls).

The real issue (that I'm getting from somebody at Blockstream reporting) is that if you just call descriptor.satisfy() then you still get two calls to the satisfier, and (I think) this is because of refactorings related to the Plan API.

[philipr-za]

I made a minimal reproduction of what I observed where the respective Satisfier method that yields a signature is called twice when you execute .satisfy(). It was an unexpected behaviour for me and meant I had to write a caching mechanism for my satisfier so as not to calculate the signature twice.

https://github.com/philipr-za/rust-miniscript/blob/philip-satisfier-issue/examples/satisfier.rs

[apoelstra]

Thanks! Confirmed that the issue is in

rust-miniscript/src/miniscript/satisfy.rs

Lines 1790 to 1792 in 2ca5a35

	Satisfaction::<Placeholder<Pk>>::build_template(term, &stfr, root_has_sig, leaf_hash)
	.try_completing(stfr)
	.expect("the same satisfier should manage to complete the template")

where both build_template and try_completing query the satisfier. We shouldn't do this. If we have actual signatures at template-build time then we should use them rather than throwing them away and re-requesting them.

I have a followup prepared to #849 which rewrites the satisfier to be non-recursive (eliminating a ton of overly-generic code and giant copy/pasted function argument lists). It preserves this behavior but hopefully this issue will be easier to deal with after that change.