Skip to content

Commit

Permalink
✨ (keyring-eth) [DSDK-537]: Reconstruct V for legacy transactions (#457)
Browse files Browse the repository at this point in the history
  • Loading branch information
@paoun-ledger
paoun-ledger authored Oct 29, 2024
2 parents 8765d42 + baa0206 commit 1c8113b
Show file tree
Hide file tree
Showing 13 changed files with 229 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/metal-wombats-relate.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ledgerhq/device-signer-kit-ethereum": patch
---

Reconstruct V full value for legacy transactions
4 changes: 3 additions & 1 deletion packages/signer/keyring-eth/src/api/app-binder/SignTransactionDeviceActionTypes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from "@ledgerhq/device-management-kit";

import { Signature } from "@api/model/Signature";
import { Transaction } from "@api/model/Transaction";
import { Transaction, TransactionType } from "@api/model/Transaction";
import { TransactionOptions } from "@api/model/TransactionOptions";
import { ProvideTransactionContextTaskErrorCodes } from "@internal/app-binder/task/ProvideTransactionContextTask";
import { TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService";
Expand Down Expand Up @@ -51,6 +51,8 @@ export type SignTransactionDAInternalState = {
readonly challenge: string | null;
readonly clearSignContexts: ClearSignContextSuccess[] | null;
readonly serializedTransaction: Uint8Array | null;
readonly chainId: number | null;
readonly transactionType: TransactionType | null;
readonly signature: Signature | null;
};

Expand Down
11 changes: 11 additions & 0 deletions packages/signer/keyring-eth/src/api/model/Transaction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,14 @@ import { type Transaction as EthersV5Transaction } from "ethers-v5";
import { type Transaction as EthersV6Transaction } from "ethers-v6";

export type Transaction = EthersV5Transaction | EthersV6Transaction;

/**
* The ethereum transaction type as according to eip-2718 transactions
* https://github.com/ethereum/EIPs/blob/master/EIPS/eip-2718.md
*/
export enum TransactionType {
LEGACY = 0,
EIP2930 = 1,
EIP1559 = 2,
EIP4844 = 3,
}
12 changes: 11 additions & 1 deletion ...-eth/src/internal/app-binder/device-action/SignTransaction/SignTransactionDeviceAction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
SignTransactionDAOutput,
} from "@api/app-binder/SignTransactionDeviceActionTypes";
import { Signature } from "@api/model/Signature";
import { Transaction } from "@api/model/Transaction";
import { Transaction, TransactionType } from "@api/model/Transaction";
import { TransactionOptions } from "@api/model/TransactionOptions";
import {
GetChallengeCommand,
Expand Down Expand Up @@ -65,6 +65,8 @@ export type MachineDependencies = {
input: {
derivationPath: string;
serializedTransaction: Uint8Array;
chainId: number;
transactionType: TransactionType;
isLegacy: boolean;
};
}) => Promise<CommandResult<Signature>>;
Expand Down Expand Up @@ -137,6 +139,8 @@ export class SignTransactionDeviceAction extends XStateDeviceAction<
error: null,
clearSignContexts: null,
serializedTransaction: null,
chainId: null,
transactionType: null,
challenge: null,
signature: null,
},
Expand Down Expand Up @@ -243,6 +247,8 @@ export class SignTransactionDeviceAction extends XStateDeviceAction<
...context._internalState,
clearSignContexts: event.output.clearSignContexts!,
serializedTransaction: event.output.serializedTransaction,
chainId: event.output.chainId,
transactionType: event.output.transactionType,
}),
}),
],
Expand Down Expand Up @@ -309,6 +315,8 @@ export class SignTransactionDeviceAction extends XStateDeviceAction<
derivationPath: context.input.derivationPath,
serializedTransaction:
context._internalState.serializedTransaction!,
chainId: context._internalState.chainId!,
transactionType: context._internalState.transactionType!,
isLegacy: true, // TODO: use ETHEREUM app version to determine if legacy
}),
onDone: {
Expand Down Expand Up @@ -379,6 +387,8 @@ export class SignTransactionDeviceAction extends XStateDeviceAction<
input: {
derivationPath: string;
serializedTransaction: Uint8Array;
chainId: number;
transactionType: TransactionType;
isLegacy: boolean;
};
}) => new SendSignTransactionTask(internalApi, arg0.input).run();
Expand Down
11 changes: 11 additions & 0 deletions packages/signer/keyring-eth/src/internal/app-binder/task/BuildTransactionContextTask.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ describe("BuildTransactionContextTask", () => {
const mapperResult: TransactionMapperResult = {
subset: { chainId: 1, to: undefined, data: "0x" },
serializedTransaction,
type: 0,
};
mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult));
contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts);
Expand All @@ -62,6 +63,8 @@ describe("BuildTransactionContextTask", () => {
expect(result).toEqual({
clearSignContexts,
serializedTransaction,
chainId: 1,
transactionType: 0,
});
});

