From e5ddcfedd9db347139a0d428fc4fe3e4bb5fc16b Mon Sep 17 00:00:00 2001 From: jiyuzhuang Date: Thu, 22 Aug 2024 15:13:53 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=8E=A8=20(signer):=20Improve=20code?= =?UTF-8?q?=20visibility?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../ExternalPluginContextLoader.test.ts | 57 ++++++++++--------- .../domain/ExternalPluginContextLoader.ts | 27 ++++++--- .../domain/ForwardDomainContextLoader.test.ts | 11 ++-- .../domain/ForwardDomainContextLoader.ts | 11 ++-- .../src/nft/domain/NftContextLoader.test.ts | 11 ++-- .../src/nft/domain/NftContextLoader.ts | 30 +++++++--- .../src/shared/model/ClearSignContext.ts | 16 +++++- .../token/domain/TokenContextLoader.test.ts | 12 +++- .../src/token/domain/TokenContextLoader.ts | 19 +++++-- .../command/ProvideDomainNameCommand.test.ts | 15 +++-- .../command/ProvideDomainNameCommand.ts | 11 ++-- .../ProvideNFTInformationCommand.test.ts | 6 +- .../command/ProvideNFTInformationCommand.ts | 7 +-- .../command/ProvideTokenInformationCommand.ts | 14 ++--- .../command/SendEIP712StructImplemCommand.ts | 2 +- .../command/SetExternalPluginCommand.test.ts | 16 ++++-- .../command/SetExternalPluginCommand.ts | 22 +++---- .../command/SetPluginCommand.test.ts | 6 +- .../app-binder/command/SetPluginCommand.ts | 7 +-- 19 files changed, 184 insertions(+), 116 deletions(-) diff --git a/packages/signer/context-module/src/external-plugin/domain/ExternalPluginContextLoader.test.ts b/packages/signer/context-module/src/external-plugin/domain/ExternalPluginContextLoader.test.ts index 88c3c5c04..4433eeb92 100644 --- a/packages/signer/context-module/src/external-plugin/domain/ExternalPluginContextLoader.test.ts +++ b/packages/signer/context-module/src/external-plugin/domain/ExternalPluginContextLoader.test.ts @@ -6,6 +6,7 @@ import { ExternalPluginDataSource } from "@/external-plugin/data/ExternalPluginD import { ExternalPluginContextLoader } from "@/external-plugin/domain/ExternalPluginContextLoader"; import { DappInfos } from "@/external-plugin/model/DappInfos"; import { SelectorDetails } from "@/external-plugin/model/SelectorDetails"; +import { ClearSignContextType } from "@/shared/model/ClearSignContext"; import { TransactionContext } from "@/shared/model/TransactionContext"; import { TokenDataSource } from "@/token/data/TokenDataSource"; @@ -135,7 +136,7 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "externalPlugin", + type: ClearSignContextType.EXTERNAL_PLUGIN, payload: "1234567890", }, ]); @@ -164,11 +165,11 @@ describe("ExternalPluginContextLoader", () => { expect(result).toEqual( expect.arrayContaining([ { - type: "externalPlugin", + type: ClearSignContextType.EXTERNAL_PLUGIN, payload: "1234567890", }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48", }, ]), @@ -200,11 +201,11 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error("error"), }, { - type: "externalPlugin", + type: ClearSignContextType.EXTERNAL_PLUGIN, payload: "1234567890", }, ]); @@ -234,15 +235,15 @@ describe("ExternalPluginContextLoader", () => { expect(result).toEqual( expect.arrayContaining([ { - type: "externalPlugin", + type: ClearSignContextType.EXTERNAL_PLUGIN, payload: "1234567890", }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48", }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xdAC17F958D2ee523a2206206994597C13D831ec7", }, ]), @@ -280,25 +281,25 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48", }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xdAC17F958D2ee523a2206206994597C13D831ec7", }, // fromToken.2 { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xB8c77482e45F1F44dE1745F52C74426C631bDD52", }, // fromToken.-1 { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xB8c77482e45F1F44dE1745F52C74426C631bDD52", }, { - type: "externalPlugin", + type: ClearSignContextType.EXTERNAL_PLUGIN, payload: "1234567890", }, ]); @@ -329,11 +330,11 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error("error"), }, { - type: "externalPlugin", + type: ClearSignContextType.EXTERNAL_PLUGIN, payload: "1234567890", }, ]); @@ -361,13 +362,13 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error( "[ContextModule] ExternalPluginContextLoader: Unable to parse abi", ), }, { - type: "externalPlugin", + type: ClearSignContextType.EXTERNAL_PLUGIN, payload: "1234567890", }, ]); @@ -395,7 +396,7 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error( "[ContextModule] ExternalPluginContextLoader: Unable to get address", ), @@ -429,7 +430,7 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new RangeError("out of result range"), }, ]); @@ -483,31 +484,31 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48", }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xdAC17F958D2ee523a2206206994597C13D831ec7", }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0x95aD61b0a150d79219dCF64E1E6Cc01f0B64C4cE", }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0x95aD61b0a150d79219dCF64E1E6Cc01f0B64C4cE", }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xB8c77482e45F1F44dE1745F52C74426C631bDD52", }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xB8c77482e45F1F44dE1745F52C74426C631bDD52", }, { - type: "externalPlugin", + type: ClearSignContextType.EXTERNAL_PLUGIN, payload: "1234567890", }, ]); @@ -528,7 +529,7 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error("error"), }, ]); @@ -547,7 +548,7 @@ describe("ExternalPluginContextLoader", () => { // THEN expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error("Invalid selector"), }, ]); diff --git a/packages/signer/context-module/src/external-plugin/domain/ExternalPluginContextLoader.ts b/packages/signer/context-module/src/external-plugin/domain/ExternalPluginContextLoader.ts index 4e262ae17..014a7b61c 100644 --- a/packages/signer/context-module/src/external-plugin/domain/ExternalPluginContextLoader.ts +++ b/packages/signer/context-module/src/external-plugin/domain/ExternalPluginContextLoader.ts @@ -6,7 +6,10 @@ import { Either, EitherAsync, Left, Right } from "purify-ts"; import type { ExternalPluginDataSource } from "@/external-plugin/data/ExternalPluginDataSource"; import { externalPluginTypes } from "@/external-plugin/di/externalPluginTypes"; import { ContextLoader } from "@/shared/domain/ContextLoader"; -import { ClearSignContext } from "@/shared/model/ClearSignContext"; +import { + ClearSignContext, + ClearSignContextType, +} from "@/shared/model/ClearSignContext"; import { TransactionContext } from "@/shared/model/TransactionContext"; import type { TokenDataSource } from "@/token/data/TokenDataSource"; import { tokenTypes } from "@/token/di/tokenTypes"; @@ -25,7 +28,7 @@ export class ExternalPluginContextLoader implements ContextLoader { this._tokenDataSource = tokenDataSource; } - async load(transaction: TransactionContext) { + async load(transaction: TransactionContext): Promise { if (!transaction.to || !transaction.data || transaction.data === "0x") { return []; } @@ -33,7 +36,12 @@ export class ExternalPluginContextLoader implements ContextLoader { const selector = transaction.data.slice(0, 10); if (!isHexaString(selector)) { - return [{ type: "error" as const, error: new Error("Invalid selector") }]; + return [ + { + type: ClearSignContextType.ERROR, + error: new Error("Invalid selector"), + }, + ]; } const eitherDappInfos = await this._externalPluginDataSource.getDappInfos({ @@ -52,7 +60,7 @@ export class ExternalPluginContextLoader implements ContextLoader { } const externalPluginContext: ClearSignContext = { - type: "externalPlugin", + type: ClearSignContextType.EXTERNAL_PLUGIN, payload: dappInfos.selectorDetails.serializedData.concat( dappInfos.selectorDetails.signature, ), @@ -68,7 +76,10 @@ export class ExternalPluginContextLoader implements ContextLoader { // but also the externalPluginContext because it is still valid if (decodedCallData.isLeft()) { return [ - { type: "error", error: decodedCallData.extract() }, + { + type: ClearSignContextType.ERROR, + error: decodedCallData.extract(), + }, externalPluginContext, ]; } @@ -93,15 +104,15 @@ export class ExternalPluginContextLoader implements ContextLoader { // map the payload or the error to a ClearSignContext const contexts: ClearSignContext[] = tokensPayload.map((eitherToken) => eitherToken.caseOf({ - Left: (error) => ({ type: "error", error }), - Right: (payload) => ({ type: "token", payload }), + Left: (error) => ({ type: ClearSignContextType.ERROR, error }), + Right: (payload) => ({ type: ClearSignContextType.TOKEN, payload }), }), ); return [...contexts, externalPluginContext]; }).caseOf({ // parse all errors into ClearSignContext - Left: (error) => [{ type: "error", error }], + Left: (error) => [{ type: ClearSignContextType.ERROR, error }], Right: (contexts) => contexts, }); } diff --git a/packages/signer/context-module/src/forward-domain/domain/ForwardDomainContextLoader.test.ts b/packages/signer/context-module/src/forward-domain/domain/ForwardDomainContextLoader.test.ts index 022a888a5..789ead0d6 100644 --- a/packages/signer/context-module/src/forward-domain/domain/ForwardDomainContextLoader.test.ts +++ b/packages/signer/context-module/src/forward-domain/domain/ForwardDomainContextLoader.test.ts @@ -2,6 +2,7 @@ import { Left, Right } from "purify-ts"; import { ForwardDomainDataSource } from "@/forward-domain/data/ForwardDomainDataSource"; import { ForwardDomainContextLoader } from "@/forward-domain/domain/ForwardDomainContextLoader"; +import { ClearSignContextType } from "@/shared/model/ClearSignContext"; import { TransactionContext } from "@/shared/model/TransactionContext"; describe("ForwardDomainContextLoader", () => { @@ -39,7 +40,7 @@ describe("ForwardDomainContextLoader", () => { expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error( "[ContextModule] ForwardDomainLoader: invalid domain", ), @@ -59,7 +60,7 @@ describe("ForwardDomainContextLoader", () => { expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error( "[ContextModule] ForwardDomainLoader: invalid domain", ), @@ -80,7 +81,7 @@ describe("ForwardDomainContextLoader", () => { expect(result).toEqual([ { - type: "domainName", + type: ClearSignContextType.DOMAIN_NAME, payload: "payload", }, ]); @@ -103,7 +104,9 @@ describe("ForwardDomainContextLoader", () => { const result = await loader.load(transaction); // THEN - expect(result).toEqual([{ type: "error", error: new Error("error") }]); + expect(result).toEqual([ + { type: ClearSignContextType.ERROR, error: new Error("error") }, + ]); }); }); }); diff --git a/packages/signer/context-module/src/forward-domain/domain/ForwardDomainContextLoader.ts b/packages/signer/context-module/src/forward-domain/domain/ForwardDomainContextLoader.ts index f4b7a7faf..769f548c6 100644 --- a/packages/signer/context-module/src/forward-domain/domain/ForwardDomainContextLoader.ts +++ b/packages/signer/context-module/src/forward-domain/domain/ForwardDomainContextLoader.ts @@ -3,7 +3,10 @@ import { inject, injectable } from "inversify"; import type { ForwardDomainDataSource } from "@/forward-domain/data/ForwardDomainDataSource"; import { forwardDomainTypes } from "@/forward-domain/di/forwardDomainTypes"; import { ContextLoader } from "@/shared/domain/ContextLoader"; -import { ClearSignContext } from "@/shared/model/ClearSignContext"; +import { + ClearSignContext, + ClearSignContextType, +} from "@/shared/model/ClearSignContext"; import { TransactionContext } from "@/shared/model/TransactionContext"; @injectable() @@ -29,7 +32,7 @@ export class ForwardDomainContextLoader implements ContextLoader { if (!this.isDomainValid(domain)) { return [ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error( "[ContextModule] ForwardDomainLoader: invalid domain", ), @@ -45,11 +48,11 @@ export class ForwardDomainContextLoader implements ContextLoader { return [ payload.caseOf({ Left: (error): ClearSignContext => ({ - type: "error", + type: ClearSignContextType.ERROR, error: error, }), Right: (value): ClearSignContext => ({ - type: "domainName", + type: ClearSignContextType.DOMAIN_NAME, payload: value, }), }), diff --git a/packages/signer/context-module/src/nft/domain/NftContextLoader.test.ts b/packages/signer/context-module/src/nft/domain/NftContextLoader.test.ts index b5c098c79..72c44fbcd 100644 --- a/packages/signer/context-module/src/nft/domain/NftContextLoader.test.ts +++ b/packages/signer/context-module/src/nft/domain/NftContextLoader.test.ts @@ -2,6 +2,7 @@ import { Left, Right } from "purify-ts"; import { NftDataSource } from "@/nft/data/NftDataSource"; import { NftContextLoader } from "@/nft/domain/NftContextLoader"; +import { ClearSignContextType } from "@/shared/model/ClearSignContext"; import { TransactionContext } from "@/shared/model/TransactionContext"; describe("NftContextLoader", () => { @@ -71,7 +72,7 @@ describe("NftContextLoader", () => { expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error("Invalid selector"), }, ]); @@ -88,7 +89,7 @@ describe("NftContextLoader", () => { expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error("error"), }, ]); @@ -106,7 +107,7 @@ describe("NftContextLoader", () => { expect(result).toEqual([ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error("error"), }, ]); @@ -124,11 +125,11 @@ describe("NftContextLoader", () => { expect(result).toEqual([ { - type: "plugin", + type: ClearSignContextType.PLUGIN, payload: "payload1", }, { - type: "nft", + type: ClearSignContextType.NFT, payload: "payload2", }, ]); diff --git a/packages/signer/context-module/src/nft/domain/NftContextLoader.ts b/packages/signer/context-module/src/nft/domain/NftContextLoader.ts index a8fd3bc44..95846f5ef 100644 --- a/packages/signer/context-module/src/nft/domain/NftContextLoader.ts +++ b/packages/signer/context-module/src/nft/domain/NftContextLoader.ts @@ -4,7 +4,10 @@ import { inject, injectable } from "inversify"; import type { NftDataSource } from "@/nft/data/NftDataSource"; import { nftTypes } from "@/nft/di/nftTypes"; import { ContextLoader } from "@/shared/domain/ContextLoader"; -import { ClearSignContext } from "@/shared/model/ClearSignContext"; +import { + ClearSignContext, + ClearSignContextType, +} from "@/shared/model/ClearSignContext"; import { TransactionContext } from "@/shared/model/TransactionContext"; enum ERC721_SUPPORTED_SELECTOR { @@ -44,7 +47,12 @@ export class NftContextLoader implements ContextLoader { const selector = transaction.data.slice(0, 10); if (!isHexaString(selector)) { - return [{ type: "error", error: new Error("Invalid selector") }]; + return [ + { + type: ClearSignContextType.ERROR, + error: new Error("Invalid selector"), + }, + ]; } if (!this.isSelectorSupported(selector)) { @@ -63,13 +71,16 @@ export class NftContextLoader implements ContextLoader { const pluginPayload = getPluginPayloadResponse.caseOf({ Left: (error): ClearSignContext => ({ - type: "error", + type: ClearSignContextType.ERROR, error, }), - Right: (value): ClearSignContext => ({ type: "plugin", payload: value }), + Right: (value): ClearSignContext => ({ + type: ClearSignContextType.PLUGIN, + payload: value, + }), }); - if (pluginPayload.type === "error") { + if (pluginPayload.type === ClearSignContextType.ERROR) { return [pluginPayload]; } @@ -83,13 +94,16 @@ export class NftContextLoader implements ContextLoader { const nftInfosPayload = getNftInfosPayloadResponse.caseOf({ Left: (error): ClearSignContext => ({ - type: "error", + type: ClearSignContextType.ERROR, error, }), - Right: (value): ClearSignContext => ({ type: "nft", payload: value }), + Right: (value): ClearSignContext => ({ + type: ClearSignContextType.NFT, + payload: value, + }), }); - if (nftInfosPayload.type === "error") { + if (nftInfosPayload.type === ClearSignContextType.ERROR) { return [nftInfosPayload]; } diff --git a/packages/signer/context-module/src/shared/model/ClearSignContext.ts b/packages/signer/context-module/src/shared/model/ClearSignContext.ts index 52a03889f..78a039d17 100644 --- a/packages/signer/context-module/src/shared/model/ClearSignContext.ts +++ b/packages/signer/context-module/src/shared/model/ClearSignContext.ts @@ -1,10 +1,22 @@ +export enum ClearSignContextType { + TOKEN = "token", + NFT = "nft", + DOMAIN_NAME = "domainName", + PLUGIN = "plugin", + EXTERNAL_PLUGIN = "externalPlugin", + ERROR = "error", +} + export type ClearSignContextSuccess = { - type: "token" | "nft" | "domainName" | "plugin" | "externalPlugin"; + type: Exclude; + /** + * Hexadecimal string representation of the payload. + */ payload: string; }; export type ClearSignContextError = { - type: "error"; + type: ClearSignContextType.ERROR; error: Error; }; diff --git a/packages/signer/context-module/src/token/domain/TokenContextLoader.test.ts b/packages/signer/context-module/src/token/domain/TokenContextLoader.test.ts index 7e1e23640..5f1cc09a1 100644 --- a/packages/signer/context-module/src/token/domain/TokenContextLoader.test.ts +++ b/packages/signer/context-module/src/token/domain/TokenContextLoader.test.ts @@ -1,5 +1,6 @@ import { Left, Right } from "purify-ts"; +import { ClearSignContextType } from "@/shared/model/ClearSignContext"; import { TransactionContext } from "@/shared/model/TransactionContext"; import { TokenDataSource } from "@/token/data/TokenDataSource"; import { TokenContextLoader } from "@/token/domain/TokenContextLoader"; @@ -85,7 +86,10 @@ describe("TokenContextLoader", () => { // THEN expect(result).toEqual([ - { type: "error", error: new Error("Invalid selector") }, + { + type: ClearSignContextType.ERROR, + error: new Error("Invalid selector"), + }, ]); }); @@ -104,7 +108,9 @@ describe("TokenContextLoader", () => { const result = await loader.load(transaction); // THEN - expect(result).toEqual([{ type: "error", error: new Error("error") }]); + expect(result).toEqual([ + { type: ClearSignContextType.ERROR, error: new Error("error") }, + ]); }); it("should return a correct response", async () => { @@ -121,7 +127,7 @@ describe("TokenContextLoader", () => { // THEN expect(result).toEqual([ { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-0xdAC17F958D2ee523a2206206994597C13D831ec7", }, ]); diff --git a/packages/signer/context-module/src/token/domain/TokenContextLoader.ts b/packages/signer/context-module/src/token/domain/TokenContextLoader.ts index 9ce08bf4e..b2f589f50 100644 --- a/packages/signer/context-module/src/token/domain/TokenContextLoader.ts +++ b/packages/signer/context-module/src/token/domain/TokenContextLoader.ts @@ -2,7 +2,10 @@ import { HexaString, isHexaString } from "@ledgerhq/device-sdk-core"; import { inject, injectable } from "inversify"; import { ContextLoader } from "@/shared/domain/ContextLoader"; -import { ClearSignContext } from "@/shared/model/ClearSignContext"; +import { + ClearSignContext, + ClearSignContextType, +} from "@/shared/model/ClearSignContext"; import { TransactionContext } from "@/shared/model/TransactionContext"; import type { TokenDataSource } from "@/token/data/TokenDataSource"; import { tokenTypes } from "@/token/di/tokenTypes"; @@ -32,7 +35,12 @@ export class TokenContextLoader implements ContextLoader { const selector = transaction.data.slice(0, 10); if (!isHexaString(selector)) { - return [{ type: "error", error: new Error("Invalid selector") }]; + return [ + { + type: ClearSignContextType.ERROR, + error: new Error("Invalid selector"), + }, + ]; } if (!this.isSelectorSupported(selector)) { @@ -47,10 +55,13 @@ export class TokenContextLoader implements ContextLoader { return [ payload.caseOf({ Left: (error): ClearSignContext => ({ - type: "error", + type: ClearSignContextType.ERROR, error, }), - Right: (value): ClearSignContext => ({ type: "token", payload: value }), + Right: (value): ClearSignContext => ({ + type: ClearSignContextType.TOKEN, + payload: value, + }), }), ]; } diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.test.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.test.ts index 0de4c64f4..2a87c04f0 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.test.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.test.ts @@ -17,8 +17,8 @@ describe("ProvideDomainNameCommand", () => { it("should return the raw APDU", () => { // GIVEN const args: ProvideDomainNameCommandArgs = { - data: "00064C6564676572", - index: 0, + data: FIRST_CHUNK_APDU.slice(5), + isFirstChunk: true, }; // WHEN const command = new ProvideDomainNameCommand(args); @@ -36,7 +36,10 @@ describe("ProvideDomainNameCommand", () => { statusCode: Buffer.from([0x6a, 0x80]), // Invalid status code }; // WHEN - const command = new ProvideDomainNameCommand({ data: "", index: 0 }); + const command = new ProvideDomainNameCommand({ + data: Uint8Array.from([]), + isFirstChunk: true, + }); const result = command.parseResponse(response); // THEN expect(isSuccessCommandResult(result)).toBe(false); @@ -49,8 +52,12 @@ describe("ProvideDomainNameCommand", () => { statusCode: Buffer.from([0x90, 0x00]), // Success status code }; // WHEN - const command = new ProvideDomainNameCommand({ data: "", index: 0 }); + const command = new ProvideDomainNameCommand({ + data: Uint8Array.from([]), + isFirstChunk: true, + }); const result = command.parseResponse(response); + // THEN expect(isSuccessCommandResult(result)).toBe(true); }); }); diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.ts index 08b53b37e..f48d3658d 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.ts @@ -17,11 +17,11 @@ export type ProvideDomainNameCommandArgs = { * If the index equals 0, the first two bytes are the length of the domain name, else all the bytes are the chunk data. * @example "00064C6564676572" (hexa for "Ledger", first chunk and only chunk) */ - data: string; + data: Uint8Array; /** * The index of the chunk. */ - index: number; + isFirstChunk: boolean; }; /** @@ -30,19 +30,18 @@ export type ProvideDomainNameCommandArgs = { export class ProvideDomainNameCommand implements Command { - constructor(private args: ProvideDomainNameCommandArgs) {} + constructor(private readonly args: ProvideDomainNameCommandArgs) {} getApdu(): Apdu { - const isFirstChunk = this.args.index === 0; const apduBuilderArgs: ApduBuilderArgs = { cla: 0xe0, ins: 0x22, - p1: isFirstChunk ? 0x01 : 0x00, + p1: this.args.isFirstChunk ? 0x01 : 0x00, p2: 0x00, }; return new ApduBuilder(apduBuilderArgs) - .addHexaStringToData(this.args.data) + .addBufferToData(this.args.data) .build(); } diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.test.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.test.ts index c3f15dbe1..496a35318 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.test.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.test.ts @@ -29,7 +29,7 @@ describe("ProvideNFTInformationCommand", () => { it("should return the raw APDU", () => { // GIVEN const args: ProvideNFTInformationCommandArgs = { - data: NFT_INFORMATION_PAYLOAD, + payload: NFT_INFORMATION_PAYLOAD, }; // WHEN const command = new ProvideNFTInformationCommand(args); @@ -47,7 +47,7 @@ describe("ProvideNFTInformationCommand", () => { statusCode: Buffer.from([0x6d, 0x00]), // Invalid status code }; // WHEN - const command = new ProvideNFTInformationCommand({ data: "" }); + const command = new ProvideNFTInformationCommand({ payload: "" }); const result = command.parseResponse(response); // THEN expect(isSuccessCommandResult(result)).toBe(false); @@ -60,7 +60,7 @@ describe("ProvideNFTInformationCommand", () => { statusCode: Buffer.from([0x90, 0x00]), // Success status code }; // WHEN - const command = new ProvideNFTInformationCommand({ data: "" }); + const command = new ProvideNFTInformationCommand({ payload: "" }); const result = command.parseResponse(response); // THEN expect(isSuccessCommandResult(result)).toBe(true); diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.ts index a17f7987f..3b3395801 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.ts @@ -20,7 +20,7 @@ export type ProvideNFTInformationCommandArgs = { /** * The stringified hexa representation of the NFT data. */ - data: string; + payload: string; }; export type ProvideNFTInformationCommandErrorCodes = "6d00"; @@ -44,7 +44,7 @@ export class ProvideNFTInformationCommand ProvideNFTInformationCommandErrorCodes > { - constructor(private args: ProvideNFTInformationCommandArgs) {} + constructor(private readonly args: ProvideNFTInformationCommandArgs) {} getApdu(): Apdu { const apduBuilderArgs: ApduBuilderArgs = { @@ -53,9 +53,8 @@ export class ProvideNFTInformationCommand p1: 0x00, p2: 0x00, }; - return new ApduBuilder(apduBuilderArgs) - .addHexaStringToData(this.args.data) + .addHexaStringToData(this.args.payload) .build(); } diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideTokenInformationCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideTokenInformationCommand.ts index e6a1607fd..71edc9333 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideTokenInformationCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideTokenInformationCommand.ts @@ -2,7 +2,7 @@ import { Apdu, ApduBuilder, - ApduBuilderArgs, + type ApduBuilderArgs, ApduParser, ApduResponse, Command, @@ -28,11 +28,7 @@ export class ProvideTokenInformationCommand ProvideTokenInformationCommandArgs > { - args: ProvideTokenInformationCommandArgs; - - constructor(args: ProvideTokenInformationCommandArgs) { - this.args = args; - } + constructor(private readonly args: ProvideTokenInformationCommandArgs) {} getApdu(): Apdu { const getEthAddressArgs: ApduBuilderArgs = { @@ -41,9 +37,9 @@ export class ProvideTokenInformationCommand p1: 0x00, p2: 0x00, }; - const builder = new ApduBuilder(getEthAddressArgs); - builder.addHexaStringToData(this.args.payload); - return builder.build(); + return new ApduBuilder(getEthAddressArgs) + .addHexaStringToData(this.args.payload) + .build(); } parseResponse( diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SendEIP712StructImplemCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SendEIP712StructImplemCommand.ts index 5ca6f12c9..156c3ecb2 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SendEIP712StructImplemCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SendEIP712StructImplemCommand.ts @@ -31,7 +31,7 @@ export type SendEIP712StructImplemCommandArgs = value: { /** * The chunk of the data that is ready to send, that is to say, prefixed by its length in two bytes. - * Eg. 01020304 => [0x00, 0x04, 0x01, 0x02, 0x03, 0x04] where 0x00, 0x04 are the length of the data. + * Eg. 01020304 => [0x00, 0x04, 0x01, 0x02, 0x03, 0x04] where [0x00, 0x04] are the length of the data. */ data: Uint8Array; isLastChunk: boolean; diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.test.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.test.ts index 2d7d4d454..54a003b51 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.test.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.test.ts @@ -44,8 +44,12 @@ describe("Set External plugin", () => { it("should retrieve correct apdu", () => { // given const command = new SetExternalPluginCommand({ - payload: Uint8Array.from(SET_EXTERNAL_PLUGIN_PAYLOAD), - signature: Uint8Array.from(SET_EXTERNAL_PLUGIN_SIGNATURE), + payload: SET_EXTERNAL_PLUGIN_PAYLOAD.map((x) => + x.toString(16).padStart(2, "0"), + ).join(""), + signature: SET_EXTERNAL_PLUGIN_SIGNATURE.map((x) => + x.toString(16).padStart(2, "0"), + ).join(""), }); // when const apdu = command.getApdu(); @@ -86,8 +90,8 @@ describe("Set External plugin", () => { it("should return a global error", () => { // given const command = new SetExternalPluginCommand({ - payload: Uint8Array.from([]), - signature: Uint8Array.from([]), + payload: "", + signature: "", }); // when const apduResponse = new ApduResponse({ @@ -105,8 +109,8 @@ describe("Set External plugin", () => { it("should return void if status is success", () => { // given const command = new SetExternalPluginCommand({ - payload: Uint8Array.from([]), - signature: Uint8Array.from([]), + payload: "", + signature: "", }); // when const apduResponse = new ApduResponse({ diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.ts index 025beeaf4..a428fe550 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.ts @@ -1,9 +1,8 @@ // https://github.com/LedgerHQ/app-ethereum/blob/develop/doc/ethapp.adoc#set-external-plugin - import { Apdu, ApduBuilder, - ApduBuilderArgs, + type ApduBuilderArgs, ApduParser, ApduResponse, Command, @@ -18,8 +17,8 @@ import { } from "@ledgerhq/device-sdk-core"; type SetExternalPluginCommandArgs = { - payload: Uint8Array; - signature: Uint8Array; + payload: string; + signature?: string; }; type SetExternalPluginCommandErrorCodes = "6a80" | "6984" | "6d00"; @@ -51,18 +50,21 @@ export class SetExternalPluginCommand constructor(private readonly args: SetExternalPluginCommandArgs) {} getApdu(): Apdu { - const { payload, signature } = this.args; const setExternalPluginBuilderArgs: ApduBuilderArgs = { cla: 0xe0, ins: 0x12, p1: 0x00, p2: 0x00, }; - const builder = new ApduBuilder(setExternalPluginBuilderArgs); - builder.addBufferToData(payload); - builder.addBufferToData(signature); - - return builder.build(); + return ( + new ApduBuilder(setExternalPluginBuilderArgs) + .addHexaStringToData(this.args.payload) + /** + * The signature is normally integrated in the payload, but keeping this step for safety reasons and will be removed in the future. + */ + .addHexaStringToData(this.args.signature ?? "") + .build() + ); } parseResponse( diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.test.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.test.ts index 1bd683dd1..3a60082af 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.test.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.test.ts @@ -30,7 +30,7 @@ describe("SetPluginCommand", () => { it("returns the correct APDU", () => { // GIVEN const args: SetPluginCommandArgs = { - data: SET_PLUGIN_COMMAND_PAYLOAD, + payload: SET_PLUGIN_COMMAND_PAYLOAD, }; // WHEN const command = new SetPluginCommand(args); @@ -52,7 +52,7 @@ describe("SetPluginCommand", () => { data: Uint8Array.from([]), statusCode: apduResponseCode, }); - const command = new SetPluginCommand({ data: "" }); + const command = new SetPluginCommand({ payload: "" }); // WHEN const result = command.parseResponse(response); // THEN @@ -71,7 +71,7 @@ describe("SetPluginCommand", () => { statusCode: Buffer.from([0x90, 0x00]), // Success status code }; // WHEN - const command = new SetPluginCommand({ data: "" }); + const command = new SetPluginCommand({ payload: "" }); const result = command.parseResponse(response); // THEN expect(isSuccessCommandResult(result)).toBe(true); diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.ts index 68604940c..feb93a327 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.ts @@ -27,7 +27,7 @@ export type SetPluginCommandArgs = { /** * The stringified hexa representation of the plugin signature. */ - data: string; + payload: string; }; export class SetPluginCommandError extends DeviceExchangeError { @@ -39,7 +39,7 @@ export class SetPluginCommandError extends DeviceExchangeError { - constructor(private args: SetPluginCommandArgs) {} + constructor(private readonly args: SetPluginCommandArgs) {} getApdu(): Apdu { const apduBuilderArgs: ApduBuilderArgs = { @@ -48,9 +48,8 @@ export class SetPluginCommand p1: 0x00, p2: 0x00, }; - return new ApduBuilder(apduBuilderArgs) - .addHexaStringToData(this.args.data) + .addHexaStringToData(this.args.payload) .build(); } From 16b84b04413ad9602f1dad6b8229d8d0afec185b Mon Sep 17 00:00:00 2001 From: jiyuzhuang Date: Mon, 26 Aug 2024 13:03:43 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=E2=9C=A8=20(keyring-eth):=20Add=20ProvideT?= =?UTF-8?q?ransactionContextTask?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .changeset/hungry-parents-chew.md | 5 + .changeset/slow-eggs-act.md | 5 + .../command/ProvideDomainNameCommand.ts | 15 +- .../command/ProvideNFTInformationCommand.ts | 6 +- .../command/ProvideTokenInformationCommand.ts | 4 +- .../command/SendEIP712StructImplemCommand.ts | 2 +- .../command/SetExternalPluginCommand.test.ts | 32 +-- .../command/SetExternalPluginCommand.ts | 10 +- .../command/SetPluginCommand.test.ts | 16 +- .../app-binder/command/SetPluginCommand.ts | 8 +- .../task/BuildTransactionContextTask.test.ts | 17 +- .../ProvideTransactionContextTask.test.ts | 205 ++++++++++++++++++ .../task/ProvideTransactionContextTask.ts | 179 +++++++++++++++ .../task/SendEIP712StructImplemTask.ts | 4 +- 14 files changed, 456 insertions(+), 52 deletions(-) create mode 100644 .changeset/hungry-parents-chew.md create mode 100644 .changeset/slow-eggs-act.md create mode 100644 packages/signer/keyring-eth/src/internal/app-binder/task/ProvideTransactionContextTask.test.ts create mode 100644 packages/signer/keyring-eth/src/internal/app-binder/task/ProvideTransactionContextTask.ts diff --git a/.changeset/hungry-parents-chew.md b/.changeset/hungry-parents-chew.md new file mode 100644 index 000000000..977ef55dd --- /dev/null +++ b/.changeset/hungry-parents-chew.md @@ -0,0 +1,5 @@ +--- +"@ledgerhq/keyring-eth": patch +--- + +Implement ProvideTransactionContextTask diff --git a/.changeset/slow-eggs-act.md b/.changeset/slow-eggs-act.md new file mode 100644 index 000000000..f06298dfc --- /dev/null +++ b/.changeset/slow-eggs-act.md @@ -0,0 +1,5 @@ +--- +"@ledgerhq/context-module": patch +--- + +Improve code visibility and update command implementations diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.ts index f48d3658d..2575a6954 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideDomainNameCommand.ts @@ -5,25 +5,22 @@ import { type ApduBuilderArgs, ApduResponse, type Command, - CommandResult, + type CommandResult, CommandResultFactory, CommandUtils, GlobalCommandErrorHandler, } from "@ledgerhq/device-sdk-core"; export type ProvideDomainNameCommandArgs = { - /** - * The chunk of the stringified hexa representation of the domain name prefixed by its length in two bytes. - * If the index equals 0, the first two bytes are the length of the domain name, else all the bytes are the chunk data. - * @example "00064C6564676572" (hexa for "Ledger", first chunk and only chunk) - */ data: Uint8Array; - /** - * The index of the chunk. - */ isFirstChunk: boolean; }; +/** + * The length of the payload will take 2 bytes in the APDU. + */ +export const PAYLOAD_LENGTH_BYTES = 2; + /** * The command that provides a chunk of the domain name to the device. */ diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.ts index 3b3395801..5399f6971 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideNFTInformationCommand.ts @@ -6,9 +6,9 @@ import { ApduParser, ApduResponse, type Command, - CommandErrorArgs, - CommandErrors, - CommandResult, + type CommandErrorArgs, + type CommandErrors, + type CommandResult, CommandResultFactory, CommandUtils, DeviceExchangeError, diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideTokenInformationCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideTokenInformationCommand.ts index 71edc9333..cbe77ef8b 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideTokenInformationCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/ProvideTokenInformationCommand.ts @@ -5,8 +5,8 @@ import { type ApduBuilderArgs, ApduParser, ApduResponse, - Command, - CommandResult, + type Command, + type CommandResult, CommandResultFactory, CommandUtils, GlobalCommandErrorHandler, diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SendEIP712StructImplemCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SendEIP712StructImplemCommand.ts index 156c3ecb2..23e2414eb 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SendEIP712StructImplemCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SendEIP712StructImplemCommand.ts @@ -5,7 +5,7 @@ import { type ApduBuilderArgs, ApduResponse, type Command, - CommandResult, + type CommandResult, CommandResultFactory, CommandUtils, GlobalCommandErrorHandler, diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.test.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.test.ts index 54a003b51..f7b781b74 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.test.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.test.ts @@ -10,12 +10,13 @@ import { SetExternalPluginCommandError, } from "@internal/app-binder/command/SetExternalPluginCommand"; -/** Test payload contains: +/** + * Test payload contains: * Length of plugin name : 08 * Plugin Name : Paraswap * contract address: 0xdef171fe48cf0115b1d80b88dc8eab59176fee57 * method selector: 0xa9059cbb - * **/ + */ const SET_EXTERNAL_PLUGIN_PAYLOAD = [ 0x08, 0x50, 0x61, 0x72, 0x61, 0x73, 0x77, 0x61, 0x70, 0xde, 0xf1, 0x71, 0xfe, 0x48, 0xcf, 0x01, 0x15, 0xb1, 0xd8, 0x0b, 0x88, 0xdc, 0x8e, 0xab, 0x59, 0x17, @@ -67,24 +68,28 @@ describe("Set External plugin", () => { ${Uint8Array.from([0x6d, 0x00])} | ${"6d00"} `( "should return an error for the response status code $errorCode", - ({ apduResponseCode, errorCode }) => { + ({ + apduResponseCode, + errorCode, + }: Record<"apduResponseCode" | "errorCode", Uint8Array>) => { // GIVEN const response = new ApduResponse({ data: Uint8Array.from([]), statusCode: apduResponseCode, }); const command = new SetExternalPluginCommand({ - payload: Uint8Array.from([]), - signature: Uint8Array.from([]), + payload: "", + signature: "", }); // WHEN const result = command.parseResponse(response); // THEN expect(isSuccessCommandResult(result)).toBe(false); - // @ts-ignore - expect(result.error).toBeInstanceOf(SetExternalPluginCommandError); - // @ts-ignore - expect(result.error.errorCode).toStrictEqual(errorCode); + if (!isSuccessCommandResult(result)) { + expect(result.error).toBeInstanceOf(SetExternalPluginCommandError); + if (result.error instanceof SetExternalPluginCommandError) + expect(result.error.errorCode).toStrictEqual(errorCode); + } }, ); it("should return a global error", () => { @@ -101,10 +106,11 @@ describe("Set External plugin", () => { // then const result = command.parseResponse(apduResponse); expect(isSuccessCommandResult(result)).toBe(false); - // @ts-ignore - expect(result.error).toBeInstanceOf(GlobalCommandError); - // @ts-ignore - expect(result.error.errorCode).toStrictEqual("5515"); + if (!isSuccessCommandResult(result)) { + expect(result.error).toBeInstanceOf(GlobalCommandError); + if (result.error instanceof GlobalCommandError) + expect(result.error.errorCode).toStrictEqual("5515"); + } }); it("should return void if status is success", () => { // given diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.ts index a428fe550..f99cbc20f 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SetExternalPluginCommand.ts @@ -5,10 +5,10 @@ import { type ApduBuilderArgs, ApduParser, ApduResponse, - Command, - CommandErrorArgs, - CommandErrors, - CommandResult, + type Command, + type CommandErrorArgs, + type CommandErrors, + type CommandResult, CommandResultFactory, CommandUtils, DeviceExchangeError, @@ -21,7 +21,7 @@ type SetExternalPluginCommandArgs = { signature?: string; }; -type SetExternalPluginCommandErrorCodes = "6a80" | "6984" | "6d00"; +export type SetExternalPluginCommandErrorCodes = "6a80" | "6984" | "6d00"; const SET_EXTERNAL_PLUGIN_ERRORS: CommandErrors = { diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.test.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.test.ts index 3a60082af..d34079b38 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.test.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.test.ts @@ -5,7 +5,7 @@ import { import { SetPluginCommand, - SetPluginCommandArgs, + type SetPluginCommandArgs, SetPluginCommandError, } from "./SetPluginCommand"; @@ -46,7 +46,10 @@ describe("SetPluginCommand", () => { ${Uint8Array.from([0x6d, 0x00])} | ${"6d00"} `( "should return an error for the response status code $errorCode", - ({ apduResponseCode, errorCode }) => { + ({ + apduResponseCode, + errorCode, + }: Record<"apduResponseCode" | "errorCode", Uint8Array>) => { // GIVEN const response = new ApduResponse({ data: Uint8Array.from([]), @@ -57,10 +60,11 @@ describe("SetPluginCommand", () => { const result = command.parseResponse(response); // THEN expect(isSuccessCommandResult(result)).toBe(false); - // @ts-ignore - expect(result.error).toBeInstanceOf(SetPluginCommandError); - // @ts-ignore - expect(result.error.errorCode).toStrictEqual(errorCode); + if (!isSuccessCommandResult(result)) { + expect(result.error).toBeInstanceOf(SetPluginCommandError); + if (result.error instanceof SetPluginCommandError) + expect(result.error.errorCode).toStrictEqual(errorCode); + } }, ); diff --git a/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.ts b/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.ts index feb93a327..c5c2b9954 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/command/SetPluginCommand.ts @@ -6,9 +6,9 @@ import { ApduParser, ApduResponse, type Command, - CommandErrorArgs, - CommandErrors, - CommandResult, + type CommandErrorArgs, + type CommandErrors, + type CommandResult, CommandResultFactory, CommandUtils, DeviceExchangeError, @@ -16,7 +16,7 @@ import { isCommandErrorCode, } from "@ledgerhq/device-sdk-core"; -type SetPluginCommandErrorCodes = "6984" | "6d00"; +export type SetPluginCommandErrorCodes = "6984" | "6d00"; const SET_PLUGIN_ERRORS: CommandErrors = { "6984": { message: "The requested plugin is not installed on the device" }, diff --git a/packages/signer/keyring-eth/src/internal/app-binder/task/BuildTransactionContextTask.test.ts b/packages/signer/keyring-eth/src/internal/app-binder/task/BuildTransactionContextTask.test.ts index 7f6ed9f39..060d584f8 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/task/BuildTransactionContextTask.test.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/task/BuildTransactionContextTask.test.ts @@ -1,4 +1,7 @@ -import { ClearSignContext } from "@ledgerhq/context-module"; +import { + ClearSignContext, + ClearSignContextType, +} from "@ledgerhq/context-module"; import { Transaction } from "ethers-v6"; import { Left, Right } from "purify-ts"; @@ -61,11 +64,11 @@ describe("BuildTransactionContextTask", () => { const serializedTransaction = new Uint8Array([0x01, 0x02, 0x03]); const clearSignContexts: ClearSignContext[] = [ { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-1", }, { - type: "nft", + type: ClearSignContextType.NFT, payload: "payload-2", }, ]; @@ -169,19 +172,19 @@ describe("BuildTransactionContextTask", () => { const serializedTransaction = new Uint8Array([0x01, 0x02, 0x03]); const clearSignContexts: ClearSignContext[] = [ { - type: "error", + type: ClearSignContextType.ERROR, error: new Error("error"), }, { - type: "token", + type: ClearSignContextType.TOKEN, payload: "payload-1", }, { - type: "error", + type: ClearSignContextType.ERROR, error: new Error("error"), }, { - type: "nft", + type: ClearSignContextType.NFT, payload: "payload-2", }, ]; diff --git a/packages/signer/keyring-eth/src/internal/app-binder/task/ProvideTransactionContextTask.test.ts b/packages/signer/keyring-eth/src/internal/app-binder/task/ProvideTransactionContextTask.test.ts new file mode 100644 index 000000000..5a776d40b --- /dev/null +++ b/packages/signer/keyring-eth/src/internal/app-binder/task/ProvideTransactionContextTask.test.ts @@ -0,0 +1,205 @@ +import { ClearSignContextType } from "@ledgerhq/context-module"; +import { + CommandResultFactory, + UnknownDeviceExchangeError, +} from "@ledgerhq/device-sdk-core"; + +import { ProvideDomainNameCommand } from "@internal/app-binder/command/ProvideDomainNameCommand"; +import { ProvideNFTInformationCommand } from "@internal/app-binder/command/ProvideNFTInformationCommand"; +import { ProvideTokenInformationCommand } from "@internal/app-binder/command/ProvideTokenInformationCommand"; +import { SetExternalPluginCommand } from "@internal/app-binder/command/SetExternalPluginCommand"; +import { SetPluginCommand } from "@internal/app-binder/command/SetPluginCommand"; +import { makeDeviceActionInternalApiMock } from "@internal/app-binder/device-action/__test-utils__/makeInternalApi"; + +import { + type ErrorCodes, + ProvideTransactionContextTask, + type ProvideTransactionContextTaskArgs, +} from "./ProvideTransactionContextTask"; + +describe("ProvideTransactionContextTask", () => { + const api = makeDeviceActionInternalApiMock(); + const successResult = CommandResultFactory({ + data: undefined, + }); + const errorResult = CommandResultFactory({ + data: undefined, + error: {} as UnknownDeviceExchangeError, + }); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe("run", () => { + const args: ProvideTransactionContextTaskArgs = { + clearSignContexts: [ + { + type: ClearSignContextType.PLUGIN, + payload: "706c7567696e", // "plugin" + }, + { + type: ClearSignContextType.EXTERNAL_PLUGIN, + payload: "65787465726e616c506c7567696e", // "externalPlugin" + }, + { + type: ClearSignContextType.NFT, + payload: "6e6674", // "nft" + }, + { + type: ClearSignContextType.TOKEN, + payload: "746f6b656e", // "token" + }, + ], + }; + afterEach(() => { + jest.restoreAllMocks(); + }); + it("should send relative commands when receiving ClearSignContexts of type not domainName", async () => { + api.sendCommand.mockResolvedValue(successResult); + // GIVEN + const task = new ProvideTransactionContextTask(api, args); + // WHEN + await task.run(); + // THEN + expect(api.sendCommand).toHaveBeenCalledTimes(4); + expect(api.sendCommand).toHaveBeenNthCalledWith( + args.clearSignContexts.findIndex( + (c) => c.type === ClearSignContextType.PLUGIN, + ) + 1, + expect.objectContaining( + new SetPluginCommand({ payload: "706c7567696e" }), + ), + ); + expect(api.sendCommand).toHaveBeenNthCalledWith( + args.clearSignContexts.findIndex( + (c) => c.type === ClearSignContextType.EXTERNAL_PLUGIN, + ) + 1, + expect.objectContaining( + new SetExternalPluginCommand({ + payload: "65787465726e616c506c7567696e", + }), + ), + ); + expect(api.sendCommand).toHaveBeenNthCalledWith( + args.clearSignContexts.findIndex( + (c) => c.type === ClearSignContextType.NFT, + ) + 1, + expect.objectContaining( + new ProvideNFTInformationCommand({ payload: "6e6674" }), + ), + ); + expect(api.sendCommand).toHaveBeenNthCalledWith( + args.clearSignContexts.findIndex( + (c) => c.type === ClearSignContextType.TOKEN, + ) + 1, + expect.objectContaining( + new ProvideTokenInformationCommand({ payload: "746f6b656e" }), + ), + ); + }); + it("should return the command error result and stop when the command fails", async () => { + api.sendCommand.mockReset(); + api.sendCommand.mockResolvedValueOnce(errorResult); + // GIVEN + const task = new ProvideTransactionContextTask(api, args); + // WHEN + const result = await task.run(); + // THEN + expect(api.sendCommand).toHaveBeenCalledTimes(1); + expect(result.isJust()).toBe(true); + expect(result.extract()).toStrictEqual(errorResult); + }); + it("should call provideDomainNameTask when receiving a ClearSignContext of type domainName", async () => { + jest + .spyOn(ProvideTransactionContextTask.prototype, "provideDomainNameTask") + .mockResolvedValueOnce(CommandResultFactory({ data: undefined })); + // GIVEN + const task = new ProvideTransactionContextTask(api, { + clearSignContexts: [ + { + type: ClearSignContextType.DOMAIN_NAME, + payload: "646f6d61696e4e616d65", // "domainName" + }, + ], + }); + // WHEN + await task.run(); + // THEN + expect( + ProvideTransactionContextTask.prototype.provideDomainNameTask, + ).toHaveBeenCalledTimes(1); + expect( + ProvideTransactionContextTask.prototype.provideDomainNameTask, + ).toHaveBeenCalledWith("646f6d61696e4e616d65"); + }); + it("should return the command error result and stop when provideDomainNameTask fails", async () => { + jest + .spyOn(ProvideTransactionContextTask.prototype, "provideDomainNameTask") + .mockResolvedValueOnce( + CommandResultFactory({ + data: undefined, + error: {} as UnknownDeviceExchangeError, + }), + ); + // GIVEN + const task = new ProvideTransactionContextTask(api, { + clearSignContexts: [ + { + type: ClearSignContextType.DOMAIN_NAME, + payload: "646f6d61696e4e616d65", // "domainName" + }, + { + type: ClearSignContextType.PLUGIN, + payload: "706c7567696e", // "plugin" + }, + ], + }); + // WHEN + const result = await task.run(); + // THEN + expect(result.isJust()).toBe(true); + expect(result.extract()).toStrictEqual(errorResult); + }); + }); + + describe("provideDomainNameTask", () => { + it("should send the multiple ProvideDomainNameCommand to the device", async () => { + // GIVEN + api.sendCommand.mockResolvedValue(successResult); + const task = new ProvideTransactionContextTask(api, { + clearSignContexts: [], + }); + // WHEN + const domainName = "646f6d61696e4e616d65"; // "domainName" + await task.provideDomainNameTask(domainName); + // THEN + expect(api.sendCommand).toHaveBeenCalledTimes(1); + expect(api.sendCommand).toHaveBeenNthCalledWith( + 1, + expect.objectContaining( + new ProvideDomainNameCommand({ + data: Uint8Array.from([ + 0x00, 0x0a, 0x64, 0x6f, 0x6d, 0x61, 0x69, 0x6e, 0x4e, 0x61, 0x6d, + 0x65, + ]), + isFirstChunk: true, + }), + ), + ); + }); + it("should return the error and stop when command fails", async () => { + // GIVEN + api.sendCommand.mockResolvedValueOnce(errorResult); + const task = new ProvideTransactionContextTask(api, { + clearSignContexts: [], + }); + // WHEN + const domainName = "646f6d61696e4e616d65"; // "domainName" + const res = await task.provideDomainNameTask(domainName); + //THEN + expect(api.sendCommand).toHaveBeenCalledTimes(1); + expect(res).toStrictEqual(errorResult); + }); + }); +}); diff --git a/packages/signer/keyring-eth/src/internal/app-binder/task/ProvideTransactionContextTask.ts b/packages/signer/keyring-eth/src/internal/app-binder/task/ProvideTransactionContextTask.ts new file mode 100644 index 000000000..313fa2c2c --- /dev/null +++ b/packages/signer/keyring-eth/src/internal/app-binder/task/ProvideTransactionContextTask.ts @@ -0,0 +1,179 @@ +import { + type ClearSignContextSuccess, + ClearSignContextType, +} from "@ledgerhq/context-module"; +import { + APDU_MAX_PAYLOAD, + ByteArrayBuilder, + type CommandErrorResult, + CommandResult, + CommandResultFactory, + hexaStringToBuffer, + type InternalApi, + isSuccessCommandResult, + type SdkError, +} from "@ledgerhq/device-sdk-core"; +import { HexaStringEncodeError } from "@ledgerhq/device-sdk-core/src/api/apdu/utils/AppBuilderError.js"; +import { Just, Maybe, Nothing } from "purify-ts"; + +import { + PAYLOAD_LENGTH_BYTES, + ProvideDomainNameCommand, +} from "@internal/app-binder/command/ProvideDomainNameCommand"; +import { + ProvideNFTInformationCommand, + type ProvideNFTInformationCommandErrorCodes, +} from "@internal/app-binder/command/ProvideNFTInformationCommand"; +import { + ProvideTokenInformationCommand, + ProvideTokenInformationCommandResponse, +} from "@internal/app-binder/command/ProvideTokenInformationCommand"; +import { + SetExternalPluginCommand, + type SetExternalPluginCommandErrorCodes, +} from "@internal/app-binder/command/SetExternalPluginCommand"; +import { + SetPluginCommand, + type SetPluginCommandErrorCodes, +} from "@internal/app-binder/command/SetPluginCommand"; + +export type ProvideTransactionContextTaskArgs = { + /** + * The valid clear sign contexts offerred by the `BuildTrancationContextTask`. + */ + clearSignContexts: ClearSignContextSuccess[]; +}; + +/** + * Temporary error type to be used in the `ProvideTransactionContextTask` in order to not forget to handle the error cases. + */ +export class ProvideTransactionContextTaskError implements SdkError { + readonly _tag = "ProvideTransactionContextTaskError"; + readonly originalError: Error; + + constructor(message?: string) { + this.originalError = new Error( + message ?? "Unknow error in ProvideTransactionContextTaskError", + ); + } +} + +/** + * The exported type here is just for testing purposes, use the concret command error codes instead for the real implementation. + */ +export type ErrorCodes = + | void + | SetExternalPluginCommandErrorCodes + | SetPluginCommandErrorCodes + | ProvideNFTInformationCommandErrorCodes; + +/** + * This task is responsible for providing the transaction context to the device. + * It will send the 5 necessary commands: + * - `SetPluginCommand` (single command) + * - `SetExternalPluginCommand` (single command) + * - `ProvideNFTInformationCommand` (single command) + * - `ProvideTokenInformationCommand` (single command) + * - `ProvideDomainNameCommand` (__mulpitle commands__) + * + * The method `provideDomainNameTask` is dedicated to send the multiple `ProvideDomainNameCommand`. + */ +export class ProvideTransactionContextTask { + constructor( + private api: InternalApi, + private args: ProvideTransactionContextTaskArgs, + ) {} + + async run(): Promise>> { + for (const context of this.args.clearSignContexts) { + const res = await this.provideContext(context); + if (!isSuccessCommandResult(res)) { + return Just(res); + } + } + return Nothing; + } + + /** + * This method will send a command according to the clear sign context type and return the command result if only one command + * is sent, otherwise it will return the result of the `provideDomainNameTask`. + * + * @param context The clear sign context to provide. + * @returns A promise that resolves when the command is sent or result of the `provideDomainNameTask`. + */ + async provideContext({ + type, + payload, + }: ClearSignContextSuccess): Promise< + CommandResult + > { + switch (type) { + case ClearSignContextType.PLUGIN: { + return await this.api.sendCommand(new SetPluginCommand({ payload })); + } + case ClearSignContextType.EXTERNAL_PLUGIN: { + return await this.api.sendCommand( + new SetExternalPluginCommand({ payload }), + ); + } + case ClearSignContextType.NFT: { + return await this.api.sendCommand( + new ProvideNFTInformationCommand({ payload }), + ); + } + case ClearSignContextType.TOKEN: { + return await this.api.sendCommand( + new ProvideTokenInformationCommand({ payload }), + ); + } + case ClearSignContextType.DOMAIN_NAME: { + return await this.provideDomainNameTask(payload); + } + default: { + const uncoveredType: never = type; + throw new ProvideTransactionContextTaskError( + `The context type [${uncoveredType}] is not covered`, + ); + } + } + } + + /** + * This method is responsible for chunking the domain name if necessary and sending `ProvideDomainNameCommand` to the device. + * It will return the result of the last command sent if all the commands are successful, otherwise it will return the first + * error result encountered. + * + * @param domainName Hexa representation of the domain name. + * @returns A promise that resolves when the command is sent. + */ + async provideDomainNameTask( + domainName: string, + ): Promise> { + const buffer = hexaStringToBuffer(domainName); + + if (buffer === null || buffer.length === 0) { + throw new HexaStringEncodeError("provideDomainNameTask"); + } + + const data = new ByteArrayBuilder(buffer.length + PAYLOAD_LENGTH_BYTES) + .add16BitUIntToData(buffer.length) + .addBufferToData(buffer) + .build(); + + let result = CommandResultFactory({ data: undefined }); + + for (let i = 0; i < data.length; i += APDU_MAX_PAYLOAD) { + result = await this.api.sendCommand( + new ProvideDomainNameCommand({ + data: data.slice(i, i + APDU_MAX_PAYLOAD), + isFirstChunk: i === 0, + }), + ); + if (!isSuccessCommandResult(result)) { + return result; + } + } + + return result; + } +} diff --git a/packages/signer/keyring-eth/src/internal/app-binder/task/SendEIP712StructImplemTask.ts b/packages/signer/keyring-eth/src/internal/app-binder/task/SendEIP712StructImplemTask.ts index 0db0c1e35..bbb02555a 100644 --- a/packages/signer/keyring-eth/src/internal/app-binder/task/SendEIP712StructImplemTask.ts +++ b/packages/signer/keyring-eth/src/internal/app-binder/task/SendEIP712StructImplemTask.ts @@ -1,9 +1,9 @@ import { APDU_MAX_PAYLOAD, ByteArrayBuilder, - CommandResult, + type CommandResult, CommandResultFactory, - InternalApi, + type InternalApi, isSuccessCommandResult, } from "@ledgerhq/device-sdk-core";