Discourage CSV as NOP when locktime disable is set & discourage unknown nSequence (
validation, tx fees and policy) Sep 22, 2021
The PR branch HEAD was e8eab74719 at the time of this review club meeting.
Bitcoin has various “allow any” consensus rules that can be repurposed in future soft
forks, such as upgradable NOPs and upgradable witness versions. In the future, if we
repurpose a NOP opcode (e.g. NOP4 as
OP_CTV) or define new semantics
under a witness version (e.g. witness version 1 for taproot), nodes that don’t upgrade immediately will still be able
to stay in consensus when the new rules activate.
Bitcoin Core’s policy
discourages usage of upgradable NOPs,
witness versions, taproot leaf versions, etc. This prevents nodes from accepting to-be-invalid
transactions into their mempool prior to activation and miners from building consensus-invalid
blocks if they don’t upgrade.
consensus-enforced semantics on the nSequence field of a transaction input
OP_CHECKSEQUENCEVERIFY opcode to enable relative lock-time spending
It specifies the most significant bit of the nSequence field as the
disable locktime flag such that, if that bit is
set, nodes do not apply consensus meaning to the sequence number.
The disable locktime flag was
as leaving room for “future extensibility,” i.e. we might create new relative locktime rules where
the disable locktime flag is set.
However, as there was
at least one
application in the Bitcoin
ecosystem using the nSequence field - including the 31st bit - use of this bit was intentionally
not discouraged in policy.
PR #22871 proposes a policy change to discourage
use of this bit in nSequence numbers and as arguments to OP_CSV in the interpreter.
The PR author has also written a
post about this unexpected
The author of the BIP112 implementation
explained why this
policy was not added originally and
noted that this change makes
sense now only if there are no more applications using the disable locktime flag for non-consensus
purposes in their nSequence numbers, as it would prohibit their transactions from propagating.
The PR author
posted to the mailing
soliciting feedback on this proposal.
noted on the
mailing list that upgrades to relative timelock semantics would also be gated on a different nVersion number.
that, since nVersion numbers greater than 2 are discouraged in policy, this method of upgrading
relative timelocks is preserved without discouraging use of the locktime disable flag.
on the mailing list that the Lightning Network protocol uses nSequence numbers to encode
commitment transaction numbers, including the disable locktime flag. Questions
Did you review the PR?
Concept ACK, approach ACK, tested ACK, or
Can you summarize the changes being proposed in this PR?
What is the difference between policy and consensus rules?
Why do we discourage the usage of upgradable NOPs in policy? How is this implemented? (Hint: grep
SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS in src/script/interpreter.cpp).
What are some reasons to discourage use of the locktime disable flag in policy?
What are some reasons
not to discourage use of the locktime disable flag in policy?
What do you think of the
upgrading the relative timelock semantics by increasing the nVersion number?
Do you think this change (discouragement of setting the most significant bit) should be applied
to nSequence numbers, CSV values, both, or neither? Why?
which removes the
SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS script verification flag from static
OP_CSV tests, needed?
What do you think of the approach of reusing the
script verification flag to discourage use of the locktime disable flag?
1 17:00 <glozow> #startmeeting
5 17:00 <glozow> hello friends! welcome to PR review club!
6 17:00 <willcl_ark> hellooo
7 17:00 <dunxen> yay, policy!
11 17:01 <glozow> did anyone get a chance to review the PR? y/n
17 17:01 <dunxen> not in depth
19 17:02 <glozow> o! is it anyone’s first time btw? :)
20 17:02 <ziggie> yes mine
21 17:02 <shoryak> yes mine first time
22 17:02 <jnewbery> welcome ziggie! welcome shoryak!
23 17:02 <glozow> OOO welcome ziggie and shoryak!!
24 17:03 <jeremyrubin> 👋 shoryak
26 17:03 <svav> To the new guys, how did you find out about PR club?
28 17:03 <jeremyrubin> note the blog post is out of date to the PR w.r.t. the solution
31 17:04 <glozow> okie dokie: Can anyone who reviewed the PR summarize the changes being proposed?
32 17:04 <Azorcode> Hello everyone
33 17:05 <willcl_ark> We want to modify mempool acceptance so that un-upgraded nodes from the future might not accidentally create consensus-invalid blocks using transactions that entered their mempool
34 17:06 <glozow> willcl_ark: yes! to be more specific though, what consensus rule(s) would these un-upgraded nodes be missing?
35 17:07 <svav> NOP - SOmething Op Code what does the N stand for?
36 17:07 <willcl_ark> If we tightened the rules (with a soft fork), then they would still blindly accept future-invalid transactions as valid
37 17:07 <dunxen> consensus rules around the unused bits of nSequence?
38 17:08 <willcl_ark> NOP = No Operation to me :)
40 17:08 <lightlike> they could use OP_CSV in combination with the disablelocktime flag freely, thinking it does nothing, while the upgraded nodes enforce some new consensus rules
41 17:08 <glozow> lightlike: exactly!
42 17:09 <jeremyrubin> yep that's one part; the PR also does another thing too
43 17:09 <Sachin> Could it also be that the nSequence triggers other rules as well?
44 17:09 <jeremyrubin> yep!
45 17:10 <glozow> yes, we should highlight that this is for nSequence numbers and for arguments to OP_CSV
46 17:10 <ziggie> so are we going to declare some transactions as non-standard with this PR ?
47 17:11 <glozow> ziggie: correct, it means transactions that set the locktime disable flag are now non-standard
48 17:11 <glozow> (with the changes in this PR, i mean)
49 17:11 <ziggie> glozow thanks
50 17:11 <glozow> quick conceptual question: What is the difference between policy and consensus rules?
51 17:12 <willcl_ark> So it's the case that currently people can pass (or store) arbitrary data for CSV, as long as they've disable it, and it just gets OP_NOP-ed (and they use the data for their own purpose)?
52 17:12 <shoryak> is policy for mempool acceptance and consensus for block acceptance ?
53 17:13 <jeremyrubin> it's actually a bit worse than that willcl_ark
54 17:13 <Sachin> willcl_ark I believe that is what this PR implements. It should've been the case before
55 17:13 <jeremyrubin> it's that any data that is not defined to be interpreted in the CSV argument or nSequence (which are, tho similar, very different fields) are uninterpreted
56 17:13 <dunxen> policy is around what we accept into our mempool and propagate through the network, some policy invalid txs might still be consensus valid (unless some future soft fork makes that not the case)
58 17:14 <ziggie> willcl_ark so a clever/aother way of doing a OP_RETRUN?
59 17:14 <jeremyrubin> doesn't really matter if disabled/enabled, but disabled is a wide set of ones with no rules
60 17:14 <glozow> shoryak: dunxen: correcto! policy is only applied to unconfirmed transactions we're evaluating for our mempool. and policy is strictly stricter than consensus.
61 17:15 <willcl_ark> It's a nice model though, to have a stricter mempool which you "know" you can freely select only valid transactions from when constructing a block.
62 17:15 <jeremyrubin> ziggie yeah, the Lightning Network currently does this using the nSequence (not CSV arg) field
63 17:15 <glozow> next question: Why do we discourage the usage of upgradable NOPs in policy? How is this implemented?
64 17:15 <ziggie> policy = declare a tx as standard, consensus= declare tx as valid ?
66 17:17 <Sachin> To protect users from potentially creating transactions that are later restricted by a softfork, making them harder (or impossible) to spend
67 17:17 <lightlike> is the term "policy" only used in the context of tx acceptance? Or could it be used for anything that is our own business and doesn't violate consensus, e.g. p2p stuff etc.?
68 17:18 <michaelfolkson> hi
69 17:18 <willcl_ark> +1 Sachin
70 17:18 <jeremyrubin> Sachin are there other users who would have issues other than spenders?
71 17:19 <glozow> lightlike: do you mean like, if we have policies that aren't about transaction validation?
72 17:20 <willcl_ark> I also associate "policy" with my local node settings, which might be configurable to some degree whilst still remaining consensus-valid. Although, as most nodes run the same (default) policy rules, if you want your tx to be propagated then you want your policy to match others' policies too
73 17:20 <glozow> we have limits on the number of ancestor/descendants transactions can have in our mempool, which is policy but not necessarily related to the tx itself
74 17:20 <lightlike> i don't know, we have many local rules that are similar, i was just asking if they would be called policy, or whether that term is usually reserved for mempool acceptance issues
75 17:21 <glozow> we sometimes configure our node with transaction rules that aren't really policy-related, like our wallet maximum fee
76 17:21 <Sachin> jeremyrubin I guess receivers could unknowingly receive to a script that they dont fully understand?
77 17:21 <glozow> i also wouldn't consider something like "i will only keep 100 orphan transactions" policy
78 17:21 <jeremyrubin> Sachin well not quite, if you make your own addresses
79 17:22 <jeremyrubin> think about how the issue would effect miners
80 17:22 <lightlike> jeremyrubin: also miners that have not upgraded might mine blocks that are seen as invalid by upgraded nodes and therefore not accepted
81 17:22 <jeremyrubin> lightlike yep! you got it
82 17:22 <Sachin> Ah, thank you
83 17:23 <glozow> good answers :)
84 17:24 <glozow> ok let's now go over some pros and cons of this PR, for the sake of deciding whether we want to "concpet ack" it
85 17:24 <willcl_ark> Would it be fair to say that a lot of policy rules are local anti-DOS measures?
87 17:24 <glozow> What are some reasons to discourage use of the locktime disable flag in policy? What are some reasons NOT to discourage use of the locktime disable flag in policy?
88 17:25 <glozow> willcl_ark: yes, a lot of them are
89 17:25 <Sachin> willcl_ark many, but some are also for protecting users and making soft forks safer
90 17:25 <Sachin> and miners
91 17:25 <willcl_ark> (except the ones we are discussing today!)
93 17:25 <jeremyrubin> glozow i think it makes sense to talk about the nSequence and CSV commits seprately in terms of pro/con
94 17:25 <willcl_ark> right
95 17:25 <sipa> michaelfolkson: not solely
96 17:25 <michaelfolkson> But as a user I guess you could broaden the definition of policy
97 17:25 <michaelfolkson> sipa: Oh cool
99 17:27 <michaelfolkson> Looks good :)
100 17:27 <glozow> jeremyrubin: fair enough, i'll rephrase. We have 4 questions to talk about: what are the pros/cons of discouraging the use of the locktime disable flag in nSequence/CSV args?
102 17:29 <dunxen> i saw that lightning uses the disable bit and some unused bits for encoding the commitment number, so that would prevent commitment transactions being broadcast?
103 17:29 <lightlike> a con would be some people out there already using it for something. this seems to be the case for the nSequence/lightning?
104 17:30 <glozow> dunxen: indeed. discouraging something in policy means that Bitcoin Core nodes will not relay them, so we have to be careful not to exclude entire applications' transactions if they rely on being able to use nSequence as they please
105 17:31 <Sachin> Would this pr truly prevent commitment txs from being broadcast? or only commitment Txs which use CSV
106 17:32 <svav> What is Bitcoin locktime please?
107 17:33 <ziggie> svav makes a transaction not valid until a specific time in the future is reached
108 17:33 <willcl_ark> I think of sanket1729's questions posted before the meeting by jeremy at this point? Whilst the interpreter changes seem logical, do we really envision further overloading these OP codes with more rules in the future?
109 17:33 <svav> Thanks ziggie!
110 17:34 <glozow> svav: "locktime" generally refers to spending conditions that are like "this UTXO cannot be spent until X time in the future"
111 17:34 <jeremyrubin> svav confusingly there's a concept of a 'lock time' and a nLockTime, which is a specific kind of 'lock time'
112 17:34 <jeremyrubin> always good to clarify which one is being discussed
113 17:34 <glozow> CSV is for relative locktime, so the time constraint is based on the time/#blocks between the time the UTXO is confirmed and when it's spent
114 17:34 <ziggie> so we have to be very conscious in the feature, introducing something like BIP 68, allowing something to be disabled?
115 17:34 <ziggie> *future
116 17:35 <lightlike> I also didn't understand the effect on the lightnig network: would this PR basically shut down the LN in its current form, could there be some exceptions for the specific LN use case?
117 17:35 <ziggie> because people will use it for their own purposes, and nobody knows I they will
118 17:35 <michaelfolkson> Sachin: Commitment transactions only go onchain if close isn't cooperative. Otherwise a simple 2-of-2
119 17:35 <jeremyrubin> ziggie i would go as far as to say if we can't make this policy change now, we can never use these bits ever again in the future
120 17:35 <jeremyrubin> (for a consensus purpose)
121 17:36 <jeremyrubin> the PR has no impact on the lightning network, does anyone know why
122 17:36 <michaelfolkson> (I think... doubting myself now). So only affects uncooperative closes and justice transactions
123 17:36 <glozow> jeremyrubin: i disagree with that. we can still use these bits in the future, gated on a different nVersion number, which doesn't have the "not discouraged in policy" problem
124 17:36 <jeremyrubin> glozow i don't think that's true actually
125 17:37 <glozow> michaelfolkson: to me, it seems _more_ dangerous for Bitcoin Core tx relay policy to only impact non-cooperative Lightning Network closes
126 17:38 <glozow> or just in general, tx relay being inconsistent like that
127 17:38 <jeremyrubin> the pr has no effect on lightning network closes?
128 17:38 <michaelfolkson> glozow: Well yeah those are the ones which are most important. Cooperative closes aren't emergency or time pressured
129 17:38 <glozow> this might be a good time to bring in sanket's suggested question: Does it make sense to change something for an unplanned upgrade?
130 17:39 <sipa> 13:35:44 < jeremyrubin> ziggie i would go as far as to say if we can't make this policy change now, we can never use these bits ever again in the future
131 17:39 <sipa> i don't understand this
132 17:40 <sipa> why can't the policy change be made when a use for the bits is being rolled out?
133 17:40 <jeremyrubin> So there are a couple reasons
134 17:40 <michaelfolkson> glozow: Upgradeability is a nice to have if there are no downsides (even if you can't imagine what the upgrade will be). But there does appear to be downsides with this
135 17:41 <jeremyrubin> A) When you make an upgrade in the future, you want un-upgraded mining nodes to not accept invalid txns to the mempool
136 17:41 <michaelfolkson> I personally can't imagine what the future upgrade would be here (though interested in ideas)
137 17:41 <sipa> jeremyrubin: sure, it'd need to time - but consensus changes take time anyway
138 17:41 <jeremyrubin> so in order for that to be the case, you want the tightening of rules to occur far in the past so that there's plenty of upgrade time
139 17:42 <jeremyrubin> The longer you wait, the more likely it is that more metadata use cases proliferate too, so it's better to make expectations clearer
140 17:42 <glozow> indeed, this is a very tight restriction on our tx relay policy. i don't think the downsides are even very clear right now; efforts should be made to understand whether applications are relying on an assumption that they can use the 31st bit of nSequence
142 17:42 <willcl_ark> But that could boil down to "because we didn't do this 5 years ago we can't do that now", right?
143 17:42 <dunxen> i'm struggling to see what was changed so this PR does not affect unilateral closes on lightning
144 17:43 <jeremyrubin> Essentially, you can't really control for tx version at the script level (IMO) to fix this.
146 17:44 <jeremyrubin> see case CTxIn::SEQUENCE_ROOT_TYPE::UNCHECKED_METADATA
147 17:45 <jeremyrubin> that exempts the LN's bolt defined use case of 0x80-------- nSequences as being just metadata with no meaning for consensus
148 17:45 <sipa> jeremyrubin: hmm, and couldn't a different opcode be used instead?
149 17:45 <glozow> it also seems like an abstraction violation to force our policy code to enumerate the types of LN sequences...
150 17:45 <sipa> (a new OP_CSV2, say)
151 17:45 <michaelfolkson> willcl_ark: I think if there was a proposed upgrade in future that people wanted the fact that this change wasn't done 5 years ago wouldn't impact it. Of course it would take longer than if we did the change now (but we don't know what the upgrade would be now if it ever arrives)
152 17:46 <jeremyrubin> glozow well we already have policy exemptions for LN in the mempool, this is similar. any app can use the seq metadata field
153 17:46 <glozow> are there other applications that use nSequence? have we observed 0 usage of the disable locktime flag for a few years?
154 17:46 <jeremyrubin> sipa a new CSV opcode would work; but i also think the impacts on the CSV arg are lesser than the nSequence field
155 17:47 <jeremyrubin> I think the nSequence field semantics as it pertains to v1 CSV have to be fixed given that v1 CSV is tx.nVersion >= 2
156 17:47 <sipa> jeremyrubin: right, but for the nSequence field, new semantics can be introduced with tx version
158 17:47 <jeremyrubin> nope I do not think so
159 17:47 <jeremyrubin> since it would allow stealing funds prematurely from a csv were the semantics to change
160 17:47 <glozow> jeremyrubin: could you explain why, for the nSequence field specifically, it wouldn't be sufficient to gate on a new nVersion number?
161 17:48 <jeremyrubin> Yes
162 17:48 <jeremyrubin> Imagine that *today* i create an output which is IF <2 years> CSV <backuip> Checksig ELSE <normal> Checksig ENDIF
163 17:48 <glozow> ok since we're running low on time, next question for the review clubbies is: Do you think this change (discouragement of setting the most significant bit) should be applied to nSequence numbers, CSV values, both, or neither? Why?
164 17:48 <lightlike> has someone done analysis of the blockchain if (and with which values) nSequence is currently used?
165 17:49 <jeremyrubin> and then you switch to tx.nVersion 3
166 17:49 <jeremyrubin> my output is spendable in tx.nVersion 3
167 17:49 <jeremyrubin> and if nVersion 3 undefines the CSV semantics as they were prior
168 17:49 <jeremyrubin> then you'd trigger backup prematurely
169 17:49 <glozow> lightlike: good question. I would also feel much more comfortable reasoning about this PR if there was such an analysis done
170 17:49 <jeremyrubin> so whatever new semantic for CSV exists in nversion 3 has to be compatible with old scripts
171 17:50 <jeremyrubin> you could do something like nVersion 3 must only be segwit v2 (not v1, v0) and that might work? But that hurts fungibility
172 17:51 <Sachin> jeremyrubin Sorry to backtrack but I don't understand why this doesnt affect LN commitment txs
173 17:51 <jeremyrubin> Sachin LN commitment txns specifically use 0x80 prefixed sequence numbers
174 17:51 <glozow> why wouldn't a new semantic for CSV scripts be compatible with old scripts?
175 17:51 <jeremyrubin> the PR applies no rules when the top bits are exactly 0x80
176 17:51 <Sachin> ah, thank you
177 17:52 <jeremyrubin> because if an old output could be spent in tx.nVersion 3 with different semantics, it would disrupt the timing of that old output
178 17:52 <michaelfolkson> Meh not convinced on the fungibility hurt, every SegWit version introduces new rules that supposedly hurt fungibility (of course they do when they are first introduced but that's the cost of a new SegWit version)
179 17:53 <jeremyrubin> michaelfolkson it's different, you're talking about fungibility from privacy v.s. cospendable fungibility
180 17:53 <jeremyrubin> one is much worse than the other
181 17:53 <michaelfolkson> Hmm ok
182 17:53 <jeremyrubin> glozow so the backup clause could either be made available prematurely or too late (or never) under tx.nVersion 3
183 17:53 <glozow> cospendability? you mean spending a "old CSV" and "new CSV" in the same tx?
184 17:54 <jeremyrubin> nope, it would be any pre segwit v2 (not v1, v0) output in the same tx as new (maybe could limit to leaf versions in v1)
185 17:54 <jeremyrubin> too late is fine, never is fine (just use tx nversion 2), but too early breaks the spec
186 17:56 <jeremyrubin> it maybe makes it more intuitive to think about it like "could we define signatures as always being valid in tx.nVersion 3?"
187 17:56 <jeremyrubin> we cannot do that, because then any miner could mine a block stealing all the coins\
188 17:57 <jeremyrubin> we need to preserve some semantics across tx version for output types.
189 17:57 <jeremyrubin> and nSequence and CSV arg are a part of that, or else it may premit theft
190 17:58 <glozow> afaik the current CSV semantics allow any nversion >=2
191 17:58 <jeremyrubin> yep; that's part of why this problem exists
192 17:58 <dunxen> glozow: i think i'd need to look at this more closely before I have an answer for your last question haha
193 17:59 <jeremyrubin> i think harding thought it was just nVersion == 2
194 18:00 <jeremyrubin> but because it's >= 2, the rules need to stay largely the same on all future versions unless we do the tx.nVersion blocks inputs that are not segwit v2 or newer or something
195 18:00 <glozow> haha no problem. i hope people have some food for thought around how to reason about this PR conceptually
196 18:00 <lightlike> so you think it was a mistake not to just use nVersion=2?
197 18:01 <glozow> given that BIP68 uses nVersion >= 2, i'd say there isn't really a problem with having an "old CSV" input inside a transaction with version 3
198 18:01 <jeremyrubin> well there isn't if you don't change nSequence semantics :)
199 18:01 <jeremyrubin> but that's the crux of this, which is preserving upgradability
200 18:02 <glozow> looks like we're out of time - the final 2 questions are around the approach of the PR, since it requires a lot of unit tests to be loosened
201 18:02 <glozow> they are left as exercise to the reader i guess :P
202 18:02 <glozow> #endmeeting