Skip to content
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

Generate extended public keys from an extended key #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cosmin-harangus
Copy link

Given an extended public/private key generate the corresponding extended public keys in "legacy", "p2sh-segwit" or "bech32" address types (if supported)
These can then be used by hd-addr to check the generated addresses for funds as mentioned in issue #28 .

@dan-da
Copy link
Owner

dan-da commented Oct 4, 2019

This patch doesn't seem to work.

Firstly, the get-extended param is not read from the command-line. maybe you forgot to commit changes in Util.php?

Even after I added that, I get an error that $key is undefined on line 54 of hd-wallet-derive.php. After fixing that, I got another error.

If you fix the PR so that I can test it out, I will evaluate further. thanks.

@cosmin-harangus
Copy link
Author

You're right, I missed a few things there... Too eager to push I guess. I will fix it in the new few days and update the PR. These changes were taken from another project where I needed to run this package as a JSON server instead of through the CLI.

@dan-da
Copy link
Owner

dan-da commented Oct 14, 2019

ok, yeah I'll take a look once you update...

@cosmin-harangus
Copy link
Author

I've updated the PR with the fixes and tested the solution. Please confirm and let me know if there are other other changes that I should do. I tried to used the same naming conventions in order to match the current usage style.


// store results here
$extkeys = array();

Copy link
Contributor

@melaxon melaxon Oct 15, 2019

Choose a reason for hiding this comment

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

May I suggest you arrange these pieces of code as foreach somehow like this:

    public function getExtendedPublicKeys($key) {
        $params = $this->get_params();
        $coin = $params['coin'];
        list($symbol) = explode('-', $coin);
        $networkCoinFactory = new NetworkCoinFactory();
        $network = $networkCoinFactory->getNetworkCoinInstance($coin);
        Bitcoin::setNetwork($network);
        // get initial key type for the coin
        $initial_key_type = $this->getKeyTypeFromCoinAndKey($coin, $key);
        $addrTypes = [
            'x' => 'legacy',
            'y' => 'p2sh-segwit',
            'z' => 'bech32'
        ];
        $types = ['x', 'y', 'z'];
        $extkeys = array();
        foreach ($types as $type) {
            $this->params['addr-type'] = $addrTypes[$type];
            $key_type = $type;
            $xyzPub = $type . 'pub';
            $master = $this->fromExtended($coin, $key, $network, $initial_key_type);
            if ( $this->networkSupportsKeyType($network, $key_type, $coin ) && method_exists($master, 'getPublicKey') ) {
                $extkeys[] = [ $xyzPub => $this->toExtendedKey($coin, $master->withoutPrivateKey(), $network, $key_type)];
            }
        }
        return $extkeys;

    }


I would also suggest you squash all the commits into a single one to keep nice clean history

Copy link
Owner

Choose a reason for hiding this comment

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

yes I concur with @melaxon's suggestions.

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.

3 participants