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