--- Day changed Fri Aug 28 2020 00:00 -!- jonatack [~jon@37.164.15.34] has joined ##miniscript 01:43 -!- jeremyrubin [~jr@2601:645:c200:f539:10b:1840:c70a:412a] has quit [Ping timeout: 240 seconds] 02:07 -!- jonatack [~jon@37.164.15.34] has quit [Read error: Connection reset by peer] 02:39 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has joined ##miniscript 03:34 -!- midnight [~midnight@unaffiliated/midnightmagic] has quit [Ping timeout: 244 seconds] 03:37 -!- midnight [~midnight@unaffiliated/midnightmagic] has joined ##miniscript 04:42 -!- roconnor [~roconnor@host-184-164-25-9.dyn.295.ca] has joined ##miniscript 06:24 < andytoshi> ah excellent 06:53 -!- shesek [~shesek@164.90.217.137] has joined ##miniscript 06:53 -!- shesek [~shesek@164.90.217.137] has quit [Changing host] 06:53 -!- shesek [~shesek@unaffiliated/shesek] has joined ##miniscript 07:40 < andytoshi> sanket1729: reviewed 70 again. it is still full of unnecessary allocations and unidiomatic stuff :/ 07:43 < andytoshi> it'll be a while before we get it in ... if you need it for something it may be best to just rewrite it 07:44 < andytoshi> i don't want to be too mean to dr-orlovsky, and the code does get better on every iteration, but it's going to take a lot of iterations and it seems like he isn't thinking abuot the efficiency of his code at all 09:18 -!- jeremyrubin [~jr@2601:645:c200:f539:10b:1840:c70a:412a] has joined ##miniscript 11:51 < sanket1729> nah, I don't need it. 12:29 < darosior> andytoshi: re https://github.com/rust-bitcoin/rust-miniscript/pull/123#discussion_r479495313, i thought `scriptCode` was introduced with bip143 ? 12:32 < andytoshi> darosior: no, it's been around forever 12:32 < andytoshi> see https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1064 12:32 < andytoshi> where `scriptCode` is defined even before any checks for segwit or non-segwit are done 12:32 < darosior> Ok, thanks 12:32 < andytoshi> pre-segwit the scriptCode is modified in this insane way by the `FindAndDelete` function 12:32 < andytoshi> but otherwise it's the same 12:33 < andytoshi> (it is the currently executed script, from the most recent OP_CODESEPARATOR to the end) 12:33 < darosior> aah ok this is the one :) 12:33 < andytoshi> i *think* in all cases, for miniscript, the scriptCode is identical to the witness script 12:34 < darosior> No, because the witness script for WPkh returns script_pukey 12:35 < andytoshi> ah nice, i see 12:35 < andytoshi> that is so weird 12:36 < andytoshi> ok i'll open an issue for this, i think we want to modify your PR to (a) compute the scriptcode even for pre-segwit things; (b) to add a comment citing BIP143 12:36 < andytoshi> funny that it's even written as "the scriptCode is 0x1976a914{20-byte-pubkey-hash}88ac" without even pretending there's a logical reason for this 12:37 < darosior> I quoted bip143 for each witness case though 12:37 < darosior> // The item 5: 12:37 < darosior> // - For P2WPKH witness program, the scriptCode is `0x1976a914{20-byte-pubkey-hash}88ac`. 12:37 < andytoshi> oh look at that 12:37 < andytoshi> ok cool, i'll just open an issue about the non-segwit cases 12:37 < darosior> Do you want use to return sciprt_pubkey() for non-segwit ? 12:37 < darosior> us* 12:38 < andytoshi> i think so, yeah 12:38 < andytoshi> err .. 12:38 < andytoshi> script_pubkey for Bare and for Pk and PkH 12:38 < darosior> Actually i hesitated between erroring and returning script_pubkey() for "legacy", but.. It seems even more misleading 12:38 < andytoshi> for Sh i think we want to do d.encode()? 12:38 < darosior> As the function was defined (and documented as) BIP143 in mind 12:39 < andytoshi> ah yeah, i think we should change the docs to say that BIP143 only describes the segwit scriptcodes 12:39 < andytoshi> but that this will return legacy scptcodes too 12:41 < andytoshi> i wonder whether this function ought to go on Miniscript instead of Descriptor 12:42 < andytoshi> well, that can't quite work 12:43 < andytoshi> i guess, for all the cases that we *could* define this on Miniscript, it would be identical to Miniscript::encode. so never mind 12:43 < sanket1729> I just implemented sighash(), and witness_script is all what we need for signing 12:43 < sanket1729> I guess, we have script_code because it is written in the BIP 12:44 < sanket1729> And I think the script_code should also there for ShWpkh and ShWsh 12:44 < andytoshi> i think it'd be quick and easy to have sighash() call script_code() 12:45 < andytoshi> we don't need to do that now, we can have a followup pr once both the sighash() and script_code() prs are merged 12:46 < sanket1729> I think sighash belongs in the descriptor. I am unable to make use of the strong_types in a nice way. 12:46 < sanket1729> https://github.com/rust-bitcoin/rust-miniscript/pull/119/commits/0315fa5bd74a9fd92a978988075f0eb124ef76a0 This commit 12:46 < sanket1729> I also don't recollect why were wanting to have sighash inside context 12:47 < sanket1729> Because the context is already available from the descriptor 12:47 < andytoshi> because you don't always have a full descriptor available 12:48 < andytoshi> the goal was that you shouldn't need to compute a descriptor 12:48 < andytoshi> you could just infer the context from a transaction (or PSBT or whatever) and do everything from that 12:49 < andytoshi> e.g. in PSBT you can determine the context by looking at the available fields. you never have anything that looks like a "descriptor" becasue PSBT predates descriptors 12:49 < sanket1729> We need the descriptor/script_pubkey for computing though 12:49 < andytoshi> for computing what? 12:49 < sanket1729> computing sighash 12:49 < andytoshi> why isn't the context enough? 12:50 < sanket1729> Basically the same thing. We need either pbst or 2)tx + spending utxo 12:50 < sanket1729> tx + spending utxo is just descriptor 12:50 < sanket1729> Ah I see, you don't want to have the struct descriptor in the API? 12:51 < andytoshi> right 12:53 < sanket1729> So, it's again akward 12:53 < andytoshi> hmmm 12:53 < sanket1729> The segwitv0 one would have a different signature as it requires script_pubkey and value 12:53 < sanket1729> And legacy one does not need the value 12:53 < andytoshi> right i see 12:53 < sanket1729> What is the signature that you are thinking of? 12:54 < andytoshi> well, if we implement separate functions on the different Ctx structs, we can have different signatures 12:54 < andytoshi> but that feels kinda weird 12:54 < sanket1729> Yeah, that what's I did currently in this commit https://github.com/rust-bitcoin/rust-miniscript/pull/119/commits/0315fa5bd74a9fd92a978988075f0eb124ef76a0 12:55 < andytoshi> right, i'm looking at that 12:56 < andytoshi> maybe this code just doesn't belong in rust-miniscript 12:56 < andytoshi> or rather, the two functions that you implemented in that commit shuold be in rust-bitcoin 12:57 < andytoshi> and we should have a method on Descriptor, and a separate method on Psbt, which call into those based on which one makes sense for a given input 12:57 < andytoshi> and these methods would be a little bit redundant 12:59 < sanket1729> We can have the method sig always take value 12:59 < sanket1729> even for legacy case 12:59 < andytoshi> well, we're going to have a similar issue with tapscript 12:59 < andytoshi> where the tapscript sighash takes even more data 13:00 < andytoshi> it's also a bit misleading in the legacy case to have a "signature hash" method that doesn't actually commit to some of the data that you give it 13:00 < sanket1729> We need a UTXO struct. 13:00 < sanket1729> as an arugment 13:01 < sanket1729> right now all the API separate script_pubkey and value. 13:01 < sanket1729> I guess I mean TxOut 13:01 < andytoshi> it's fine to have them be separate 13:03 < sanket1729> I was considering that TxOut unifies this for segwit, legacy and taproot(I guess)? 13:03 < sanket1729> No, it does not. 13:05 < andytoshi> no, for taproot you still need more stuff, you need the other inputs' values and scriptpubkeys, and you need the merkle path 13:05 < andytoshi> also legacy does not sign the value 13:05 < andytoshi> and i really do not want an API that takes the value and just drops it on the ground 13:05 < andytoshi> that seems dangerously misleading 13:07 < sanket1729> If we want to have an API under ScriptContext, then it must take a unification of the all the possible things it needs. 13:07 < andytoshi> yeah. so i guess we don't want an API under ScriptContext 13:07 < sanket1729> Perhaps a struct with different enums as per Legacy, Segwit, Taproot etc 13:08 < andytoshi> maybe - but then that belongs in rust-bitcoin i think 13:08 < andytoshi> since we're forcing the user to determine what kind of data is needed 13:10 < sanket1729> The high level workflow I am considering is a user provides a descriptor and unsigned tx(psbt), we provide sighash if possible. 13:10 < sanket1729> If it is taproot spend, and the user does not provide merkle path we err 13:11 < andytoshi> if they have a psbt then they shouldn't have to provide a descriptor 13:11 < sanket1729> Yeah 13:11 < sanket1729> I think that does not belong in ScriptContext 13:11 < andytoshi> so i think it's reasonable to have two APIs, one that works with a descriptor + unsigned tx 13:11 < sanket1729> * I agree 13:11 < andytoshi> and one that works with a PSBT 13:11 < andytoshi> and yeah, neither of these make sense to be in ScriptContext 13:11 < andytoshi> and maybe the PSBT one belongs in rust-bitcoin rather than rust-miniscrit 13:12 < sanket1729> agreed. The psbt one is easier because it has all the info. 13:13 < sanket1729> We run the amount issue for unsigned tx + des one 13:13 < andytoshi> ah right 13:13 < andytoshi> sigh lol 13:14 < andytoshi> maybe we should only have the PSBT API 13:15 < sanket1729> Yup, makes most sense. 13:15 < sanket1729> So, there is nothing to in rust-miniscript? 13:16 < andytoshi> yeah, i think so, maybe we should remove all the signing logic from all the PRs 13:17 < andytoshi> becasue having descriptors/scripts is neither necessary nor sufficient to compute sighashes or signatures 13:18 < sanket1729> I like it. 13:18 < sanket1729> Stay away from private key apis 13:19 < andytoshi> lol, well, we still probably do need private key APIs 13:19 < andytoshi> to match Core 13:19 < sanket1729> for descriptors? 13:20 < andytoshi> yeah 13:20 < sanket1729> But signing would still go to rust-bitcoin? 13:21 < sanket1729> We just get the descriptor and return the key to the user 13:21 < sanket1729> They have to use rust-secp/rust-bitcoin for singing? 13:21 < sanket1729> *signing 13:22 < andytoshi> yeah 13:23 < andytoshi> basically all we need is the ability to parse and serialize descriptors that have secret keys in them 13:24 < sanket1729> What do you think about consensus testing to miniscripts generated by rust-miniscript against rust bindings of libbitcoinconsensus? 13:26 < sanket1729> Also about detecting duplicate keys 13:26 < sanket1729> I think we should only do for bitcoin::PublicKey 13:26 < sanket1729> Nah 13:26 < sanket1729> It does make sense 13:26 < sanket1729> *does not 13:26 < sanket1729> lol 13:36 < andytoshi> re consensus testing i think it's a great idea 13:37 < andytoshi> i think we could write a fuzz test that did this 13:37 < andytoshi> re detecting duplicate keys .... i'm torn 13:38 < andytoshi> i think in practice this might wind up being overly restrictive ... like, even though having duplicate keys breaks all our analysis it'll still "work" and maybe users who do this can reason through why they're OK 13:42 < sanket1729> I guess we can a generic function for detecting duplicates that does not work on DummyKeys 13:43 < sanket1729> By adding some helper function to MiniscriptKey, something like is_dummykey() 13:43 < sanket1729> That is default false 13:46 < andytoshi> heh 13:46 < andytoshi> no, i think that's too messy 13:46 < andytoshi> DummyKey is really there just for testing, we don't want its existence to mess up the real API 15:17 -!- jeremyrubin [~jr@2601:645:c200:f539:10b:1840:c70a:412a] has quit [Ping timeout: 240 seconds] 16:47 -!- jeremyrubin [~jr@c-24-4-56-108.hsd1.ca.comcast.net] has joined ##miniscript 18:20 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has quit [Ping timeout: 240 seconds] 18:20 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has joined ##miniscript 19:19 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has quit [Ping timeout: 264 seconds] 22:01 -!- sgeisler [sid356034@gateway/web/irccloud.com/x-xovlpgilwcahhtkw] has quit [Ping timeout: 240 seconds] 22:02 -!- sgeisler [sid356034@gateway/web/irccloud.com/x-vbvikcdqkjouvktk] has joined ##miniscript 22:12 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has joined ##miniscript 22:18 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has quit [Read error: Connection reset by peer] 22:18 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has joined ##miniscript 22:53 -!- shesek [~shesek@unaffiliated/shesek] has quit [Remote host closed the connection]