Skip to content

Conversation

@karelbilek
Copy link
Contributor

I have added some checks on hardening indexes.

You don't want to accidentally put in twice hardened index, or a negative number to harden (which produces non-hardened index), etc.

if (index >= highestIndex) {
throw new Error('Could not derive with too high index')
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@Runn1ng it might be easiest to make a custom typeforce type for this :)

See types.js in src/ (of this repo) for some examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to say I don't understand typeforce that much, but it seems it's actually pretty easy to do that in there. How do you want to name the types... UnsignedInt31 and UnsignedInt32? :) (can typeforce right now to check if the number is integer by the way?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also would you want these types to be added into typeforce source directly, or rather somewhere in bitcoin.js?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...oh, it seems you actually do have the types already in bitcoinjs (well not the 31 bit one, but that can be added). OK, will use that instead

As a consequence, it will not allow accidentally double-hardened indexes.

It also won't allow strings or forgotten parameters.
@karelbilek
Copy link
Contributor Author

yep you were right, it's easier with typeforce types

@karelbilek karelbilek changed the title Adding some checks on hardening indexes Adding some checks on deriving indexes Feb 6, 2016
dcousens added a commit that referenced this pull request Feb 7, 2016
Adding some checks on deriving indexes
@dcousens dcousens merged commit b3b2397 into bitcoinjs:master Feb 7, 2016
@dcousens
Copy link
Contributor

dcousens commented Feb 7, 2016

Nice @Runn1ng 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants