fix!: Compressing private key should ensure string format

hirosystems · Sep 24, 2024 · 8ea4aa7 · 8ea4aa7
1 parent 557e2ba
commit 8ea4aa7
Show file tree

Hide file tree

Showing 8 changed files with 24 additions and 27 deletions.
diff --git a/packages/cli/src/keys.ts b/packages/cli/src/keys.ts
@@ -6,7 +6,6 @@ const c32check = require('c32check');
 
 import { HDKey } from '@scure/bip32';
 import * as scureBip39 from '@scure/bip39';
-import { bytesToHex } from '@stacks/common';
 
 import {
   compressPrivateKey,
@@ -152,7 +151,7 @@ export async function getStacksWalletKeyInfo(
   const child = master.derive(derivationPath);
   const pubkey = Buffer.from(child.publicKey!);
   const privkeyBuffer = Buffer.from(child.privateKey!);
-  const privkey = bytesToHex(compressPrivateKey(privkeyBuffer));
+  const privkey = compressPrivateKey(privkeyBuffer);
   const wifVersion = network.isTestnet() ? BITCOIN_WIF_TESTNET : BITCOIN_WIF;
   const walletImportFormat = wif.encode(wifVersion, privkeyBuffer, true);
 

diff --git a/packages/common/src/constants.ts b/packages/common/src/constants.ts
@@ -7,8 +7,7 @@ export const GAIA_URL = 'https://hub.blockstack.org';
 // todo: deduplicate magic variables
 
 /** @ignore internal */
-export const PRIVATE_KEY_COMPRESSED_LENGTH = 33;
-// todo: `next` make length consts more consistent in naming
+export const PRIVATE_KEY_BYTES_COMPRESSED = 33;
 
 /** @ignore internal */
-export const PRIVATE_KEY_UNCOMPRESSED_LENGTH = 32;
+export const PRIVATE_KEY_BYTES_UNCOMPRESSED = 32;
diff --git a/packages/encryption/src/keys.ts b/packages/encryption/src/keys.ts
@@ -2,7 +2,7 @@ import { hmac } from '@noble/hashes/hmac';
 import { sha256 } from '@noble/hashes/sha256';
 import { getPublicKey as nobleGetPublicKey, signSync, utils } from '@noble/secp256k1';
 import {
-  PRIVATE_KEY_COMPRESSED_LENGTH,
+  PRIVATE_KEY_BYTES_COMPRESSED,
   PrivateKey,
   bytesToHex,
   concatBytes,
@@ -13,6 +13,7 @@ import {
 import base58 from 'bs58';
 import { hashRipemd160 } from './hashRipemd160';
 import { hashSha256Sync } from './sha2Hash';
+import { privateKeyToHex } from '../../transactions/src';
 
 const BITCOIN_PUBKEYHASH = 0x00;
 
@@ -122,10 +123,10 @@ export function isValidPrivateKey(privateKey: PrivateKey): boolean {
 /**
  * @ignore
  */
-export function compressPrivateKey(privateKey: PrivateKey): Uint8Array {
-  const privateKeyBytes = privateKeyToBytes(privateKey);
+export function compressPrivateKey(privateKey: PrivateKey): string {
+  privateKey = privateKeyToHex(privateKey);
 
-  return privateKeyBytes.length == PRIVATE_KEY_COMPRESSED_LENGTH
-    ? privateKeyBytes // leave compressed
-    : concatBytes(privateKeyBytes, new Uint8Array([1])); // compress
+  return privateKey.length == PRIVATE_KEY_BYTES_COMPRESSED * 2
+    ? privateKey // leave compressed
+    : `${privateKey}01`; // compress
 }
diff --git a/packages/encryption/tests/keys.test.ts b/packages/encryption/tests/keys.test.ts
@@ -1,8 +1,8 @@
 import { utils } from '@noble/secp256k1';
 import {
+  PRIVATE_KEY_BYTES_UNCOMPRESSED,
   bytesToHex,
   hexToBytes,
-  PRIVATE_KEY_UNCOMPRESSED_LENGTH,
   utf8ToBytes,
 } from '@stacks/common';
 import { address, ECPair, networks } from 'bitcoinjs-lib';
@@ -24,7 +24,7 @@ test('makeECPrivateKey', () => {
 
   expect(privateKey).toBeTruthy();
   expect(typeof privateKey).toEqual('string');
-  expect(privateKey.length).toEqual(PRIVATE_KEY_UNCOMPRESSED_LENGTH * 2);
+  expect(privateKey.length).toEqual(PRIVATE_KEY_BYTES_UNCOMPRESSED * 2);
   expect(utils.isValidPrivateKey(privateKey)).toBeTruthy();
 });
 
@@ -115,14 +115,14 @@ describe(compressPrivateKey, () => {
     const privateKeyCompressed =
       '00cdce6b5f87d38f2a830cae0da82162e1b487f07c5affa8130f01fe1a2a25fb01';
 
-    expect(compressPrivateKey(privateKeyCompressed)).toEqual(hexToBytes(privateKeyCompressed));
+    expect(compressPrivateKey(privateKeyCompressed)).toEqual(privateKeyCompressed);
   });
 
   it('compresses uncompressed key', () => {
     const privateKey = '00cdce6b5f87d38f2a830cae0da82162e1b487f07c5affa8130f01fe1a2a25fb';
     const privateKeyCompressed =
       '00cdce6b5f87d38f2a830cae0da82162e1b487f07c5affa8130f01fe1a2a25fb01';
 
-    expect(compressPrivateKey(privateKey)).toEqual(hexToBytes(privateKeyCompressed));
+    expect(compressPrivateKey(privateKey)).toEqual(privateKeyCompressed);
   });
 });
diff --git a/packages/transactions/src/keys.ts b/packages/transactions/src/keys.ts
@@ -13,7 +13,7 @@ import {
   hexToBytes,
   intToHex,
   parseRecoverableSignatureVrs,
-  PRIVATE_KEY_COMPRESSED_LENGTH,
+  PRIVATE_KEY_BYTES_COMPRESSED,
   PrivateKey,
   privateKeyToBytes,
   PublicKey,
@@ -116,7 +116,7 @@ export const isPrivateKeyCompressed = privateKeyIsCompressed;
 /** @deprecated Use {@link isPrivateKeyCompressed} instead */
 export function privateKeyIsCompressed(privateKey: PrivateKey): boolean {
   const length = typeof privateKey === 'string' ? privateKey.length / 2 : privateKey.byteLength;
-  return length === PRIVATE_KEY_COMPRESSED_LENGTH;
+  return length === PRIVATE_KEY_BYTES_COMPRESSED;
 }
 
 /**

diff --git a/packages/transactions/src/wire/types.ts b/packages/transactions/src/wire/types.ts
@@ -281,8 +281,7 @@ export interface TransactionAuthFieldWire {
 
 export type TransactionAuthFieldContentsWire = PublicKeyWire | MessageSignatureWire;
 
-// todo: `next` refactor to match wire format more precisely eg https://github.com/jbencin/sips/blob/sip-02x-non-sequential-multisig-transactions/sips/sip-02x/sip-02x-non-sequential-multisig-transactions.md
-//  "A spending authorization field is encoded as follows:" ...
+/** @see {@link AuthFieldType} */
 export interface TransactionAuthFieldWire {
   type: StacksWireType.TransactionAuthField;
   pubKeyEncoding: PubKeyEncoding;

diff --git a/packages/transactions/tests/builder.test.ts b/packages/transactions/tests/builder.test.ts
@@ -1,7 +1,7 @@
 import {
   HIRO_MAINNET_URL,
   HIRO_TESTNET_URL,
-  PRIVATE_KEY_COMPRESSED_LENGTH,
+  PRIVATE_KEY_BYTES_COMPRESSED,
   bytesToHex,
   bytesToUtf8,
   createApiKeyMiddleware,
@@ -98,6 +98,7 @@ import {
   transactionToHex,
 } from '../src/transaction';
 import { cloneDeep, randomBytes } from '../src/utils';
+import { compressPrivateKey } from '../../encryption/src';
 
 function setSignature(
   unsignedTransaction: StacksTransaction,
@@ -2351,10 +2352,8 @@ describe('multi-sig', () => {
   describe('working non-sequential multi-sig', () => {
     const pk1 = makeRandomPrivKey();
     const pk2 = makeRandomPrivKey();
-    const pk3 = bytesToHex(randomBytes(32)) + '01';
-    const pk4 = bytesToHex(randomBytes(32)) + '01';
-
-    // todo: add compressPrivateKey helper `next`
+    const pk3 = compressPrivateKey(makeRandomPrivKey());
+    const pk4 = compressPrivateKey(makeRandomPrivKey());
 
     const CASES = [
       { signers: [pk1, pk2, pk3], signing: [pk1, pk2], required: 2 },
@@ -2458,7 +2457,7 @@ describe('multi-sig', () => {
           );
         });
         tx.auth.spendingCondition.fields[fieldIdx] = createTransactionAuthField(
-          hexToBytes(signerKey).byteLength === PRIVATE_KEY_COMPRESSED_LENGTH
+          hexToBytes(signerKey).byteLength === PRIVATE_KEY_BYTES_COMPRESSED
             ? PubKeyEncoding.Compressed
             : PubKeyEncoding.Uncompressed,
           createMessageSignature(nextSig)

diff --git a/packages/wallet-sdk/src/derive.ts b/packages/wallet-sdk/src/derive.ts
@@ -242,13 +242,13 @@ export const derivePrivateKeyByType = ({
 export const deriveStxPrivateKey = ({ rootNode, index }: { rootNode: HDKey; index: number }) => {
   const childKey = rootNode.derive(STX_DERIVATION_PATH).deriveChild(index);
   assertIsTruthy(childKey.privateKey);
-  return bytesToHex(compressPrivateKey(childKey.privateKey));
+  return compressPrivateKey(childKey.privateKey);
 };
 
 export const deriveDataPrivateKey = ({ rootNode, index }: { rootNode: HDKey; index: number }) => {
   const childKey = rootNode.derive(DATA_DERIVATION_PATH).deriveChild(index + HARDENED_OFFSET);
   assertIsTruthy(childKey.privateKey);
-  return bytesToHex(compressPrivateKey(childKey.privateKey));
+  return compressPrivateKey(childKey.privateKey);
 };
 
 export const deriveAccount = ({