-
Notifications
You must be signed in to change notification settings - Fork 13
Add native crypto lib #3314
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
base: main
Are you sure you want to change the base?
Add native crypto lib #3314
Conversation
# Conflicts: # yarn.lock fix set problems stash work # Conflicts: # yarn.lock stash work # Conflicts: # packages/core-mobile/index.js performance tracking # Conflicts: # yarn.lock stash work make it more solid fix patch for ios update sentry fix patches
3a99462 to
2f2e959
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you remove these wallet connect patches? they should already be the other wallet connect pr.
packages/core-mobile/metro.config.js
Outdated
| stream: require.resolve('./node_modules/stream-browserify'), | ||
| '@noble/hashes': require.resolve('./node_modules/@noble/hashes') | ||
| '@noble/hashes': require.resolve('./node_modules/@noble/hashes'), | ||
| "react-native-nitro-avalabs-crypto": nitroCryptoPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need to declare react-native-nitro-avalabs-crypto here. it should already be picked up automatically by metro since it belongs to this workspace.
| nodeModulesPaths: [ | ||
| path.resolve(workspaceRoot, 'node_modules'), | ||
| path.resolve(projectRoot, 'node_modules'), | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this?
b8c5e23 to
24d1dd5
Compare
24d1dd5 to
bda4068
Compare
# Conflicts: # packages/core-mobile/android/app/src/main/AndroidManifest.xml # yarn.lock
0688ea9 to
865584d
Compare
8b63168 to
ce5e3bc
Compare
| "clean": "rm -rf android/build node_modules/**/android/build lib", | ||
| "lint": "eslint \"**/*.{js,ts,tsx}\" --fix", | ||
| "lint-ci": "eslint \"**/*.{js,ts,tsx}\" -f @jamesacarr/github-actions", | ||
| "typescript": "tsc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "typescript": "tsc", | |
| "tsc": "tsc", |
since we run yarn tsc for all packages in this repo
| "lint": "eslint \"**/*.{js,ts,tsx}\" --fix", | ||
| "lint-ci": "eslint \"**/*.{js,ts,tsx}\" -f @jamesacarr/github-actions", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "lint": "eslint \"**/*.{js,ts,tsx}\" --fix", | |
| "lint-ci": "eslint \"**/*.{js,ts,tsx}\" -f @jamesacarr/github-actions", | |
| "lint": "eslint .", |
let's keep only 1 lint here. no need for the --fix also since we already do that when committing.
| "README.md" | ||
| ], | ||
| "scripts": { | ||
| "postinstall": "tsc || exit 0;", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "postinstall": "tsc || exit 0;", |
I don't think we need to run typescript post install
| "eslintConfig": { | ||
| "root": true, | ||
| "extends": [ | ||
| "@react-native", | ||
| "prettier" | ||
| ], | ||
| "plugins": [ | ||
| "prettier" | ||
| ], | ||
| "rules": { | ||
| "prettier/prettier": [ | ||
| "warn", | ||
| { | ||
| "quoteProps": "consistent", | ||
| "singleQuote": true, | ||
| "tabWidth": 2, | ||
| "trailingComma": "es5", | ||
| "useTabs": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already have our own lint setup actually so there is no need to specify eslint rules here. could you follow the same setup as in the packages in this repo? an example is the packages/k2-alpine/.eslintrc.js
| @@ -0,0 +1 @@ | |||
| export * from './Crypto'; No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are 2 index files. looks like one can be removed
| if (len & 0x80) { | ||
| const n = len & 0x7f | ||
| if (n === 0 || n > 2) throw new TypeError('Invalid DER: long len too big') | ||
| if (p + n > sig.length) throw new TypeError('Invalid DER: length overflow') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add tests for these functions?
a9e1f6b to
1e11650
Compare
Description
This pull request introduces native Nitro crypto integration for React Native, updates cryptography dependencies to use patched versions with native support, and improves monorepo compatibility and configuration for the mobile app. The main changes focus on enabling fast, native-backed cryptographic operations (ECDSA, Schnorr) in the mobile environment, updating dependency patching for
@noble/curvesand@bitcoinerlab/secp256k1, and ensuring the Metro bundler is aware of the new native crypto workspace.Native crypto integration and dependency patching:
@bitcoinerlab/secp256k1and@noble/curvesvia custom patches, allowing operations likesign,verify,signSchnorr, andverifySchnorrto use thereact-native-nitro-avalabs-cryptomodule when available, with fallback to JS implementations. (.yarn/patches/@bitcoinerlab-secp256k1-npm-1.2.0-1098d4b329.patch[1].yarn/patches/@noble-curves-npm-1.9.7-2b9efc8ab4.patch[2]package.jsonto patch all references to@noble/curvesand@bitcoinerlab/secp256k1to ensure the mobile app uses the Nitro-enabled versions. (package.jsonpackage.jsonL55-R73)Monorepo and Metro configuration:
packages/core-mobile/metro.config.jsto add the Nitro crypto workspace to Metro's watch folders and resolver, ensuring correct module resolution for the new native crypto module. (packages/core-mobile/metro.config.js[1] [2]react-native-nitro-avalabs-cryptoas a workspace dependency inpackages/core-mobile/package.json. (packages/core-mobile/package.jsonpackages/core-mobile/package.jsonR202-R203)Platform compatibility and workflow improvements:
package.jsonfiles, improving compatibility across platforms. (.yarn/patches/@bitcoinerlab-secp256k1-npm-1.2.0-1098d4b329.patch[1].yarn/patches/@noble-curves-npm-1.9.7-2b9efc8ab4.patch[2].yarn/patches/jail-monkey-npm-2.8.0-77e4d06b40.patch[3]autoconf,automake,libtool) for native module compilation. (bitrise.ymlbitrise.ymlR218-R220)Minor improvements and fixes:
package.json. (package.jsonpackage.jsonR16).yarn/patches/jail-monkey-npm-2.8.0-77e4d06b40.patch.yarn/patches/jail-monkey-npm-2.8.0-77e4d06b40.patchL19-R31)These changes collectively enable high-performance native cryptography for React Native, ensure correct dependency resolution across the monorepo, and improve platform compatibility for mobile builds.
Testing
Dev Testing (if applicable)
QA Testing (if applicable)
Checklist
Please check all that apply (if applicable)