Skip to content

[BC/CWC] Wallet private key updates [1]#4068

Open
MichaelAJay wants to merge 29 commits intobitpay:masterfrom
MichaelAJay:wallet-privatekey-ref
Open

[BC/CWC] Wallet private key updates [1]#4068
MichaelAJay wants to merge 29 commits intobitpay:masterfrom
MichaelAJay:wallet-privatekey-ref

Conversation

@MichaelAJay
Copy link
Contributor

@MichaelAJay MichaelAJay commented Dec 12, 2025

bitcore-client

Encryption

  • adds encryptBuffer - enables normalized buffer input
  • adds decryptToBuffer - enables improved security by not decrypting directly to string

Storage

  • adds addKeysSafe - incoming keys' private key is encrypted, so not immediately available to be used to retrieve pubkey, which should be available anyway. Throws if missing a pubkey

Wallet

  • Adds version: 2 & version checks for backwards compatibility (note: to create an old Wallet would require version: 0 - so 1 is skipped - looking for feedback on numbering/versioning).
  • create encrypts HDPrivateKey xprivkey and privateKey so they can be decrypted as buffers instead of serializing the whole masterKey and THEN encrypting
  • importKeys does the same for signing keys
  • signTx decrypts to buffers then uses deriver for chain-aware decoding. In the next phase, this decoding should be made unnecessary by passing the buffer all the way through to use, if possible

Tests

  • update prior tests to include versioning
  • introduce new tests for version 2 specifics

crypto-wallet-core

Derivation

  • Add two passthrough methods on DeriverProxy to buffer & serialize private keys
  • Adds privateKeyToBuffer and privateKeyBufferToNativePrivateKey to IDeriver & implement for each Deriver - no need to override for any of the extending classes (UTXOs to AbstractBitcoreLibDeriver & EVM to EthDeriver)

@kajoseph kajoseph added CWC This pull request modifies the crypto-wallet-core package BCC This pull request modifies the bitcore-client package labels Dec 30, 2025
@MichaelAJay MichaelAJay changed the title Draft: [BC/CWC] Wallet private key updates [1] [BC/CWC] Wallet private key updates [1] Jan 6, 2026
Copy link
Collaborator

@kajoseph kajoseph left a comment

Choose a reason for hiding this comment

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

I think a better approach may be to auto migrate wallets with version < 2 to v2. That way we can avoid carve-outs while actively push better security.

loadWallet() {
  const wallet = read wallet file;
  if (wallet.version < 2) {
    convert to v2 wallet;
    backup wallet file to .bak;
    overwrite wallet file with v2;
  }
}

@kajoseph kajoseph added the bitcore-lib This pull request modifies the bitcore-lib package label Feb 4, 2026
}
}

export function encryptPrivateKey(privKey, pubKey, encryptionKey) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we mark as @deprecated and use encryptBuffer instead? We could also just make this a wrapper for encryptBuffer...

describe('signTx v2 key handling', function() {
let txStub: sinon.SinonStub;
afterEach(async function() {
txStub?.restore();
Copy link
Collaborator

Choose a reason for hiding this comment

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

sandbox.resolve()


/**
* New methods
* TODO: Deprecate above as necessary
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we mark the above as @deprecated?

const pubKey = CWC.Deriver.getPublicKey('ETH', wallet.network, privBufForPubKey);
privBufForPubKey.fill(0);
const privBuf = CWC.Deriver.privateKeyToBuffer('ETH', privHex);
// v2 key encryption uses the key's pubKey as the IV salt (not the wallet pubKey)
Copy link
Collaborator

Choose a reason for hiding this comment

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

?? As far as I can tell, they both set the IV the same way.

return this.get(chain).privateKeyToBuffer(privateKey);
}

privateKeyBufferToNativePrivateKey(chain: string, network: string, buf: Buffer): any {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Avoid any. Every fn returns a string - might as well type it.

/**
* Temporary - converts decrypted private key buffer to chain-native private key format
*/
privateKeyBufferToNativePrivateKey(buf: Buffer, network: string): any;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sort of. It depends on how much we want THIS PR to do.

The point of this is that it pushes the buffer private keys all the way to the boundary - Wallet side.

Next would be updating the called libraries and sending buffers straight on through.

I think that should be a second effort after this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCC This pull request modifies the bitcore-client package bitcore-lib This pull request modifies the bitcore-lib package CWC This pull request modifies the crypto-wallet-core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants