summaryrefslogtreecommitdiff
path: root/d9/1c65856fbfe6d1d36b9d879dcdec02a44647f3
blob: 2a9dc01cf529cbf6f6bb92f922a08c52dd8abc9d (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
Received: from sog-mx-1.v43.ch3.sourceforge.com ([172.29.43.191]
	helo=mx.sourceforge.net)
	by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76)
	(envelope-from <zooko@leastauthority.com>) id 1YEQrx-0002a4-Jo
	for bitcoin-development@lists.sourceforge.net;
	Thu, 22 Jan 2015 23:08:57 +0000
X-ACL-Warn: 
Received: from mail-pa0-f48.google.com ([209.85.220.48])
	by sog-mx-1.v43.ch3.sourceforge.com with esmtps (TLSv1:RC4-SHA:128)
	(Exim 4.76) id 1YEQrw-00022h-Fg
	for bitcoin-development@lists.sourceforge.net;
	Thu, 22 Jan 2015 23:08:57 +0000
Received: by mail-pa0-f48.google.com with SMTP id ey11so4744058pad.7
	for <bitcoin-development@lists.sourceforge.net>;
	Thu, 22 Jan 2015 15:08:51 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
	d=1e100.net; s=20130820;
	h=x-gm-message-state:mime-version:in-reply-to:references:date
	:message-id:subject:from:to:cc:content-type;
	bh=M/8Jr0xTN0nNFE18ZqAZQdbYkqWvRVTbasEk0v2e0YY=;
	b=S1uezsYwIx8G/Q/oXn6PLmr4i1udO0UIFoXPg7Oxg72x5jTbGMeNUNS6MM4A8s94vm
	aG/9jsjeZqQYgwjAJ7UxQE7Yr5BPwocYpQQBnE0OeiH06Y4ZKc3iZKintwWlwvdMokbU
	JErcYm6CK3JkF2vZf3iIjyTrxlxl1yyi/1H1SwSrBa8o9dgTz6XAt6Mbc1f30tPMREfi
	Ljm9UWjxWRcYTNgu1N1PATLHeMf2n4PRU9DsMHUseFBaJ+7bwgYg2NLH/lK9L5L+w55o
	zE02LxtXh870U8ptnqjUqxuy7pHbLSt7Kr75Gc/YyIPHa2YDI9ERFDoUpGE/a2VmFZtj
	ryRw==
X-Gm-Message-State: ALoCoQnnZY2DvKnms3vVhplyjsKWm2oAdZzHH/OxTIVsb49Jr8q3e3UdNxiu+LFgMEZKSEic3F1l
MIME-Version: 1.0
X-Received: by 10.70.98.200 with SMTP id ek8mr5732341pdb.163.1421966507370;
	Thu, 22 Jan 2015 14:41:47 -0800 (PST)
Received: by 10.70.51.130 with HTTP; Thu, 22 Jan 2015 14:41:47 -0800 (PST)
In-Reply-To: <CAPg+sBhk7F2OHT64i2LNSjv8DR5tD3RJkLJGzPGZW8OPQTCjQw@mail.gmail.com>
References: <CAPg+sBhk7F2OHT64i2LNSjv8DR5tD3RJkLJGzPGZW8OPQTCjQw@mail.gmail.com>
Date: Thu, 22 Jan 2015 22:41:47 +0000
Message-ID: <CAM_a8Jzt=Q8m=-7wur1UdscqQRQmg+TBFNtneUtRH3nh+TOTDQ@mail.gmail.com>
From: Zooko Wilcox-OHearn <zooko@leastauthority.com>
To: Pieter Wuille <pieter.wuille@gmail.com>
Content-Type: text/plain; charset=UTF-8
X-Spam-Score: 0.0 (/)
X-Spam-Report: Spam Filtering performed by mx.sourceforge.net.
	See http://spamassassin.org/tag/ for more details.
X-Headers-End: 1YEQrw-00022h-Fg
Cc: Bitcoin Dev <bitcoin-development@lists.sourceforge.net>
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: <bitcoin-development.lists.sourceforge.net>
List-Unsubscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
	<mailto:bitcoin-development-request@lists.sourceforge.net?subject=unsubscribe>
List-Archive: <http://sourceforge.net/mailarchive/forum.php?forum_name=bitcoin-development>
List-Post: <mailto:bitcoin-development@lists.sourceforge.net>
List-Help: <mailto:bitcoin-development-request@lists.sourceforge.net?subject=help>
List-Subscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
	<mailto:bitcoin-development-request@lists.sourceforge.net?subject=subscribe>
X-List-Received-Date: Thu, 22 Jan 2015 23:08:57 -0000

.Hi there. Thank you for your work on this.

I've looked over https://gist.github.com/sipa/5d12c343746dad376c80 and
https://github.com/sipa/bitcoin/commit/bipstrictder . I didn't
actually audit the included reference implementation of
IsValidSignatureEncoding(), and I didn't check whether the test
vectors in https://github.com/sipa/bitcoin/commit/f94e806f8bfa007a3de4b45fa3c9860f2747e427
exercise all of the branches that are changed by this patch.

I have the following comments:

* It seems like a good idea to do this.

* I don't see any problem with using the upgrade mechanism from BIP 34
for this. It's cool! I'm happy that such a mechanism seems to work in
practice.

* 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
.

* 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?

* 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.

* 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.

* missing comment:
https://github.com/sipa/bitcoin/commit/e186f6a80161f9fa45fbced82ab1d22f081b942c#commitcomment-9406643

Okay, that's all I've got. Hope it helps! Thanks again for your good work!

Regards,

Zooko