-
Notifications
You must be signed in to change notification settings - Fork 130
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
API improvements suggestions #545
Comments
Definitely ACK your second suggestion. For the first, I don't really see a sensible way to distinguish between taproot and non-taproot descriptors. In your suggestion, what would happen if the user calls |
Perhaps making the
The rationale for this suggestion is that there are transactions each with several hundreds of non-taproot inputs. If the program using miniscript API needs to call |
That's a great point about the extra compute to build a I think your API makes sense. I'd also like to change the existing |
We used miniscript to identify which M of N PubKeys correspond to the M signatures in a spent M of N multisig output, for the P2SH, P2WSH and P2TR (script path) outputs.
I wanted to make two suggestions about potential Miniscript API improvements:
The
interpreter.verify_sig()
function requiresprevouts
argument, that is a vector of all outpoints corresponding to all the transaction’s inputs. Only in the case of Taproot the signed message commits to the scriptPubKeys of all outputs spent by the transaction, per BIP 341. Even for the Witness case per BIP 143, only the amount from a single outpoint corresponding to the input being verified is required (and not the whole vector). For the legacy P2SH or bare script cases, no outpoints are required. Preparing thisprevouts
Vec
for all legacy cases is an unnecessary computation in the program that is using the miniscript API.We suggest either:
ALL(& [T])
(for taproot),ONE(T)
(for witness) orNONE
(for legacy)interpreter.verify_sig
function with three functionsinterpreter.verify_sig_taproot()
,interpreter.verify_sig_witness()
,interpreter.verify_sig_legacy()
.interpreter.verify_sig()
returnsfalse
instead of a concrete error. For example, we mistakenly passed aprevouts
vector that did not correspond to inputs length. The Error::PrevoutsSize thatcache.taproot_script_spend_signature_hash()
returned was transformed intofalse
here, and returned.It would be desirable to return a
Result<bool, sighash::Error>
frominterpreter.verify_sig()
, instead ofbool
.The text was updated successfully, but these errors were encountered: