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

feat: unify import/export private key format #15415

Merged
merged 11 commits into from
Jan 30, 2024
Merged

feat: unify import/export private key format #15415

merged 11 commits into from
Jan 30, 2024

Conversation

joyqvq
Copy link
Contributor

@joyqvq joyqvq commented Dec 18, 2023

Description

SIP: sui-foundation/sips#15

CLI changes

sui keytool convert: this converts hex, base64 to bech32
sui keytool import: can only import as bech32
sui keytoo export: new, export as bech32

SDK changes

/**
 * This returns a Bech32 encoded string starting with `suiprivkey`,
 * encoding 33-byte `flag || bytes` for the given the 32-byte private
 * key and its signature scheme.
 */
export function encodeSuiPrivateKey(bytes: Uint8Array, scheme: SignatureScheme): string

/**
 * This returns an ParsedKeypair object based by validating the
 * 33-byte Bech32 encoded string starting with `suiprivkey`, and
 * parse out the signature scheme and the private key in bytes.
 */
 export function decodeSuiPrivateKey(value: string): ParsedKeypair

Wallet changes

Export UI supports Bech32 only.
Import UI supports both Hex and Bech32. Hex importing shows warning for deprecation for March 1, 2024.

image

Test Plan

  • unit test

  • CLI scenario

# import bech32 works 
target/debug/sui keytool import suiprivkey1q8typ5p96jtw3s3cqlnxulcg6me26cswjcpa8984z6k4ey4dwefcwlssmpg ed25519
Keys saved as Base64 with 33 bytes `flag || privkey` ($BASE64_STR). 
        To see Bech32 format encoding, use `sui keytool export --address $ADDR` where 
        $ADDR can be found with `sui keytool list`. Or use `sui keytool convert $BASE64_STR`.
╭─────────────────┬──────────────────────────────────────────────────────────────────────╮
│ alias           │                                                                      │
│ suiAddress      │  0x1b87a727f58830d9ba2bfe6ecdc8fb49aa96fa2a2bbe175e128bfee13f6895ff  │
│ publicBase64Key │  AQMSxGmd92VJuD30t5sAOVMO//UWqx85kbuOMDRrUCzWfw==                    │
│ keyScheme       │  secp256k1                                                           │
│ flag            │  1                                                                   │
│ peerId          │                                                                      │
╰─────────────────┴──────────────────────────────────────────────────────────────────────╯

# import with alias and bech32 works
 target/debug/sui keytool import suiprivkey1q8typ5p96jtw3s3cqlnxulcg6me26cswjcpa8984z6k4ey4dwefcwlssmpg --alias test-pr ed25519
Keys saved as Base64 with 33 bytes `flag || privkey` ($BASE64_STR). 
        To see Bech32 format encoding, use `sui keytool export --address $ADDR` where 
        $ADDR can be found with `sui keytool list`. Or use `sui keytool convert $BASE64_STR`.
╭─────────────────┬──────────────────────────────────────────────────────────────────────╮
│ alias           │                                                                      │
│ suiAddress      │  0x1b87a727f58830d9ba2bfe6ecdc8fb49aa96fa2a2bbe175e128bfee13f6895ff  │
│ publicBase64Key │  AQMSxGmd92VJuD30t5sAOVMO//UWqx85kbuOMDRrUCzWfw==                    │
│ keyScheme       │  secp256k1                                                           │
│ flag            │  1                                                                   │
│ peerId          │                                                                      │
╰─────────────────┴──────────────────────────────────────────────────────────────────────╯

# import hex does not work
target/debug/sui keytool import 0x1b87a727f58830d9ba2bfe6ecdc8fb49aa96fa2a2bbe175e128bfee13f6895ff ed25519
Sui Keystore and Sui Wallet no longer support importing 
                    private key as Hex, if you are sure your private key is encoded in Hex, use 
                    `sui keytool convert $HEX` to convert first then import the Bech32 encoded 
                    private key starting with `suiprivkey`.

# export works
target/debug/sui keytool export 0x1b87a727f58830d9ba2bfe6ecdc8fb49aa96fa2a2bbe175e128bfee13f6895ff
╭────────────────────┬────────────────────────────────────────────────────────────────────────────────────────────╮
│ exportedPrivateKey │  suiprivkey1q8typ5p96jtw3s3cqlnxulcg6me26cswjcpa8984z6k4ey4dwefcwlssmpg                    │
│ key                │ ╭─────────────────┬──────────────────────────────────────────────────────────────────────╮ │
│                    │ │ alias           │                                                                      │ │
│                    │ │ suiAddress      │  0x1b87a727f58830d9ba2bfe6ecdc8fb49aa96fa2a2bbe175e128bfee13f6895ff  │ │
│                    │ │ publicBase64Key │  AQMSxGmd92VJuD30t5sAOVMO//UWqx85kbuOMDRrUCzWfw==                    │ │
│                    │ │ keyScheme       │  secp256k1                                                           │ │
│                    │ │ flag            │  1                                                                   │ │
│                    │ │ peerId          │                                                                      │ │
│                    │ ╰─────────────────┴──────────────────────────────────────────────────────────────────────╯ │
╰────────────────────┴────────────────────────────────────────────────────────────────────────────────────────────╯

