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

Add example for using in production #1239

Merged
merged 2 commits into from
Dec 31, 2018
Merged

Conversation

ak1103dev
Copy link
Contributor

No description provided.

@dcousens
Copy link
Contributor

dcousens commented Oct 6, 2018

Maybe we can force rng to throw if it isn't run within a tape or mocha env?

@ak1103dev
Copy link
Contributor Author

ak1103dev commented Oct 6, 2018

The address.js is test script. And it is example for using the library. I mean remove "rng" function for using it in real code.

@junderw
Copy link
Member

junderw commented Oct 9, 2018

@dcousens I agree.

Adding more comments probably won't help a person who copy pastes without understanding what's going on.

Adding logic to prevent rng setting would be good.

IMO, this should be something like:
(Though not sure how we'd check if running in test. Dumb idea below)

} while (!ecc.isPrivate(d))

if (typeof global.it !== 'function' &&
    d.equals(Buffer.from('zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz'))) {
  throw new Error('DO NOT COPY PASTE THE EXAMPLE RNG FROM TESTS!!!')
}

return fromPrivateKey(d, options)

@dcousens
Copy link
Contributor

dcousens commented Oct 9, 2018

We put it in the rng provided in the tests, not ECPair.
PR accepted 👍

@dcousens dcousens closed this Oct 9, 2018
@dcousens dcousens reopened this Oct 9, 2018
@rlaace423
Copy link
Contributor

Finding default value of rng attribute doesn't take much time, but I used my time for searching it. It will be nice if there's an comments in testcode like this PR.

In case someone copy pastes, instead of getting a dangerous key, they will get a random key, and a console error.
@junderw junderw merged commit 1d73823 into bitcoinjs:master Dec 31, 2018
@dcousens
Copy link
Contributor

dcousens commented Dec 31, 2018

Why not throw, don't encourage copy pasting

@junderw
Copy link
Member

junderw commented Dec 31, 2018

Because I have seen copy-pasters in action, and they will:

  1. Notice the throw was from code they pasted.
  2. See a simple if that just throws the error.
  3. "Hmm, what happens if I delete this if statement 3 lines.
  4. "Yay it worked!"

That said, maybe there's a better way...

But sitting around thinking of the ways someone can shoot themselves in the foot seems less productive then making sure the gun they use only shoots nerf foam darts...

I'll make another PR with another suggestion that:

  1. Will throw if 100% copy-pasted
  2. Will be hard to tease apart and reason through. Making it hard to remove the throw.
  3. If the throw is somehow removed, randombytes will be used.

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.

4 participants