Initialize PrecomputedTransactionData in CheckInputScripts and Move single-sig checking EvalScript code to EvalChecksig (consensus, taproot)

https://github.com/bitcoin/bitcoin/pulls/18401

Host: jnewbery  -  PR author: sipa

The PR branch HEAD was b409b611 at the time of this review club meeting.

This week, we’ll review two (small) PRs:

Notes

  • A few weeks ago, we looked at PRs 16902 and 18002, which had both been pulled out of PR 17977, the WIP schnorr/taproot implementation. This week, we’ll look at two more small refactor PRs that have also been pulled out of that PR.

  • The same general considerations apply to these PRs: ordinarily, these refactor changes to consensus code wouldn’t meet the bar for review cost and risk -vs- benefit. However, splitting them off from the main schnorr/taproot PR makes reviewing the logical changes to consensus in the main PR easier and safer.

PR 18401: Initialize PrecomputedTransactionData in CheckInputScripts

  • When a signature for a transaction input is created or verified, different parts of the transaction are hashed into a transaction digest. This is the e value in e = Hash(m) that is used in the signing/verification algorithm. The transaction digest algorithm for pre-segwit transaction inputs is documented here (in particular, look at the verifyThisStr string in step 9 of the diagram, which is the data that is hashed).

  • For pre-segwit transaction inputs, the transaction digest has to be recalculated from scratch for each signature. This means that in the worst case the amount of data that needs to be hashed is quadratic in the size of the transaction. See Sergio Demian Lerner’s bitcoin talk post for more details of this quadratic hashing issue. The Bitcoin Core blog and Rusty Russell’s blog also have good explanations.

  • To resolve the quadratic hashing problem, the way that the transaction digest is calculated was changed for segwit inputs. See BIP 143 for the motivation and specification.

  • The segwit transaction digest is designed so that parts of the digest are shared between all the signatures in a transaction. This means that the total amount of data to be hashed for the transaction is linear in the size of the transaction.

  • This shared data between all signatures of a transaction is stored in the PrecomputedTransactionData object, which was added in PR 8524.

  • Segwit v1 (taproot) makes some very minor changes to the transaction digest algorithm. Specifically, the transaction digest commits to the amounts of all inputs (instead of just the amount of the input being signed) and to the scriptPubKey of the output being spent. See BIP 341 for the full description of the transaction digest.

  • Because of these changes, more data needs to be stored in the PrecomputedTransactionData object. This PR changes the way that the object is constructed and initialized so that a future commit in PR 17977 can add that data to the object.

PR 18422: Move single-sig checking EvalScript code to EvalChecksig

  • Tapscript is the scripting language used for scripts in a taproot tree (taproot has versioning for scripts, so a future soft fork could introduce a different scripting version or language).

  • Tapscript has almost the same semantics as Bitcoin Script. One notable difference is that OP_CHECKSIG and OP_CHECKSIGVERIFY both verify schnorr signatures instead of ECDSA signatures.

  • This PR extracts most of the logic for OP_CHECKSIG and OP_CHECKSIGVERIFY from the very large EvalScript() function.

  • A future commit in PR 17977 modifies this function to switch on which flags have been passed into the script interpreter in order to determine whether to do schnorr or ECDSA signature verification.

Questions

  1. Did you review the PRs? Concept ACK, approach ACK, tested ACK, or NACK? (Don’t forget to put your PR review on GitHub.)

  2. A new PrecomputedTransactionData default constructor is added which doesn’t initialize the data members. Where do they get initialized instead?

  3. What new data members are added to PrecomputedTransationData in the subsequent commit in the taproot PR 17977?

  4. Is the old PrecomputedTransactionData constructor still used anywhere?

  5. Why is the transaction digest algorithm changed for taproot? What problems does the new algorithm solve?

  6. What is the difference between EvalCheckSig() returning false and setting the return parameter fSuccess to false? When would those different failure modes be used?

  7. Why isn’t the code for OP_CHECKMULTISIG and OP_CHECKMULTISIGVERIFY also extracted out in PR 18401?

