--- Log opened Wed Dec 09 00:00:42 2020 00:36 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has quit [Read error: Connection reset by peer] 00:38 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has joined ##miniscript 01:12 -!- jeremyrubin [~jr@c-73-15-215-148.hsd1.ca.comcast.net] has quit [Ping timeout: 240 seconds] 03:09 -!- darosior9 is now known as darosior 08:05 < darosior> sanket1729: why in finalize here https://github.com/rust-bitcoin/rust-miniscript/blob/b342833844a1062b54f0fd14297161157ed111e8/src/psbt/finalizer.rs#L252-L293 you only check for empty sig / non-DER-encoded sigs *iff* an optional sighash flag is part of the PSBT input ? iiuc it's checked with the interpreter next, but still i don't get why 08:05 < darosior> conditioning the first verification 08:20 -!- jeremyrubin [~jr@c-73-15-215-148.hsd1.ca.comcast.net] has joined ##miniscript 08:22 < darosior> Also, what do you think about checking against libbitcoinconsensus in https://github.com/rust-bitcoin/rust-miniscript/blob/b342833844a1062b54f0fd14297161157ed111e8/src/psbt/finalizer.rs#L329 ? I did that in my homebrewed satisfier 15:31 < darosior> So my second comment is actually in the specs so nvm 17:00 < sanket1729> darosior: you are right, that was some legacy code which we had earlier. There should be no conditional sighash check. 17:00 < sanket1729> in the first part, because the same check is done later 17:03 < sanket1729> I have a PR for integration tests which tests satisfier against bitcoin core consensus and standard rules. The issue that we cannot check again libbitcoinconsensus because it does not expose standardness rule 17:03 < sanket1729> Maybe, there is no harm to check only consensus rules there. andytoshi how do you feel about adding libbitcoinconsensus dependancy 17:05 < andytoshi> we can add it as a dev-dependency 17:06 < andytoshi> i'm surprised that libbitcoinconsensus does not expose standardness flags 17:06 < andytoshi> it would almost be extra effort for it to not.. 17:06 < sipa> it's intentional 17:06 < andytoshi> ah hmm 17:06 < sipa> but there has been discussion about exposing standardness as well 17:07 < sipa> the problem is that it's inherently incomplete 17:07 < sipa> it can't observe mempool condtions etc 17:07 < andytoshi> yeah fair enough 17:18 < sipa> or rather, standardness on its own is only relevant to the extent it's an approximation of relayability... and that is inherently incomplete without actually asking a network-connected node 17:24 < sanket1729> andytoshi: I think darosior was suggesting doing the actual check in while do the interpreter check and not a test/dev-dependancy. 17:34 < andytoshi> lol absolutely not 17:35 < andytoshi> the point of rust miniscript is that we can work with script without linking into core's script interpreter 18:12 < sanket1729> I also pushed 204 when can be included in the patch release 18:35 < andytoshi> nice 18:36 < andytoshi> can you use tree.args.is_empty() rather than tree.args.len() == 9 18:36 < andytoshi> == 0 rather :P 18:37 < sanket1729> cool 18:38 < sanket1729> done.waiting CI 18:39 < andytoshi> cool. i also need to run some checks locally ... just reading the psbt commit now 18:39 < andytoshi> having trouble seeing what you did here 18:39 < andytoshi> oh, it looks like the old logic didn't default to sighash_all? 18:41 < andytoshi> lol weird that you can have a sighash flag on the sig itself, and that there's also a psbt field for it 18:41 < sipa> the psbt field is a suggestion for signers what sighash flag to use 18:42 < andytoshi> ah! 18:42 < sipa> it's kind of pointless i think 18:42 < andytoshi> yeah, ok, but that makes sense as a purpose 18:42 < andytoshi> even if it's unclear that it's useful for any real protocol 18:42 < andytoshi> and bip174 does indeed say that you should refuse to use sigs that don't match the psbt field 18:42 < andytoshi> so it's a bit more than a suggestion 18:44 < andytoshi> and i think you can jam a psbt by putting bogus sighash flags for everyone's signatures, then getting a Combiner to combine it with the psbt 18:44 < andytoshi> this is out of scope for this particular rust-miniscript PR, which i think is correct 18:44 < andytoshi> but it mildly concerns me 20:03 < sanket1729> yeah, I also found it a bit restricting because you cannot different sigs with different sighash flags for the same input. I don't know why one would do it, but it does not much sense in rejecting those 21:01 < andytoshi> i suspect this came from a workflow andy was thinking about before psbt had real-world usage 21:48 -!- shesek [~shesek@unaffiliated/shesek] has quit [Remote host closed the connection] --- Log closed Thu Dec 10 00:00:45 2020