Return-Path: Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id E6EA6C000D for ; Thu, 9 Sep 2021 01:05:09 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id D68A1605E6 for ; Thu, 9 Sep 2021 01:05:09 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org X-Spam-Flag: NO X-Spam-Score: -4.199 X-Spam-Level: X-Spam-Status: No, score=-4.199 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XOm26uPUF6en for ; Thu, 9 Sep 2021 01:05:08 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) by smtp3.osuosl.org (Postfix) with ESMTPS id 214BA605BC for ; Thu, 9 Sep 2021 01:05:07 +0000 (UTC) Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com [209.85.167.43]) (authenticated bits=0) (User authenticated as jlrubin@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 189155iL029796 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NOT) for ; Wed, 8 Sep 2021 21:05:06 -0400 Received: by mail-lf1-f43.google.com with SMTP id x27so207630lfu.5 for ; Wed, 08 Sep 2021 18:05:06 -0700 (PDT) X-Gm-Message-State: AOAM533WIfu9bZiRY77/h8/vFZsHWIod/g6jvKDl/t+OzV11+n4bAWtQ 9GrTE7v0JPrggk8mnCrmI9hdFUwOMctbPTZk+0U= X-Google-Smtp-Source: ABdhPJw0KM1H1W0BpXbwmDCWjVizQwHo8wqPgxrQ+LImMVeT08Ow3bmTPMZjyXXYkk6bcYBPXHPJnGMeSyh5UxTDLIQ= X-Received: by 2002:a19:c7c3:: with SMTP id x186mr335857lff.175.1631149504670; Wed, 08 Sep 2021 18:05:04 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Jeremy Date: Wed, 8 Sep 2021 18:04:53 -0700 X-Gmail-Original-Message-ID: Message-ID: To: Antoine Riard Content-Type: multipart/alternative; boundary="000000000000cd305c05cb85967f" Cc: Bitcoin Protocol Discussion Subject: Re: [bitcoin-dev] Note on Sequence Lock Upgrades Defect X-BeenThere: bitcoin-dev@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Bitcoin Protocol Discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Sep 2021 01:05:10 -0000 --000000000000cd305c05cb85967f Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable See the current patchset proposed: https://github.com/bitcoin/bitcoin/pull/22871/commits Two things are happening that are separate: 1) Fixing the semantics of arg in OP_CHECKSEQUENCEVERIFY 2) Fixing the semantics on nSequence in each tx input There is no sense in conditioning part 1 on RBF or anything else, since it's only loosely related to 2. I think it should be a class-2 rollout as you describe above since it's a rule tightening. For part 2, I think the way the patches handle it currently (which is defining 1 byte type prefix followed by 3 bytes application data) is sufficient for immediate deployment. I agree with you that a class-2 rollout might be appropriate for it, but that can be followed by removing the SEQUENCE_ROOT_TYPE::SPECIAL field later as a class-1 rollout. However, so long as it's not being used for any particular constants, there is no need to deallocate SEQUENCE_ROOT_TYPE::SPECIAL tag as long as no new use case must overlap it's range. With respect to the SEQUENCE_ROOT_TYPE::UNCHECKED_METADATA, it is in fact *not* mempool data, but is a special type of metadata which is required for the counterparty to efficiently respond to a unilateral channel closure (se= e bolt-3 This obscures the number of commitments made on the channel in the case of unilateral close, yet still provides a useful index for both nodes (who know the payment_basepoints) to quickly find a revoked commitment transaction.) I understand wanting to remove full-rbf, but I think that fixing the upgradability of sequences is much less controversial among the userbase and worth doing expediently. That part 1 is doable now -- albeit as a class 2 -- means that it would not be unreasonable to bundle parts 1 and 2 so that we don't double burden the community with an upgrade effort. Further, RBF can be disabled on a purely ad-hoc node-by-node policy layer, whereas this restriction requires more community coordination/awareness. -- @JeremyRubin On Wed, Sep 8, 2021 at 5:03 PM Antoine Riard wrote: > Hi Jeremy, > > Answering here from #22871 discussions. > > I agree on the general principle to not blur mempool policies signaling i= n > committed transaction data. Beyond preserving upgradeability, another goo= d > argument is to let L2 nodes update the mempool policies signaling their > pre-signed transactions non-interactively. If one of the transaction fiel= ds > is assigned mempool semantics, in case of tightening policy changes, you > will need to re-sign or bear the risks of having non-propagating > transactions which opens the door for exploitation by a malicious > counterparty. I think this point is kinda relevant if we have future > cross-layer coordinated safety fixes to deal with a la CVE-2021-31876. > > Even further, a set of L2 counterparties would like to pick up divergent > tx-relay/mempool policies, having the signaling fields as part of the > signature force them to come to consensus. > > I think we can take the opportunity of p2p packages to introduce a new > field to signal policy. Of course, a malicious tx-relay peer could modify > its content to jam your transaction's propagation but in that case it is > easier to just drop it. > > One issue with taking back the `nSequence` field for consensus-semantic > sounds is depriving the application-layer from a discrete, zero-cost > payload (e.g the LN obfuscated commitment number watermark). This might b= e > controversial as we'll increase the price of such applications if they're > still willingly to relay application specific data through the p2p networ= k > (e.g force them to use a costly OP_RETURN output or payer/payee > interactions to setup a pay-to-contract) > > W.r.t flag day activation to smooth policy deployment, I think that's > something we might rely on in the future though we could distinguish few > types of policy deployments : > 1) loosening changes (e.g full-rbf/dust threshold removal), a transaction > which was relaying under > the former policy should relay under the new one > 2) tightening changes (e.g #22871), a transaction which was relaying unde= r > the former policy > might not relay under the new one > 3) new feature introduced (e.g packages), a transaction is offered a new > mode of relay > > I think 1) doesn't need that level of ecosystem coordination as > applications/second-layers should always benefit from such changes. Maybe > with the exception of full-rbf, where we have historical 0-conf softwares= , > with (broken) security assumptions made on the opt-out RBF mechanism. Sam= e > with 3), better to have new features deployed gradually, a flag day > activation day in this case won't mean that all higher stacks will jump t= o > use package-relay ? > > Where a flag day might make sense would be for 2) ? It would create a > higher level of commitment by the base layer software instead of a pure > communication on the ML/GH, which might not be concretized in the announc= ed > release due to slow review process/feature freeze/rebase conflicts... > Reversing the process and asking for Bitcoin applications/higher layers t= o > update first might get us in the trap of never doing the change, as someo= ne > might have a small use-case in the corner relying on a given policy > behavior. > > That said, w.r.t to the proposed policy change in #22871, I think it's > better to deploy full-rbf first, then give a time buffer to higher > applications to free up the `nSequence` field and finally start to > discourage the usage. Otherwise, by introducing new discouragement waiver= s, > e.g not rejecting the usage of the top 8 bits, I think we're moving away > from the policy design principle we're trying to establish (separation of > mempool policies signaling from consensus data) > > Le ven. 3 sept. 2021 =C3=A0 23:32, Jeremy via bitcoin-dev < > bitcoin-dev@lists.linuxfoundation.org> a =C3=A9crit : > >> Hi Bitcoin Devs, >> >> I recently noticed a flaw in the Sequence lock implementation with >> respect to upgradability. It might be the case that this is protected >> against by some transaction level policy (didn't see any in policy.cpp, = but >> if not, I've put up a blogpost explaining the defect and patching it >> https://rubin.io/bitcoin/2021/09/03/upgradable-nops-flaw/ >> >> I've proposed patching it here >> https://github.com/bitcoin/bitcoin/pull/22871, it is proper to widely >> survey the community before patching to ensure no one is depending on th= e >> current semantics in any live application lest this tightening of >> standardness rules engender a confiscatory effect. >> >> Best, >> >> Jeremy >> >> -- >> @JeremyRubin >> >> _______________________________________________ >> bitcoin-dev mailing list >> bitcoin-dev@lists.linuxfoundation.org >> https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev >> > --000000000000cd305c05cb85967f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
See the current patchset = proposed:=C2=A0https://github.com/bitcoin/bitcoin/pull/22871/commits