Meeting Log

  113:00 <jnewbery> #startmeeting
  213:00 <jnewbery> Hi folks! Welcome to Bitcoin Core Review club. Feel free to say hi to let everyone know you're here.
  313:00 <fjahr> hi
  413:00 <nehan_> hi
  513:00 <josh-bushwick> hi
  613:00 <michaelfolkson> Bitcoin Corrrr Review Club
  713:00 <michaelfolkson> hi
  813:00 <theStack> hi
  913:00 <jonatack> hi
 1013:00 <emzy> hi
 1113:00 <jnewbery> Special welcome to everyone who's at their first review club meeting.
 1213:01 <amiti> hi
 1313:01 <r251d> hi
 1413:01 <pinheadmz> hi
 1513:01 <AlistairMann> hi
 1613: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.
 1713:01 <jkczyz> hi
 1813:01 <jnewbery> Notes in the usual place: https://bitcoincore.reviews/18401.html
 1913:02 <jnewbery> Who had a chance to review the PRs this week?
 2013:02 <jnewbery> y/n
 2113:02 <pinheadmz> o/
 2213:02 <fjahr> y
 2313:02 <nehan_> y
 2413:02 <michaelfolkson> y
 2513:02 <theStack> y
 2613:02 <jkczyz> y
 2713:02 <r251d> y
 2813:02 <jonatack> y
 2913:02 <instagibbs> y
 3013: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.
 3113:03 <jnewbery> I'll ask questions, but like I said earlier, feel free to jump in at any time with your own questions/comments
 3213:03 <jnewbery> first question: A new PrecomputedTransactionData default constructor is added which doesn’t initialize the data members. Where do they get initialized instead?
 3313:04 <jnewbery> (this is in PR 18401: Initialize PrecomputedTransactionData in CheckInputScripts)
 3413:04 <jkczyz> In it's Init method
 3513:04 <jnewbery> jkczyz: right, and where is that called?
 3613:04 <pinheadmz> in the script interpreter ?
 3713:04 <fjahr> As the PR title says, in CheckInputScripts :)
 3813:04 <theStack> it's called in the function CheckInputScripts()
 3913:04 <jnewbery> fjahr: exactly!
 4013:04 <jkczyz> CheckInputScripts
 4113:05 <michaelfolkson> That is it's Init method?
 4213:05 <jnewbery> why are we calling Init there instead of the ctor?
 4313:06 <michaelfolkson> Because the initialization needs to happen later...
 4413:06 <instagibbs> we need valid (uninitialized) caches in place for pointer validity?
 4513:06 <nehan_> to set things up for a later change which requires knowing about spent outputs
 4613:06 <jnewbery> nehan_: yes, that's right
 4713: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
 4813:07 <nehan_> how do we know it's ok to wait until then? Functions like PolicyScriptChecks() use the uninitialized txdata
 4913:07 <jnewbery> instagibbs: you're talking about this comment: https://github.com/bitcoin/bitcoin/pull/18401/commits/b409b611eb0fc6c71f107b5313ab79ecaf57f479#diff-24efdb00bfbe56b140fb006b562cc70bR2079 ?
 5013:08 <instagibbs> I answered the wrong question, but yeah
 5113:08 <jnewbery> That's unchanged by this PR (although I've expanded the comment a bit). Can you explain what's going on there?
 5213:08 <instagibbs> you asked why uninitialized then :)
 5313:09 <jnewbery> (or anyone else)
 5413:09 <pinheadmz> is it because some of the data isnt needed if its not taproot?
 5513:09 <nehan_> control might run later and relies on some of the data in txdata.
 5613:09 <instagibbs> the multithreaded part(control) needs access to the state
 5713: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)
 5813:10 <nehan_> jnewbery: yeah your comment is clearer
 5913:10 <jnewbery> nehan_ instagibbs: exactly correct
 6013:11 <instagibbs> used to be that when control took over script validation jobs, you'd std::swap the pointers in
 6113:11 <jnewbery> You can see that control might not get run until later: https://github.com/bitcoin/bitcoin/blob/d52ba21dfff99173abb927bc964ce7ceb711d789/src/validation.cpp#L2164
 6213: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
 6313:11 <nehan_> (this was in the original PR)
 6413:12 <jnewbery> nehan_: in the new code, they're constructed in place when the vector is created
 6513:12 <jnewbery> but the data inside them is initialized inside CheckInputScripts
 6613: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`?
 6713:13 <nehan_> ok. previously the data inside them was initialized here, which was what confused me.
 6813: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
 6913: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"
 7013:14 <jnewbery> so I moved it next to control and expanded the comment
 7113:14 <nehan_> I went down a rabbit hole trying to confirm that the fields inside the items were initialized somewhere.
 7213:15 <instagibbs> to be fair you'll notice it quite fast I think :)
 7313:15 <theStack> jnewbery: thanks for clarifying, that makes sense
 7413:15 <jnewbery> instagibbs: I guess lots of tests would start failing!
 7513:16 <jnewbery> ok, does anyone want to ask any more questions about control or CCheckQueueControl? What's happening is quite interesting
 7613:16 <jnewbery> (in terms of when the script validation is running and on which threads)
 7713: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?
 7813:18 <fjahr> the spent outputs of the tx: m_spent_outputs
 7913:18 <theStack> there's a new member m_spent_outputs introduced
 8013:18 <sipa> hi!
 8113:18 <jnewbery> fjahr theStack: yes. We cache the spent outputs
 8213:18 <jnewbery> why?
 8313:18 <jnewbery> hi sipa!
 8413:19 <pinheadmz> a vector of all tx outputs
 8513:19 <fjahr> because we need them for taproot sig message
 8613:19 <pinheadmz> er spent outputs
 8713:20 <fjahr> at least the amounts
 8813:20 <jnewbery> right. signatures in taproot commit to a couple of extra pieces of data. What are they?
 8913:21 <jkczyz> amount of all inputs and scriptPubKey of output being spent
 9013:21 <pinheadmz> the annex :-)
 9113:22 <jnewbery> jkczyz: right
 9213:22 <pinheadmz> and the scriptpubkey
 9313:22 <jnewbery> pinheadmz: ah yes, also the annex. Have a bonus point
 9413:22 <pinheadmz> ty, i could use a few
 9513:22 <michaelfolkson> Why do we need the single hashes in addition to the double hashes?
 9613:23 <sipa> the real question is: what things does the sighash commit to that is *not* part of the spending transaction
 9713: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. "
 9813:23 <jnewbery> (difficult question) what potential problems does this solve? Why are we committing to the scriptPubKey and all ammounts?
 9913:23 <pinheadmz> i think the extra commitments help offline signers
10013:23 <michaelfolkson> Thanks pinheadmz
10113:24 <jnewbery> pinheadmz: yes! How?
10213: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)
10313: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
10413:24 <nehan_> er, instead of maintain legacy behavior, maybe I should have said reduce changes
10513:24 <jnewbery> nehan_: you mean this stack overflow answer: https://bitcoin.stackexchange.com/questions/53486/what-is-the-point-of-including-the-scriptcode-of-the-input-in-the-transaction-di/53493#53493 ?
10613:25 <pinheadmz> jnewbery: i thought it was posisble beofre this change to trick a hardware wallt into giving away a huge fee
10713:25 <jonatack_> to prevent lying to offline signing devices about the output being spent?
10813:25 <nehan_> jnewbery: yes
10913:25 <sipa> nehan_: the current sighashes do not commit to the sPK being spent, actually
11013:25 <sipa> scriptCode is not the same as scriptPubKey
11113:25 <instagibbs> jonatack_, yes
11213:25 <nehan_> sipa: true! I meant scriptCode
11313: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
11413:26 <nehan_> jnewbery: ah, thanks
11513:26 <jnewbery> and yes, all of this is to prevent being able to lie to an offline signer about fees
11613: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
11713:27 <nehan_> sipa: side question -- I think this answer is incorrect: https://bitcoin.stackexchange.com/questions/53486/what-is-the-point-of-including-the-scriptcode-of-the-input-in-the-transaction-di/90921#90921. it seems to me the code path is implicitly committed to without committing to scriptCode. Am I missing something?
11813:27 <instagibbs> those two sigs combined is a valid tx with an unknown fee
11913: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
12013:28 <jnewbery> instagibbs: that's right
12113:29 <sipa> nehan_: imagine you have a script that checks two signatures with the same key
12213:29 <sipa> nehan_: could you satsify it with 2 identical signatures?
12313:30 <sipa> so <key> OP_CHeCKSIGVERIFY <key> OP_CHECKSIG, say
12413: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.
12513:31 <jnewbery> committing to the scriptPubKey prevents all attacks in this class
12613:32 <sipa> nehan_: or maybe a better example: IF <key> CHECKSIG ELSE <key> CHECKSIG ENDIF
12713: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?
12813:33 <jnewbery> r251d: we'll get onto 18422 in just a moment
12913:33 <nehan_> sipa: that last one requires one signature? and doesn't use CODESEPARATOR as indicated is necessary in that comment?
13013:34 <sipa> nehan_: indeed
13113:34 <sipa> nehan_: but imagine you're given a satisfying witness 1 <sig>
13213:34 <sipa> can you change the 1 into a 0?
13313: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?
13413:35 <sipa> theStack: i believe it was an oversight in P2SH that was maintained in P2WSH
13513: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?
13613:37 <sipa> jnewbery: indeed
13713:37 <sipa> as i think nehan_ believed the CODESEP was unnecessary for that purpose
13813: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.
13913:38 <sipa> nehan_: ah!
14013:38 <sipa> you are right, but we're talking about a number of different things
14113: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)
14213: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
14313: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
14413:39 <sipa> giving the entire previous transaction is not cheap
14513:39 <nehan_> sure
14613:39 <nehan_> but outside of that
14713: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
14813:39 <sipa> secondly, the "code path" referred to in the stack exchange answer is about code without the scriptCode
14913:39 <sipa> say, what branch of an IF a particular checksig is executed in
15013:40 <sipa> s/without/inside/
15113: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 :)
15213: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?
15313:41 <jnewbery> (here: https://github.com/bitcoin/bitcoin/pull/18422/commits/14e8cf974a7a317796ef8e97e5cf9c355ceff0ee#diff-be2905e2f5218ecdbe4e55637dac75f3R350)
15413:41 <pinheadmz> return false means the script threw an error.
15513:42 <r251d> jnewbery: Thanks for asking that. That's the essence of my question above.
15613:42 <pinheadmz> the fsuccess indicates the signature evalutaion
15713: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
15813:42 <jnewbery> pinheadmz: correct
15913:42 <sipa> theStack: s/script/signature/ maybe
16013: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.
16113:43 <jnewbery> theStack: yes, except I think you meant to write signature instead of script at the end
16213:43 <theStack> sipa: yes, thanks
16313: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
16413:44 <jnewbery> ok, so why would evaluating a signature sometimes cause a script to fail instantly, and sometimes not?
16513:44 <jnewbery> in the case where it's not a valid signature
16613:44 <sipa> nehan_: russell points out that OP_CODESEPARATOR can optionally be used for that purpose, because it modifies the scriptCode
16713:44 <sipa> and the scriptCode is committed to
16813:45 <michaelfolkson> In a multisig case providing an invalid signature shouldn't cause the script to fail?
16913:45 <pinheadmz> jnewbery: if the sig or pubkey break rules set in bip340
17013:45 <pinheadmz> size of pubkey and secp field size limits etc
17113:45 <sipa> pinheadmz: not really
17213:46 <theStack> jnewbery: with "fail instantly" you mean that false is returned, i.e. before CheckSig() is called?
17313:46 <sipa> BIP340 treats the signature and public key as byte arrays
17413:46 <jnewbery> theStack: I mean we fail the script instantly and the transaction is invalid
17513:46 <sipa> so secp field limits are not really relevant; a signature is valid or invalid
17613:46 <sipa> and there is no longer a distinction between invalid encoding for a signature vs signature just being wrong
17713:47 <pinheadmz> did i get filed size mixed up with curve order?
17813:47 <pinheadmz> ah
17913:47 <jnewbery> Take a look here: https://github.com/bitcoin/bitcoin/pull/18422/commits/14e8cf974a7a317796ef8e97e5cf9c355ceff0ee#diff-be2905e2f5218ecdbe4e55637dac75f3R368-R371
18013:47 <sipa> neither of those matter
18113:47 <jnewbery> anyone want to guess what's going on here?
18213:47 <sipa> a signature is valid or invalid according to BIP340
18313:48 <jnewbery> sipa: we're still discussing pre-taproot sig validation
18413:48 <sipa> oh
18513:48 <sipa> oops
18613:48 <sipa> ignore me
18713:48 <jnewbery> here: https://github.com/bitcoin/bitcoin/pull/18422/commits/14e8cf974a7a317796ef8e97e5cf9c355ceff0ee#diff-be2905e2f5218ecdbe4e55637dac75f3R350
18813:48 <sipa> pinheadmz: i confused you
18913:48 <jnewbery> which in the taproot PR eventually will be called EvalChecksigPreTapscript()
19013:48 <theStack> so pinheadmz was right? i think the names "CheckSignatureEncoding" and "CheckPubKeyEncoding" speak for themselves :)
19113:49 <sipa> theStack: indeed
19213:49 <sipa> he did mention bip340 though ;)
19313:49 <pinheadmz> sipa do you mean that in taproot bc theres soft forkable pubkey versions that CheckPubKeyEncoding wont happen?
19413:49 <sipa> pinheadmz: correct
19513:49 <pinheadmz> yeah no i wasnt really confused i was getting ahead of the pr
19613:50 <pinheadmz> ah ok
19713:50 <sipa> nor is there a concept of an "incorrectly encoded signature"
19813:50 <pinheadmz> right for the wrong reason perhaps 🏆
19913:50 <pinheadmz> ok
20013:50 <sipa> a signature is a byte array, and it's valid or invalid
20113:50 <sipa> this was a huge source of problems with ECDSA
20213:50 <sipa> that ECDSA signatures are abstract objects, with some complex encoding, ...
20313: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
20413:51 <pinheadmz> so in the future EvalChecksigPreTapscript() will check pubkey and sig encoding, but EvalChecksigTapscript() doesnt
20513:52 <sipa> pinheadmz: indeed
20613:52 <pinheadmz> cheers
20713:52 <sipa> nehan_: to continue..
20813: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.
20913: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.
21013:52 <sipa> nehan_: in pre-taproot, the scriptCode is committed to directly as well
21113:53 <sipa> nehan_: and the scriptCode is modified by OP_CODESEP operators
21213:53 <nehan_> sipa: yes. That seems redudant to me :)
21313: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
21413:53 <jnewbery> that's what the SCRIPT_VERIFY_NULLFAIL flag is all about
21513:53 <sipa> nehan_: russell's answer shows why it is not entirely redundant: it allows committing to the execution path
21613:54 <jnewbery> Final question from me. Why isn’t the code for OP_CHECKMULTISIG and OP_CHECKMULTISIGVERIFY also extracted out in PR 18401?
21713:54 <sipa> the sPK will be identical in all execution paths, but the scriptCode won't be
21813:55 <pinheadmz> jnewbery: those are removed from tapscript
21913: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)
22013:55 <jnewbery> 5 minute warning. If you have a question but have been keeping quiet, now's the time to speak up
22113:55 <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?
22213:56 <michaelfolkson> jnewbery: So they are deprecated in BIP Taproot. The aim is to do these PRs piecemeal
22313:56 <fjahr> they are disabled in taproot
22413: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.
22513:56 <sipa> amiti: say a 1-of-2 multisig
22613:56 <amiti> oh lol
22713:56 <jnewbery> amiti: good question! Any suggestions?
22813:56 <sipa> nehan_: go back to my 0 <sig> vs 1 <sig> exame
22913:57 <sipa> nehan_: it's a toy example, but it's a form of malleability
23013:57 <jnewbery> sipa: in that case only one signature (plus dummy empty stack item) is provided
23113:57 <instagibbs> amiti, if you want to do some big threshhold using script
23213:57 <pinheadmz> amiti: jnewbery seems silly, but you could have a script be valid if the sig is false
23313:58 <sipa> nehan_: i think i need to write this out in more detail
23413:58 <amiti> cool. thanks :)
23513:58 <pinheadmz> maybe especially in a checksigadd scneario ?
23613: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
23713:58 <jnewbery> pinheadmz: not silly at all. I think optimized lightning uses a similar construction.
23813:58 <sipa> nehan_: you're right that in simple scenarios it is likely not useful
23913:59 <michaelfolkson> Lightning uses a false sig construction jnewbery?!
24013: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
24113:59 <jnewbery> a basic lightning script might be OP_IF <hashlock branch with checksig> OP_ELSE <timelock branch with checksig> OP_ENDIF
24214:00 <sipa> there is no failing checksig there afaik
24314:00 <sipa> just a branch that executes only one of the checksigs?
24414: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
24514:00 <sipa> oh right
24614:00 <jnewbery> ok, that's time!
24714:01 <jnewbery> Thanks everyone
24814:01 <theStack> thanks, was interesting and instructive
24914:01 <pinheadmz> thanks jnewbery and sipa !
25014:01 <jonatack_> thanks jnewbery, sipa, and everyone!
25114:01 <fjahr> thanks!
25214:01 <jkczyz> thanks!
25314:01 <amiti> thanks!
25414:01 <AlistairMann> Awesome - learned so much from just reading and researching as you went. Thanks!
25514:01 <sipa> thanks!
25614:01 <jnewbery> thanks for joining us, sipa!
25714:01 <emzy> Thanks! And stay save.
25814:01 <sipa> yw
25914:01 <nehan_> thanks! sorry for derailing!
26014:02 <michaelfolkson> Good one, thanks. Nice discussion neha and sipa
26114:02 <nehan_> sipa: ok i see that someone could get me to sign a 0 <sig> and then replace it with a 1 <sig>. bad! but I don't see how committing to scriptCode changes that.
26214:02 <michaelfolkson> nehan_
26314:02 <jnewbery> nehan_: not derailing at all. OP_CODESEPARATOR is gnarly
26414:02 <nehan_> jnewbery: but we haven't even talked about an example with that yet :/
26514:02 <sipa> nehan_: if one of the branches has a OP_CODESEP it is no longer possible to do that mauling
26614:03 <jnewbery> I know Nicolas Dorier was using it in his tumblebit implementation a couple of years ago
26714:03 <sipa> because the two sigs will be verified against a different sighash