Received-SPF: pass (sog-mx-1.v43.ch3.sourceforge.com: domain of gmail.com
	designates 209.85.223.170 as permitted sender)
	client-ip=209.85.223.170; envelope-from=pieter.wuille@gmail.com;
	helo=mail-ie0-f170.google.com; 
MIME-Version: 1.0
In-Reply-To: <CAM_a8Jzt=Q8m=-7wur1UdscqQRQmg+TBFNtneUtRH3nh+TOTDQ@mail.gmail.com>
References: <CAPg+sBhk7F2OHT64i2LNSjv8DR5tD3RJkLJGzPGZW8OPQTCjQw@mail.gmail.com>
	<CAM_a8Jzt=Q8m=-7wur1UdscqQRQmg+TBFNtneUtRH3nh+TOTDQ@mail.gmail.com>
Date: Sun, 25 Jan 2015 12:57:23 -0400
Message-ID: <CAPg+sBi7OnV9Tm06VBoHv54opQy3RJHzmyD+imcaUq8sEw=6qQ@mail.gmail.com>
From: Pieter Wuille <pieter.wuille@gmail.com>
To: Zooko Wilcox-OHearn <zooko@leastauthority.com>
Content-Type: text/plain; charset=ISO-8859-1
Cc: Bitcoin Dev <bitcoin-development@lists.sourceforge.net>
Subject: Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
Precedence: list

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