From: Pieter Wuille <pieter.wuille@gmail•com>
To: Zooko Wilcox-OHearn <zooko@leastauthority•com>
Cc: Bitcoin Dev <bitcoin-development@lists•sourceforge.net>
Subject: Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
Date: Sun, 25 Jan 2015 12:57:23 -0400 [thread overview]
Message-ID: <CAPg+sBi7OnV9Tm06VBoHv54opQy3RJHzmyD+imcaUq8sEw=6qQ@mail.gmail.com> (raw)
In-Reply-To: <CAM_a8Jzt=Q8m=-7wur1UdscqQRQmg+TBFNtneUtRH3nh+TOTDQ@mail.gmail.com>
On Thu, Jan 22, 2015 at 6:41 PM, Zooko Wilcox-OHearn
<zooko@leastauthority•com> wrote:
> * Should the bipstrictder give a rationale or link to why accept the
> 0-length sig as correctly-encoded-but-invalid? I guess the rationale
> is an efficiency issue as described in the log entry for
> https://github.com/sipa/bitcoin/commit/041f1e3597812c250ebedbd8f4ef1565591d2c34
I've lately been updating the BIP text without updating the code in
the repository; I've synced them now. The sigsize=0 case was actually
already handled elsewhere already, so I removed the code and added a
comment about it now in the BIP text.
> * Does this mean there are still multiple ways to encode a correctly
> encoded but invalid signature, one of which is the 0-length string?
> Would it make sense for this change to also treat any *other*
> correctly-encoded-but-invalid sig (besides the 0-length string) as
> incorrectly-encoded? Did I just step in some BIP62?
You didn't miss anything; that's correct. In fact, Peter Todd already
pointed out the possibility of making non-empty invalid signatures
illegal. The reason for not doing it yet is that I'd like this BIP to
be minimal and uncontroversial - it's a real problem we want to fix as
fast as is reasonable. It wouldn't be hard to make this a standardness
rule though, and perhaps later softfork it in as consensus rule if
there was sufficient agreement about it.
> * It would be good to verify that all the branches of the new
> IsDERSignature() from
> https://github.com/sipa/bitcoin/commit/0c427135151a6bed657438ffb2e670be84eb3642
> are tested by the test vectors in
> https://github.com/sipa/bitcoin/commit/f94e806f8bfa007a3de4b45fa3c9860f2747e427
> . Eyeballing it, there are about 20 branches touched by the patch, and
> about 24 new test vectors.
A significiant part of DERSIG behaviour (which didn't change, only the
cases in which it is enforced) was already tested, in fact. Some
branches remained untested however; I've added extra test cases in the
repository. They give 100% coverage for IsValidSignatureEncoding (the
new name for IsDERSignature) now (tested with gcov).
> * It would be good to finish the TODOs in
> https://github.com/sipa/bitcoin/commit/b7986119a5d41337fea1e83804ed6223438158ec
> so that it was actually testing the upgrade behavior.
I agree, but that requires very significant changes to the codebase,
as we currently have no way to mine blocks with non-acceptable
transactions. Ideally, the RPC tests gain some means of
building/mining blocks from without the Python test framework. Things
like that would make the code changes also hard to backport, which we
definitely will need to do to roll this out quickly.
> * missing comment:
> https://github.com/sipa/bitcoin/commit/e186f6a80161f9fa45fbced82ab1d22f081b942c#commitcomment-9406643
Fixed.
> Okay, that's all I've got. Hope it helps! Thanks again for your good work!
Thanks!
--
Pieter
next prev parent reply other threads:[~2015-01-25 16:57 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-21 0:35 Pieter Wuille
2015-01-21 4:45 ` Rusty Russell
2015-01-21 16:49 ` Pieter Wuille
2015-01-21 19:10 ` Peter Todd
2015-01-21 19:29 ` Douglas Roark
2015-01-21 20:30 ` Pieter Wuille
2015-01-21 20:39 ` Douglas Roark
2015-01-21 20:37 ` Gavin Andresen
2015-01-21 20:52 ` Douglas Roark
2015-01-21 21:22 ` Pieter Wuille
2015-01-21 20:27 ` Andrew Poelstra
2015-01-21 22:57 ` Dave Collins
2015-01-22 0:32 ` Rusty Russell
2015-01-22 3:12 ` David Vorick
2015-01-22 4:18 ` Matt Whitlock
2015-01-22 4:20 ` Pieter Wuille
2015-01-25 14:34 ` Pieter Wuille
2015-01-25 14:48 ` Gregory Maxwell
2015-02-03 0:44 ` Pieter Wuille
2015-02-03 2:21 ` Gregory Maxwell
2015-02-03 12:00 ` Wladimir
2015-02-03 14:30 ` Alex Morcos
2015-02-03 18:15 ` Pieter Wuille
2015-02-03 18:19 ` Gavin Andresen
2015-02-03 19:22 ` Jeff Garzik
2015-02-03 23:38 ` Pieter Wuille
2015-01-22 22:41 ` Zooko Wilcox-OHearn
2015-01-25 16:57 ` Pieter Wuille [this message]
2015-01-26 5:14 ` Pieter Wuille
2015-01-26 18:35 ` Gregory Maxwell
2015-01-28 6:24 ` Wladimir
2015-02-06 21:38 ` Pieter Wuille
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='CAPg+sBi7OnV9Tm06VBoHv54opQy3RJHzmyD+imcaUq8sEw=6qQ@mail.gmail.com' \
--to=pieter.wuille@gmail$(echo .)com \
--cc=bitcoin-development@lists$(echo .)sourceforge.net \
--cc=zooko@leastauthority$(echo .)com \
/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