Two t= hings are happening=C2=A0that are separate:

1) Fixing the semantics= =C2=A0of arg in <arg> OP_CHECKSEQUENCEVERIFY
2) Fixing the semantics on nSequence in each tx input

T= here is no sense in conditioning part 1 on RBF or anything else, since it&#= 39;s only loosely related to 2. I think it should be a class-2 rollout as y= ou describe above since it's a rule tightening.

For part 2, I thi= nk the way the patches handle it currently (which is defining 1 byte type p= refix followed by 3 bytes application data) is sufficient for immediate dep= loyment.

I agree with you that a class-2 rollout might be appropriate= for it, but that can be followed by removing the SEQUENCE_ROOT_TYPE::SPECI= AL field later as a class-1 rollout. However, so long as it's not being= used for any particular constants, there is no need to deallocate SEQUENCE= _ROOT_TYPE::SPECIAL tag as long as no new use case must overlap it's ra= nge.

With r= espect to the SEQUENCE_ROOT_TYPE::UNCHECKED_METADATA, it is in fact *not* m= empool data, but is a special type of metadata which is required for the counterp= arty to efficiently respond to a unilateral channel closure=C2=A0(se= e bolt-3 This obscures the number of commitments made on the channel in the= case of unilateral close, yet still provides a useful index for both nodes= (who know the payment_basepoints) to quickly find a revoked commitment tra= nsaction.)

