--- Day changed Wed Apr 01 2020 00:06 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews 00:41 -!- vasild_ [~vd@gateway/tor-sasl/vasild] has joined #bitcoin-core-pr-reviews 00:44 -!- vasild [~vd@gateway/tor-sasl/vasild] has quit [Ping timeout: 240 seconds] 00:44 -!- vasild_ is now known as vasild 01:45 -!- Talkless [~Talkless@hst-227-49.splius.lt] has joined #bitcoin-core-pr-reviews 02:00 -!- ncantu [~ncantu@37.171.37.61] has quit [Ping timeout: 265 seconds] 02:00 -!- ncantu [~ncantu@37.173.187.40] has joined #bitcoin-core-pr-reviews 02:50 -!- masterdonx2 [~masterdon@213.183.54.167] has joined #bitcoin-core-pr-reviews 02:54 -!- MasterdonX [~masterdon@103.108.117.132] has quit [Ping timeout: 256 seconds] 03:05 -!- Theron71Borer [~Theron71B@ns334669.ip-5-196-64.eu] has joined #bitcoin-core-pr-reviews 03:08 -!- ncantu [~ncantu@37.173.187.40] has quit [Ping timeout: 256 seconds] 03:11 -!- Theron71Borer [~Theron71B@ns334669.ip-5-196-64.eu] has quit [Ping timeout: 256 seconds] 04:00 -!- apaval_ [~apaval@124.108.23.110] has quit [Remote host closed the connection] 04:00 -!- apaval_ [~apaval@124.108.23.110] has joined #bitcoin-core-pr-reviews 04:52 -!- sipa [~pw@gateway/tor-sasl/sipa1024] has quit [Ping timeout: 240 seconds] 04:54 -!- sipa [~pw@gateway/tor-sasl/sipa1024] has joined #bitcoin-core-pr-reviews 05:28 -!- slivera_ [~slivera@116.206.228.243] has quit [Remote host closed the connection] 07:07 < ryanofsky> gwillen: thanks, that worked! I had no idea the Tree: widget on /commits/ pages let you create new branches 07:20 -!- shesek [~shesek@unaffiliated/shesek] has quit [Read error: Connection reset by peer] 08:14 -!- apaval_ [~apaval@124.108.23.110] has quit [Ping timeout: 265 seconds] 08:50 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:11be:312f:adcf:42dd] has joined #bitcoin-core-pr-reviews 08:51 -!- molly [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 08:54 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 258 seconds] 08:55 -!- molz_ [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 08:58 -!- molly [~molly@unaffiliated/molly] has quit [Ping timeout: 258 seconds] 09:02 -!- lightlike [~lightlike@p200300C7EF12F30068A69683A0FF44F3.dip0.t-ipconnect.de] has joined #bitcoin-core-pr-reviews 09:12 -!- AlistairMann [56b41c67@host86-180-28-103.range86-180.btcentralplus.com] has joined #bitcoin-core-pr-reviews 09:13 -!- molz_ [~molly@unaffiliated/molly] has quit [Ping timeout: 258 seconds] 09:18 < jnewbery> ryanofsky: wrong channel 09:18 < jnewbery> we'll get started in about 40 minutes 09:22 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has quit [Ping timeout: 252 seconds] 09:24 -!- jonatack [~jon@37.171.65.83] has joined #bitcoin-core-pr-reviews 09:46 -!- theStack [~theStack@81.223.165.6] has joined #bitcoin-core-pr-reviews 09:56 -!- josh-bushwick [457a83ef@ool-457a83ef.dyn.optonline.net] has joined #bitcoin-core-pr-reviews 09:57 < gwillen> ryanofsky: heh, as observed by jnewbery this is indeed a different channel, but I'm glad it worked! You are very welcome. I guess I should make a PR to add this to one of the documents of developer tips. 09:57 -!- r251d [~r251d@162-196-59-192.lightspeed.irvnca.sbcglobal.net] has joined #bitcoin-core-pr-reviews 09:59 -!- Guest41 [56c00d0d@gateway/web/cgi-irc/kiwiirc.com/ip.86.192.13.13] has joined #bitcoin-core-pr-reviews 10:00 < jnewbery> #startmeeting 10:00 < jnewbery> Hi folks! Welcome to Bitcoin Core Review club. Feel free to say hi to let everyone know you're here. 10:00 < fjahr> hi 10:00 < nehan_> hi 10:00 < josh-bushwick> hi 10:00 < michaelfolkson> Bitcoin Corrrr Review Club 10:00 < michaelfolkson> hi 10:00 < theStack> hi 10:00 < jonatack> hi 10:00 < emzy> hi 10:00 < jnewbery> Special welcome to everyone who's at their first review club meeting. 10:01 -!- jesseposner [~jesseposn@c-67-188-220-154.hsd1.ca.comcast.net] has joined #bitcoin-core-pr-reviews 10:01 < amiti> hi 10:01 < r251d> hi 10:01 < pinheadmz> hi 10:01 < AlistairMann> hi 10:01 -!- yrok [54112948@84.17.41.72] has joined #bitcoin-core-pr-reviews 10:01 < jnewbery> Feel free to jump in at any time to ask questions. There are no stupid questions here. We're all here to learn. 10:01 < jkczyz> hi 10:01 -!- Guest41 [56c00d0d@gateway/web/cgi-irc/kiwiirc.com/ip.86.192.13.13] has quit [Remote host closed the connection] 10:01 -!- Guest41 [56c00d0d@gateway/web/cgi-irc/kiwiirc.com/ip.86.192.13.13] has joined #bitcoin-core-pr-reviews 10:01 -!- lightlike [~lightlike@p200300C7EF12F30068A69683A0FF44F3.dip0.t-ipconnect.de] has quit [Ping timeout: 240 seconds] 10:01 < jnewbery> Notes in the usual place: https://bitcoincore.reviews/18401.html 10:02 -!- excess [~excess@45.162.229.132] has joined #bitcoin-core-pr-reviews 10:02 < jnewbery> Who had a chance to review the PRs this week? 10:02 < jnewbery> y/n 10:02 < pinheadmz> o/ 10:02 < fjahr> y 10:02 < nehan_> y 10:02 < michaelfolkson> y 10:02 < theStack> y 10:02 < jkczyz> y 10:02 < r251d> y 10:02 < jonatack> y 10:02 < instagibbs> y 10:03 < jnewbery> fantastic! The code changes are quite small and mechanical, but there's a _lot_ of context, so I thought it'd be fun to dig into the surrounding concepts a bit. 10:03 -!- lightlike [~lightlike@p200300C7EF12F30068A69683A0FF44F3.dip0.t-ipconnect.de] has joined #bitcoin-core-pr-reviews 10:03 < jnewbery> I'll ask questions, but like I said earlier, feel free to jump in at any time with your own questions/comments 10:03 < jnewbery> first question: A new PrecomputedTransactionData default constructor is added which doesn’t initialize the data members. Where do they get initialized instead? 10:04 < jnewbery> (this is in PR 18401: Initialize PrecomputedTransactionData in CheckInputScripts) 10:04 < jkczyz> In it's Init method 10:04 < jnewbery> jkczyz: right, and where is that called? 10:04 < pinheadmz> in the script interpreter ? 10:04 < fjahr> As the PR title says, in CheckInputScripts :) 10:04 < theStack> it's called in the function CheckInputScripts() 10:04 -!- gloooooo [a5e33c1c@165.227.60.28] has joined #bitcoin-core-pr-reviews 10:04 < jnewbery> fjahr: exactly! 10:04 < jkczyz> CheckInputScripts 10:05 < michaelfolkson> That is it's Init method? 10:05 < jnewbery> why are we calling Init there instead of the ctor? 10:05 -!- chanho [b8980f30@cpe-184-152-15-48.nyc.res.rr.com] has joined #bitcoin-core-pr-reviews 10:05 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has joined #bitcoin-core-pr-reviews 10:06 < michaelfolkson> Because the initialization needs to happen later... 10:06 < instagibbs> we need valid (uninitialized) caches in place for pointer validity? 10:06 < nehan_> to set things up for a later change which requires knowing about spent outputs 10:06 < jnewbery> nehan_: yes, that's right 10:07 < jnewbery> you can see in the later commit in the taproot PR that we add spent_outputs to the txdata object: https://github.com/bitcoin/bitcoin/pull/17977/commits/6dcc85e3347fe8a0c5e3e578176fd38fa093df39#diff-24efdb00bfbe56b140fb006b562cc70bR1515 10:07 < nehan_> how do we know it's ok to wait until then? Functions like PolicyScriptChecks() use the uninitialized txdata 10:07 < jnewbery> instagibbs: you're talking about this comment: https://github.com/bitcoin/bitcoin/pull/18401/commits/b409b611eb0fc6c71f107b5313ab79ecaf57f479#diff-24efdb00bfbe56b140fb006b562cc70bR2079 ? 10:08 < instagibbs> I answered the wrong question, but yeah 10:08 < jnewbery> That's unchanged by this PR (although I've expanded the comment a bit). Can you explain what's going on there? 10:08 < instagibbs> you asked why uninitialized then :) 10:08 -!- ecurrencyhodler [68ac287f@cpe-104-172-40-127.socal.res.rr.com] has joined #bitcoin-core-pr-reviews 10:09 < jnewbery> (or anyone else) 10:09 < pinheadmz> is it because some of the data isnt needed if its not taproot? 10:09 < nehan_> control might run later and relies on some of the data in txdata. 10:09 < instagibbs> the multithreaded part(control) needs access to the state 10:09 < jnewbery> It tripped me up initially (I didn't understand what the previous comment was trying to say here: https://github.com/bitcoin/bitcoin/pull/18401/commits/b409b611eb0fc6c71f107b5313ab79ecaf57f479#diff-24efdb00bfbe56b140fb006b562cc70bL2086) 10:10 < nehan_> jnewbery: yeah your comment is clearer 10:10 < jnewbery> nehan_ instagibbs: exactly correct 10:11 < instagibbs> used to be that when control took over script validation jobs, you'd std::swap the pointers in 10:11 < jnewbery> You can see that control might not get run until later: https://github.com/bitcoin/bitcoin/blob/d52ba21dfff99173abb927bc964ce7ceb711d789/src/validation.cpp#L2164 10:11 < nehan_> why is it ok to remove the emplace_back(tx) though? It seems to me like you're never initializing the items in txsdata... https://github.com/bitcoin/bitcoin/pull/18401/commits/b409b611eb0fc6c71f107b5313ab79ecaf57f479#diff-24efdb00bfbe56b140fb006b562cc70bL2133 10:11 < nehan_> (this was in the original PR) 10:12 < jnewbery> nehan_: in the new code, they're constructed in place when the vector is created 10:12 < jnewbery> but the data inside them is initialized inside CheckInputScripts 10:12 < theStack> nit-question: i guess the definition moved up just for code readability reasons, to be closer to `control`, as the comment refers to both `control` and `txsdata`? 10:13 < nehan_> ok. previously the data inside them was initialized here, which was what confused me. 10:13 < jnewbery> theStack: yeah, it's a big red flag to me that control relies on txsdata but there's nothing in the code that enforces that 10:14 < jnewbery> someone might come along later and see that txsdata is only used inside the for loop and think "we don't need a vector. We can just create a new txdata in each loop iteration" 10:14 < jnewbery> so I moved it next to control and expanded the comment 10:14 < nehan_> I went down a rabbit hole trying to confirm that the fields inside the items were initialized somewhere. 10:15 < instagibbs> to be fair you'll notice it quite fast I think :) 10:15 < theStack> jnewbery: thanks for clarifying, that makes sense 10:15 < jnewbery> instagibbs: I guess lots of tests would start failing! 10:16 < jnewbery> ok, does anyone want to ask any more questions about control or CCheckQueueControl? What's happening is quite interesting 10:16 -!- lightlike [~lightlike@p200300C7EF12F30068A69683A0FF44F3.dip0.t-ipconnect.de] has quit [Quit: Leaving] 10:16 < jnewbery> (in terms of when the script validation is running and on which threads) 10:17 < jnewbery> ok, moving on (but feel free to continue asking questions on anything). What new data members are added to PrecomputedTransationData in the subsequent commit in the taproot PR 17977? 10:17 -!- yrok [54112948@84.17.41.72] has left #bitcoin-core-pr-reviews [] 10:18 < fjahr> the spent outputs of the tx: m_spent_outputs 10:18 < theStack> there's a new member m_spent_outputs introduced 10:18 < sipa> hi! 10:18 < jnewbery> fjahr theStack: yes. We cache the spent outputs 10:18 < jnewbery> why? 10:18 < jnewbery> hi sipa! 10:19 < pinheadmz> a vector of all tx outputs 10:19 < fjahr> because we need them for taproot sig message 10:19 -!- gchain [gustafmatr@gateway/shell/matrix.org/x-gsqjvcvnycybeoxp] has left #bitcoin-core-pr-reviews ["User left"] 10:19 < pinheadmz> er spent outputs 10:20 < fjahr> at least the amounts 10:20 < jnewbery> right. signatures in taproot commit to a couple of extra pieces of data. What are they? 10:21 < jkczyz> amount of all inputs and scriptPubKey of output being spent 10:21 < pinheadmz> the annex :-) 10:22 < jnewbery> jkczyz: right 10:22 < pinheadmz> and the scriptpubkey 10:22 < jnewbery> pinheadmz: ah yes, also the annex. Have a bonus point 10:22 < pinheadmz> ty, i could use a few 10:22 < michaelfolkson> Why do we need the single hashes in addition to the double hashes? 10:23 < sipa> the real question is: what things does the sighash commit to that is *not* part of the spending transaction 10:23 < pinheadmz> michaelfolkson: from bip341: "There is no expected security improvement by doubling SHA256 because this only protects against length-extension attacks against SHA256 which are not a concern for signature messages because there is no secret data. " 10:23 < jnewbery> (difficult question) what potential problems does this solve? Why are we committing to the scriptPubKey and all ammounts? 10:23 < pinheadmz> i think the extra commitments help offline signers 10:23 < michaelfolkson> Thanks pinheadmz 10:24 < jnewbery> pinheadmz: yes! How? 10:24 < fjahr> also nSequences but I guess that's less common: The signature message commits to all input nSequence if SIGHASH_NONE or SIGHASH_SINGLE are set (unless SIGHASH_ANYONECANPAY is set as well) 10:24 < nehan_> amounts are to eventually support a cold wallet. I still don't think it needs to commit to scriptPubKey but as sipa pointed out on stackoverflow it's to maintain legacy behavior 10:24 < nehan_> er, instead of maintain legacy behavior, maybe I should have said reduce changes 10:24 < jnewbery> nehan_: you mean this stack overflow answer: https://bitcoin.stackexchange.com/a/53493 ? 10:25 -!- jonatack_ [~jon@37.173.127.56] has joined #bitcoin-core-pr-reviews 10:25 < pinheadmz> jnewbery: i thought it was posisble beofre this change to trick a hardware wallt into giving away a huge fee 10:25 < jonatack_> to prevent lying to offline signing devices about the output being spent? 10:25 < nehan_> jnewbery: yes 10:25 < sipa> nehan_: the current sighashes do not commit to the sPK being spent, actually 10:25 < sipa> scriptCode is not the same as scriptPubKey 10:25 < instagibbs> jonatack_, yes 10:25 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 10:25 < nehan_> sipa: true! I meant scriptCode 10:26 -!- jonatack_ [~jon@37.173.127.56] has quit [Client Quit] 10:26 < jnewbery> nehan_: that's keeping the behaviour between pre-segwit and segwit v0 similar. In taproot, we also commit to the scriptPubKey, which is new 10:26 -!- jonatack_ [~jon@37.173.127.56] has joined #bitcoin-core-pr-reviews 10:26 < nehan_> jnewbery: ah, thanks 10:26 < jnewbery> and yes, all of this is to prevent being able to lie to an offline signer about fees 10:27 < instagibbs> f.e. two inputs, attacker lies about value of one, gets one valid sig, attacker asks for sigs again, lying about the other input's value 10:27 < nehan_> sipa: side question -- I think this answer is incorrect: https://bitcoin.stackexchange.com/a/90921/104735. it seems to me the code path is implicitly committed to without committing to scriptCode. Am I missing something? 10:27 < instagibbs> those two sigs combined is a valid tx with an unknown fee 10:27 < jnewbery> There's a bit more context here: http://www.erisian.com.au/meetbot/taproot-bip-review/2019/taproot-bip-review.2019-11-26-19.01.log.html#l-17 10:28 < jnewbery> instagibbs: that's right 10:28 -!- jonatack [~jon@37.171.65.83] has quit [Ping timeout: 265 seconds] 10:29 < sipa> nehan_: imagine you have a script that checks two signatures with the same key 10:29 < sipa> nehan_: could you satsify it with 2 identical signatures? 10:30 < sipa> so OP_CHeCKSIGVERIFY OP_CHECKSIG, say 10:30 < jnewbery> There's a (very miner) similar attack where an online host could lie to an offline signer about whether an output being spent is P2WPKH or P2WSH-P2PKH. The signer doesn't know the transaction output type and therefore the size, and a signature it provides would be valid for both. 10:31 < jnewbery> committing to the scriptPubKey prevents all attacks in this class 10:32 < sipa> nehan_: or maybe a better example: IF CHECKSIG ELSE CHECKSIG ENDIF 10:32 < r251d> In #18422 EvalChecksig communicates success by return value and by fSuccess bool ref. Are those success values only inconsistent when verifying scripts with null signatures? 10:33 < jnewbery> r251d: we'll get onto 18422 in just a moment 10:33 < nehan_> sipa: that last one requires one signature? and doesn't use CODESEPARATOR as indicated is necessary in that comment? 10:34 < sipa> nehan_: indeed 10:34 < sipa> nehan_: but imagine you're given a satisfying witness 1 10:34 < sipa> can you change the 1 into a 0? 10:34 < theStack> so the reason that scriptPubKey of the output being spent was not included in the signature was just that those attacks were not taken into consideration? or is there also a drawback in doing so? 10:35 < sipa> theStack: i believe it was an oversight in P2SH that was maintained in P2WSH 10:36 < jnewbery> sipa: I think your point is that with OP_CODESEPARATOR, the signer can provide a signature that is only valid for a certain execution branch. Is that right? 10:37 < sipa> jnewbery: indeed 10:37 < sipa> as i think nehan_ believed the CODESEP was unnecessary for that purpose 10:37 < nehan_> sipa: i'm confused. to make sure we're on the same page: I think that committing to (txid, input index) implicitly commits to a scriptCode for someone who has that data (so, not an airgapped wallet, of course). therefore commiting to the actual scriptCode in the signature seems unnecessary. 10:38 < sipa> nehan_: ah! 10:38 < sipa> you are right, but we're talking about a number of different things 10:38 < nehan_> I think this is true EVEN IF you are using CODESEPARATOR (Russell, and you with your edit on that stackoverflow comment, imply otherwise) 10:38 < jnewbery> nehan_: if the sighash only committed to (txid, index), then both of those signatures in sipa's example would be signing the same digest, so the signatures would be the same 10:39 < sipa> first of all, committing isn't enough; we need to commit in a way that is cheap to prove to an offline signing device 10:39 < sipa> giving the entire previous transaction is not cheap 10:39 < nehan_> sure 10:39 < nehan_> but outside of that 10:39 < jnewbery> if the signer only wanted to sign the IF branch, someone could take that signature and place it in the ELSE branch and it'd still be valid 10:39 < sipa> secondly, the "code path" referred to in the stack exchange answer is about code without the scriptCode 10:39 < sipa> say, what branch of an IF a particular checksig is executed in 10:40 < sipa> s/without/inside/ 10:41 < jnewbery> I'm going to ask the next question, which is about 18422, but don't feel like you need to stop discussing CODESEPARATOR :) 10:41 < jnewbery> What is the difference between EvalCheckSig() returning false and setting the return parameter fSuccess to false? When would those different failure modes be used? 10:41 < jnewbery> (here: https://github.com/bitcoin/bitcoin/pull/18422/commits/14e8cf974a7a317796ef8e97e5cf9c355ceff0ee#diff-be2905e2f5218ecdbe4e55637dac75f3R350) 10:41 < pinheadmz> return false means the script threw an error. 10:42 < r251d> jnewbery: Thanks for asking that. That's the essence of my question above. 10:42 < pinheadmz> the fsuccess indicates the signature evalutaion 10:42 < theStack> i'd roughly say: if EvalCheckSig() returns false, then the input script is invalid (e.g. invalid signature format) in some way, where as fSuccess represents the result of the evaluated script 10:42 -!- gloooooo [a5e33c1c@165.227.60.28] has quit [Remote host closed the connection] 10:42 -!- Guest41 [56c00d0d@gateway/web/cgi-irc/kiwiirc.com/ip.86.192.13.13] has quit [Remote host closed the connection] 10:42 < jnewbery> pinheadmz: correct 10:42 < sipa> theStack: s/script/signature/ maybe 10:42 < nehan_> sipa: i think my misunderstanding was the following: i did not think that the scriptSig specified which code path in a scriptPubKey to take. I thought this was determined by execution. 10:43 < jnewbery> theStack: yes, except I think you meant to write signature instead of script at the end 10:43 < theStack> sipa: yes, thanks 10:43 -!- ecurrencyhodler [68ac287f@cpe-104-172-40-127.socal.res.rr.com] has quit [Remote host closed the connection] 10:44 < sipa> nehan_: you're right - it does not: signatures so far have never directly or indirectly committed to what execution path is taken in the script 10:44 < jnewbery> ok, so why would evaluating a signature sometimes cause a script to fail instantly, and sometimes not? 10:44 < jnewbery> in the case where it's not a valid signature 10:44 < sipa> nehan_: russell points out that OP_CODESEPARATOR can optionally be used for that purpose, because it modifies the scriptCode 10:44 < sipa> and the scriptCode is committed to 10:45 < michaelfolkson> In a multisig case providing an invalid signature shouldn't cause the script to fail? 10:45 < pinheadmz> jnewbery: if the sig or pubkey break rules set in bip340 10:45 < pinheadmz> size of pubkey and secp field size limits etc 10:45 < sipa> pinheadmz: not really 10:46 < theStack> jnewbery: with "fail instantly" you mean that false is returned, i.e. before CheckSig() is called? 10:46 < sipa> BIP340 treats the signature and public key as byte arrays 10:46 < jnewbery> theStack: I mean we fail the script instantly and the transaction is invalid 10:46 < sipa> so secp field limits are not really relevant; a signature is valid or invalid 10:46 < sipa> and there is no longer a distinction between invalid encoding for a signature vs signature just being wrong 10:47 < pinheadmz> did i get filed size mixed up with curve order? 10:47 < pinheadmz> ah 10:47 < jnewbery> Take a look here: https://github.com/bitcoin/bitcoin/pull/18422/commits/14e8cf974a7a317796ef8e97e5cf9c355ceff0ee#diff-be2905e2f5218ecdbe4e55637dac75f3R368-R371 10:47 < sipa> neither of those matter 10:47 < jnewbery> anyone want to guess what's going on here? 10:47 < sipa> a signature is valid or invalid according to BIP340 10:47 -!- Lola_Dam [56c00d0d@gateway/web/cgi-irc/kiwiirc.com/ip.86.192.13.13] has joined #bitcoin-core-pr-reviews 10:48 < jnewbery> sipa: we're still discussing pre-taproot sig validation 10:48 < sipa> oh 10:48 < sipa> oops 10:48 < sipa> ignore me 10:48 < jnewbery> here: https://github.com/bitcoin/bitcoin/pull/18422/commits/14e8cf974a7a317796ef8e97e5cf9c355ceff0ee#diff-be2905e2f5218ecdbe4e55637dac75f3R350 10:48 < sipa> pinheadmz: i confused you 10:48 < jnewbery> which in the taproot PR eventually will be called EvalChecksigPreTapscript() 10:48 < theStack> so pinheadmz was right? i think the names "CheckSignatureEncoding" and "CheckPubKeyEncoding" speak for themselves :) 10:49 < sipa> theStack: indeed 10:49 < sipa> he did mention bip340 though ;) 10:49 < pinheadmz> sipa do you mean that in taproot bc theres soft forkable pubkey versions that CheckPubKeyEncoding wont happen? 10:49 < sipa> pinheadmz: correct 10:49 < pinheadmz> yeah no i wasnt really confused i was getting ahead of the pr 10:50 < pinheadmz> ah ok 10:50 < sipa> nor is there a concept of an "incorrectly encoded signature" 10:50 < pinheadmz> right for the wrong reason perhaps 🏆 10:50 < pinheadmz> ok 10:50 < sipa> a signature is a byte array, and it's valid or invalid 10:50 < sipa> this was a huge source of problems with ECDSA 10:50 < sipa> that ECDSA signatures are abstract objects, with some complex encoding, ... 10:51 < theStack> to answer the question (in pre-taproot), an invalid encoded signature or pubkey encoding could be reasons for an instant fail of the script, leading to an invalid transaction 10:51 < pinheadmz> so in the future EvalChecksigPreTapscript() will check pubkey and sig encoding, but EvalChecksigTapscript() doesnt 10:52 -!- andrewtoth [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Ping timeout: 240 seconds] 10:52 < sipa> pinheadmz: indeed 10:52 < pinheadmz> cheers 10:52 < sipa> nehan_: to continue.. 10:52 < nehan_> sipa: I don't see how OP_CODESEPARATOR could be used for that purpose -- the entire scriptPubKey is committed to in the txid. to clarify: we are talking about pre-segwit Bitcoin, and not considering airgrapped wallets and access to data is not a concern. 10:52 < jnewbery> From a high level, I think the point is that we want a way to be able to fail a signature check, but continue execution, while avoiding malleability. 10:52 < sipa> nehan_: in pre-taproot, the scriptCode is committed to directly as well 10:53 < sipa> nehan_: and the scriptCode is modified by OP_CODESEP operators 10:53 < nehan_> sipa: yes. That seems redudant to me :) 10:53 < jnewbery> in segwit v0, we do that by allowing a zero-length signature to be an invalid sig, but not cause an instant failure 10:53 < jnewbery> that's what the SCRIPT_VERIFY_NULLFAIL flag is all about 10:53 < sipa> nehan_: russell's answer shows why it is not entirely redundant: it allows committing to the execution path 10:54 < jnewbery> Final question from me. Why isn’t the code for OP_CHECKMULTISIG and OP_CHECKMULTISIGVERIFY also extracted out in PR 18401? 10:54 < sipa> the sPK will be identical in all execution paths, but the scriptCode won't be 10:55 < pinheadmz> jnewbery: those are removed from tapscript 10:55 < sipa> (scriptCode is the script being executed, from the last executed OO_CODESEP until the end of the script; and the entire executed script if no CODESEP was ever executed) 10:55 < jnewbery> 5 minute warning. If you have a question but have been keeping quiet, now's the time to speak up 10:56 < amiti> jnewbery: I don't quite follow. whats an example of why we'd want to be able to continue execution after failing a signature check? 10:56 < michaelfolkson> jnewbery: So they are deprecated in BIP Taproot. The aim is to do these PRs piecemeal 10:56 < fjahr> they are disabled in taproot 10:56 < nehan_> sipa: I might need to work out an example, because I don't see how that adds a degree of flexibility. relevant OP_CODESEPARATORs are in the scripPubKey, and the scriptCode that results is deterministic from that. Certainly the signer could provide different scriptSigs that could cause different paths to execute, but that does not change the scriptPubKey and hence the scriptCode. 10:56 < sipa> amiti: say a 1-of-2 multisig 10:56 < amiti> oh lol 10:56 < jnewbery> amiti: good question! Any suggestions? 10:56 < sipa> nehan_: go back to my 0 vs 1 exame 10:57 < sipa> nehan_: it's a toy example, but it's a form of malleability 10:57 < jnewbery> sipa: in that case only one signature (plus dummy empty stack item) is provided 10:57 < instagibbs> amiti, if you want to do some big threshhold using script 10:57 < pinheadmz> amiti: jnewbery seems silly, but you could have a script be valid if the sig is false 10:58 < sipa> nehan_: i think i need to write this out in more detail 10:58 < amiti> cool. thanks :) 10:58 < pinheadmz> maybe especially in a checksigadd scneario ? 10:58 < instagibbs> in tapscript that'd be checksigadd construction, it's ok to have some failures, provided the failures don't actually make you spend the sig validation time 10:58 < jnewbery> pinheadmz: not silly at all. I think optimized lightning uses a similar construction. 10:58 < sipa> nehan_: you're right that in simple scenarios it is likely not useful 10:59 < michaelfolkson> Lightning uses a false sig construction jnewbery?! 10:59 < sipa> nehan_: but when reasoning through malleability in miniscript we had to introduce a rule "the same public key cannot occur more than once in a script", because otherwise it's near impossible to reason about - using CODESEP that could be made permitted 10:59 < jnewbery> a basic lightning script might be OP_IF OP_ELSE OP_ENDIF 11:00 < sipa> there is no failing checksig there afaik 11:00 < sipa> just a branch that executes only one of the checksigs? 11:00 < jnewbery> it's maybe a byte or two more efficient if you do the checksig outside the if/else and then choose the branch based on whether the checksig is successful 11:00 < sipa> oh right 11:00 < jnewbery> ok, that's time! 11:01 < jnewbery> Thanks everyone 11:01 < theStack> thanks, was interesting and instructive 11:01 < pinheadmz> thanks jnewbery and sipa ! 11:01 < jonatack_> thanks jnewbery, sipa, and everyone! 11:01 < fjahr> thanks! 11:01 < jkczyz> thanks! 11:01 < amiti> thanks! 11:01 < AlistairMann> Awesome - learned so much from just reading and researching as you went. Thanks! 11:01 < sipa> thanks! 11:01 < jnewbery> thanks for joining us, sipa! 11:01 < emzy> Thanks! And stay save. 11:01 < sipa> yw 11:01 < nehan_> thanks! sorry for derailing! 11:02 < michaelfolkson> Good one, thanks. Nice discussion neha and sipa 11:02 < nehan_> sipa: ok i see that someone could get me to sign a 0 and then replace it with a 1 . bad! but I don't see how committing to scriptCode changes that. 11:02 < michaelfolkson> nehan_ 11:02 < jnewbery> nehan_: not derailing at all. OP_CODESEPARATOR is gnarly 11:02 -!- r251d [~r251d@162-196-59-192.lightspeed.irvnca.sbcglobal.net] has quit [Quit: r251d] 11:02 < nehan_> jnewbery: but we haven't even talked about an example with that yet :/ 11:02 < sipa> nehan_: if one of the branches has a OP_CODESEP it is no longer possible to do that mauling 11:03 -!- josh-bushwick [457a83ef@ool-457a83ef.dyn.optonline.net] has left #bitcoin-core-pr-reviews [] 11:03 < jnewbery> I know Nicolas Dorier was using it in his tumblebit implementation a couple of years ago 11:03 < sipa> because the two sigs will be verified against a different sighash 11:04 -!- Lola_Dam [56c00d0d@gateway/web/cgi-irc/kiwiirc.com/ip.86.192.13.13] has quit [Remote host closed the connection] 11:05 < nehan_> sipa: your example scriptPK is IF CHECKSIG ELSE CHECKSIG ENDIF. What does the version with OP_CODESEPARATOR look like? 11:05 < sipa> IF key CHECKSIG ELSE CODESEP key CHECKSIG ENDIF 11:06 < pinheadmz> has op_codesep ever been used on chain? besides the one really weird sighash_single tx where the sig was actually in the scriptPubkey? 11:07 -!- theStack [~theStack@81.223.165.6] has quit [Remote host closed the connection] 11:08 < sipa> nehan_: in the right branch, scriptCode would be "key CHECKSIG ENDIF" 11:08 < sipa> rather than the whole script 11:08 < nehan_> sipa: oh wow wow wow. i get it now. 11:10 < sipa> note that in tapscript this is changed, scriptCode goes away (but we commit to sPK directly(, and additionally commit to the instruction number of the last executed CODESEP 11:10 < nehan_> sipa: i think. argh. I think you are saying *whether or not codesep actually gets used* is dependent on the control flow logic 11:10 < sipa> nehan_: right, and which one 11:10 < nehan_> sipa: that is insane 11:10 < nehan_> the previous way, not the taproot way 11:11 < sipa> to be clear: i do not assume this is what CODESEP was designed for 11:11 < nehan_> thank you very much! I did not realize codesep depended on logic 11:11 -!- andrewtoth [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews 11:11 < sipa> yeah, it's only *executes* CODESEPs that have this effect 11:11 < sipa> *executed 11:12 < sipa> though even if they worked even in unevaluated branches you could use them for this purpose 11:12 < nehan_> sipa: i don't grok your last sentence 11:13 < sipa> nehan_: take the codesep script i gave a above 11:13 < sipa> it wouls work the same way even if codeseps had an effect even when in an unexecuted branch 11:13 -!- chanho [b8980f30@cpe-184-152-15-48.nyc.res.rr.com] has quit [Remote host closed the connection] 11:13 -!- AlistairMann [56b41c67@host86-180-28-103.range86-180.btcentralplus.com] has quit [Remote host closed the connection] 11:18 -!- AlistairMannRO59 [56b41c67@host86-180-28-103.range86-180.btcentralplus.com] has joined #bitcoin-core-pr-reviews 11:25 -!- AlistairMannRO [~am@host86-180-28-103.range86-180.btcentralplus.com] has joined #bitcoin-core-pr-reviews 11:25 -!- AlistairMannRO59 [56b41c67@host86-180-28-103.range86-180.btcentralplus.com] has quit [Remote host closed the connection] 11:26 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 250 seconds] 11:37 -!- jesseposner [~jesseposn@c-67-188-220-154.hsd1.ca.comcast.net] has quit [Remote host closed the connection] 11:40 -!- jessepos_ [~jesseposn@c-67-188-220-154.hsd1.ca.comcast.net] has joined #bitcoin-core-pr-reviews 11:40 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 11:41 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:11be:312f:adcf:42dd] has quit [Quit: Sleep mode] 11:52 -!- jessepos_ [~jesseposn@c-67-188-220-154.hsd1.ca.comcast.net] has left #bitcoin-core-pr-reviews [] 12:25 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 260 seconds] 12:28 -!- AlistairMannRO [~am@host86-180-28-103.range86-180.btcentralplus.com] has quit [Ping timeout: 250 seconds] 12:28 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 12:37 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit [Remote host closed the connection] 12:40 -!- vasild_ [~vd@gateway/tor-sasl/vasild] has joined #bitcoin-core-pr-reviews 12:44 -!- vasild [~vd@gateway/tor-sasl/vasild] has quit [Ping timeout: 240 seconds] 12:44 -!- vasild_ is now known as vasild 12:45 -!- AlistairMannRO [~am@host86-180-28-103.range86-180.btcentralplus.com] has joined #bitcoin-core-pr-reviews 12:50 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 265 seconds] 12:56 -!- AlistairMannRO [~am@host86-180-28-103.range86-180.btcentralplus.com] has quit [Quit: Leaving.] 13:04 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews 13:15 -!- ghost43_ [~daer@gateway/tor-sasl/daer] has joined #bitcoin-core-pr-reviews 13:15 -!- ghost43 [~daer@gateway/tor-sasl/daer] has quit [Ping timeout: 240 seconds] 13:19 < jnewbery> meeting log is posted: https://bitcoincore.reviews/18401.html 13:22 -!- ghost43_ [~daer@gateway/tor-sasl/daer] has quit [Remote host closed the connection] 13:23 -!- ghost43 [~daer@gateway/tor-sasl/daer] has joined #bitcoin-core-pr-reviews 13:35 -!- slivera [~slivera@116.206.228.243] has joined #bitcoin-core-pr-reviews 13:37 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 13:37 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has quit [Read error: Connection reset by peer] 13:41 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has joined #bitcoin-core-pr-reviews 13:57 -!- ncantu [~ncantu@37.170.153.9] has joined #bitcoin-core-pr-reviews 14:35 -!- ghost43 [~daer@gateway/tor-sasl/daer] has quit [Remote host closed the connection] 14:35 -!- ghost43 [~daer@gateway/tor-sasl/daer] has joined #bitcoin-core-pr-reviews 15:00 -!- ysangkok [janus@anubis.0x90.dk] has left #bitcoin-core-pr-reviews [] 15:00 -!- ryanofsky_ [~russ@jumpy.yanofsky.org] has joined #bitcoin-core-pr-reviews 15:02 -!- Talkless [~Talkless@hst-227-49.splius.lt] has quit [Quit: Konversation terminated!] 15:08 -!- Netsplit *.net <-> *.split quits: ryanofsky 15:13 -!- ghost43 [~daer@gateway/tor-sasl/daer] has quit [Remote host closed the connection] 15:14 -!- ghost43 [~daer@gateway/tor-sasl/daer] has joined #bitcoin-core-pr-reviews 15:14 -!- michaelfolkson [~textual@host86-132-64-99.range86-132.btcentralplus.com] has joined #bitcoin-core-pr-reviews 15:18 -!- michaelfolkson [~textual@host86-132-64-99.range86-132.btcentralplus.com] has quit [Client Quit] 16:45 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit [Remote host closed the connection] 18:46 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews 18:51 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit [Ping timeout: 265 seconds] 19:24 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has quit [Remote host closed the connection] 19:43 -!- excess [~excess@45.162.229.132] has quit [Quit: excess] 20:06 -!- ncantu [~ncantu@37.170.153.9] has quit [Ping timeout: 256 seconds] 21:05 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 250 seconds] 21:41 -!- andrewtoth [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Ping timeout: 240 seconds] 22:25 -!- jonatack__ [~jon@37.170.215.215] has joined #bitcoin-core-pr-reviews 22:28 -!- jonatack_ [~jon@37.173.127.56] has quit [Ping timeout: 256 seconds] 22:48 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews 22:53 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has quit [Ping timeout: 250 seconds] 23:07 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 23:32 -!- ghost43 [~daer@gateway/tor-sasl/daer] has quit [Remote host closed the connection] 23:33 -!- ghost43 [~daer@gateway/tor-sasl/daer] has joined #bitcoin-core-pr-reviews 23:52 -!- PaulTroon [~paultroon@h-5-150-248-150.NA.cust.bahnhof.se] has joined #bitcoin-core-pr-reviews