From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from sog-mx-1.v43.ch3.sourceforge.com ([172.29.43.191] helo=mx.sourceforge.net) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1YFQV6-0002L1-Pg for bitcoin-development@lists.sourceforge.net; Sun, 25 Jan 2015 16:57:28 +0000 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; Received: from mail-ie0-f170.google.com ([209.85.223.170]) by sog-mx-1.v43.ch3.sourceforge.com with esmtps (TLSv1:RC4-SHA:128) (Exim 4.76) id 1YFQV5-0003o0-Hw for bitcoin-development@lists.sourceforge.net; Sun, 25 Jan 2015 16:57:28 +0000 Received: by mail-ie0-f170.google.com with SMTP id y20so5099643ier.1 for ; Sun, 25 Jan 2015 08:57:23 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.107.166.13 with SMTP id p13mr13864048ioe.61.1422205043163; Sun, 25 Jan 2015 08:57:23 -0800 (PST) Received: by 10.50.20.229 with HTTP; Sun, 25 Jan 2015 08:57:23 -0800 (PST) In-Reply-To: References: Date: Sun, 25 Jan 2015 12:57:23 -0400 Message-ID: From: Pieter Wuille To: Zooko Wilcox-OHearn Content-Type: text/plain; charset=ISO-8859-1 X-Spam-Score: -1.6 (-) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for sender-domain 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (pieter.wuille[at]gmail.com) -0.0 SPF_PASS SPF: sender matches SPF record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.0 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1YFQV5-0003o0-Hw Cc: Bitcoin Dev Subject: Re: [Bitcoin-development] [softfork proposal] Strict DER signatures X-BeenThere: bitcoin-development@lists.sourceforge.net X-Mailman-Version: 2.1.9 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 25 Jan 2015 16:57:28 -0000 On Thu, Jan 22, 2015 at 6:41 PM, Zooko Wilcox-OHearn 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