I understand wanting to remove full-rbf, but I think that fixi= ng the upgradability of sequences is much less controversial among the user= base=C2=A0and worth doing expediently. That part 1 is doable now -- albeit = as a class 2 -- means that it would not be unreasonable to bundle parts 1 a= nd 2 so that we don't double burden the community with an upgrade effor= t. Further, RBF can be disabled on a purely ad-hoc node-by-node policy laye= r, whereas this restriction requires more community coordination/awareness.=


On Wed, Sep 8, 2021 = at 5:03 PM Antoine Riard <ant= oine.riard@gmail.com> wrote:
Hi Jeremy,

Answering here from #22871 discussions.

I agre= e on the general principle to not blur mempool policies signaling in commit= ted transaction data. Beyond preserving upgradeability, another good argume= nt is to let L2 nodes update the mempool policies signaling their pre-signe= d transactions non-interactively. If one of the transaction fields is assig= ned mempool semantics, in case of tightening policy changes, you will need = to re-sign or bear the risks of having non-propagating transactions which o= pens the door for exploitation by a malicious counterparty. I think this po= int is kinda relevant if we have future cross-layer coordinated safety fixe= s to deal with a la CVE-2021-31876.

Even further, a set of L2 counte= rparties would like to pick up divergent tx-relay/mempool policies, having = the signaling fields as part of the signature force them to come to consens= us.

I think we can take the opportunity of p2p packages to introduce= a new field to signal policy. Of course, a malicious tx-relay peer could m= odify its content to jam your transaction's propagation but in that cas= e it is easier to just drop it.

One issue with taking back the `nSeq= uence` field for consensus-semantic sounds is depriving the application-lay= er from a discrete, zero-cost payload (e.g the LN obfuscated commitment num= ber watermark). This might be controversial as we'll increase the price= of such applications if they're still willingly to relay application s= pecific data through the p2p network (e.g force them to use a costly OP_RET= URN output or payer/payee interactions to setup a pay-to-contract)

W= .r.t flag day activation to smooth policy deployment, I think that's so= mething we might rely on in the future though we could distinguish few type= s of policy deployments :
1) loosening changes (e.g full-rbf/dust thresh= old removal), a transaction which was relaying under
the former policy s= hould relay under the new one
2) tightening changes (e.g #22871), a tran= saction which was relaying under the former policy
might not relay under= the new one
3) new feature introduced (e.g packages), a transaction is = offered a new mode of relay

I think 1) doesn't need that level o= f ecosystem coordination as applications/second-layers should always benefi= t from such changes. Maybe with the exception of full-rbf, where we have hi= storical 0-conf softwares, with (broken) security assumptions made on the o= pt-out RBF mechanism. Same with 3), better to have new features deployed gr= adually, a flag day activation day in this case won't mean that all hig= her stacks will jump to use package-relay ?

Where a flag day might m= ake sense would be for 2) ? It would create a higher level of commitment by= the base layer software instead of a pure communication on the ML/GH, whic= h might not be concretized in the announced release due to slow review proc= ess/feature freeze/rebase conflicts... Reversing the process and asking for= Bitcoin applications/higher layers to update first might get us in the tra= p of never doing the change, as someone might have a small use-case in the = corner relying on a given policy behavior.

That said, w.r.t to the p= roposed policy change in #22871, I think it's better to deploy full-rbf= first, then give a time buffer to higher applications to free up the `nSeq= uence` field and finally start to discourage the usage. Otherwise, by intro= ducing new discouragement waivers, e.g not rejecting the usage of the top 8= bits, I think we're moving away from the policy design principle we= 9;re trying to establish (separation of mempool policies signaling from con= sensus data)

Le=C2=A0ven. 3 sept. 2021 =C3=A0=C2=A023:32, Jeremy via bit= coin-dev <bitcoin-dev@lists.linuxfoundation.org> a =C3=A9crit=C2= =A0:
Hi Bitcoin Devs,

I recently noticed a flaw in the Sequ= ence lock implementation with respect to upgradability. It might be the cas= e that this is protected against by some transaction level policy (didn'= ;t see any in policy.cpp, but if not, I've put up a blogpost=C2=A0expla= ining the defect and patching it=C2=A0https://rubin.io/bitcoin= /2021/09/03/upgradable-nops-flaw/

I've proposed patchin= g it here=C2=A0https://github.com/bitcoin/bitcoin/pull/22871, it is pr= oper to widely survey the community before patching to ensure no one is dep= ending on the current semantics in any live application lest this tightenin= g of standardness rules engender a confiscatory effect.

Best,

Jeremy

_______________________________________________
bitcoin-dev mailing list
= bitcoin-dev@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mail= man/listinfo/bitcoin-dev
--000000000000cd305c05cb85967f--