--- Log opened Mon Sep 27 00:00:19 2021
00:39 < darosior> Should we also restrict non minimal VERIFYs? ToScript will always minify it so it breaks roundtrip if we accept eg <pk> CHECKSIG EQUAL VERIFY
00:42 < darosior> s/EQUAL//
01:10 < darosior> Well, added a commit that does that to #72
05:04 <@sipa> darosior: nice catch
05:43 < darosior> I also think we have an off-by-one on the number of stack elements (doesn't account for the witscript push), see the opened PRs
08:48 <@sipa> darosior: i've been thinking about the IsValid/IsValidTopLevel/IsSafe/IsSafeTopLevel distinctions, and i think the situation is a bit more complex
08:49 <@sipa> e.g. i think that (ops.stat > MAX_OPS_PER_SCRIPT) can be under IsValid, because failing that will unconditionally make the script invalid
08:50 <@sipa> but GetOps() > MAX_OPS_PER_SCRIPT should probably be an IsSafe() thing, as failing that may still mean that valid branches exist - just not all of them
08:50 <@sipa> stack size should purely be an IsSafe thing
08:51 <@sipa> sanket1729: opinions on whether to include a canonical list of checks which make a miniscript unconditionally illegal ?
08:51 < darosior> Even if there is not a single path with a valid stack size?
08:52 <@sipa> darosior: we have no way of testing that, i think?
08:52 <@sipa> the current code only computes the maximum stack size
08:52 <@sipa> for any satisfaction
08:52 < darosior> Oh right
08:52 < darosior> Hmm then yes i should not have it in Decode()
08:52 <@sipa> we could also compute the minimal one of course, with just as much effort, and turn that into an IsValid check
08:53 <@sipa> but i think we should perhaps first have a discussion about what exactly we're going to outlaw unconditionally
08:54 < darosior> witnessScript > MAX_STD_P2WSH_SIZE should be an IsValid() thing i think
08:56 < darosior> Maybe it's the only one? Sounds like any satisfaction-dependent rule should be in IsSafe()
08:56  * darosior checks the Rust implem
08:57 <@sipa> i'd certainly include in those (a) invalid type [and B for toplevel] (b) ScriptSize() <= 10000
08:57 <@sipa> scriptsize <= 3600... mhmm
08:58 < darosior> (b) is implied by MAX_STD_P2WSH_SIZE
08:58 <@sipa> yes, but i'm not sure the standard witness size should be include in IsValid
08:59 <@sipa> but mostly, i think the determination of what miniscript IsValid() should be part of the language definition, so it should be documented and consistent across implementations
09:01 < darosior> Yes
09:05 < darosior> So, the Rust implementation would currently do this: for any decoding, check the type, ScriptSize() <= 10000 and for compressed keys if under Segwit context (we do this implicitly in the C++ implem by matching on 33 bytes for the pubkeys). Then, when parsing "sane" scripts (the default), it would check whether all spending paths require a
09:05 < darosior> signature, check for malleability, the satisfaction max ops count, that the witscript is < max std size, that the script has no repeated keys and no mixed timelocks.
09:18 <@sipa> i'm fine with copying that behavior
09:18 <@sipa> (the c++ code assumes p2wsh overall, so no need to account for other cases)
09:35 < darosior> Updated https://github.com/sipa/miniscript/pull/72 with this definition of validity and to only check for validity when decoding from Script
09:57 < darosior> And opened https://github.com/sipa/miniscript/pull/75 for the safety part
09:57 < darosior> Sorry sanket1729 for the conflict but it should be trivial
09:57 < sanket1729> catching up to all the discussion now. Was away for the past few days
11:18 < sanket1729> agreed. We should make a canonical list of conditions. https://github.com/rust-bitcoin/rust-miniscript/blob/master/doc/resource_limitations.md
11:18 < sanket1729> sipa, darosior: How do you like using the phrases safety, validity and sanity as described above? 
11:19 <@sipa> what is the difference between sanity and safety?
11:19 <@sipa> oh, i see
11:19 <@sipa> you're using safe to refer to the 's' property
11:19 < sanket1729> Yeah
11:20 < sanket1729> We can combine safety and sanity
11:20 <@sipa> i would not do that; the 's' type property has a very specific meaning w.r.t. malleability
11:20 <@sipa> i don't want to conflate it with the global "can we reason about this completely"
11:21 <@sipa> what does rust-miniscript do with scripts over 3600 but below 10000 bytes?
11:22 <@sipa> (when parsing, decomposing, ...)
11:22 < sanket1729> Provide separate APIs. parse_insane/ from_str_insane
11:22 <@sipa> and above 10000, even _insane fails?
11:22 < sanket1729> Yes
11:23 <@sipa> sounds reasonable to me
11:23 <@sipa> though i think that may be the only place where something nonstandard is accepted?
11:24 < sanket1729> I think andytoshi wanted a block explorer support for miniscript which is why we had support for non-standard. 
11:24 < sanket1729> We can decide to enforce standardness in the spec itself to simplify things
11:24 <@sipa> well, we _do_
11:25 <@sipa> iirc
11:25 <@sipa> the typing rules / fragments are written to assume standardness
11:25 <@sipa> e.g. the SIZE in the j: wrapper
11:25 < jeremyrubin> i think it's good to code wise keep standardness and consensus separate to the extent possible, future relaxations or changes to standardness can be more easily reviewed?
11:26 <@sipa> it's really not that simple - miniscript in general is defined to assume standardness
11:27 <@sipa> i'm not opposed to allowing this one exception (scripts above 3600), but it feels a bit inconsistent
11:27 <@sipa> unless there are other examples i'm missing
11:29 < sanket1729> Agreed. It is a bug in rust-miniscript that we allow > 3600 scripts.
16:31 -!- ksedgwic [~ksedgwicm@2001:470:69fc:105::ce1] has quit [Ping timeout: 246 seconds]
16:41 -!- ksedgwic [~ksedgwicm@2001:470:69fc:105::ce1] has joined ##miniscript
--- Log closed Tue Sep 28 00:00:20 2021