Add Groestlcoin with Tests #353
Conversation
| @@ -488,6 +488,9 @@ | |||
| function calcBip32RootKeyFromSeed(phrase, passphrase) { | |||
| seed = mnemonic.toSeed(phrase, passphrase); | |||
| bip32RootKey = bitcoinjs.bitcoin.HDNode.fromSeedHex(seed, network); | |||
| if(isGRS()) | |||
| bip32RootKey = groestlcoinjs.HDNode.fromSeedHex(seed, network); | |||
|
|
|||
| } | |||
|
|
|||
| function calcBip32RootKeyFromBase58(rootKeyBase58) { | |||
There was a problem hiding this comment.
This method does get messy with all the exception handling. We could make a separate method that handles only Groestlcoin and then call it if isGRS()==true
| } | ||
| catch (e) { | ||
| bip32RootKey = groestlcoinjs.HDNode.fromBase58(rootKeyBase58, network); | ||
| } | ||
| } | ||
|
|
||
| function calcBip32ExtendedKey(path) { |
There was a problem hiding this comment.
This method does get messy with all the exception handling. We could make a separate method that handles only Groestlcoin and then call it if isGRS()==true
There was a problem hiding this comment.
Yes I think it makes more sense to use if isGRS than nesting try/catch. if is also easier and cleaner to extend for other coins that may find the same situation in the future.
There was a problem hiding this comment.
This change was made.
|
Thanks, this is great and I appreciate the work put in to making this possible. Just mainly the small issue of style regarding |
|
@iancoleman Thank you for the feedback, I will make the changes, test and modify this pull request soon. |
HashEngineering
force-pushed
the
add-groestlcoin
branch
from
fef7ff2
to
76120cb
Compare
|
The changes have been submitted regarding the using |
|
I want to raise an issue with https://github.com/Groestlcoin/bip38grs But they do not seem to allow issues, nor does https://github.com/Groestlcoin/groestlcoinjs-lib Can you advise how to get changes into their code? |
|
Both of those repos have issues activated now. |
Purpose
This PR is to add Groestlcoin support for mainnet and testnet. Groestlcoin differs from most other Bitcoin derived coins in that it uses different hash functions for blocks, transactions and addresses.
History
We made a query about adding coins that do not use SHA256D for the checksum of addresses with #191
Methodology
The goal was to add support for Groestlcoin with the minimal amount of changes. Our first approach was to create a javascript module that would take an address that followed the bitcoin rules and convert it to an andress that followed Groestlcoin rules. This method would require many modifications to index.js for every place a base58 string was displayed (addressed, dumped private keys, extended public and private keys).
Our second approach, which is used in this PR, uses the groestlcoin equivalents of bitcoinjs-lib and bitcoinjs-bip38. When ECPair or HDNodes are created using bitcoinjs, we instead use groestlcoinjs when Groestlcoin related networks are selected by the user. This ensures that the addresses, dumped private keys, extended private and public keys are rendered using Groestlcoin Rules. Likewise in the case that the user chooses BIP38, then groestlcoinjs-bip38 is used instead of bitcoinjs-bip38 so that the encrypted private key is rendered correctly for Groestlcoin (mainnet or testnet).
Testing:
Some audit testing was done to compare the results of https://iancoleman.io/bip39/ and this PR for Bitcoin and Dash to ensure that the addresses and keys generated matched this PR. Additionally, for Groestlcoin, we used, https://groestlcoin.org/bip39/, a site forked from this repo was used to ensure that this PR generates matching output. https://groestlcoin.org/bip39/ is used as a reference for Groestlcoin software development to double check BIP32, BIP44, BIP49 and BIP84 HD path derivation. Three tests were also added to tests.js.
I will add some comments in a few places in the code.