diff --git a/.changeset/little-camels-approve.md b/.changeset/little-camels-approve.md new file mode 100644 index 000000000..deeba8581 --- /dev/null +++ b/.changeset/little-camels-approve.md @@ -0,0 +1,5 @@ +--- +"@ledgerhq/device-signer-kit-ethereum": minor +--- + +Update SignTransaction device action with generic-parser support diff --git a/packages/signer/context-module/src/transaction/data/CalldataDto.ts b/packages/signer/context-module/src/transaction/data/CalldataDto.ts index 1bcc75834..234d8c0c7 100644 --- a/packages/signer/context-module/src/transaction/data/CalldataDto.ts +++ b/packages/signer/context-module/src/transaction/data/CalldataDto.ts @@ -112,7 +112,13 @@ export interface CalldataDescriptorValueV1 { type_size?: number; } +export interface CalldataDescriptorContainerPathV1 { + type: "CONTAINER"; + value: CalldataDescriptorContainerPathTypeV1; +} + export interface CalldataDescriptorPathElementsV1 { + type: "DATA"; elements: CalldataDescriptorPathElementV1[]; } @@ -150,7 +156,7 @@ export interface CalldataDescriptorPathElementSliceV1 { end?: number; } -export type CalldataDescriptorContainerPathV1 = "FROM" | "TO" | "VALUE"; +export type CalldataDescriptorContainerPathTypeV1 = "FROM" | "TO" | "VALUE"; export type CalldataDescriptorPathLeafTypeV1 = | "ARRAY_LEAF" | "TUPLE_LEAF" diff --git a/packages/signer/context-module/src/transaction/data/HttpTransactionDataSource.test.ts b/packages/signer/context-module/src/transaction/data/HttpTransactionDataSource.test.ts index 298954ddb..c4590fc55 100644 --- a/packages/signer/context-module/src/transaction/data/HttpTransactionDataSource.test.ts +++ b/packages/signer/context-module/src/transaction/data/HttpTransactionDataSource.test.ts @@ -66,6 +66,7 @@ describe("HttpTransactionDataSource", () => { param: { value: { binary_path: { + type: "DATA", elements: [ { type: "TUPLE", @@ -83,6 +84,7 @@ describe("HttpTransactionDataSource", () => { type: "TOKEN_AMOUNT", token: { binary_path: { + type: "DATA", elements: [ { type: "ARRAY", @@ -106,7 +108,10 @@ describe("HttpTransactionDataSource", () => { fieldTrustedName = { param: { value: { - binary_path: "TO", + binary_path: { + type: "CONTAINER", + value: "TO", + }, type_family: "STRING", type_size: 20, }, @@ -121,6 +126,7 @@ describe("HttpTransactionDataSource", () => { param: { value: { binary_path: { + type: "DATA", elements: [ { type: "ARRAY", @@ -141,6 +147,7 @@ describe("HttpTransactionDataSource", () => { }, collection: { binary_path: { + type: "DATA", elements: [ { type: "REF", @@ -166,7 +173,7 @@ describe("HttpTransactionDataSource", () => { }); function createFieldWithoutReference( - binary_path: unknown, + binary_path: string, type_family: string, type: string, descriptor: string, @@ -174,7 +181,10 @@ describe("HttpTransactionDataSource", () => { return { param: { value: { - binary_path, + binary_path: { + type: "CONTAINER", + value: binary_path, + }, type_family, type_size: 32, }, @@ -310,7 +320,7 @@ describe("HttpTransactionDataSource", () => { expect(result.extract()).toEqual([ { payload: - "0001000108000000000000000102147d2768de32b0b80b7a3454c06bdac94a69ddc7a9030469328dec04207d5e9ed0004b8035b164edd9d78c37415ad6b1d123be4943d0abd5a50035cae3050857697468647261770604416176650708416176652044414f081068747470733a2f2f616176652e636f6d0a045fc4ba9c3045022100eb67599abfd9c7360b07599a2a2cb769c6e3f0f74e1e52444d788c8f577a16d20220402e92b0adbf97d890fa2f9654bc30c7bd70dacabe870f160e6842d9eb73d36f", + "0001000108000000000000000102147d2768de32b0b80b7a3454c06bdac94a69ddc7a9030469328dec04207d5e9ed0004b8035b164edd9d78c37415ad6b1d123be4943d0abd5a50035cae3050857697468647261770604416176650708416176652044414f081068747470733a2f2f616176652e636f6d0a045fc4ba9c81ff473045022100eb67599abfd9c7360b07599a2a2cb769c6e3f0f74e1e52444d788c8f577a16d20220402e92b0adbf97d890fa2f9654bc30c7bd70dacabe870f160e6842d9eb73d36f", type: "transactionInfo", }, { @@ -393,7 +403,7 @@ describe("HttpTransactionDataSource", () => { expect(result.extract()).toEqual([ { payload: - "0001000108000000000000000102147d2768de32b0b80b7a3454c06bdac94a69ddc7a9030469328dec04207d5e9ed0004b8035b164edd9d78c37415ad6b1d123be4943d0abd5a50035cae3050857697468647261770604416176650708416176652044414f081068747470733a2f2f616176652e636f6d0a045fc4ba9c3045022100eb67599abfd9c7360b07599a2a2cb769c6e3f0f74e1e52444d788c8f577a16d20220402e92b0adbf97d890fa2f9654bc30c7bd70dacabe870f160e6842d9eb73d36f", + "0001000108000000000000000102147d2768de32b0b80b7a3454c06bdac94a69ddc7a9030469328dec04207d5e9ed0004b8035b164edd9d78c37415ad6b1d123be4943d0abd5a50035cae3050857697468647261770604416176650708416176652044414f081068747470733a2f2f616176652e636f6d0a045fc4ba9c81ff473045022100eb67599abfd9c7360b07599a2a2cb769c6e3f0f74e1e52444d788c8f577a16d20220402e92b0adbf97d890fa2f9654bc30c7bd70dacabe870f160e6842d9eb73d36f", type: "transactionInfo", }, { diff --git a/packages/signer/context-module/src/transaction/data/HttpTransactionDataSource.ts b/packages/signer/context-module/src/transaction/data/HttpTransactionDataSource.ts index fe6fe74db..a2cf414a0 100644 --- a/packages/signer/context-module/src/transaction/data/HttpTransactionDataSource.ts +++ b/packages/signer/context-module/src/transaction/data/HttpTransactionDataSource.ts @@ -108,7 +108,7 @@ export class HttpTransactionDataSource implements TransactionDataSource { ]; const info: ClearSignContextSuccess = { type: ClearSignContextType.TRANSACTION_INFO, - payload: `${infoData}${infoSignature}`, + payload: this.formatTransactionInfo(infoData, infoSignature), }; const enums: ClearSignContextSuccess[] = calldataDescriptor.enums.map( (e) => ({ @@ -126,6 +126,20 @@ export class HttpTransactionDataSource implements TransactionDataSource { return Right([info, ...enums, ...fields]); } + private formatTransactionInfo( + infoData: string, + infoSignature: string, + ): string { + // Ensure correct padding + if (infoSignature.length % 2 !== 0) { + infoSignature = "0" + infoSignature; + } + // TLV encoding as according to generic parser documentation + const infoSignatureTag = "81ff"; + const infoSignatureLength = (infoSignature.length / 2).toString(16); + return `${infoData}${infoSignatureTag}${infoSignatureLength}${infoSignature}`; + } + private getReference( param: CalldataDescriptorParam, ): ClearSignContextReference | undefined { @@ -153,8 +167,8 @@ export class HttpTransactionDataSource implements TransactionDataSource { private toGenericPath( path: CalldataDescriptorContainerPathV1 | CalldataDescriptorPathElementsV1, ): GenericPath { - if (typeof path !== "object") { - return path; + if (path.type === "CONTAINER") { + return path.value; } return path.elements.map((element) => { if (element.type === "ARRAY") { @@ -259,9 +273,10 @@ export class HttpTransactionDataSource implements TransactionDataSource { ].includes(data.type_family) && (typeof data.type_size === "undefined" || typeof data.type_size === "number") && - ((typeof data.binary_path === "string" && - ["FROM", "TO", "VALUE"].includes(data.binary_path)) || - (typeof data.binary_path === "object" && + ((typeof data.binary_path === "object" && + data.binary_path.type === "CONTAINER" && + ["FROM", "TO", "VALUE"].includes(data.binary_path.value)) || + (data.binary_path.type === "DATA" && Array.isArray(data.binary_path.elements) && data.binary_path.elements.every((e) => this.isPathElementV1(e)))) ); diff --git a/packages/signer/context-module/src/trusted-name/data/HttpTrustedNameDataSource.test.ts b/packages/signer/context-module/src/trusted-name/data/HttpTrustedNameDataSource.test.ts index f30b085b8..35efe3719 100644 --- a/packages/signer/context-module/src/trusted-name/data/HttpTrustedNameDataSource.test.ts +++ b/packages/signer/context-module/src/trusted-name/data/HttpTrustedNameDataSource.test.ts @@ -173,7 +173,7 @@ describe("HttpTrustedNameDataSource", () => { // GIVEN const response = { data: { - signedDescriptor: { data: "payload", signatures: { prod: "sig" } }, + signedDescriptor: { data: "payload" }, }, }; jest.spyOn(axios, "request").mockResolvedValue(response); @@ -187,7 +187,28 @@ describe("HttpTrustedNameDataSource", () => { }); // THEN - expect(result).toEqual(Right("payloadsig")); + expect(result).toEqual(Right("payload")); + }); + + it("should return a payload with a signature", async () => { + // GIVEN + const response = { + data: { + signedDescriptor: { data: "payload", signatures: { prod: "12345" } }, + }, + }; + jest.spyOn(axios, "request").mockResolvedValue(response); + + // WHEN + const result = await datasource.getTrustedNamePayload({ + address: "0x1234", + challenge: "", + sources: ["ens"], + types: ["eoa"], + }); + + // THEN + expect(result).toEqual(Right("payload153012345")); }); }); }); diff --git a/packages/signer/context-module/src/trusted-name/data/HttpTrustedNameDataSource.ts b/packages/signer/context-module/src/trusted-name/data/HttpTrustedNameDataSource.ts index b5c4841d1..ba9816428 100644 --- a/packages/signer/context-module/src/trusted-name/data/HttpTrustedNameDataSource.ts +++ b/packages/signer/context-module/src/trusted-name/data/HttpTrustedNameDataSource.ts @@ -57,6 +57,12 @@ export class HttpTrustedNameDataSource implements TrustedNameDataSource { types, }: GetTrustedNameInfosParams): Promise> { try { + // TODO remove that filtering once https://ledgerhq.atlassian.net/browse/BACK-8075 is done + // For now we have to filter or trusted names won't work with the generic parser, because transaction + // fields descriptors can contain unsupported sources. + sources = sources.filter( + (source) => source === "ens" || source === "crypto_assets_list", + ); const response = await axios.request({ method: "GET", url: `https://nft.api.live.ledger.com/v2/names/ethereum/1/reverse/${address}?types=${types.join(",")}&sources=${sources.join(",")}&challenge=${challenge}`, @@ -65,28 +71,28 @@ export class HttpTrustedNameDataSource implements TrustedNameDataSource { }, }); const trustedName = response.data; + if (!trustedName?.signedDescriptor?.data) { + return Left( + new Error( + `[ContextModule] HttpTrustedNameDataSource: no trusted name metadata for address ${address}`, + ), + ); + } + const payload = trustedName.signedDescriptor.data; if ( - !trustedName || - !trustedName.signedDescriptor || - !trustedName.signedDescriptor.data || !trustedName.signedDescriptor.signatures || typeof trustedName.signedDescriptor.signatures[this.config.cal.mode] !== "string" ) { - return Left( - new Error( - `[ContextModule] HttpTrustedNameDataSource: no trusted name metadata for address ${address}`, - ), - ); + // If we have no separated signature but a valid descriptor, it may mean the descriptor was + // signed on-the-fly for dynamic sources such as ens + return Right(payload); } - return Right( - [ - trustedName.signedDescriptor.data, - trustedName.signedDescriptor.signatures[this.config.cal.mode], - ].join(""), - ); + const signature = + trustedName.signedDescriptor.signatures[this.config.cal.mode]!; + return Right(this.formatTrustedName(payload, signature)); } catch (_error) { return Left( new Error( @@ -95,4 +101,15 @@ export class HttpTrustedNameDataSource implements TrustedNameDataSource { ); } } + + private formatTrustedName(payload: string, signature: string): string { + // Ensure correct padding + if (signature.length % 2 !== 0) { + signature = "0" + signature; + } + // TLV encoding as according to trusted name documentation + const signatureTag = "15"; + const signatureLength = (signature.length / 2).toString(16); + return `${payload}${signatureTag}${signatureLength}${signature}`; + } } diff --git a/packages/signer/signer-eth/src/api/app-binder/SignTransactionDeviceActionTypes.ts b/packages/signer/signer-eth/src/api/app-binder/SignTransactionDeviceActionTypes.ts index e72411bc4..0c7502d80 100644 --- a/packages/signer/signer-eth/src/api/app-binder/SignTransactionDeviceActionTypes.ts +++ b/packages/signer/signer-eth/src/api/app-binder/SignTransactionDeviceActionTypes.ts @@ -15,7 +15,9 @@ import { type Signature } from "@api/model/Signature"; import { type Transaction, type TransactionType } from "@api/model/Transaction"; import { type TransactionOptions } from "@api/model/TransactionOptions"; import { type ProvideTransactionContextTaskErrorCodes } from "@internal/app-binder/task/ProvideTransactionContextTask"; +import { type GenericContext } from "@internal/app-binder/task/ProvideTransactionGenericContextTask"; import { type TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService"; +import { type TransactionParserService } from "@internal/transaction/service/parser/TransactionParserService"; export type SignTransactionDAOutput = Signature; @@ -23,6 +25,7 @@ export type SignTransactionDAInput = { readonly derivationPath: string; readonly transaction: Transaction; readonly mapper: TransactionMapperService; + readonly parser: TransactionParserService; readonly contextModule: ContextModule; readonly options: TransactionOptions; }; @@ -49,10 +52,11 @@ export type SignTransactionDAState = DeviceActionState< export type SignTransactionDAInternalState = { readonly error: SignTransactionDAError | null; readonly challenge: string | null; - readonly clearSignContexts: ClearSignContextSuccess[] | null; + readonly clearSignContexts: ClearSignContextSuccess[] | GenericContext | null; readonly serializedTransaction: Uint8Array | null; readonly chainId: number | null; readonly transactionType: TransactionType | null; + readonly isLegacy: boolean; readonly signature: Signature | null; }; diff --git a/packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.test.ts b/packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.test.ts index 02b294118..6bbd21209 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.test.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.test.ts @@ -32,6 +32,7 @@ import { import { type Signature } from "@api/model/Signature"; import { type TypedData } from "@api/model/TypedData"; import { type TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService"; +import { type TransactionParserService } from "@internal/transaction/service/parser/TransactionParserService"; import { type TypedDataParserService } from "@internal/typed-data/service/TypedDataParserService"; import { GetAddressCommand } from "./command/GetAddressCommand"; @@ -50,6 +51,9 @@ describe("EthAppBinder", () => { const mockedMapper: TransactionMapperService = { mapTransactionToSubset: jest.fn(), } as unknown as TransactionMapperService; + const mockedParser: TransactionParserService = { + extractValue: jest.fn(), + } as unknown as TransactionParserService; beforeEach(() => { jest.clearAllMocks(); @@ -81,6 +85,7 @@ describe("EthAppBinder", () => { mockedDmk, mockedContextModule, mockedMapper, + mockedParser, "sessionId", ); const { observable } = appBinder.getAddress({ @@ -135,6 +140,7 @@ describe("EthAppBinder", () => { mockedDmk, mockedContextModule, mockedMapper, + mockedParser, "sessionId", ); appBinder.getAddress(params); @@ -165,6 +171,7 @@ describe("EthAppBinder", () => { mockedDmk, mockedContextModule, mockedMapper, + mockedParser, "sessionId", ); appBinder.getAddress(params); @@ -216,6 +223,7 @@ describe("EthAppBinder", () => { mockedDmk, mockedContextModule, mockedMapper, + mockedParser, "sessionId", ); const { observable } = appBinder.signTransaction({ @@ -283,6 +291,7 @@ describe("EthAppBinder", () => { mockedDmk, mockedContextModule, mockedMapper, + mockedParser, "sessionId", ); const { observable } = appBinder.signTransaction({ @@ -350,6 +359,7 @@ describe("EthAppBinder", () => { mockedDmk, mockedContextModule, mockedMapper, + mockedParser, "sessionId", ); const { observable } = appBinder.signPersonalMessage({ @@ -424,6 +434,7 @@ describe("EthAppBinder", () => { mockedDmk, mockedContextModule, mockedMapper, + mockedParser, "sessionId", ); const { observable } = appBinder.signTypedData({ diff --git a/packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.ts b/packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.ts index a414af45f..5fa886395 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.ts @@ -18,6 +18,7 @@ import { SignTypedDataDeviceAction } from "@internal/app-binder/device-action/Si import { externalTypes } from "@internal/externalTypes"; import { transactionTypes } from "@internal/transaction/di/transactionTypes"; import { TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService"; +import { TransactionParserService } from "@internal/transaction/service/parser/TransactionParserService"; import { type TypedDataParserService } from "@internal/typed-data/service/TypedDataParserService"; import { GetAddressCommand } from "./command/GetAddressCommand"; @@ -31,6 +32,8 @@ export class EthAppBinder { @inject(externalTypes.ContextModule) private contextModule: ContextModule, @inject(transactionTypes.TransactionMapperService) private mapper: TransactionMapperService, + @inject(transactionTypes.TransactionParserService) + private parser: TransactionParserService, @inject(externalTypes.SessionId) private sessionId: DeviceSessionId, ) {} @@ -80,6 +83,7 @@ export class EthAppBinder { derivationPath: args.derivationPath, transaction: args.transaction, mapper: this.mapper, + parser: this.parser, contextModule: this.contextModule, options: args.options ?? {}, }, diff --git a/packages/signer/signer-eth/src/internal/app-binder/command/StoreTransactionCommand.ts b/packages/signer/signer-eth/src/internal/app-binder/command/StoreTransactionCommand.ts index 466a06699..b7adb08f5 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/command/StoreTransactionCommand.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/command/StoreTransactionCommand.ts @@ -39,8 +39,8 @@ export class StoreTransactionCommand const signEthTransactionArgs: ApduBuilderArgs = { cla: 0xe0, ins: 0x04, - p1: isFirstChunk ? 0x01 : 0x00, - p2: 0x02, + p1: isFirstChunk ? 0x00 : 0x80, + p2: 0x01, }; const builder = new ApduBuilder(signEthTransactionArgs); return builder.addBufferToData(serializedTransaction).build(); diff --git a/packages/signer/signer-eth/src/internal/app-binder/device-action/SignTransaction/SignTransactionDeviceAction.test.ts b/packages/signer/signer-eth/src/internal/app-binder/device-action/SignTransaction/SignTransactionDeviceAction.test.ts index b58b8c465..2d0c9b118 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/device-action/SignTransaction/SignTransactionDeviceAction.test.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/device-action/SignTransaction/SignTransactionDeviceAction.test.ts @@ -10,10 +10,12 @@ import { Transaction } from "ethers-v6"; import { Just, Nothing } from "purify-ts"; import { type SignTransactionDAState } from "@api/app-binder/SignTransactionDeviceActionTypes"; +import { TransactionType } from "@api/model/Transaction"; import { makeDeviceActionInternalApiMock } from "@internal/app-binder/device-action/__test-utils__/makeInternalApi"; import { setupOpenAppDAMock } from "@internal/app-binder/device-action/__test-utils__/setupOpenAppDAMock"; import { testDeviceActionStates } from "@internal/app-binder/device-action/__test-utils__/testDeviceActionStates"; import { type TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService"; +import { type TransactionParserService } from "@internal/transaction/service/parser/TransactionParserService"; import { SignTransactionDeviceAction } from "./SignTransactionDeviceAction"; @@ -37,15 +39,20 @@ describe("SignTransactionDeviceAction", () => { const mapperMock: TransactionMapperService = { mapTransactionToSubset: jest.fn(), } as unknown as TransactionMapperService; + const parserMock: TransactionParserService = { + extractValue: jest.fn(), + } as unknown as TransactionParserService; const getChallengeMock = jest.fn(); const buildContextMock = jest.fn(); const provideContextMock = jest.fn(); + const provideGenericContextMock = jest.fn(); const signTransactionMock = jest.fn(); function extractDependenciesMock() { return { getChallenge: getChallengeMock, buildContext: buildContextMock, provideContext: provideContextMock, + provideGenericContext: provideGenericContextMock, signTransaction: signTransactionMock, }; } @@ -73,6 +80,7 @@ describe("SignTransactionDeviceAction", () => { options: defaultOptions, contextModule: contextModuleMock, mapper: mapperMock, + parser: parserMock, }, }); @@ -90,6 +98,8 @@ describe("SignTransactionDeviceAction", () => { }, ], serializedTransaction: new Uint8Array([0x01, 0x02, 0x03]), + chainId: 1, + transactionType: TransactionType.LEGACY, }); provideContextMock.mockResolvedValueOnce(Nothing); signTransactionMock.mockResolvedValueOnce( @@ -201,6 +211,169 @@ describe("SignTransactionDeviceAction", () => { derivationPath: "44'/60'/0'/0/0", serializedTransaction: new Uint8Array([0x01, 0x02, 0x03]), isLegacy: true, + chainId: 1, + transactionType: TransactionType.LEGACY, + }, + }), + ); + }, + }); + }); + + it("should call external dependencies with the correct parameters with the generic parser", (done) => { + setupOpenAppDAMock(); + + const deviceAction = new SignTransactionDeviceAction({ + input: { + derivationPath: "44'/60'/0'/0/0", + transaction: defaultTransaction, + options: defaultOptions, + contextModule: contextModuleMock, + mapper: mapperMock, + parser: parserMock, + }, + }); + + // Mock the dependencies to return some sample data + getChallengeMock.mockResolvedValueOnce( + CommandResultFactory({ + data: { challenge: "challenge" }, + }), + ); + buildContextMock.mockResolvedValueOnce({ + clearSignContexts: { + transactionInfo: "payload-1", + transactionFields: [ + { + type: "enum", + payload: "payload-2", + }, + ], + }, + serializedTransaction: new Uint8Array([0x01, 0x02, 0x03]), + chainId: 7, + transactionType: TransactionType.EIP1559, + }); + provideGenericContextMock.mockResolvedValueOnce(Nothing); + signTransactionMock.mockResolvedValueOnce( + CommandResultFactory({ + data: { + v: 0x1c, + r: "0x8a540510e13b0f2b11a451275716d29e08caad07e89a1c84964782fb5e1ad788", + s: "0x64a0de235b270fbe81e8e40688f4a9f9ad9d283d690552c9331d7773ceafa513", + }, + }), + ); + jest + .spyOn(deviceAction, "extractDependencies") + .mockReturnValue(extractDependenciesMock()); + + // Expected intermediate values for the following state sequence: + // Initial -> OpenApp -> GetChallenge -> BuildContext -> ProvideGenericContext -> SignTransaction + const expectedStates: Array = [ + // Initial state + { + intermediateValue: { + requiredUserInteraction: UserInteractionRequired.None, + }, + status: DeviceActionStatus.Pending, + }, + // OpenApp interaction + { + intermediateValue: { + requiredUserInteraction: UserInteractionRequired.ConfirmOpenApp, + }, + status: DeviceActionStatus.Pending, + }, + // GetChallenge state + { + intermediateValue: { + requiredUserInteraction: UserInteractionRequired.None, + }, + status: DeviceActionStatus.Pending, + }, + // BuildContext state + { + intermediateValue: { + requiredUserInteraction: UserInteractionRequired.None, + }, + status: DeviceActionStatus.Pending, + }, + // ProvideContext state + { + intermediateValue: { + requiredUserInteraction: UserInteractionRequired.None, + }, + status: DeviceActionStatus.Pending, + }, + // SignTransaction state + { + intermediateValue: { + requiredUserInteraction: UserInteractionRequired.SignTransaction, + }, + status: DeviceActionStatus.Pending, + }, + // Final state + { + output: { + v: 0x1c, + r: "0x8a540510e13b0f2b11a451275716d29e08caad07e89a1c84964782fb5e1ad788", + s: "0x64a0de235b270fbe81e8e40688f4a9f9ad9d283d690552c9331d7773ceafa513", + }, + status: DeviceActionStatus.Completed, + }, + ]; + + const { observable } = testDeviceActionStates( + deviceAction, + expectedStates, + makeDeviceActionInternalApiMock(), + done, + ); + + // Verify mocks calls parameters + observable.subscribe({ + complete: () => { + expect(getChallengeMock).toHaveBeenCalled(); + expect(buildContextMock).toHaveBeenCalledWith( + expect.objectContaining({ + input: { + challenge: "challenge", + contextModule: contextModuleMock, + mapper: mapperMock, + options: defaultOptions, + transaction: defaultTransaction, + }, + }), + ); + expect(provideGenericContextMock).toHaveBeenCalledWith( + expect.objectContaining({ + input: { + chainId: 7, + context: { + transactionInfo: "payload-1", + transactionFields: [ + { + type: "enum", + payload: "payload-2", + }, + ], + }, + contextModule: contextModuleMock, + derivationPath: "44'/60'/0'/0/0", + serializedTransaction: new Uint8Array([0x01, 0x02, 0x03]), + transactionParser: parserMock, + }, + }), + ); + expect(signTransactionMock).toHaveBeenCalledWith( + expect.objectContaining({ + input: { + derivationPath: "44'/60'/0'/0/0", + serializedTransaction: new Uint8Array([0x01, 0x02, 0x03]), + isLegacy: false, + chainId: 7, + transactionType: TransactionType.EIP1559, }, }), ); @@ -220,6 +393,7 @@ describe("SignTransactionDeviceAction", () => { options: defaultOptions, contextModule: contextModuleMock, mapper: mapperMock, + parser: parserMock, }, }); @@ -269,6 +443,7 @@ describe("SignTransactionDeviceAction", () => { options: defaultOptions, contextModule: contextModuleMock, mapper: mapperMock, + parser: parserMock, }, }); @@ -328,6 +503,7 @@ describe("SignTransactionDeviceAction", () => { options: defaultOptions, contextModule: contextModuleMock, mapper: mapperMock, + parser: parserMock, }, }); @@ -387,6 +563,7 @@ describe("SignTransactionDeviceAction", () => { options: defaultOptions, contextModule: contextModuleMock, mapper: mapperMock, + parser: parserMock, }, }); @@ -458,6 +635,7 @@ describe("SignTransactionDeviceAction", () => { options: defaultOptions, contextModule: contextModuleMock, mapper: mapperMock, + parser: parserMock, }, }); @@ -547,6 +725,7 @@ describe("SignTransactionDeviceAction", () => { options: defaultOptions, contextModule: contextModuleMock, mapper: mapperMock, + parser: parserMock, }, }); @@ -634,6 +813,7 @@ describe("SignTransactionDeviceAction", () => { options: defaultOptions, contextModule: contextModuleMock, mapper: mapperMock, + parser: parserMock, }, }); diff --git a/packages/signer/signer-eth/src/internal/app-binder/device-action/SignTransaction/SignTransactionDeviceAction.ts b/packages/signer/signer-eth/src/internal/app-binder/device-action/SignTransaction/SignTransactionDeviceAction.ts index 62affa979..bfd05adb4 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/device-action/SignTransaction/SignTransactionDeviceAction.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/device-action/SignTransaction/SignTransactionDeviceAction.ts @@ -38,8 +38,13 @@ import { } from "@internal/app-binder/task/BuildTransactionContextTask"; import { ProvideTransactionContextTask } from "@internal/app-binder/task/ProvideTransactionContextTask"; import { type ProvideTransactionContextTaskErrorCodes } from "@internal/app-binder/task/ProvideTransactionContextTask"; +import { + type GenericContext, + ProvideTransactionGenericContextTask, +} from "@internal/app-binder/task/ProvideTransactionGenericContextTask"; import { SendSignTransactionTask } from "@internal/app-binder/task/SendSignTransactionTask"; import { type TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService"; +import { type TransactionParserService } from "@internal/transaction/service/parser/TransactionParserService"; export type MachineDependencies = { readonly getChallenge: () => Promise< @@ -61,6 +66,18 @@ export type MachineDependencies = { }) => Promise< Maybe> >; + readonly provideGenericContext: (arg0: { + input: { + contextModule: ContextModule; + transactionParser: TransactionParserService; + chainId: number; + derivationPath: string; + serializedTransaction: Uint8Array; + context: GenericContext; + }; + }) => Promise< + Maybe> + >; readonly signTransaction: (arg0: { input: { derivationPath: string; @@ -96,8 +113,13 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< SignTransactionDAInternalState >; - const { getChallenge, buildContext, provideContext, signTransaction } = - this.extractDependencies(internalApi); + const { + getChallenge, + buildContext, + provideContext, + provideGenericContext, + signTransaction, + } = this.extractDependencies(internalApi); return setup({ types: { @@ -112,10 +134,15 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< getChallenge: fromPromise(getChallenge), buildContext: fromPromise(buildContext), provideContext: fromPromise(provideContext), + provideGenericContext: fromPromise(provideGenericContext), signTransaction: fromPromise(signTransaction), }, guards: { noInternalError: ({ context }) => context._internalState.error === null, + isGenericContext: ({ context }) => + context._internalState.clearSignContexts !== null && + typeof (context._internalState.clearSignContexts as GenericContext) + .transactionInfo === "string", }, actions: { assignErrorFromEvent: assign({ @@ -142,6 +169,7 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< chainId: null, transactionType: null, challenge: null, + isLegacy: true, signature: null, }, }; @@ -240,7 +268,7 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< challenge: context._internalState.challenge!, }), onDone: { - target: "ProvideContext", + target: "BuildContextResultCheck", actions: [ assign({ _internalState: ({ event, context }) => ({ @@ -259,12 +287,24 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< }, }, }, + BuildContextResultCheck: { + always: [ + { + target: "ProvideGenericContext", + guard: "isGenericContext", + }, + { + target: "ProvideContext", + }, + ], + }, ProvideContext: { invoke: { id: "provideContext", src: "provideContext", input: ({ context }) => ({ - clearSignContexts: context._internalState.clearSignContexts!, + clearSignContexts: context._internalState + .clearSignContexts as ClearSignContextSuccess[], }), onDone: { actions: assign({ @@ -286,6 +326,45 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< }, }, }, + ProvideGenericContext: { + invoke: { + id: "provideGenericContext", + src: "provideGenericContext", + input: ({ context }) => ({ + contextModule: context.input.contextModule, + transactionParser: context.input.parser, + chainId: context._internalState.chainId!, + derivationPath: context.input.derivationPath, + serializedTransaction: + context._internalState.serializedTransaction!, + context: context._internalState + .clearSignContexts as GenericContext, + }), + onDone: { + actions: assign({ + _internalState: ({ event, context }) => { + const { isLegacy: _, ...rest } = context._internalState; + return event.output.caseOf({ + Just: (error) => ({ + ...rest, + isLegacy: false, + error: error.error, + }), + Nothing: () => ({ + ...rest, + isLegacy: false, + }), + }); + }, + }), + target: "ProvideContextResultCheck", + }, + onError: { + target: "Error", + actions: "assignErrorFromEvent", + }, + }, + }, ProvideContextResultCheck: { always: [ { @@ -317,7 +396,7 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< context._internalState.serializedTransaction!, chainId: context._internalState.chainId!, transactionType: context._internalState.transactionType!, - isLegacy: true, // TODO: use ETHEREUM app version to determine if legacy + isLegacy: context._internalState.isLegacy, }), onDone: { target: "SignTransactionResultCheck", @@ -372,7 +451,7 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< internalApi.sendCommand(new GetChallengeCommand()); const buildContext = async (arg0: { input: BuildTransactionContextTaskArgs; - }) => new BuildTransactionContextTask(arg0.input).run(); + }) => new BuildTransactionContextTask(internalApi, arg0.input).run(); const provideContext = async (arg0: { input: { @@ -383,6 +462,25 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< clearSignContexts: arg0.input.clearSignContexts, }).run(); + const provideGenericContext = async (arg0: { + input: { + contextModule: ContextModule; + transactionParser: TransactionParserService; + chainId: number; + derivationPath: string; + serializedTransaction: Uint8Array; + context: GenericContext; + }; + }) => + new ProvideTransactionGenericContextTask(internalApi, { + contextModule: arg0.input.contextModule, + transactionParser: arg0.input.transactionParser, + chainId: arg0.input.chainId, + derivationPath: arg0.input.derivationPath, + serializedTransaction: arg0.input.serializedTransaction, + context: arg0.input.context, + }).run(); + const signTransaction = async (arg0: { input: { derivationPath: string; @@ -397,6 +495,7 @@ export class SignTransactionDeviceAction extends XStateDeviceAction< getChallenge, buildContext, provideContext, + provideGenericContext, signTransaction, }; } diff --git a/packages/signer/signer-eth/src/internal/app-binder/task/BuildTransactionContextTask.test.ts b/packages/signer/signer-eth/src/internal/app-binder/task/BuildTransactionContextTask.test.ts index 2b8cc2644..385ca6f1e 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/task/BuildTransactionContextTask.test.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/task/BuildTransactionContextTask.test.ts @@ -2,9 +2,14 @@ import { type ClearSignContext, ClearSignContextType, } from "@ledgerhq/context-module"; +import { + DeviceSessionStateType, + DeviceStatus, +} from "@ledgerhq/device-management-kit"; import { Transaction } from "ethers-v6"; import { Left, Right } from "purify-ts"; +import { makeDeviceActionInternalApiMock } from "@internal/app-binder/device-action/__test-utils__/makeInternalApi"; import { type TransactionMapperResult } from "@internal/transaction/service/mapper/model/TransactionMapperResult"; import { type TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService"; @@ -27,6 +32,7 @@ describe("BuildTransactionContextTask", () => { }; let defaultTransaction: Transaction; let defaultArgs: BuildTransactionContextTaskArgs; + const apiMock = makeDeviceActionInternalApiMock(); beforeEach(() => { jest.clearAllMocks(); @@ -56,9 +62,18 @@ describe("BuildTransactionContextTask", () => { }; mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult)); contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.12.0" }, + }); // WHEN - const result = await new BuildTransactionContextTask(defaultArgs).run(); + const result = await new BuildTransactionContextTask( + apiMock, + defaultArgs, + ).run(); // THEN expect(result).toEqual({ @@ -89,9 +104,18 @@ describe("BuildTransactionContextTask", () => { }; mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult)); contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.12.0" }, + }); // WHEN - const result = await new BuildTransactionContextTask(defaultArgs).run(); + const result = await new BuildTransactionContextTask( + apiMock, + defaultArgs, + ).run(); // THEN expect(result).toEqual({ @@ -102,6 +126,59 @@ describe("BuildTransactionContextTask", () => { }); }); + it("should build the transaction context with generic-parser context", async () => { + // GIVEN + const serializedTransaction = new Uint8Array([0x01, 0x02, 0x03]); + const clearSignContexts: ClearSignContext[] = [ + { + type: ClearSignContextType.TRANSACTION_INFO, + payload: "payload-1", + }, + { + type: ClearSignContextType.TRANSACTION_FIELD_DESCRIPTION, + payload: "payload-2", + }, + { + type: ClearSignContextType.ENUM, + payload: "payload-3", + }, + { + type: ClearSignContextType.TRANSACTION_FIELD_DESCRIPTION, + payload: "payload-4", + }, + ]; + const mapperResult: TransactionMapperResult = { + subset: { chainId: 1, to: undefined, data: "0x" }, + serializedTransaction, + type: 2, + }; + mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult)); + contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.14.0" }, + }); + + // WHEN + const result = await new BuildTransactionContextTask( + apiMock, + defaultArgs, + ).run(); + + // THEN + expect(result).toEqual({ + clearSignContexts: { + transactionInfo: "payload-1", + transactionFields: [...clearSignContexts.slice(1)], + }, + serializedTransaction, + chainId: 1, + transactionType: 2, + }); + }); + it("should call the mapper with the transaction", async () => { // GIVEN const serializedTransaction = new Uint8Array([0x01, 0x02, 0x03]); @@ -113,9 +190,15 @@ describe("BuildTransactionContextTask", () => { }; mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult)); contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.12.0" }, + }); // WHEN - await new BuildTransactionContextTask(defaultArgs).run(); + await new BuildTransactionContextTask(apiMock, defaultArgs).run(); // THEN expect(mapperMock.mapTransactionToSubset).toHaveBeenCalledWith( @@ -134,9 +217,15 @@ describe("BuildTransactionContextTask", () => { }; mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult)); contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.12.0" }, + }); // WHEN - await new BuildTransactionContextTask(defaultArgs).run(); + await new BuildTransactionContextTask(apiMock, defaultArgs).run(); // THEN expect(contextModuleMock.getContexts).toHaveBeenCalledWith({ @@ -150,9 +239,15 @@ describe("BuildTransactionContextTask", () => { // GIVEN const error = new Error("error"); mapperMock.mapTransactionToSubset.mockReturnValueOnce(Left(error)); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.12.0" }, + }); // WHEN - const task = new BuildTransactionContextTask(defaultArgs); + const task = new BuildTransactionContextTask(apiMock, defaultArgs); // THEN await expect(task.run()).rejects.toThrow(error); @@ -186,9 +281,72 @@ describe("BuildTransactionContextTask", () => { }; mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult)); contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.12.0" }, + }); + + // WHEN + const result = await new BuildTransactionContextTask( + apiMock, + defaultArgs, + ).run(); + + // THEN + expect(result).toEqual({ + clearSignContexts: [clearSignContexts[1], clearSignContexts[3]], + serializedTransaction, + chainId: 1, + transactionType: 0, + }); + }); + + it("should exclude generic-parser contexts from the result on old apps", async () => { + // GIVEN + const serializedTransaction = new Uint8Array([0x01, 0x02, 0x03]); + const clearSignContexts: ClearSignContext[] = [ + { + type: ClearSignContextType.TRANSACTION_INFO, + payload: "transaction_info", + }, + { + type: ClearSignContextType.TOKEN, + payload: "payload-1", + }, + { + type: ClearSignContextType.TRANSACTION_FIELD_DESCRIPTION, + payload: "transaction_field", + }, + { + type: ClearSignContextType.NFT, + payload: "payload-2", + }, + { + type: ClearSignContextType.ENUM, + payload: "enum", + }, + ]; + const mapperResult: TransactionMapperResult = { + subset: { chainId: 1, to: undefined, data: "0x" }, + serializedTransaction, + type: 0, + }; + mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult)); + contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.12.0" }, + }); // WHEN - const result = await new BuildTransactionContextTask(defaultArgs).run(); + const result = await new BuildTransactionContextTask( + apiMock, + defaultArgs, + ).run(); // THEN expect(result).toEqual({ @@ -198,4 +356,107 @@ describe("BuildTransactionContextTask", () => { transactionType: 0, }); }); + + it("should exclude generic-parser contexts from the result if no transaction_info was found", async () => { + // GIVEN + const serializedTransaction = new Uint8Array([0x01, 0x02, 0x03]); + const clearSignContexts: ClearSignContext[] = [ + { + type: ClearSignContextType.TRANSACTION_FIELD_DESCRIPTION, + payload: "transaction_field", + }, + { + type: ClearSignContextType.TOKEN, + payload: "payload-1", + }, + { + type: ClearSignContextType.ENUM, + payload: "enum", + }, + { + type: ClearSignContextType.NFT, + payload: "payload-2", + }, + ]; + const mapperResult: TransactionMapperResult = { + subset: { chainId: 1, to: undefined, data: "0x" }, + serializedTransaction, + type: 0, + }; + mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult)); + contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.14.0" }, + }); + + // WHEN + const result = await new BuildTransactionContextTask( + apiMock, + defaultArgs, + ).run(); + + // THEN + expect(result).toEqual({ + clearSignContexts: [clearSignContexts[1], clearSignContexts[3]], + serializedTransaction, + chainId: 1, + transactionType: 0, + }); + }); + + it("should exclude legacy contexts from the result for generic-parser transactions", async () => { + // GIVEN + const serializedTransaction = new Uint8Array([0x01, 0x02, 0x03]); + const clearSignContexts: ClearSignContext[] = [ + { + type: ClearSignContextType.TOKEN, + payload: "payload-1", + }, + { + type: ClearSignContextType.TRANSACTION_INFO, + payload: "payload-2", + }, + { + type: ClearSignContextType.EXTERNAL_PLUGIN, + payload: "payload-3", + }, + { + type: ClearSignContextType.TRANSACTION_FIELD_DESCRIPTION, + payload: "payload-4", + }, + ]; + const mapperResult: TransactionMapperResult = { + subset: { chainId: 1, to: undefined, data: "0x" }, + serializedTransaction, + type: 2, + }; + mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult)); + contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts); + apiMock.getDeviceSessionState.mockReturnValueOnce({ + sessionStateType: DeviceSessionStateType.ReadyWithoutSecureChannel, + deviceStatus: DeviceStatus.CONNECTED, + installedApps: [], + currentApp: { name: "Ethereum", version: "1.14.0" }, + }); + + // WHEN + const result = await new BuildTransactionContextTask( + apiMock, + defaultArgs, + ).run(); + + // THEN + expect(result).toEqual({ + clearSignContexts: { + transactionInfo: "payload-2", + transactionFields: [clearSignContexts[3]], + }, + serializedTransaction, + chainId: 1, + transactionType: 2, + }); + }); }); diff --git a/packages/signer/signer-eth/src/internal/app-binder/task/BuildTransactionContextTask.ts b/packages/signer/signer-eth/src/internal/app-binder/task/BuildTransactionContextTask.ts index c844d4a17..187ec8c3c 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/task/BuildTransactionContextTask.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/task/BuildTransactionContextTask.ts @@ -1,14 +1,22 @@ import { type ClearSignContextSuccess, + ClearSignContextType, type ContextModule, } from "@ledgerhq/context-module"; +import { + DeviceSessionStateType, + type InternalApi, +} from "@ledgerhq/device-management-kit"; +import { gte } from "semver"; import { type Transaction, type TransactionType } from "@api/model/Transaction"; import { type TransactionOptions } from "@api/model/TransactionOptions"; import { type TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService"; +import { type GenericContext } from "./ProvideTransactionGenericContextTask"; + export type BuildTransactionTaskResult = { - readonly clearSignContexts: ClearSignContextSuccess[]; + readonly clearSignContexts: ClearSignContextSuccess[] | GenericContext; readonly serializedTransaction: Uint8Array; readonly chainId: number; readonly transactionType: TransactionType; @@ -23,7 +31,10 @@ export type BuildTransactionContextTaskArgs = { }; export class BuildTransactionContextTask { - constructor(private readonly args: BuildTransactionContextTaskArgs) {} + constructor( + private readonly api: InternalApi, + private readonly args: BuildTransactionContextTaskArgs, + ) {} async run(): Promise { const { contextModule, mapper, transaction, options, challenge } = @@ -43,13 +54,49 @@ export class BuildTransactionContextTask { // TODO: for now we ignore the error contexts // as we consider that they are warnings and not blocking const clearSignContextsSuccess: ClearSignContextSuccess[] = - clearSignContexts.filter((context) => context.type !== "error"); + clearSignContexts.filter( + (context) => context.type !== ClearSignContextType.ERROR, + ); + + let filteredContexts: ClearSignContextSuccess[] | GenericContext = []; + const transactionInfo = clearSignContextsSuccess.find( + (ctx) => ctx.type === ClearSignContextType.TRANSACTION_INFO, + ); + if (!this.supportsGenericParser() || transactionInfo === undefined) { + filteredContexts = clearSignContextsSuccess.filter( + (ctx) => + ctx.type !== ClearSignContextType.TRANSACTION_INFO && + ctx.type !== ClearSignContextType.TRANSACTION_FIELD_DESCRIPTION && + ctx.type !== ClearSignContextType.ENUM, + ); + } else { + const transactionFields = clearSignContextsSuccess.filter( + (ctx) => + ctx.type === ClearSignContextType.TRANSACTION_FIELD_DESCRIPTION || + ctx.type === ClearSignContextType.ENUM, + ); + filteredContexts = { + transactionInfo: transactionInfo.payload, + transactionFields, + }; + } return { - clearSignContexts: clearSignContextsSuccess, + clearSignContexts: filteredContexts, serializedTransaction, chainId: subset.chainId, transactionType: type, }; } + + private supportsGenericParser(): boolean { + const deviceState = this.api.getDeviceSessionState(); + if (deviceState.sessionStateType === DeviceSessionStateType.Connected) { + return false; + } + if (deviceState.currentApp.name !== "Ethereum") { + return false; + } + return gte(deviceState.currentApp.version, "1.14.0"); + } } diff --git a/packages/signer/signer-eth/src/internal/app-binder/task/ProvideTransactionGenericContextTask.ts b/packages/signer/signer-eth/src/internal/app-binder/task/ProvideTransactionGenericContextTask.ts index 485d7fb2f..09209d18b 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/task/ProvideTransactionGenericContextTask.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/task/ProvideTransactionGenericContextTask.ts @@ -6,14 +6,15 @@ import { } from "@ledgerhq/context-module"; import { bufferToHexaString, + ByteArrayBuilder, type CommandErrorResult, type CommandResult, CommandResultFactory, - CommandResultStatus, type InternalApi, InvalidStatusWordError, isSuccessCommandResult, } from "@ledgerhq/device-management-kit"; +import { DerivationPathUtils } from "@ledgerhq/signer-utils"; import { Just, type Maybe, Nothing } from "purify-ts"; import { GetChallengeCommand } from "@internal/app-binder/command/GetChallengeCommand"; @@ -47,6 +48,7 @@ export type ProvideTransactionGenericContextTaskArgs = { readonly contextModule: ContextModule; readonly transactionParser: TransactionParserService; readonly chainId: number; + readonly derivationPath: string; readonly serializedTransaction: Uint8Array; readonly context: GenericContext; }; @@ -64,8 +66,15 @@ export class ProvideTransactionGenericContextTask { Maybe> > { // Store the transaction in the device memory + const paths = DerivationPathUtils.splitPath(this.args.derivationPath); + const builder = new ByteArrayBuilder(); + builder.add8BitUIntToData(paths.length); + paths.forEach((path) => { + builder.add32BitUIntToData(path); + }); + builder.addBufferToData(this.args.serializedTransaction); const storeTransactionResult = await new SendCommandInChunksTask(this.api, { - data: this.args.serializedTransaction, + data: builder.build(), commandFactory: (args) => new StoreTransactionCommand({ serializedTransaction: args.chunkedData, @@ -121,15 +130,13 @@ export class ProvideTransactionGenericContextTask { reference.valuePath, ); if (values.isLeft()) { - return Just({ - status: CommandResultStatus.Error, - error: new InvalidStatusWordError( - "The clear sign context reference contains a path not found in that transaction", - ), - }); + // The path was not found in transaction payload. In that case we should raw-sign that field. + return Nothing; } for (const value of values.unsafeCoerce()) { - const address = bufferToHexaString(value.slice(0, 20)); + const address = bufferToHexaString( + value.slice(Math.max(0, value.length - 20)), + ); let context; if (reference.type === ClearSignContextType.TRUSTED_NAME) { const getChallengeResult = await this.api.sendCommand( diff --git a/packages/signer/signer-eth/src/internal/app-binder/task/SendSignTransactionTask.test.ts b/packages/signer/signer-eth/src/internal/app-binder/task/SendSignTransactionTask.test.ts index 54dbe98fe..85a08b878 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/task/SendSignTransactionTask.test.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/task/SendSignTransactionTask.test.ts @@ -8,6 +8,7 @@ import { Transaction } from "ethers-v6"; import { Just, Nothing } from "purify-ts"; import { SignTransactionCommand } from "@internal/app-binder/command/SignTransactionCommand"; +import { StartTransactionCommand } from "@internal/app-binder/command/StartTransactionCommand"; import { makeDeviceActionInternalApiMock } from "@internal/app-binder/device-action/__test-utils__/makeInternalApi"; import { SendSignTransactionTask } from "./SendSignTransactionTask"; @@ -88,6 +89,7 @@ describe("SendSignTransactionTask", () => { serializedTransaction: SIMPLE_TRANSACTION, chainId: 1, transactionType: 1, + isLegacy: true, }; apiMock.sendCommand.mockResolvedValueOnce(resultOk); @@ -109,6 +111,29 @@ describe("SendSignTransactionTask", () => { expect((result as any).data).toStrictEqual(signature); }); + it("Generic-parser transaction should be signed without payload", async () => { + // GIVEN + const args = { + derivationPath: "44'/60'/0'/0/0", + serializedTransaction: SIMPLE_TRANSACTION, + chainId: 1, + transactionType: 1, + isLegacy: false, + }; + apiMock.sendCommand.mockResolvedValueOnce(resultOk); + + // WHEN + const result = await new SendSignTransactionTask(apiMock, args).run(); + + // THEN + expect(apiMock.sendCommand.mock.calls).toHaveLength(1); + expect(apiMock.sendCommand.mock.calls[0]![0]).toStrictEqual( + new StartTransactionCommand(), + ); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((result as any).data).toStrictEqual(signature); + }); + it("should send the transaction in chunks", async () => { // GIVEN const args = { @@ -171,6 +196,7 @@ describe("SendSignTransactionTask", () => { serializedTransaction: serialized, chainId, transactionType: 0, + isLegacy: true, }; apiMock.sendCommand.mockResolvedValueOnce(resultNothing); apiMock.sendCommand.mockResolvedValueOnce(resultNothing); @@ -234,6 +260,31 @@ describe("SendSignTransactionTask", () => { ); }); + it("should return an error if the generic-parser command fails", async () => { + // GIVEN + const args = { + derivationPath: "44'/60'/0'/0/0", + serializedTransaction: SIMPLE_TRANSACTION, + chainId: 1, + transactionType: 1, + isLegacy: false, + }; + apiMock.sendCommand.mockResolvedValueOnce(resultNothing); + + // WHEN + const result = await new SendSignTransactionTask(apiMock, args).run(); + + // THEN + expect(apiMock.sendCommand.mock.calls).toHaveLength(1); + expect(apiMock.sendCommand.mock.calls[0]![0]).toStrictEqual( + new StartTransactionCommand(), + ); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((result as any).error).toStrictEqual( + new InvalidStatusWordError("no signature returned"), + ); + }); + it("should return an error if the command fails in the middle of the transaction", async () => { // GIVEN const args = { @@ -286,6 +337,7 @@ describe("SendSignTransactionTask", () => { serializedTransaction: SIMPLE_TRANSACTION, chainId: 56, transactionType: 0, + isLegacy: true, }; apiMock.sendCommand.mockResolvedValueOnce( CommandResultFactory({ @@ -312,6 +364,7 @@ describe("SendSignTransactionTask", () => { serializedTransaction: SIMPLE_TRANSACTION, chainId: 56, transactionType: 0, + isLegacy: true, }; apiMock.sendCommand.mockResolvedValueOnce( CommandResultFactory({ @@ -338,6 +391,7 @@ describe("SendSignTransactionTask", () => { serializedTransaction: SIMPLE_TRANSACTION, chainId: 11297108109, transactionType: 0, + isLegacy: true, }; apiMock.sendCommand.mockResolvedValueOnce( CommandResultFactory({ @@ -364,6 +418,7 @@ describe("SendSignTransactionTask", () => { serializedTransaction: SIMPLE_TRANSACTION, chainId: 11297108109, transactionType: 0, + isLegacy: true, }; apiMock.sendCommand.mockResolvedValueOnce( CommandResultFactory({ diff --git a/packages/signer/signer-eth/src/internal/app-binder/task/SendSignTransactionTask.ts b/packages/signer/signer-eth/src/internal/app-binder/task/SendSignTransactionTask.ts index ab632bee6..60d4fe985 100644 --- a/packages/signer/signer-eth/src/internal/app-binder/task/SendSignTransactionTask.ts +++ b/packages/signer/signer-eth/src/internal/app-binder/task/SendSignTransactionTask.ts @@ -18,6 +18,7 @@ import { SignTransactionCommand, type SignTransactionCommandResponse, } from "@internal/app-binder/command/SignTransactionCommand"; +import { StartTransactionCommand } from "@internal/app-binder/command/StartTransactionCommand"; const PATH_SIZE = 4; @@ -26,6 +27,7 @@ type SendSignTransactionTaskArgs = { serializedTransaction: Uint8Array; chainId: number; transactionType: TransactionType; + isLegacy: boolean; }; export class SendSignTransactionTask { @@ -35,6 +37,23 @@ export class SendSignTransactionTask { ) {} async run(): Promise> { + // For generic-parser transactions, the derivation path and transaction were previously sent + if (!this.args.isLegacy) { + const signature = await this.api.sendCommand( + new StartTransactionCommand(), + ); + if (!isSuccessCommandResult(signature)) { + return signature; + } + return this.recoverSignature(signature.data).mapOrDefault( + (data) => CommandResultFactory({ data }), + CommandResultFactory({ + error: new InvalidStatusWordError("no signature returned"), + }), + ); + } + + // For other transactions, add derivation path and transaction to the payload const { derivationPath, serializedTransaction } = this.args; const paths = DerivationPathUtils.splitPath(derivationPath); const builder = new ByteArrayBuilder( diff --git a/packages/signer/signer-eth/src/internal/transaction/di/transactionModule.ts b/packages/signer/signer-eth/src/internal/transaction/di/transactionModule.ts index 892c45132..6ac7fb199 100644 --- a/packages/signer/signer-eth/src/internal/transaction/di/transactionModule.ts +++ b/packages/signer/signer-eth/src/internal/transaction/di/transactionModule.ts @@ -4,6 +4,7 @@ import { transactionTypes } from "@internal/transaction/di/transactionTypes"; import { EthersV5TransactionMapper } from "@internal/transaction/service/mapper/EthersV5TransactionMapper"; import { EthersV6TransactionMapper } from "@internal/transaction/service/mapper/EthersV6TransactionMapper"; import { TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService"; +import { TransactionParserService } from "@internal/transaction/service/parser/TransactionParserService"; import { SignTransactionUseCase } from "@internal/transaction/use-case/SignTransactionUseCase"; export const transactionModuleFactory = () => @@ -21,6 +22,9 @@ export const transactionModuleFactory = () => bind(transactionTypes.TransactionMapperService).to( TransactionMapperService, ); + bind(transactionTypes.TransactionParserService).to( + TransactionParserService, + ); bind(transactionTypes.TransactionMappers).to(EthersV5TransactionMapper); bind(transactionTypes.TransactionMappers).to(EthersV6TransactionMapper); }, diff --git a/packages/signer/signer-eth/src/internal/transaction/di/transactionTypes.ts b/packages/signer/signer-eth/src/internal/transaction/di/transactionTypes.ts index d4af26bb2..f51fa13d9 100644 --- a/packages/signer/signer-eth/src/internal/transaction/di/transactionTypes.ts +++ b/packages/signer/signer-eth/src/internal/transaction/di/transactionTypes.ts @@ -1,5 +1,6 @@ export const transactionTypes = { SignTransactionUseCase: Symbol.for("SignTransactionUseCase"), + TransactionParserService: Symbol.for("TransactionParserService"), TransactionMapperService: Symbol.for("TransactionMapperService"), TransactionMappers: Symbol.for("TransactionMappers"), }; diff --git a/packages/signer/signer-eth/src/internal/transaction/service/parser/TransactionParserService.ts b/packages/signer/signer-eth/src/internal/transaction/service/parser/TransactionParserService.ts index 47a47a6d3..5ec6e5d0a 100644 --- a/packages/signer/signer-eth/src/internal/transaction/service/parser/TransactionParserService.ts +++ b/packages/signer/signer-eth/src/internal/transaction/service/parser/TransactionParserService.ts @@ -15,6 +15,7 @@ import { hexaStringToBuffer, } from "@ledgerhq/device-management-kit"; import { Transaction } from "ethers-v6"; +import { injectable } from "inversify"; import { Either, Left, Maybe, Right } from "purify-ts"; /** @@ -72,6 +73,7 @@ import { Either, Left, Maybe, Right } from "purify-ts"; const SELECTOR_LENGTH = 4; const CHUNK_SIZE = 32; +@injectable() export class TransactionParserService { public extractValue( serializedTransaction: Uint8Array,