Smaller key size when testing #198
Conversation
ghost
assigned 
| const args = ['init', '-b', initOpts.keysize || 2048] | ||
| let keysize = initOpts.keysize | ||
| if (!keysize) { | ||
| if (typeof global.it === 'function') { |
There was a problem hiding this comment.
@richardschneider @diasdavid I'm really not sure we should do this.
- Why do we have to change this for test only?
- Can we be sure that this is not going to hide some subtle issues by running with different defaults in tests vs prod?
There was a problem hiding this comment.
Why do we have to change this for test only?
Because small key sizes are unsafe. If someone is using this interface to create a 'real' peer then proper key sizes should be used. This is just hack to get tests to run faster.
libp2p-keychain and libp2p-crypto tests use real key sizes.
Can we be sure that this is not going to hide some subtle issues by running with different defaults in tests vs prod?
I did think about this and came to the conclusion that because none of the ipfs an ipfs-api tests specify a key size it should be safe. But running the tests against this PR is the only sure way.
There was a problem hiding this comment.
also, checking for global.it ties us to mocha and feels a bit iffy...
There was a problem hiding this comment.
@dryajov Open to any suggestions. But the current tests are in mocha. Worse case, someone writes a new a test in another framework and all they get is bigger key sizes.
There was a problem hiding this comment.
@richardschneider can we use an ENV variable to set the keysize, that way, in test we can just set it, rather than checking for global.it?
i.e.
IPFS_KEYSIZE=1024 npm test
There was a problem hiding this comment.
that's true, maybe this is a change that needs to happen in aegir, where we set an ENV variable signaling that we're in test - global.it, IMO would just end up being too brittle.
i.e.
process.env['AEGIR_TEST']
There was a problem hiding this comment.
Thinking about it some more, I really think we should just have IPFS_KEYSIZE_[JS|GO|PROC]. This carries the most changes, but it's also the most consistent and gives us more flexibility down the road.
There was a problem hiding this comment.
The reason I say this, is because ipfsd-ctl is not used only in tests, and there might be other use cases and applications that can benefit from this functionality as well.
There was a problem hiding this comment.
An other app, could simply set the keysize. I expect an other app, would init an repo only once.
Our tests are an exception, as there are many of creating many repos..
There was a problem hiding this comment.
I agree. Let's not peg on globals that are used by specific test frameworks.
If another batch of tests wants to drop down the keysize, then it should do that explicitly rather than having this module making the decision.
| const args = ['init', '-b', initOpts.keysize || 2048] | ||
| let keysize = initOpts.keysize | ||
| if (!keysize) { | ||
| if (typeof global.it === 'function') { |
There was a problem hiding this comment.
I agree. Let's not peg on globals that are used by specific test frameworks.
If another batch of tests wants to drop down the keysize, then it should do that explicitly rather than having this module making the decision.
| let keysize = initOpts.keysize | ||
| if (!keysize) { | ||
| if (typeof global.it === 'function') { | ||
| keysize = this.opts.type === 'go' ? 1024 : 512 |
There was a problem hiding this comment.
@richardschneider why are we setting different sizes for go and js, could you please remind me?
There was a problem hiding this comment.
RTFM the top comment on this PR
|
closing in favor of #203 |
A 2048 bit RSA key can take up to 20 seconds to generate. When testing, a smaller key size can be used.
For
js-ipfsa 512 bit key size is used. Forgo-ipfs, because it enforces minimum key size, 1024 bits is used.