Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ (signer-eth) [DSDK-617]: Remove ethers dependency from api and accept only raw tx #591

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/nine-readers-roll.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ledgerhq/device-signer-kit-ethereum": minor
---

Breaking change: remove ethers from api dependency and use only raw tx for signTransaction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if 'breaking change' wording is compatible to minor versioning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree but basically for signers we shouldn't have release v1.0 it was more 1.0-rc

5 changes: 5 additions & 0 deletions .changeset/tame-crabs-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ledgerhq/device-signer-kit-ethereum": patch
---

Update signTransaction transaction parameter with new api
1 change: 0 additions & 1 deletion apps/sample/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
"@ledgerhq/react-ui": "^0.17.0",
"@playwright/test": "^1.49.0",
"@sentry/nextjs": "^8.42.0",
"ethers": "6.13.4",
"next": "14.2.15",
"react": "^18.3.1",
"react-dom": "^18.3.1",
Expand Down
16 changes: 10 additions & 6 deletions apps/sample/src/components/SignerEthView/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useMemo } from "react";
import { hexaStringToBuffer } from "@ledgerhq/device-management-kit";
import {
type GetAddressDAError,
type GetAddressDAIntermediateValue,
Expand All @@ -14,7 +15,6 @@ import {
type SignTypedDataDAOutput,
type TypedData,
} from "@ledgerhq/device-signer-kit-ethereum";
import { ethers } from "ethers";

import { DeviceActionsList } from "@/components/DeviceActionsView/DeviceActionsList";
import { type DeviceActionProps } from "@/components/DeviceActionsView/DeviceActionTester";
Expand Down Expand Up @@ -103,11 +103,15 @@ export const SignerEthView: React.FC<{ sessionId: string }> = ({
if (!signer) {
throw new Error("Signer not initialized");
}
return signer.signTransaction(
derivationPath,
ethers.Transaction.from(transaction),
{ domain: recipientDomain },
);

const tx = hexaStringToBuffer(transaction);
if (!tx) {
throw new Error("Invalid transaction format");
}

return signer.signTransaction(derivationPath, tx, {
domain: recipientDomain,
});
},
initialValues: {
derivationPath: "44'/60'/0'/0/0",
Expand Down
3 changes: 1 addition & 2 deletions packages/signer/signer-eth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@
},
"dependencies": {
"@ledgerhq/signer-utils": "workspace:*",
"ethers-v5": "npm:ethers@^5.7.2",
"ethers-v6": "npm:ethers@6.13.4",
"ethers": "6.13.4",
"inversify": "^6.2.1",
"inversify-logger-middleware": "^3.1.0",
"purify-ts": "^2.1.0",
Expand Down
3 changes: 1 addition & 2 deletions packages/signer/signer-eth/src/api/SignerEth.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { type GetAddressDAReturnType } from "@api/app-binder/GetAddressDeviceActionTypes";
import { type SignTypedDataDAReturnType } from "@api/app-binder/SignTypedDataDeviceActionTypes";
import { type AddressOptions } from "@api/model/AddressOptions";
import { type Transaction } from "@api/model/Transaction";
import { type TransactionOptions } from "@api/model/TransactionOptions";
import { type TypedData } from "@api/model/TypedData";

Expand All @@ -11,7 +10,7 @@ import { type SignTransactionDAReturnType } from "./app-binder/SignTransactionDe
export interface SignerEth {
signTransaction: (
derivationPath: string,
transaction: Transaction,
transaction: Uint8Array,
options?: TransactionOptions,
) => SignTransactionDAReturnType;
signMessage: (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import {
} from "@ledgerhq/device-management-kit";

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 TransactionType } from "@api/model/TransactionType";
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";
Expand All @@ -23,7 +23,7 @@ export type SignTransactionDAOutput = Signature;

export type SignTransactionDAInput = {
readonly derivationPath: string;
readonly transaction: Transaction;
readonly transaction: Uint8Array;
readonly mapper: TransactionMapperService;
readonly parser: TransactionParserService;
readonly contextModule: ContextModule;
Expand Down
2 changes: 1 addition & 1 deletion packages/signer/signer-eth/src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export {
export * from "@api/model/Address";
export * from "@api/model/AddressOptions";
export * from "@api/model/Signature";
export * from "@api/model/Transaction";
export * from "@api/model/TransactionOptions";
export * from "@api/model/TransactionType";
export * from "@api/model/TypedData";
export * from "@api/SignerEth";
export * from "@api/SignerEthBuilder";
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { addressTypes } from "./address/di/addressTypes";
import { messageTypes } from "./message/di/messageTypes";
import { transactionTypes } from "./transaction/di/transactionTypes";
import { typedDataTypes } from "./typed-data/di/typedDataTypes";
import { type Transaction, type TypedData } from "..";
import { type TypedData } from "..";
import { DefaultSignerEth } from "./DefaultSignerEth";

describe("DefaultSignerEth", () => {
Expand Down Expand Up @@ -46,7 +46,7 @@ describe("DefaultSignerEth", () => {
it("should sign a transaction", async () => {
// GIVEN
const derivationPath = "derivationPath";
const transaction = {} as Transaction;
const transaction = new Uint8Array(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[COULD] I think the 0 parameter is not necessary to initialise a fake data, new Uint8Array() is OK, but not important.


// WHEN
const result = await signer.signTransaction(derivationPath, transaction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { type SignPersonalMessageDAReturnType } from "@api/app-binder/SignPerson
import { type SignTransactionDAReturnType } from "@api/app-binder/SignTransactionDeviceActionTypes";
import { type SignTypedDataDAReturnType } from "@api/app-binder/SignTypedDataDeviceActionTypes";
import { type AddressOptions } from "@api/model/AddressOptions";
import { type Transaction } from "@api/model/Transaction";
import { type TransactionOptions } from "@api/model/TransactionOptions";
import { type TypedData } from "@api/model/TypedData";
import { type SignerEth } from "@api/SignerEth";
Expand Down Expand Up @@ -39,7 +38,7 @@ export class DefaultSignerEth implements SignerEth {

signTransaction(
derivationPath: string,
transaction: Transaction,
transaction: Uint8Array,
options?: TransactionOptions,
): SignTransactionDAReturnType {
return this._container
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import { type ContextModule } from "@ledgerhq/context-module";
import {
type DeviceActionState,
type DeviceManagementKit,
hexaStringToBuffer,
} from "@ledgerhq/device-management-kit";
import { DeviceActionStatus } from "@ledgerhq/device-management-kit";
import { SendCommandInAppDeviceAction } from "@ledgerhq/device-management-kit";
import { UserInteractionRequired } from "@ledgerhq/device-management-kit";
import { Transaction } from "ethers-v6";
import { Transaction } from "ethers";
import { from } from "rxjs";

import {
Expand Down Expand Up @@ -199,9 +200,12 @@ describe("EthAppBinder", () => {
s: `0xBEEF`,
v: 0,
};
const transaction: Transaction = new Transaction();
transaction.to = "0x1234567890123456789012345678901234567890";
transaction.value = 0n;
const transaction: Uint8Array = hexaStringToBuffer(
Transaction.from({
to: "0x1234567890123456789012345678901234567890",
value: 0n,
}).unsignedSerialized,
)!;
const options = {};

jest.spyOn(mockedDmk, "executeDeviceAction").mockReturnValue({
Expand Down Expand Up @@ -268,9 +272,12 @@ describe("EthAppBinder", () => {
s: `0xBEEF`,
v: 0,
};
const transaction: Transaction = new Transaction();
transaction.to = "0x1234567890123456789012345678901234567890";
transaction.value = 0n;
const transaction: Uint8Array = hexaStringToBuffer(
Transaction.from({
to: "0x1234567890123456789012345678901234567890",
value: 0n,
}).unsignedSerialized,
)!;

jest.spyOn(mockedDmk, "executeDeviceAction").mockReturnValue({
observable: from([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { type GetAddressDAReturnType } from "@api/app-binder/GetAddressDeviceAct
import { type SignPersonalMessageDAReturnType } from "@api/app-binder/SignPersonalMessageDeviceActionTypes";
import { type SignTransactionDAReturnType } from "@api/app-binder/SignTransactionDeviceActionTypes";
import { type SignTypedDataDAReturnType } from "@api/app-binder/SignTypedDataDeviceActionTypes";
import { type Transaction } from "@api/model/Transaction";
import { type TransactionOptions } from "@api/model/TransactionOptions";
import { type TypedData } from "@api/model/TypedData";
import { SignTypedDataDeviceAction } from "@internal/app-binder/device-action/SignTypedData/SignTypedDataDeviceAction";
Expand Down Expand Up @@ -73,7 +72,7 @@ export class EthAppBinder {

signTransaction(args: {
derivationPath: string;
transaction: Transaction;
transaction: Uint8Array;
options?: TransactionOptions;
}): SignTransactionDAReturnType {
return this.dmk.executeDeviceAction({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ import { type ContextModule } from "@ledgerhq/context-module";
import {
CommandResultFactory,
DeviceActionStatus,
hexaStringToBuffer,
UnknownDAError,
UserInteractionRequired,
} from "@ledgerhq/device-management-kit";
import { InvalidStatusWordError } from "@ledgerhq/device-management-kit";
import { Transaction } from "ethers-v6";
import { Transaction } from "ethers";
import { Just, Nothing } from "purify-ts";

import { type SignTransactionDAState } from "@api/app-binder/SignTransactionDeviceActionTypes";
import { TransactionType } from "@api/model/Transaction";
import { TransactionType } from "@api/model/TransactionType";
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";
Expand Down Expand Up @@ -59,14 +60,16 @@ describe("SignTransactionDeviceAction", () => {
const defaultOptions = {
domain: "domain-name.eth",
};
let defaultTransaction: Transaction;
const defaultTransaction: Uint8Array = hexaStringToBuffer(
Transaction.from({
chainId: 1n,
nonce: 0,
data: "0x",
}).unsignedSerialized,
)!;

beforeEach(() => {
jest.clearAllMocks();
defaultTransaction = new Transaction();
defaultTransaction.chainId = 1n;
defaultTransaction.nonce = 0;
defaultTransaction.data = "0x";
});

describe("Happy path", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import {
type SignTransactionDAOutput,
} from "@api/app-binder/SignTransactionDeviceActionTypes";
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 TransactionType } from "@api/model/TransactionType";
import {
GetChallengeCommand,
type GetChallengeCommandResponse,
Expand Down Expand Up @@ -54,7 +54,7 @@ export type MachineDependencies = {
input: {
contextModule: ContextModule;
mapper: TransactionMapperService;
transaction: Transaction;
transaction: Uint8Array;
options: TransactionOptions;
challenge: string;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import {
import {
DeviceSessionStateType,
DeviceStatus,
hexaStringToBuffer,
} from "@ledgerhq/device-management-kit";
import { Transaction } from "ethers-v6";
import { Transaction } from "ethers";
import { Left, Right } from "purify-ts";

import { makeDeviceActionInternalApiMock } from "@internal/app-binder/device-action/__test-utils__/makeInternalApi";
Expand All @@ -30,18 +31,20 @@ describe("BuildTransactionContextTask", () => {
const defaultOptions = {
domain: "domain-name.eth",
};
let defaultTransaction: Transaction;
const defaultTransaction: Uint8Array = hexaStringToBuffer(
Transaction.from({
chainId: 1n,
nonce: 0,
data: "0x",
}).unsignedSerialized,
)!;

let defaultArgs: BuildTransactionContextTaskArgs;
const apiMock = makeDeviceActionInternalApiMock();

beforeEach(() => {
jest.clearAllMocks();

defaultTransaction = new Transaction();
defaultTransaction.chainId = 1n;
defaultTransaction.nonce = 0;
defaultTransaction.data = "0x";

defaultArgs = {
contextModule: contextModuleMock,
mapper: mapperMock as unknown as TransactionMapperService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import {
} 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 TransactionType } from "@api/model/TransactionType";
import { type TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService";

import { type GenericContext } from "./ProvideTransactionGenericContextTask";
Expand All @@ -26,7 +26,7 @@ export type BuildTransactionTaskResult = {
export type BuildTransactionContextTaskArgs = {
readonly contextModule: ContextModule;
readonly mapper: TransactionMapperService;
readonly transaction: Transaction;
readonly transaction: Uint8Array;
readonly options: TransactionOptions;
readonly challenge: string;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
hexaStringToBuffer,
InvalidStatusWordError,
} from "@ledgerhq/device-management-kit";
import { Transaction } from "ethers-v6";
import { Transaction } from "ethers";
import { Just, Nothing } from "purify-ts";

import { SignTransactionCommand } from "@internal/app-binder/command/SignTransactionCommand";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import {
isSuccessCommandResult,
} from "@ledgerhq/device-management-kit";
import { DerivationPathUtils } from "@ledgerhq/signer-utils";
import { decodeRlp, encodeRlp } from "ethers-v6";
import { decodeRlp, encodeRlp } from "ethers";
import { Nothing } from "purify-ts";

import { type Signature } from "@api/index";
import { TransactionType } from "@api/model/Transaction";
import { TransactionType } from "@api/model/TransactionType";
import {
SignTransactionCommand,
type SignTransactionCommandResponse,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ describe("transactionModuleFactory", () => {
expect(mod).toBeDefined();
});

it("should bind a list of transaction mappers", () => {
it("should bind a transaction mapper", () => {
expect(
container.getAll(transactionTypes.TransactionMappers),
).toHaveLength(2);
).toHaveLength(1);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { ContainerModule } from "inversify";

import { transactionTypes } from "@internal/transaction/di/transactionTypes";
import { EthersV5TransactionMapper } from "@internal/transaction/service/mapper/EthersV5TransactionMapper";
import { EthersV6TransactionMapper } from "@internal/transaction/service/mapper/EthersV6TransactionMapper";
import { EthersRawTransactionMapper } from "@internal/transaction/service/mapper/EthersRawTransactionMapper";
import { TransactionMapperService } from "@internal/transaction/service/mapper/TransactionMapperService";
import { TransactionParserService } from "@internal/transaction/service/parser/TransactionParserService";
import { SignTransactionUseCase } from "@internal/transaction/use-case/SignTransactionUseCase";
Expand All @@ -25,7 +24,6 @@ export const transactionModuleFactory = () =>
bind(transactionTypes.TransactionParserService).to(
TransactionParserService,
);
bind(transactionTypes.TransactionMappers).to(EthersV5TransactionMapper);
bind(transactionTypes.TransactionMappers).to(EthersV6TransactionMapper);
bind(transactionTypes.TransactionMappers).to(EthersRawTransactionMapper);
},
);
Loading
Loading