Expand All @@ -81,6 +84,7 @@ describe("BuildTransactionContextTask", () => {
const mapperResult: TransactionMapperResult = {
subset: { chainId: 1, to: undefined, data: "0x" },
serializedTransaction,
type: 2,
};
mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult));
contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts);
Expand All @@ -92,6 +96,8 @@ describe("BuildTransactionContextTask", () => {
expect(result).toEqual({
clearSignContexts,
serializedTransaction,
chainId: 1,
transactionType: 2,
});
});

Expand All @@ -102,6 +108,7 @@ describe("BuildTransactionContextTask", () => {
const mapperResult: TransactionMapperResult = {
subset: { chainId: 1, to: undefined, data: "0x" },
serializedTransaction,
type: 0,
};
mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult));
contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts);
Expand All @@ -122,6 +129,7 @@ describe("BuildTransactionContextTask", () => {
const mapperResult: TransactionMapperResult = {
subset: { chainId: 1, to: undefined, data: "0x" },
serializedTransaction,
type: 0,
};
mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult));
contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts);
Expand Down Expand Up @@ -173,6 +181,7 @@ describe("BuildTransactionContextTask", () => {
const mapperResult: TransactionMapperResult = {
subset: { chainId: 1, to: undefined, data: "0x" },
serializedTransaction,
type: 0,
};
mapperMock.mapTransactionToSubset.mockReturnValueOnce(Right(mapperResult));
contextModuleMock.getContexts.mockResolvedValueOnce(clearSignContexts);
Expand All @@ -184,6 +193,8 @@ describe("BuildTransactionContextTask", () => {
expect(result).toEqual({
clearSignContexts: [clearSignContexts[1], clearSignContexts[3]],
serializedTransaction,
chainId: 1,
transactionType: 0,
});
});
});
8 changes: 6 additions & 2 deletions packages/signer/keyring-eth/src/internal/app-binder/task/BuildTransactionContextTask.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ import {
ContextModule,
} from "@ledgerhq/context-module";

import { Transaction } from "@api/model/Transaction";
import { Transaction, TransactionType } from "@api/model/Transaction";
import { TransactionOptions } from "@api/model/TransactionOptions";
import { TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService";

export type BuildTransactionTaskResult = {
readonly clearSignContexts: ClearSignContextSuccess[];
readonly serializedTransaction: Uint8Array;
readonly chainId: number;
readonly transactionType: TransactionType;
};

export type BuildTransactionContextTaskArgs = {
Expand All @@ -30,7 +32,7 @@ export class BuildTransactionContextTask {
parsed.ifLeft((err) => {
throw err;
});
const { subset, serializedTransaction } = parsed.unsafeCoerce();
const { subset, serializedTransaction, type } = parsed.unsafeCoerce();

const clearSignContexts = await contextModule.getContexts({
challenge,
Expand All @@ -46,6 +48,8 @@ export class BuildTransactionContextTask {
return {
clearSignContexts: clearSignContextsSuccess,
serializedTransaction,
chainId: subset.chainId,
transactionType: type,
};
}
}
112 changes: 112 additions & 0 deletions packages/signer/keyring-eth/src/internal/app-binder/task/SendSignTransactionTask.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ describe("SendSignTransactionTask", () => {
const args = {
derivationPath: "44'/60'/0'/0/0",
serializedTransaction: SIMPLE_TRANSACTION,
chainId: 1,
transactionType: 1,
};
apiMock.sendCommand.mockResolvedValueOnce(resultOk);

Expand All @@ -110,6 +112,8 @@ describe("SendSignTransactionTask", () => {
const args = {
derivationPath: "44'/60'/0'/0/0",
serializedTransaction: BIG_TRANSACTION,
chainId: 1,
transactionType: 1,
isLegacy: true,
};
apiMock.sendCommand.mockResolvedValueOnce(resultNothing);
Expand Down Expand Up @@ -147,6 +151,8 @@ describe("SendSignTransactionTask", () => {
const args = {
derivationPath: "44'/60'/0'/0/0",
serializedTransaction: SIMPLE_TRANSACTION,
chainId: 1,
transactionType: 1,
isLegacy: true,
};
apiMock.sendCommand.mockResolvedValueOnce(resultNothing);
Expand Down Expand Up @@ -176,6 +182,8 @@ describe("SendSignTransactionTask", () => {
const args = {
derivationPath: "44'/60'/0'/0/0",
serializedTransaction: BIG_TRANSACTION,
chainId: 1,
transactionType: 1,
isLegacy: true,
};
apiMock.sendCommand.mockResolvedValueOnce(resultNothing);
Expand Down Expand Up @@ -213,5 +221,109 @@ describe("SendSignTransactionTask", () => {
new InvalidStatusWordError("An error"),
);
});

it("legacy transaction with small chainId", async () => {
// GIVEN
const args = {
derivationPath: "44'/60'/0'/0/0",
serializedTransaction: SIMPLE_TRANSACTION,
chainId: 56,
transactionType: 0,
};
apiMock.sendCommand.mockResolvedValueOnce(
CommandResultFactory({
data: Just({
v: 147,
r: "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
s: "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
}),
}),
);

// WHEN
const result = await new SendSignTransactionTask(apiMock, args).run();

// THEN
// eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((result as any).data.v).toStrictEqual(147);
});

it("legacy transaction with small chainId with positive parity", async () => {
// GIVEN
const args = {
derivationPath: "44'/60'/0'/0/0",
serializedTransaction: SIMPLE_TRANSACTION,
chainId: 56,
transactionType: 0,
};
apiMock.sendCommand.mockResolvedValueOnce(
CommandResultFactory({
data: Just({
v: 148,
r: "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
s: "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
}),
}),
);

// WHEN
const result = await new SendSignTransactionTask(apiMock, args).run();

// THEN
// eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((result as any).data.v).toStrictEqual(148);
});

it("legacy transaction with big chainId", async () => {
// GIVEN
const args = {
derivationPath: "44'/60'/0'/0/0",
serializedTransaction: SIMPLE_TRANSACTION,
chainId: 11297108109,
transactionType: 0,
};
apiMock.sendCommand.mockResolvedValueOnce(
CommandResultFactory({
data: Just({
v: 131,
r: "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
s: "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
}),
}),
);

// WHEN
const result = await new SendSignTransactionTask(apiMock, args).run();

// THEN
// eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((result as any).data.v).toStrictEqual(22594216253);
});

it("legacy transaction with big chainId with positive parity", async () => {
// GIVEN
const args = {
derivationPath: "44'/60'/0'/0/0",
serializedTransaction: SIMPLE_TRANSACTION,
chainId: 11297108109,
transactionType: 0,
};
apiMock.sendCommand.mockResolvedValueOnce(
CommandResultFactory({
data: Just({
v: 132,
r: "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
s: "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
}),
}),
);

// WHEN
const result = await new SendSignTransactionTask(apiMock, args).run();

// THEN
// eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((result as any).data.v).toStrictEqual(22594216254);
});
});
});
Loading

0 comments on commit 1c8113b

Please sign in to comment.