Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Replacement of node-scrypt by crypto scrypt #2642

Closed
wants to merge 1 commit into from
Closed

Replacement of node-scrypt by crypto scrypt #2642

wants to merge 1 commit into from

Conversation

Ugarz
Copy link

@Ugarz Ugarz commented Apr 3, 2019

Description

Hi,
The changes here are to replace the node-scrypt module by the node standard library crypto scrypt. I went accross some issues with the node-scrypt module, like others users. I first searched online and found it was coming from a deprecated version of scrypt. In the issue!2385 @Kaisle suggests to move to a standard library.
Here is my attempt.

Resources

Type of change

  • Bug fix
  • Enhancement

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no warnings.
  • I have updated or added types for all modules I've changed
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run test in the root folder with success and extended the tests if necessary.
  • I ran npm run build in the root folder and tested it in the browser and with node.
  • I ran npm run dtslint in the root folder and tested that all my types are correct
  • I have tested my code on an ethereum test network.

@nivida nivida added the Enhancement Includes improvements or optimizations label Apr 3, 2019
@Ugarz
Copy link
Author

Ugarz commented Apr 17, 2019

Hi,
I am currently trying to pass the tests in local environnement to merge my request.
I'm facing some errors on passing the tests, here is what i get :

lerna ERR! npm run test stderr:
FAIL tests/AbiCoderTest.js
  ● Test suite failed to run

    Cannot find module 'web3-utils' from 'AbiCoderTest.js'

      4 |
      5 | // Mocks
    > 6 | jest.mock('web3-utils');
        |      ^
      7 | jest.mock('ethers/utils/abi-coder');
      8 |
      9 | /**

      at Resolver.resolveModule (../../node_modules/jest/node_modules/jest-resolve/build/index.js:229:17)
      at Object.mock (tests/AbiCoderTest.js:6:6)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        8.407s

In the contributing section I did not found informations on how to make the test pass correctly. What am I missing ? I saw you are using lerna, but I did not used it before. Do i have to install packages in another way than npm install ?

In the pipeline I saw this error coming :

lerna ERR! npm run test --coverage stderr:
FAIL tests/src/models/AccountTest.js
  ● AccountTest › calls toV3Keystore and returns the expected object
    TypeError: Cannot read property 'slice' of undefined
      167 |         }
      168 | 
    > 169 |         const cipher = crypto.createCipheriv(options.cipher || 'aes-128-ctr', derivedKey.slice(0, 16), iv);
          |                                                                                          ^
      170 |         if (!cipher) {
      171 |             throw new Error('Unsupported cipher');
      172 |         }
      at Account.slice [as toV3Keystore] (src/models/Account.js:169:90)
      at Object.toV3Keystore (tests/src/models/AccountTest.js:368:45)
Jest: "global" coverage threshold for statements (80%) not met: 35.98%
Jest: "global" coverage threshold for lines (80%) not met: 36.65%
Jest: "global" coverage threshold for functions (80%) not met: 20%
Test Suites: 1 failed, 1 of 4 total
Tests:       1 failed, 14 passed, 15 total
Snapshots:   0 total
Time:        5.329s
Ran all test suites.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test: `jest`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

You can see the pipeline for node 11 here 😃

@nivida nivida closed this Jun 12, 2019
@nivida
Copy link
Contributor

nivida commented Jun 12, 2019

This PR got closed because of the reorganisation of the branches. Feel free to open a new PR for the 2.x branch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Enhancement Includes improvements or optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants