Skip to content

Add BIP32 bitcoinjs implementation - #310

Closed
dcousens wants to merge 5 commits into
bitcoin:masterfrom
dcousens:patch-4
Closed

Add BIP32 bitcoinjs implementation#310
dcousens wants to merge 5 commits into
bitcoin:masterfrom
dcousens:patch-4

Conversation

@dcousens

Copy link
Copy Markdown
Contributor

Also formats the list for readability

@dcousens dcousens changed the title Add bitcoinjs reference implementation Add BIP32 bitcoinjs reference implementation Jan 29, 2016
@dcousens dcousens changed the title Add BIP32 bitcoinjs reference implementation Add BIP32 bitcoinjs implementation Jan 29, 2016
@luke-jr luke-jr added the Proposed BIP modification PR by non-owner to update BIP content label Jan 29, 2016
@dcousens

Copy link
Copy Markdown
Contributor Author

ping @sipa, thoughts?

@sipa

sipa commented Jul 16, 2016

Copy link
Copy Markdown
Member

Per https://bitcoincore.org/en/meetings/2016/05/12/ please link to actual implementation code for BIP32, and not just applications that support it.

@dcousens

dcousens commented Jul 16, 2016

Copy link
Copy Markdown
Contributor Author

@sipa not sure what you mean? The link added was implementation code for BIP32?

This was the link added: https://github.com/bitcoinjs/bitcoinjs-lib/blob/v2.1.4/src/hdnode.js
Perhaps I'll rebase and isolate the formatting commit from the addition.

Jcorgan proposes to link to an URL and commit hash, to make sure the linked code reflects the implementation.

Is the commit hash preferred over the tagged version used above? If so, I'll change it.

@afk11

afk11 commented Jul 16, 2016

Copy link
Copy Markdown
Contributor

I haven't seen commit hashes included much before, just a link to the implementation's file or repo.

Re sipa's comment, probably refers to mentioning blockchain.info's use of the bitcoin-js implementation; that information is not typically included in BIP documents. 681c160

There is also another PR (#293) which reformats the BIP32 implementation list, perhaps coordinate so there's only one large update?

@dcousens

dcousens commented Jul 17, 2016

Copy link
Copy Markdown
Contributor Author

Re sipa's comment, probably refers to mentioning blockchain.info's use of the bitcoin-js implementation; that information is not typically included in BIP documents. 681c160

Right! I totally forgot that those commits were included into this.
I'll re-submit a new PR that performs the implementation reference.

If #293 is just a formatting PR, perfect. Otherwise, I'll submit that independently.

@dcousens
dcousens deleted the patch-4 branch July 17, 2016 04:42
luke-jr pushed a commit to luke-jr/bips that referenced this pull request Jan 20, 2018
(1) addressed review items from @rustyrussell and @toadlyBroodle ; and (2) added table of contents courtesy of @bcongdon in bitcoin#310
luke-jr pushed a commit to luke-jr/bips that referenced this pull request Jan 20, 2018
More line editing and clarifications, also includes typo-fix from @dimitris-t in bitcoin#306 and table of contents from @bcongdon in bitcoin#310.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Proposed BIP modification PR by non-owner to update BIP content

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants