path: root/35/d0ed137ada11d7e448db9f7700cb7cc89e297d

Return-Path: <jlrubin@mit.edu>
Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138])
 by lists.linuxfoundation.org (Postfix) with ESMTP id 4C984C001E
 for <bitcoin-dev@lists.linuxfoundation.org>;
 Tue, 11 Jan 2022 04:38:59 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
 by smtp1.osuosl.org (Postfix) with ESMTP id 2C96C83459
 for <bitcoin-dev@lists.linuxfoundation.org>;
 Tue, 11 Jan 2022 04:38:59 +0000 (UTC)
X-Virus-Scanned: amavisd-new at osuosl.org
X-Spam-Flag: NO
X-Spam-Score: -4.197
X-Spam-Level: 
X-Spam-Status: No, score=-4.197 tagged_above=-999 required=5
 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_MED=-2.3,
 RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001,
 SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from smtp1.osuosl.org ([127.0.0.1])
 by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)
 with ESMTP id pz2ndCsO64zO
 for <bitcoin-dev@lists.linuxfoundation.org>;
 Tue, 11 Jan 2022 04:38:57 +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 smtp1.osuosl.org (Postfix) with ESMTPS id 21B4682D04
 for <bitcoin-dev@lists.linuxfoundation.org>;
 Tue, 11 Jan 2022 04:38:56 +0000 (UTC)
Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com
 [209.85.167.52]) (authenticated bits=0)
 (User authenticated as jlrubin@ATHENA.MIT.EDU)
 by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 20B4csKS004467
 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NOT)
 for <bitcoin-dev@lists.linuxfoundation.org>; Mon, 10 Jan 2022 23:38:55 -0500
Received: by mail-lf1-f52.google.com with SMTP id i31so51851530lfv.10
 for <bitcoin-dev@lists.linuxfoundation.org>;
 Mon, 10 Jan 2022 20:38:55 -0800 (PST)
X-Gm-Message-State: AOAM533wq3CFxT+KuxAlpygUXnsULTu5+yYZYTUjbO7Tov+WlA+Vh4iB
 iIsMwP1tTL4BHXMYRhjQcdF/4Y3Y1bKANual/f4=
X-Google-Smtp-Source: ABdhPJzsCFF1SxACUkj2ynEkKbItqOhASGhD6wCpP1k22Xc+mETon2bE4ddfeFTJNlnaQEK8pNWVWv1JfBEsVwyLa0g=
X-Received: by 2002:a05:651c:160a:: with SMTP id
 f10mr1691013ljq.212.1641875933697; 
 Mon, 10 Jan 2022 20:38:53 -0800 (PST)
MIME-Version: 1.0
References: <XuO20TMFGBqz53WYWxi9bgAdB3iGmqEIUE84AupRxCpHQVd3-YbGVzZUFz21dOgb_AgwlGWaruzE8NGxhes6HCKHpRZLmL1d1kNu1yobAIU=@protonmail.com>
 <YdrJJ3VxoxHVgg7Y@petertodd.org>
 <CAD5xwhi=H0Nft4Jbqhd3=89BhAB2JLoTc=mPhdcQkoQxa1sAUg@mail.gmail.com>
In-Reply-To: <CAD5xwhi=H0Nft4Jbqhd3=89BhAB2JLoTc=mPhdcQkoQxa1sAUg@mail.gmail.com>
From: Jeremy <jlrubin@mit.edu>
Date: Mon, 10 Jan 2022 20:38:42 -0800
X-Gmail-Original-Message-ID: <CAD5xwhi9gveSgAp2Vuswzq8hXNLaUGjHeOb7LbyWq5_A_n9GMw@mail.gmail.com>
Message-ID: <CAD5xwhi9gveSgAp2Vuswzq8hXNLaUGjHeOb7LbyWq5_A_n9GMw@mail.gmail.com>
To: Jeremy <jlrubin@mit.edu>
Content-Type: multipart/alternative; boundary="000000000000cb1c7d05d54707a9"
Cc: Bitcoin Protocol Discussion <bitcoin-dev@lists.linuxfoundation.org>
Subject: Re: [bitcoin-dev] Stumbling into a contentious soft fork activation
	attempt
X-BeenThere: bitcoin-dev@lists.linuxfoundation.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: Bitcoin Protocol Discussion <bitcoin-dev.lists.linuxfoundation.org>
List-Unsubscribe: <https://lists.linuxfoundation.org/mailman/options/bitcoin-dev>, 
 <mailto:bitcoin-dev-request@lists.linuxfoundation.org?subject=unsubscribe>
List-Archive: <http://lists.linuxfoundation.org/pipermail/bitcoin-dev/>
List-Post: <mailto:bitcoin-dev@lists.linuxfoundation.org>
List-Help: <mailto:bitcoin-dev-request@lists.linuxfoundation.org?subject=help>
List-Subscribe: <https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev>, 
 <mailto:bitcoin-dev-request@lists.linuxfoundation.org?subject=subscribe>
X-List-Received-Date: Tue, 11 Jan 2022 04:38:59 -0000

--000000000000cb1c7d05d54707a9
Content-Type: text/plain; charset="UTF-8"

Please see the following bips PRs which are follow ups to the concrete
actionables raised by Peter. Thanks for bringing these up, it certainly
improves the reviewability of the BIP.

https://github.com/bitcoin/bips/pull/1271
https://github.com/bitcoin/bips/pull/1272

--
@JeremyRubin <https://twitter.com/JeremyRubin>
<https://twitter.com/JeremyRubin>


On Mon, Jan 10, 2022 at 7:42 PM Jeremy <jlrubin@mit.edu> wrote:

> Hi Peter,
>
> Thank you for your review and feedback.
>
> Apologies for the difficulties in reviewing. The branch linked from the
> BIP is not the latest, the branch in the PR is what should be considered
> https://github.com/bitcoin/bitcoin/pull/21702 for review and has more
> thorough well documented tests and test vectors. The version you reviewed
> should still be compatible with the current branch as there have not been
> any spec changes, though.
>
> I'm not sure what best practice is w.r.t. linking to BIPs and
> implementations given need to rebase and respond to feedback with changes.
> Appreciate any pointers on how to better solve this. For the time being, I
> will suggest an edit to point it to the PR, although I recognize this is
> not ideal. I understand your preference for a commit hash and can do one
> if it helps. For what it's worth, the taproot BIPs do not link to a
> reference implementation of Taproot so I'm not sure what best practice is
> considered these days.
>
> One note that is unfortunate in your review is that there is a
> discrepancy between the BIP and the implementation (the original reference
> or the current PR either) in that caching and DoS is not addressed. This
> was an explicit design goal of CTV and for it not to be mentioned in the
> BIP (and just the reference) is an oversight on my part to not aid
> reviewers more explicitly. Compounding this, I accepted a third-party PR to
> make the BIP more clear as to what is required to implement it that does
> not have caching (functional correctness), that exposes the issue if
> implemented by the BIP directly and not by the reference implementation. I
> have explained this in a review last year to pyskell
> <https://github.com/bitcoin/bitcoin/pull/21702#discussion_r616853690> on
> the PR that caching is required for non-DoS. I will add a note to the BIP
> about the importance of caching to avoid DoS as that should make third
> party implementers aware of the issue.
>
> That said, this is not a mis-considered part of CTV. The reference
> implementation is specifically designed to not have quadratic hashing and
> CTV is designed to be friendly to caching to avoid denial of service. It's
> just a part of the BIP that can be more clear. I will make a PR to more
> clearly describe how that should happen.
>
>