# export alias works
target/debug/sui keytool export --alias nostalgic-hiddenite                                ✔  10119  10:40:40
╭────────────────────┬────────────────────────────────────────────────────────────────────────────────────────────╮
│ exportedPrivateKey │  suiprivkey1q8typ5p96jtw3s3cqlnxulcg6me26cswjcpa8984z6k4ey4dwefcwlssmpg                    │
│ key                │ ╭─────────────────┬──────────────────────────────────────────────────────────────────────╮ │
│                    │ │ alias           │                                                                      │ │
│                    │ │ suiAddress      │  0x1b87a727f58830d9ba2bfe6ecdc8fb49aa96fa2a2bbe175e128bfee13f6895ff  │ │
│                    │ │ publicBase64Key │  AQMSxGmd92VJuD30t5sAOVMO//UWqx85kbuOMDRrUCzWfw==                    │ │
│                    │ │ keyScheme       │  secp256k1                                                           │ │
│                    │ │ flag            │  1                                                                   │ │
│                    │ │ peerId          │                                                                      │ │
│                    │ ╰─────────────────┴──────────────────────────────────────────────────────────────────────╯ │
╰────────────────────┴────────────────────────────────────────────────────────────────────────────────────────────╯

# convert works

target/debug/sui keytool convert 0x1b87a727f58830d9ba2bfe6ecdc8fb49aa96fa2a2bbe175e128bfee13f6895ff
╭────────────────┬──────────────────────────────────────────────────────────────────────────╮
│ bech32WithFlag │  suiprivkey1qqdc0fe87kyrpkd690lxanwgldy649h69g4mu967z29lacfldz2l766z9q2  │
│ base64WithFlag │  ABuHpyf1iDDZuiv+bs3I+0mqlvoqK74XXhKL/uE/aJX/                            │
│ hexWithoutFlag │  1b87a727f58830d9ba2bfe6ecdc8fb49aa96fa2a2bbe175e128bfee13f6895ff        │
│ scheme         │  ed25519                                                                 │
╰────────────────┴──────────────────────────────────────────────────────────────────────────╯


If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

  • protocol change
  • user-visible impact
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

Sui Keystore CLI no longer supports an import of a private key as a 32-byte hex string. It now only supports import or export Bech32-string encoded 33-byte flag || private_key starting with suiprivkey. See usage for sui keytool convert -h if you want to see all formatted private keys. See sui keytool export -h if you need to export a private key in bech32 format. This matches the import and export private key format in Sui Wallet and SDK. See SIP for more standard details.

@joyqvq joyqvq requested review from plam-ml and Nikhil-Mysten and removed request for a team December 18, 2023 20:56
Copy link

vercel bot commented Dec 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-core ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 29, 2024 11:25pm
sui-typescript-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 29, 2024 11:25pm
4 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2024 11:25pm
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2024 11:25pm
mysten-ui ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2024 11:25pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2024 11:25pm

sdk/typescript/src/cryptography/keypair.ts Outdated Show resolved Hide resolved
sdk/typescript/src/keypairs/ed25519/keypair.ts Outdated Show resolved Hide resolved
sdk/typescript/src/keypairs/ed25519/keypair.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@pchrysochoidis pchrysochoidis left a comment

Choose a reason for hiding this comment

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

After having another look, it seems ExportedKeyPair and Keypair.export are not necessary anymore, so I opened this PR to deprecate them.

@@ -0,0 +1,5 @@
---
'@mysten/sui.js': major
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if we do major updates for the sdk - noting for @hayes-mysten or @Jordan-Mysten to confirm

Copy link
Contributor

Choose a reason for hiding this comment

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

yes everything should be a minor until we get to 1.0

@@ -135,6 +135,7 @@
"@sentry/browser": "^7.61.0",
"@tanstack/react-query": "^5.0.0",
"@tanstack/react-query-persist-client": "^4.29.25",
"bech32": "^2.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this might not be needed anymore?

return keypair;
}

export function fromExportedKeypair(keypair: ExportedKeypair, legacySupport = false): Keypair {
Copy link
Contributor

Choose a reason for hiding this comment

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

When will we ever drop legacy support? I'm not sure it makes sense for this to be configurable?

Copy link
Contributor

Choose a reason for hiding this comment

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

This was added for wallet storage backwards compatibility, cc @pchrysochoidis

Copy link
Contributor

Choose a reason for hiding this comment

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

We will need to run a storage migration to get rid of this. Although I've this PR #15833 that changes it a bit. We should review/merge that one first.

export function fromExportedKeypair(keypair: ExportedKeypair): Keypair {
const secretKey = fromB64(keypair.privateKey);
export function validateExportedKeypair(keypair: ExportedKeypair): ExportedKeypair {
const _kp = decodeSuiPrivateKey(keypair.privateKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can just call the function and not grab a return value.

Copy link
Contributor

Choose a reason for hiding this comment

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

I removed it in #15833

try {
privateKeyBytes = hexToBytes(hexValue);
decodeSuiPrivateKey(privateKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

Given it's going to take a while for wallets to update, we probably don't want to ship with this, and should be able to detect legacy formats and upgrade them seamlessly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

allowing both hex and bech32 in the current impl

joyqvq and others added 6 commits January 29, 2024 18:16
Co-authored-by: stefan-mysten <135084671+stefan-mysten@users.noreply.github.com>
* this allows supporting keys that are stored in base64 format already in the wallet storage
@joyqvq joyqvq merged commit a34f1cb into main Jan 30, 2024
41 checks passed
@joyqvq joyqvq deleted the privkey branch January 30, 2024 15:37
@joyqvq joyqvq mentioned this pull request Jan 30, 2024
7 tasks
joyqvq added a commit that referenced this pull request Jan 30, 2024
## Description 

followup to #15415

## Test Plan 

How did you test the new or updated feature?

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants