New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implemented (lax) signature parsing in Bitcoin-S #1446
Conversation
…rsion to most recent
…now! Deleted C-like version in place of this one after property based tests showed them to be equivalent
650aaee
to
2ea9ffc
Compare
@@ -235,6 +205,108 @@ sealed abstract class DERSignatureUtil { | |||
require(DERSignatureUtil.isLowS(sigLowS)) | |||
sigLowS | |||
} | |||
|
|||
/** Scala implementation of https://github.com/bitcoin/bitcoin/blob/master/src/pubkey.cpp#L16-L165 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: instead of using master, use the current HEAD commit so if there are changes it doesn't make this comment outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any test cases from bitcoin core that be ported off to specifically test this stuff? It would be nice to have a few sanity unit tests at least that can directly test this
* Implemented parseDERLax signature parsing and bumped bouncy castle version to most recent * implemented using iterators * Factored out common functionality and added comments, it is readable now! Deleted C-like version in place of this one after property based tests showed them to be equivalent * Made compatible with scala 2.12 * Make compatible with scala 2.11 * Added tests for lax DER signatures
This allows us to finally update bouncy castle!
The current version is literally just me mimicking (
var
s,Array
s and all) the code here line for line: https://github.com/bitcoin/bitcoin/blob/master/src/pubkey.cpp#L16-L165I will attempt to clean this up and scala-fy it tomorrow, though I've already attempted it once and tbh I don't fully understand the business with the first bit of length bytes (I know it has to do with leading zeroes appended to r and s values, but need to get a better grasp on this), which makes it quite hard to re-write.
That said, at the time I did not actually have a working integration and now that I do it should be easier to write a new function that (should do) does the same thing and compare.
I still haven't been able to access the Gemini fork and see what is going on over there but hopefully this can help on their end!
EDIT:
I have now implemented it in a clean and readable way that is still efficient using iterators!