🐛 (signer-eth) [LIVE-25852]: Use chainId in getAddress only when checkOnDevice is enabled by mbertin-ledger · Pull Request #1298 · LedgerHQ/device-sdk-ts

mbertin-ledger · 2026-02-11T14:00:30Z

📝 Description

This PR ensures the optional chainId parameter for Ethereum signer getAddress is only used when checkOnDevice is enabled. When checkOnDevice is false, chainId is ignored: it is not sent in the getAddress APDU and dynamic network context is not loaded. When checkOnDevice is true, chainId is sent to the device for address verification and used to load dynamic network context (e.g. network name and icon); it defaults to 1 (Ethereum mainnet) when omitted.

Changes:

GetAddressCommand: Chain ID is included in the APDU data only when checkOnDevice is true.
SendGetAddressTask: Uses an effective chainId (chainId when checkOnDevice is true, undefined otherwise) for loading dynamic network context and for the command.
Sample app: Get address form only passes chainId in options when checkOnDevice is true and the value is valid.
Docs: AddressOptions documents chainId and the requirement that it is only used when checkOnDevice is true.

❓ Context

JIRA or GitHub link: LIVE-25852
Feature: getAddress chainId with checkOnDevice

✅ Checklist

Covered by automatic tests (GetAddressCommand and SendGetAddressTask tests updated/added)
Changeset is provided
Documentation is up-to-date (eth signer reference updated)
Impact of the changes:
- Ethereum getAddress: behavior change only when checkOnDevice is false and caller passed chainId (now ignored).
- Sample app: chainId field only affects behavior when "Check on device" is enabled.
- QA: verify getAddress with checkOnDevice on/off and with/without chainId on a device.

🧐 Checklist for the PR Reviewers

The code aligns with the requirements described in the linked JIRA or GitHub issue.
The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
There are no undocumented trade-offs, technical debt, or maintainability issues.
The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
Any new dependencies have been justified and documented.

vercel · 2026-02-11T14:00:34Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
device-sdk-ts-sample	Ready	Preview, Comment	Feb 11, 2026 3:48pm

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
doc-device-management-kit	Ignored		Feb 11, 2026 3:48pm

github-actions · 2026-02-11T14:03:16Z

Danger Check Results

Messages

✅	Danger: All checks passed successfully! 🎉

Generated by 🚫 dangerJS against d72803d

Copilot

Pull request overview

This PR updates the Ethereum signer getAddress flow so chainId is only considered when checkOnDevice is enabled, including conditional APDU encoding and conditional dynamic network context loading.

Changes:

Update GetAddressCommand APDU encoding to include a 64-bit chainId only when checkOnDevice is true (defaulting to 1 when omitted).
Introduce SendGetAddressTask and route EthAppBinder.getAddress through it to optionally load/provide dynamic network context before sending getAddress.
Propagate/document chainId through API types, sample app, and docs.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/signer/signer-eth/src/internal/app-binder/task/SendGetAddressTask.ts	New task that optionally loads dynamic network context and then sends `GetAddressCommand`.
packages/signer/signer-eth/src/internal/app-binder/task/SendGetAddressTask.test.ts	Adds unit tests for the new task behavior around context loading and command sending.
packages/signer/signer-eth/src/internal/app-binder/command/GetAddressCommand.ts	Encodes chainId (uint64) only when `checkOnDevice` is enabled, with defaulting behavior.
packages/signer/signer-eth/src/internal/app-binder/command/GetAddressCommand.test.ts	Updates APDU fixtures/expectations and adds chainId-specific tests.
packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.ts	Switches `getAddress` to execute a task (`CallTaskInAppDeviceAction`) instead of sending the command directly.
packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.test.ts	Updates binder tests to match the new task-based device action shape.
packages/signer/signer-eth/src/internal/address/use-case/GetAddressUseCase.ts	Propagates `chainId` from public options down into the binder call.
packages/signer/signer-eth/src/api/model/AddressOptions.ts	Documents the new `chainId` option on the public options type.
packages/signer/signer-eth/src/api/app-binder/GetAddressCommandTypes.ts	Adds `chainId?: number` to the command args type.
packages/signer/context-module/src/dynamic-network/domain/DynamicNetworkContextLoader.ts	Exports `DYNAMIC_NETWORK_CONTEXT_TYPES` for consumers to request the correct context types.
packages/device-management-kit/src/api/apdu/utils/ApduBuilder.ts	Adds `add64BitUIntToData` helper used for chainId encoding.
apps/sample/src/components/SignerEthView/index.tsx	Only forwards `chainId` to `getAddress` when `checkOnDevice` is enabled and input is parseable.
apps/docs/pages/docs/references/signers/eth.mdx	Documents `chainId` and clarifies it is only used when `checkOnDevice` is `true`.
.changeset/slow-suits-relax.md	Declares a patch release note for the behavior change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T14:05:57Z

packages/signer/signer-eth/src/internal/app-binder/task/SendGetAddressTask.ts

+    const effectiveChainId =
+      checkOnDevice && chainId !== undefined ? chainId : undefined;


effectiveChainId is only set when chainId is explicitly provided. When checkOnDevice is true and chainId is omitted, GetAddressCommand will still default and send chainId=1, but this task won’t load dynamic network context for mainnet (and also won’t pass chainId to the command args), which contradicts the documented behavior. Compute an effective chain id as checkOnDevice ? (chainId ?? 1) : undefined and use it both for context loading and for the command arguments.

Suggested change

const effectiveChainId =

checkOnDevice && chainId !== undefined ? chainId : undefined;

const effectiveChainId = checkOnDevice ? (chainId ?? 1) : undefined;

Copilot · 2026-02-11T14:05:58Z

packages/signer/signer-eth/src/internal/app-binder/task/SendGetAddressTask.test.ts

+    it("should send GetAddressCommand with correct args (checkOnDevice, returnChainCode, chainId)", async () => {
+      contextModuleMock.getContexts.mockResolvedValue([]);
+
+      const task = new SendGetAddressTask(apiMock, {
+        ...baseArgs,
+        checkOnDevice: true,
+        returnChainCode: true,
+        chainId: 137,
+      });
+      await task.run();
+
+      expect(apiMock.sendCommand).toHaveBeenLastCalledWith(
+        expect.objectContaining({
+          name: "getAddress",
+          args: expect.objectContaining({
+            derivationPath: DEFAULT_DERIVATION_PATH,
+            checkOnDevice: true,
+            returnChainCode: true,
+            chainId: 137,
+          }),
+        }),
+      );
+    });
+
+    it("should load dynamic network context and provide contexts when chainId is defined and checkOnDevice is true", async () => {
+      const dynamicNetworkContext: ClearSignContextSuccess<ClearSignContextType.DYNAMIC_NETWORK> =
+        {


Missing test case for checkOnDevice: true with chainId omitted. Given GetAddressCommand defaults chainId to 1, the task should also load dynamic network context with chainId=1 and send the command with chainId=1; add an assertion for contextModule.getContexts/api.getDeviceModel calls in that scenario.

Copilot · 2026-02-11T14:05:59Z

packages/signer/signer-eth/src/api/model/AddressOptions.ts

  checkOnDevice?: boolean;
  returnChainCode?: boolean;
  skipOpenApp?: boolean;
+  /** Optional chain ID for address derivation (e.g. 1 for Ethereum mainnet). When omitted, defaults to 1. */


The new JSDoc for chainId is misleading/inaccurate: chainId does not affect “address derivation”, and it is ignored when checkOnDevice is false. Update the comment to state it’s only used for on-device verification (and dynamic network context) when checkOnDevice is true, and only defaults to 1 in that mode.

Suggested change

/** Optional chain ID for address derivation (e.g. 1 for Ethereum mainnet). When omitted, defaults to 1. */

/** Optional chain ID used only for on-device address verification and dynamic network context when `checkOnDevice` is true (e.g. 1 for Ethereum mainnet). When omitted in that mode, defaults to 1. */

aussedatlo · 2026-02-11T14:04:02Z

apps/sample/src/components/SignerEthView/index.tsx

        },
        initialValues: {
          derivationPath: "44'/60'/0'/0/0",
+          chainId: "",


[COULD] be 1 by default

Nope as we want to keep it undefined by default

so => [WONT DO]

aussedatlo · 2026-02-11T14:06:38Z

packages/signer/signer-eth/src/internal/app-binder/command/GetAddressCommand.ts

-    builder.add32BitUIntToData(1);
+    // Chain ID is only sent when checkOnDevice is true; default to 1 (Ethereum mainnet) when omitted.
+    if (this.args.checkOnDevice) {
+      const chainId = this.args.chainId ?? 1;


[COULD] The set to default value 1 could be in packages/signer/signer-eth/src/internal/address/use-case/GetAddressUseCase.ts like with other values: https://github.com/LedgerHQ/device-sdk-ts/pull/1298/changes#diff-c6696bcd6dc71e14305689964fec360ba66258ae46bec83dd3b1554c3e71d508R25

Nope because in command this has no impact on dynamic network call.
If we are putting 1, the tasks will ask for dynamic network and we should not as it's optional.

Co-authored-by: Cursor <[email protected]>

…NetworkContextLoader Co-authored-by: Cursor <[email protected]>

Co-authored-by: Cursor <[email protected]>

github-actions bot assigned mbertin-ledger Feb 11, 2026

mbertin-ledger marked this pull request as ready for review February 11, 2026 14:00

mbertin-ledger requested a review from a team as a code owner February 11, 2026 14:00

Copilot AI review requested due to automatic review settings February 11, 2026 14:00

Copilot started reviewing on behalf of mbertin-ledger February 11, 2026 14:01 View session

vercel bot deployed to Preview – device-sdk-ts-sample February 11, 2026 14:04 View deployment

Copilot AI reviewed Feb 11, 2026

View reviewed changes

aussedatlo approved these changes Feb 11, 2026

View reviewed changes

vercel bot deployed to Preview – device-sdk-ts-sample February 11, 2026 14:25 View deployment

mbertin-ledger and others added 10 commits February 11, 2026 16:43

✨ (device-management-kit): Add add64BitUIntToData to ApduBuilder

9776caa

Co-authored-by: Cursor <[email protected]>

✨ (signer-eth): Add optional chainId parameter to getAddress

81712fc

Co-authored-by: Cursor <[email protected]>

✨ (context-module): Export DYNAMIC_NETWORK_CONTEXT_TYPES from Dynamic…

db2a1ca

…NetworkContextLoader Co-authored-by: Cursor <[email protected]>

✨ (signer-eth): Load dynamic network context before getAddress command

c32f487

Co-authored-by: Cursor <[email protected]>

✨ (signer-eth): Use chainId only when checkOnDevice is enabled

4252ee1

Co-authored-by: Cursor <[email protected]>

✅ (signer-eth): Update getAddress tests for chainId with checkOnDevice

462e466

Co-authored-by: Cursor <[email protected]>

📝 (docs): Document chainId in getAddress and checkOnDevice requirement

a1a19e5

Co-authored-by: Cursor <[email protected]>

📦 (chore): Add changeset for getAddress chainId with checkOnDevice

026a706

Co-authored-by: Cursor <[email protected]>

📝 (signer-eth): Add comment for chainId usage in GetAddressUseCase

2d82a0a

Co-authored-by: Cursor <[email protected]>

✅ (signer-eth): Update GetAddress and SendGetAddressTask tests

d72803d

Co-authored-by: Cursor <[email protected]>

mbertin-ledger force-pushed the bugfix/LIVE-25852-add-chain-id-eth-getaddress branch from 50dae65 to d72803d Compare February 11, 2026 15:44

vercel bot deployed to Preview – device-sdk-ts-sample February 11, 2026 15:48 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 (signer-eth) [LIVE-25852]: Use chainId in getAddress only when checkOnDevice is enabled#1298

🐛 (signer-eth) [LIVE-25852]: Use chainId in getAddress only when checkOnDevice is enabled#1298
mbertin-ledger wants to merge 10 commits intodevelopfrom
bugfix/LIVE-25852-add-chain-id-eth-getaddress

mbertin-ledger commented Feb 11, 2026

Uh oh!

vercel bot commented Feb 11, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Feb 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

aussedatlo Feb 11, 2026

Uh oh!

mbertin-ledger Feb 11, 2026

Uh oh!

mbertin-ledger Feb 11, 2026

Uh oh!

aussedatlo Feb 11, 2026

Uh oh!

mbertin-ledger Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const effectiveChainId =
		checkOnDevice && chainId !== undefined ? chainId : undefined;

	const effectiveChainId =
	checkOnDevice && chainId !== undefined ? chainId : undefined;
	const effectiveChainId = checkOnDevice ? (chainId ?? 1) : undefined;

	/** Optional chain ID for address derivation (e.g. 1 for Ethereum mainnet). When omitted, defaults to 1. */
	/** Optional chain ID used only for on-device address verification and dynamic network context when `checkOnDevice` is true (e.g. 1 for Ethereum mainnet). When omitted in that mode, defaults to 1. */

Conversation

mbertin-ledger commented Feb 11, 2026

📝 Description

❓ Context

✅ Checklist

🧐 Checklist for the PR Reviewers

Uh oh!

vercel bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Danger Check Results

Messages

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

aussedatlo Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

mbertin-ledger Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

mbertin-ledger Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

aussedatlo Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

mbertin-ledger Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel bot commented Feb 11, 2026 •

edited

Loading

github-actions bot commented Feb 11, 2026 •

edited

Loading