-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Bitcore vault #4039
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: master
Are you sure you want to change the base?
Bitcore vault #4039
Conversation
| * Override signTx to require vault access | ||
| */ | ||
| public async signTx(params: any): Promise<any> { | ||
| return this.withVaultAccess(params.passphrase, super.signTx.bind(this), params); |
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.
this one is interesting withVaultAccess unlocks using the passphrase. but the passphrase still needs to be sent via params?
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.
Good catch. Updated.
| } | ||
|
|
||
| private getPublicKey(): string { | ||
| return this.publicKey.export({ type: 'spki', format: 'pem' }).toString(); |
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.
add this.publicKey null check
| if (!walletEntry) { | ||
| throw new Error(`Wallet not found: ${name}`); | ||
| } | ||
|
|
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.
consider privateKey null check
if (!this.privateKey) {
throw new Error('Private key not available - secure process not initialized or already cleaned up');
}
| }); | ||
|
|
||
| await this.sendMessage<void>('initialize', {}); | ||
| const publicKeyPem = await this.sendMessage<string>('getPublicKey', {}); |
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.
consider validating retrieved key
if (!publicKeyPem || typeof publicKeyPem !== 'string' || publicKeyPem.trim().length === 0) {
throw new Error('Invalid public key received from secure process');
}
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.
Good call. typeof publicKeyPem !== 'string' || !publicKey.trim().length is functionally similar - any other falsy value (including undefined) than "" wouldn't make it past the type check, and the trim().length takes care of "".
No description provided.