Skip to content

Conversation

@dcousens
Copy link
Contributor

Only breaking change in terms of exact exception messages.

The messages are now consistent in the https://github.com/bitcoinjs/bip66 module, and adhering to SEMVER.

The fact I can now test for all of them using:

/Expected DER (integer|sequence)|(R|S) value (excessively padded|is negative)|(R|S|DER sequence) length is (zero|too short|too long|invalid)/

Is a huge bonus compared to the huge regex that was necessary prior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, huge performance improvement here. Added tests to cover it, overall script tests running in half the time, isolated benchmark gives >200% improvement.

In terms of validity, this is now on par with bitcoin-core: https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L67-L87

dcousens added a commit that referenced this pull request Aug 25, 2015
@dcousens dcousens merged commit ec1195b into master Aug 25, 2015
@dcousens dcousens deleted the bip66 branch August 25, 2015 04:17
@dcousens
Copy link
Contributor Author

@weilu please post-ACK

@weilu
Copy link
Contributor

weilu commented Sep 5, 2015

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants