On Thu, Jan 22, 2015 at 6:41 PM, Zooko Wilcox-OHearn <zo...@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 ------------------------------------------------------------------------------ New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet _______________________________________________ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
