From 1c7a8f6593c2070ec086d66e70a4f64a5665fd36 Mon Sep 17 00:00:00 2001 From: Rushil Kapoor Date: Wed, 28 Jun 2023 11:58:29 +0530 Subject: [PATCH] feat(abstract-utxo): update the keySignature verification network BG-79423 --- modules/abstract-utxo/src/abstractUtxoCoin.ts | 8 +++- .../test/v2/unit/coins/abstractUtxoCoin.ts | 37 +++++++++++++++---- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 833aa8ac68..522c212b31 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -102,6 +102,7 @@ const { getExternalChainCode, isChainCode, scriptTypeForChain, outputScripts } = type Unspent = bitgo.Unspent; type RootWalletKeys = bitgo.RootWalletKeys; + export interface VerifyAddressOptions extends BaseVerifyAddressOptions { chain: number; index: number; @@ -677,7 +678,7 @@ export abstract class AbstractUtxoCoin extends BaseCoin { * @param {VerifyKeySignaturesOptions} params * @return {{backup: boolean, bitgo: boolean}} */ - protected verifyKeySignature(params: VerifyKeySignaturesOptions): boolean { + public verifyKeySignature(params: VerifyKeySignaturesOptions): boolean { // first, let's verify the integrity of the user key, whose public key is used for subsequent verifications const { userKeychain, keychainToVerify, keySignature } = params; if (!userKeychain) { @@ -695,10 +696,13 @@ export abstract class AbstractUtxoCoin extends BaseCoin { // verify the signature against the user public key assert(userKeychain.pub); const publicKey = bip32.fromBase58(userKeychain.pub).publicKey; + // Due to interface of `bitcoinMessage`, we need to convert the public key to an address. + // Note that this address has no relationship to on-chain transactions. We are + // only interested in the address as a representation of the public key. const signingAddress = utxolib.address.toBase58Check( utxolib.crypto.hash160(publicKey), utxolib.networks.bitcoin.pubKeyHash, - this.network + utxolib.networks.bitcoin ); // BG-5703: use BTC mainnet prefix for all key signature operations diff --git a/modules/bitgo/test/v2/unit/coins/abstractUtxoCoin.ts b/modules/bitgo/test/v2/unit/coins/abstractUtxoCoin.ts index 81e45d883d..5fecaa8db1 100644 --- a/modules/bitgo/test/v2/unit/coins/abstractUtxoCoin.ts +++ b/modules/bitgo/test/v2/unit/coins/abstractUtxoCoin.ts @@ -1,7 +1,7 @@ import * as utxolib from '@bitgo/utxo-lib'; import * as should from 'should'; import * as sinon from 'sinon'; -import { UnexpectedAddressError, VerificationOptions } from '@bitgo/sdk-core'; +import { Keychain, UnexpectedAddressError, VerificationOptions } from '@bitgo/sdk-core'; import { TestBitGo } from '@bitgo/sdk-test'; import { BitGo } from '../../../../src/bitgo'; import { @@ -82,7 +82,10 @@ describe('Abstract UTXO Coin:', () => { it('should classify outputs which spend change back to a v1 wallet base address as external ' + 'if considerMigratedFromAddressInternal is set and false', async function () { - return runClassifyOutputsTest(wallet.migratedFrom(), { ...verification, considerMigratedFromAddressInternal: false }, true); + return runClassifyOutputsTest(wallet.migratedFrom(), { + ...verification, + considerMigratedFromAddressInternal: false, + }, true); }); it('should classify outputs which spend to addresses not on the wallet as external', async function () { @@ -96,7 +99,12 @@ describe('Abstract UTXO Coin:', () => { it('should classify outputs with external address in recipients as explicit', async function () { const externalAddress = '2NAuziD75WnPPHJVwnd4ckgY4SuJaDVVbMD'; - return runClassifyOutputsTest(externalAddress, verification, true, { recipients: [{ address: externalAddress, amount: outputAmount }] }); + return runClassifyOutputsTest(externalAddress, verification, true, { + recipients: [{ + address: externalAddress, + amount: outputAmount, + }], + }); }); }); @@ -239,7 +247,7 @@ describe('Abstract UTXO Coin:', () => { }; // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const sign = async (key, keychain) => (await coin.signMessage(keychain, key.pub!)).toString('hex'); + const sign = async (key, keychain, coinToSignFor = coin) => (await coinToSignFor.signMessage(keychain, key.pub!)).toString('hex'); const signUser = (key) => sign(key, userKeychain); const signOther = (key) => sign(key, otherKeychain); const passphrase = 'test_passphrase'; @@ -252,7 +260,10 @@ describe('Abstract UTXO Coin:', () => { badKey: { keychains: { // user public key swapped out - user: { pub: otherKeychain.pub, encryptedPrv: bitgo.encrypt({ input: userKeychain.prv, password: passphrase }) }, + user: { + pub: otherKeychain.pub, + encryptedPrv: bitgo.encrypt({ input: userKeychain.prv, password: passphrase }), + }, }, needsCustomChangeKeySignatureVerification: true, }, @@ -423,8 +434,7 @@ describe('Abstract UTXO Coin:', () => { it('should not allow any implicit external outputs if paygo outputs are disallowed', async () => { const coinMock = sinon.stub(coin, 'parseTransaction').resolves({ - keychains: { - }, + keychains: {}, keySignatures: {}, outputs: [], missingOutputs: [], @@ -516,5 +526,18 @@ describe('Abstract UTXO Coin:', () => { coinMock.restore(); bitcoinMock.restore(); }); + + it('should verify key signature of ZEC', async () => { + const zecCoin = bitgo.coin('tzec') as AbstractUtxoCoin; + const userKeychain = await zecCoin.keychains().create(); + const otherKeychain = await zecCoin.keychains().create(); + + await zecCoin.verifyKeySignature({ + userKeychain: (userKeychain as unknown) as Keychain, + keychainToVerify: (otherKeychain as unknown) as Keychain, + keySignature: await sign(userKeychain, otherKeychain, zecCoin), + }).should.be.true(); + }); + }); });