Conversation
|
Warning MetaMask internal reviewing guidelines:
Ignoring alerts on:
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…-keys, share-transfer, seed-phrase from BN/elliptic to bigint/noble
| this.encPubKey = Buffer.from(testEncPubKey.data); | ||
| if (encPubKey instanceof Uint8Array) { | ||
| this.encPubKey = encPubKey; | ||
| } else if ((encPubKey as BufferObj).type === "Buffer") { |
There was a problem hiding this comment.
not sure here, it depends on our serialization
…into feat/stack-upgrades
|
@cursor can you do a review? (don't commit anything) |
|
Here is my code review of the Code Review:
|
ieow
left a comment
There was a problem hiding this comment.
lgtm
But it would be great to add backward compability tests
matthiasgeihs
left a comment
There was a problem hiding this comment.
I've skimmed over it and looks good at first glance.
For a thorough review I'd probably need half a day to a day which is hard for me to allocate at the moment, and not sure if it's needed. I let the Cursor bot double check the PR. It found some potential issues, which I think are worth looking into. Nothing major though, I believe. (1. should be easy to fix, 2. is a false positive, I believe.)
There was a problem hiding this comment.
I assume removal of TSS package is intentional because we dropped support for it?


Jira Link
https://consensyssoftware.atlassian.net/browse/W3APD-5328
Description
Migrate the entire tkey SDK from legacy cryptography libraries (
bn.js,elliptic,Buffer,@toruslabs/tweetnacl-js) to modern alternatives (bigint,@noble/curves,Uint8Array).Key changes:
bn.js(BN) with nativebigintacross all packages (common-types, core, service-providers, modules)ellipticsecp256k1 operations with@noble/curves/secp256k1@toruslabs/tweetnacl-jsED25519 operations with@noble/curves/ed25519Bufferusage withUint8ArraythroughoutbigIntReplacerutility for JSON serialization ofbigintvalues (sinceJSON.stringifycannot serializebigintnatively)ThresholdKey.fromJSON()deserialization to properly reconstructShareStore,AuthMetadata, andPublicShareinstances from JSON@tkey/tsspackage (to be migrated separately)Packages affected: common-types, core, default, private-keys, security-questions, seed-phrase, service-provider-base, service-provider-torus, service-provider-sfa, share-serialization, share-transfer, storage-layer-torus, chrome-storage, web-storage
How has this been tested?
bigint/@noble/curvesAPIsnpm run buildpasses for all 14 packagesnpm run test -w packages/common-types— all passingnpm run test -w packages/core— all passingnpm run test -w packages/share-serialization— all passingnpm run test -w packages/default— 264 passing, 0 failingScreenshots (if appropriate)
N/A — no UI changes
Types of changes
Checklist