From 777e367c3f600f18d2b07036139c19b604d8a6d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomislav=20Hora=C4=8Dek?= Date: Mon, 21 Aug 2023 12:37:12 +0200 Subject: [PATCH 1/2] test(wallet): fix trezor transaction mismatching issue --- packages/key-management/src/TrezorKeyAgent.ts | 9 ++- .../hardware/trezor/TrezorKeyAgent.test.ts | 66 ++++++++++--------- 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/packages/key-management/src/TrezorKeyAgent.ts b/packages/key-management/src/TrezorKeyAgent.ts index a394df32893..3b3fd0aed25 100644 --- a/packages/key-management/src/TrezorKeyAgent.ts +++ b/packages/key-management/src/TrezorKeyAgent.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import * as Crypto from '@cardano-sdk/crypto'; -import { AuthenticationError, TransportError } from './errors'; +import { AuthenticationError, HwMappingError, TransportError } from './errors'; import { Cardano, NotImplementedError, coreToCml } from '@cardano-sdk/core'; import { CardanoKeyConst, @@ -147,7 +147,7 @@ export class TrezorKeyAgent extends KeyAgentBase { ); } - async signTransaction({ body }: Cardano.TxBodyWithHash): Promise { + async signTransaction({ body, hash }: Cardano.TxBodyWithHash): Promise { const scope = new ManagedFreeableScope(); try { await this.isTrezorInitialized; @@ -167,6 +167,11 @@ export class TrezorKeyAgent extends KeyAgentBase { } const signedData = result.payload; + + if (signedData.hash !== hash) { + throw new HwMappingError('Trezor computed a different transaction id'); + } + return new Map( await Promise.all( signedData.witnesses.map(async (witness) => { diff --git a/packages/wallet/test/hardware/trezor/TrezorKeyAgent.test.ts b/packages/wallet/test/hardware/trezor/TrezorKeyAgent.test.ts index dcf00c385d1..cc57efb32e1 100644 --- a/packages/wallet/test/hardware/trezor/TrezorKeyAgent.test.ts +++ b/packages/wallet/test/hardware/trezor/TrezorKeyAgent.test.ts @@ -2,16 +2,16 @@ import * as Crypto from '@cardano-sdk/crypto'; import { AddressType, CommunicationType, - GroupedAddress, SerializableTrezorKeyAgentData, TrezorKeyAgent, util } from '@cardano-sdk/key-management'; import { AssetId, createStubStakePoolProvider, mockProviders as mocks } from '@cardano-sdk/util-dev'; import { CML, Cardano } from '@cardano-sdk/core'; +import { InitializeTxProps, InitializeTxResult } from '@cardano-sdk/tx-construction'; import { PersonalWallet, setupWallet } from '../../../src'; import { dummyLogger as logger } from 'ts-log'; -import { mockKeyAgentDependencies, stakeKeyDerivationPath } from '../../../../key-management/test/mocks'; +import { mockKeyAgentDependencies } from '../../../../key-management/test/mocks'; describe('TrezorKeyAgent', () => { let keyAgent: TrezorKeyAgent; @@ -29,38 +29,28 @@ describe('TrezorKeyAgent', () => { txSubmitProvider = mocks.mockTxSubmitProvider(); ({ keyAgent, wallet } = await setupWallet({ bip32Ed25519: new Crypto.CmlBip32Ed25519(CML), - - createKeyAgent: async (dependencies) => { - const trezorKeyAgent = await TrezorKeyAgent.createWithDevice( + createKeyAgent: async (dependencies) => + await TrezorKeyAgent.createWithDevice( { chainId: Cardano.ChainIds.LegacyTestnet, trezorConfig }, dependencies - ); - const groupedAddress: GroupedAddress = { - accountIndex: 0, - address: mocks.utxo[0][0].address, - index: 0, - networkId: Cardano.NetworkId.Testnet, - rewardAccount: mocks.rewardAccount, - stakeKeyDerivationPath, - type: AddressType.External - }; - trezorKeyAgent.deriveAddress = jest.fn().mockResolvedValue(groupedAddress); - trezorKeyAgent.knownAddresses.push(groupedAddress); - return trezorKeyAgent; - }, + ), createWallet: async (trezorKeyAgent) => { + const { address, rewardAccount } = await trezorKeyAgent.deriveAddress( + { index: 0, type: AddressType.External }, + 0 + ); const assetProvider = mocks.mockAssetProvider(); const stakePoolProvider = createStubStakePoolProvider(); const networkInfoProvider = mocks.mockNetworkInfoProvider(); - const rewardsProvider = mocks.mockRewardsProvider(); - const chainHistoryProvider = mocks.mockChainHistoryProvider(); - const utxoProvider = mocks.mockUtxoProvider(); + const utxoProvider = mocks.mockUtxoProvider({ address }); + const rewardsProvider = mocks.mockRewardsProvider({ rewardAccount }); + const chainHistoryProvider = mocks.mockChainHistoryProvider({ rewardAccount }); const asyncKeyAgent = util.createAsyncKeyAgent(trezorKeyAgent); return new PersonalWallet( - { name: 'Trezor Wallet' }, + { name: 'HW Wallet' }, { assetProvider, chainHistoryProvider, @@ -114,7 +104,7 @@ describe('TrezorKeyAgent', () => { expect(typeof keyAgent.extendedAccountPublicKey).toBe('string'); }); - test('sign tx', async () => { + describe('sign transaction', () => { const outputs = [ { address: Cardano.PaymentAddress( @@ -132,15 +122,31 @@ describe('TrezorKeyAgent', () => { } } ]; - const props = { + const props: InitializeTxProps = { outputs: new Set(outputs) }; - const txInternals = await wallet.initializeTx(props); - const signatures = await keyAgent.signTransaction({ - body: txInternals.body, - hash: txInternals.hash + let txInternals: InitializeTxResult; + + beforeAll(async () => { + txInternals = await wallet.initializeTx(props); + }); + + it('successfully signs a transaction with assets and validity interval', async () => { + const signatures = await keyAgent.signTransaction({ + body: txInternals.body, + hash: txInternals.hash + }); + expect(signatures.size).toBe(2); + }); + + it('throws if signed transaction hash doesnt match hash computed by the wallet', async () => { + await expect( + keyAgent.signTransaction({ + body: txInternals.body, + hash: 'non-matching' as unknown as Cardano.TransactionId + }) + ).rejects.toThrow(); }); - expect(signatures.size).toBe(2); }); describe('serializableData', () => { From d045670ed2eb5eee8efa1acaf97823ebb7676922 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomislav=20Hora=C4=8Dek?= Date: Thu, 24 Aug 2023 08:46:36 +0200 Subject: [PATCH 2/2] chore(hardware-trezor): add transaction hash check --- packages/hardware-trezor/src/TrezorKeyAgent.ts | 5 +++++ .../wallet/test/hardware/trezor/TrezorKeyAgent.test.ts | 9 ++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/hardware-trezor/src/TrezorKeyAgent.ts b/packages/hardware-trezor/src/TrezorKeyAgent.ts index b19d1b4da97..49ed4270f60 100644 --- a/packages/hardware-trezor/src/TrezorKeyAgent.ts +++ b/packages/hardware-trezor/src/TrezorKeyAgent.ts @@ -185,6 +185,11 @@ export class TrezorKeyAgent extends KeyAgentBase { } const signedData = result.payload; + + if (signedData.hash !== tx.hash) { + throw new errors.HwMappingError('Trezor computed a different transaction id'); + } + return new Map( await Promise.all( signedData.witnesses.map(async (witness) => { diff --git a/packages/wallet/test/hardware/trezor/TrezorKeyAgent.test.ts b/packages/wallet/test/hardware/trezor/TrezorKeyAgent.test.ts index d28ff039025..e05bf76b9c0 100644 --- a/packages/wallet/test/hardware/trezor/TrezorKeyAgent.test.ts +++ b/packages/wallet/test/hardware/trezor/TrezorKeyAgent.test.ts @@ -1,10 +1,5 @@ import * as Crypto from '@cardano-sdk/crypto'; -import { - AddressType, - CommunicationType, - SerializableTrezorKeyAgentData, - util -} from '@cardano-sdk/key-management'; +import { AddressType, CommunicationType, SerializableTrezorKeyAgentData, util } from '@cardano-sdk/key-management'; import { AssetId, createStubStakePoolProvider, mockProviders as mocks } from '@cardano-sdk/util-dev'; import { CML, Cardano } from '@cardano-sdk/core'; import { InitializeTxProps, InitializeTxResult } from '@cardano-sdk/tx-construction'; @@ -131,7 +126,7 @@ describe('TrezorKeyAgent', () => { txInternals = await wallet.initializeTx(props); }); - it('successfully signs a transaction with assets and validity interval', async () => { + it('successfully signs a transaction with assets', async () => { const signatures = await keyAgent.signTransaction({ body: txInternals.body, hash: txInternals.hash