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

deps: modernization and upgrades #584

Merged
merged 21 commits into from
Jun 30, 2021
Merged

deps: modernization and upgrades #584

merged 21 commits into from
Jun 30, 2021

Conversation

tomholford
Copy link
Collaborator

@tomholford tomholford commented Jun 28, 2021

Context

The JS ecosystem moves quickly 🏎️ This PR brings Bridge up-to-date in several crypto libs, and removes unused development libs (since replaced by latest version of react-scripts). It also seems to resolve the issue we were having with unreliable builds.

Changes

  • upgrade: keccak, keythereum, secp256k1, truffle, web3
  • remove: react-hot-loader, @hot-loader/react-dom
  • converted a handful of files to Typescript and added typings
  • reorganized some crypto utils and constants from the Wallet.ts file
  • update README with correct Node version

Testing

Since there are no automated tests (yet!), we manually tested these changes by following these steps:

  1. rm -rf node_modules/ - first, remove existing deps
  2. npm install - reinstall JS packages and rebuild native extensions
  3. npm run build - generate production-optimized build
  4. npm run pilot - confirm that local development environment starts successfully

Once confirming the app was building and loading successfully, we then tested the following flows:

  • Mnemonic Login
  • Galaxy Creation
  • Star Creation
  • Planet Creation
  • Invite Acceptance
  • Passport Download
  • New Master Ticket (from Passport) Login

Fixes #410

Copy link
Member

@Fang- Fang- left a comment

Choose a reason for hiding this comment

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

Nice! Tested this locally, seems to work fine for all the broad/important cases. Some questions and comments below, and out of band.

DEVELOPMENT.md Outdated Show resolved Hide resolved
DEVELOPMENT.md Outdated Show resolved Hide resolved
src/lib/constants.ts Show resolved Hide resolved
src/lib/flowCommand.ts Outdated Show resolved Hide resolved
src/lib/utils/crypto.ts Outdated Show resolved Hide resolved
src/store/lib/useControlledPointsStore.js Outdated Show resolved Hide resolved
src/lib/utils/crypto.ts Outdated Show resolved Hide resolved
src/types/bridge-wallet.type.ts Outdated Show resolved Hide resolved
@tomholford tomholford marked this pull request as ready for review June 29, 2021 05:59
@tomholford tomholford requested a review from Fang- June 29, 2021 05:59
tomholford and others added 17 commits June 28, 2021 23:24
- upgrade: keccak, keythereum, secp256k1, truffle, web3
- remove: react-hot-loader, @hot-loader/react-dom
- update README with correct Node version
included changes:
- converted some files to Typescript and added typings to aid debugging
- reorganized some crypto utilities and constants
- added our first test! ... currently being skipped for now (:
Use sha256 instead of keccak256 to ensure 32-byte output for ecdsaSign
- update Development doc
- renamings
- remove dead code
- update Testing note per fang's feedback
Add context to error message logging

Co-authored-by: fang <github@fang.io>
@Fang- Fang- linked an issue Jun 30, 2021 that may be closed by this pull request
Copy link
Member

@Fang- Fang- left a comment

Choose a reason for hiding this comment

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

As discussed out of band, this now works just fine across flows and devices, as far as I can tell. Great work here! Can't wait to finally push out minimized builds again. (:

@Fang- Fang- added the code qol label Jun 30, 2021
@tomholford tomholford merged commit 81070fa into master Jun 30, 2021
@tomholford tomholford deleted the deps/upgrade branch June 30, 2021 22:49
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.

Very slow to load in incognito mode Should update web3 (toWei errors in minified builds only)
2 participants