From: Jeremy <jlrubin@mit•edu>
To: Jeremy <jlrubin@mit•edu>
Cc: Bitcoin Protocol Discussion <bitcoin-dev@lists•linuxfoundation.org>
Subject: Re: [bitcoin-dev] Stumbling into a contentious soft fork activation attempt
Date: Mon, 10 Jan 2022 20:38:42 -0800 [thread overview]
Message-ID: <CAD5xwhi9gveSgAp2Vuswzq8hXNLaUGjHeOb7LbyWq5_A_n9GMw@mail.gmail.com> (raw)
In-Reply-To: <CAD5xwhi=H0Nft4Jbqhd3=89BhAB2JLoTc=mPhdcQkoQxa1sAUg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2748 bytes --]
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.
>
>
[-- Attachment #2: Type: text/html, Size: 5093 bytes --]
next prev parent reply other threads:[~2022-01-11 4:38 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-03 2:05 Michael Folkson
2022-01-09 11:38 ` Peter Todd
2022-01-11 3:42 ` Jeremy
2022-01-11 4:38 ` Jeremy [this message]
2022-01-04 11:53 Prayank
2022-01-04 14:15 ` Michael Folkson
2022-01-04 15:06 ` Prayank
2022-01-04 16:48 ` Michael Folkson
2022-01-04 17:07 ` Prayank
2022-01-04 14:42 ` Christian Decker
2022-01-04 15:45 ` Prayank
2022-01-18 1:57 Prayank
2022-02-18 23:41 ` Peter Todd
2022-02-20 18:35 ` Erik Aronesty
2022-02-21 3:03 ` Prayank
2022-02-21 9:02 ` ZmnSCPxj
2022-02-21 9:11 ` ZmnSCPxj
2022-02-21 9:48 ` Prayank
2022-02-22 12:57 ` Billy Tetrud
2022-02-21 9:09 ` ZmnSCPxj
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAD5xwhi9gveSgAp2Vuswzq8hXNLaUGjHeOb7LbyWq5_A_n9GMw@mail.gmail.com \
--to=jlrubin@mit$(echo .)edu \
--cc=bitcoin-dev@lists$(echo .)linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox