doc: Add comments to better explaining the meaning of "recId" for pubkey recovery#3361
doc: Add comments to better explaining the meaning of "recId" for pubkey recovery#3361johnzweng wants to merge 1 commit into
Conversation
msgilligan
left a comment
msgilligan
left a comment
There was a problem hiding this comment.
Looks good! I have few minor suggestions, though.
| * Decompress a compressed public key (x co-ord and low-bit of y-coord). | ||
| * (if <code>yBit</code> is <code>true</code>, the odd y coordinte will be used, | ||
| * if it is <code>false</code> the even y coordinate will be used) | ||
| */ |
There was a problem hiding this comment.
Since this is a JavaDoc comment, you could use @param tags to document yBit and add xBN as well.
There was a problem hiding this comment.
Done: Added the parameters as @param tags, also changed the overall text (existing inconsistent wording, x co-ord vs. y-coord).
And also renamed the parameter name from xBN to xCoordinate. 🙂
There was a problem hiding this comment.
I think the parameter rename should have gone into a separate PR or commit. Reading your PR description, this PR is about comments, has no code changes.
There was a problem hiding this comment.
Oh, yes, you are totally right. I will remove it again from here.
There was a problem hiding this comment.
@schildbach Done ✅
Now this PR should only contain comment changes (again). 🙂
There was a problem hiding this comment.
Squashed the commits together, now it's one single commit, without code changes.
|
Thanks @msgilligan for looking over the PR. I changed your suggestions. 🙂 ✅ |
johnzweng
force-pushed
the
better_comments-for-recId
branch
from
cec14cc to
39a7524
Compare
|
Merged. Thanks for improving our documentation! Contributions like this are much appreciated. |
3 participants
Background
Some time ago I already merged this PR into bitcoinJ where I added the functionality of signing text messages with SegWit addresses.
What I missed back than, was proper documentation of the value
recIdin the signatures header byte. 🙁As the meaning of
recIdis not that obvious I tried to explain it with some added inline documentation.Description:
recIdin the header byte of Compact signatures (like used in the signature of bitcoin signed messages)