--000000000000cb1c7d05d54707a9
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div class=3D"gmail_default" style=3D"fon=
t-family:arial,helvetica,sans-serif;font-size:small;color:#000000">Please s=
ee the following bips PRs which are follow ups to the concrete actionables =
raised by Peter. Thanks for bringing these up, it certainly improves the re=
viewability of the BIP.</div><div class=3D"gmail_default" style=3D"font-fam=
ily:arial,helvetica,sans-serif;font-size:small;color:#000000"><br></div><di=
v class=3D"gmail_default" style=3D"font-family:arial,helvetica,sans-serif;f=
ont-size:small;color:#000000"><a href=3D"https://github.com/bitcoin/bips/pu=
ll/1271">https://github.com/bitcoin/bips/pull/1271</a><br></div><div class=
=3D"gmail_default" style=3D"font-family:arial,helvetica,sans-serif;font-siz=
e:small;color:#000000"><a href=3D"https://github.com/bitcoin/bips/pull/1272=
">https://github.com/bitcoin/bips/pull/1272</a><br></div><div class=3D"gmai=
l_default" style=3D"font-family:arial,helvetica,sans-serif;font-size:small;=
color:#000000"><br></div><div><div dir=3D"ltr" class=3D"gmail_signature" da=
ta-smartmail=3D"gmail_signature"><div dir=3D"ltr">--<br><a href=3D"https://=
twitter.com/JeremyRubin" target=3D"_blank">@JeremyRubin</a><a href=3D"https=
://twitter.com/JeremyRubin" target=3D"_blank"></a></div></div></div><br></d=
iv><br><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">On =
Mon, Jan 10, 2022 at 7:42 PM Jeremy &lt;<a href=3D"mailto:jlrubin@mit.edu">=
jlrubin@mit.edu</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" s=
tyle=3D"margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:so=
lid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><=
div style=3D"font-family:arial,helvetica,sans-serif;font-size:small;color:r=
gb(0,0,0)">Hi Peter,</div><div style=3D"font-family:arial,helvetica,sans-se=
rif;font-size:small;color:rgb(0,0,0)"><br></div><div style=3D"font-family:a=
rial,helvetica,sans-serif;font-size:small;color:rgb(0,0,0)">Thank you for y=
our review and feedback.</div><div style=3D"font-family:arial,helvetica,san=
s-serif;font-size:small;color:rgb(0,0,0)"><br></div><div style=3D"font-fami=
ly:arial,helvetica,sans-serif;font-size:small;color:rgb(0,0,0)">Apologies f=
or the difficulties in reviewing. The branch linked from the BIP is not the=
 latest, the branch in the PR is what should be considered=C2=A0<a href=3D"=
https://github.com/bitcoin/bitcoin/pull/21702" target=3D"_blank">https://gi=
thub.com/bitcoin/bitcoin/pull/21702</a> for review and has more thorough we=
ll documented tests and test vectors. The version you reviewed should still=
 be compatible with the current branch as there have not been any spec chan=
ges, though.</div><div style=3D"font-family:arial,helvetica,sans-serif;font=
-size:small;color:rgb(0,0,0)"><br></div>I&#39;m not sure what best practice=
 is w.r.t. linking to BIPs and implementations given need to rebase and res=
pond to feedback with changes. Appreciate any pointers on how to better sol=
ve this. For the time being, I will suggest an edit to point it to the PR, =
although I recognize this is not ideal. I understand your preference for a =
commit hash and can do one<span class=3D"gmail_default" style=3D"font-famil=
y:arial,helvetica,sans-serif;font-size:small;color:rgb(0,0,0)"> if it helps=
. For what it&#39;s worth, the taproot BIPs do not link to a reference impl=
ementation of Taproot so I&#39;m not sure what best practice is considered =
these days.</span><div><br></div><div><div style=3D"font-family:arial,helve=
tica,sans-serif;font-size:small;color:rgb(0,0,0)">One note that is unfortun=
ate in your review is that there is a discrepancy=C2=A0between the BIP and =
the implementation (the original reference or the current PR either) in tha=
t caching and DoS is not addressed. This was an explicit design goal of CTV=
 and for it not to be mentioned in the BIP (and just the reference) is an o=
versight on my part to not aid reviewers more explicitly. Compounding this,=
 I accepted a third-party PR to make the BIP more clear as to what is requi=
red to implement it that does not have caching (functional correctness), th=
at exposes the issue if implemented by the BIP directly and not by the refe=
rence implementation. I have explained this in a <a href=3D"https://github.=
com/bitcoin/bitcoin/pull/21702#discussion_r616853690" target=3D"_blank">rev=
iew last year to pyskell</a> on the PR that caching is required for non-DoS=
. I will add a note to the BIP about the importance of caching to avoid DoS=
 as that should make third party implementers aware of the issue.</div><div=
 style=3D"font-family:arial,helvetica,sans-serif;font-size:small;color:rgb(=
0,0,0)"><br></div><div style=3D"font-family:arial,helvetica,sans-serif;font=
-size:small;color:rgb(0,0,0)">That said, this is not a mis-considered=C2=A0=
part of CTV. The reference implementation is specifically designed to not h=
ave quadratic hashing and CTV is designed to be friendly to caching to avoi=
d denial of service. It&#39;s just a part of the BIP that can be more clear=
. I will make a PR to more clearly describe how that should happen.</div><d=
iv style=3D"font-family:arial,helvetica,sans-serif;font-size:small;color:rg=
b(0,0,0)"><br></div><div style=3D"font-size:small"></div></div></div>
</blockquote></div></div>

--000000000000cb1c7d05d54707a9--