Skip to content

Conversation

@dcousens
Copy link
Contributor

@dcousens dcousens commented Jul 7, 2015

See pseudo code for BIP66 here, with ordering amendments.

The constant amendments are because we ignore the SIGHASH byte in this function, which is why the constants are 8, 72 for the min/max sizes, and 6 instead of 7 for the total length offset.

@dcousens
Copy link
Contributor Author

ping @weilu @jprichardson

@jprichardson
Copy link
Member

Since we discussed creating packages that represent BIPs, do you think that it makes sense to turn BIP66 functionality into a package?

@dcousens
Copy link
Contributor Author

@jprichardson that sounds like a great idea, however, lets not let it slow down this PR? We can extract it very easily after :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should extract all above validation into a function validateDER with reference to bip66?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will later, in fact I might even pull this out to a bip66 module, that way it could be super clear.

@weilu
Copy link
Contributor

weilu commented Jul 17, 2015

The implementation looks right to me compared to the reference code. Do we have test case for each one of them though?

@dcousens
Copy link
Contributor Author

@weilu we do have test cases for each.

dcousens added a commit that referenced this pull request Jul 19, 2015
@dcousens dcousens merged commit a8f36ba into master Jul 19, 2015
@dcousens dcousens deleted the bip66adherance branch July 19, 2015 23:20
@dcousens dcousens mentioned this pull request Jul 28, 2015
Closed
dcousens added a commit that referenced this pull request Sep 14, 2015
dcousens added a commit that referenced this pull request Sep 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants