feat(evm): compute intrinsic gas limit for send on evm by Moustafa-Koterba · Pull Request #14801 · LedgerHQ/ledger-live

Moustafa-Koterba · 2026-02-25T16:23:36Z

✅ Checklist

npx changeset was attached.
Covered by automatic tests.
Impact of the changes:
- Gas limit can changed now

📝 Description

We were using a static value for the default minimal gas needed for sending ETH. This task aims to improve this behaviour and be compliant with https://github.com/wolflo/evm-opcodes/blob/main/gas.md#a0-0-intrinsic-gas. The following formula has been implemented:

intrinsic gas limit = 21 000 + 4 * null bytes count + 16 * non null bytes count

For basic ethereum, the gas limit stays at 21 000
For ethereum tokens, the above formula is used

❓ Context

JIRA or GitHub link: LIVE-26166

🧐 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.
Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

Copilot

Pull request overview

This PR implements dynamic computation of the intrinsic gas limit for EVM transactions, replacing the previous static 21,000 gas limit with a formula that accounts for transaction data: 21000 + 4 * zero_bytes + 16 * non_zero_bytes. This aligns with the Ethereum Yellow Paper specification for intrinsic gas costs.

Changes:

Adds computeIntrinsicGasLimit function to calculate gas based on call data
Updates validateIntent to use computed intrinsic gas limits instead of static values
Modifies getCallData to return empty buffer when recipient is missing (preventing invalid ERC20 data generation)

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`libs/coin-modules/coin-evm/src/logic/computeGasLimit.ts`	New file implementing intrinsic gas limit calculation based on callData byte counts
`libs/coin-modules/coin-evm/src/logic/computeGasLimit.test.ts`	Test suite for intrinsic gas computation with various ERC20 transfer scenarios
`libs/coin-modules/coin-evm/src/logic/validateIntent.ts`	Updated gas validation to use computed intrinsic limits; changed `jest.restoreAllMocks` to `jest.clearAllMocks`
`libs/coin-modules/coin-evm/src/logic/validateIntent.test.ts`	Added mock setup for `computeIntrinsicGasLimit` and updated test cases
`libs/coin-modules/coin-evm/src/logic/common.ts`	Exported `getCallData` and added `!intent.recipient` guard
`libs/coin-modules/coin-evm/tsconfig.json`	Added newline at end of file (formatting)
`.changeset/eleven-countries-pull.md`	Changeset documenting the feature addition

libs/coin-modules/coin-evm/src/logic/validateIntent.test.ts

francois-guerin-ledger · 2026-02-26T16:55:56Z

libs/coin-modules/coin-evm/src/logic/common.ts

+  return isNative(intent.asset) || !intent.recipient
+    ? Buffer.from([])
+    : getErc20Data(intent.recipient, intent.amount);


Do we have cases with falsy recipients (like "") ?

as discussed IRL, it happens sometimes on Ledger Wallet, when starting a send

francois-guerin-ledger · 2026-02-26T17:00:16Z

libs/coin-modules/coin-evm/src/logic/validateIntent.test.ts

            }),
          );
+
+          expect(computeGasLimitModule.computeIntrinsicGasLimit).toHaveBeenCalledTimes(1);


It is not useful to know which internal function is called. We don't want to test the implementation

Suggested change

expect(computeGasLimitModule.computeIntrinsicGasLimit).toHaveBeenCalledTimes(1);

as discussed IRL, need to be discussed with the team

We never discussed that. We only talked about mocking dependencies, not about expecting dependencies are called X amount of times. Because this info is not crucial for the test itself, it means we are testing the implementation ... which is in any case not what we want.

github-actions · 2026-02-27T16:11:46Z

⚠️ E2E tests are required

Changes detected require e2e testing before merge (even before asking for any review).

🖥️ Desktop

-> Run Desktop E2E

Select "Run workflow"
Branch: feature/LIVE-26166
Device: nanoSP or stax

📱 Mobile

-> Run Mobile E2E

Select "Run workflow"
Branch: feature/LIVE-26166
Device: nanoX

Affected coins modules: evm

francois-guerin-ledger

We must not check if recipient is empty string an silently failing here. Instead, we could have another PR

francois-guerin-ledger · 2026-03-01T21:48:13Z

libs/coin-modules/coin-evm/src/logic/common.test.ts

+      } as unknown as TransactionIntent<MemoNotSupported, BufferTxData>;
+
+      const result = getCallData(intent);
+      expect(result).toEqual(intent.data.value);


Using some part of the function input as output expectation is not a good practice. Imagine getCallData mutates intent, before returning the result. Values will be equal indeed, but it won't be what you did input due to the internal mutation.
Best practice is always work with static values when dealing with tests

francois-guerin-ledger · 2026-03-01T21:49:32Z

libs/coin-modules/coin-evm/src/logic/common.test.ts

+    it("should return empty buffer for native asset", () => {
+      const intent = {
+        asset: {
+          type: "native",
+        },
+      } as unknown as TransactionIntent<MemoNotSupported, BufferTxData>;
+
+      const result = getCallData(intent);
+      expect(result).toEqual(Buffer.from([]));
+    });
+
+    it("should return empty buffer for non native asset and no recipient", () => {
+      const intent = {
+        asset: {
+          type: "erc20",
+        },
+      } as unknown as TransactionIntent<MemoNotSupported, BufferTxData>;
+
+      const result = getCallData(intent);
+      expect(result).toEqual(Buffer.from([]));
+    });
+
+    it("should return empty buffer for non native asset with recipient and no amount", () => {
+      const intent = {
+        asset: {
+          type: "erc20",
+        },
+        recipient: "0x66c4371aE8FFeD2ec1c2EBbbcCfb7E494181E1E3",
+      } as unknown as TransactionIntent<MemoNotSupported, BufferTxData>;
+
+      const result = getCallData(intent);
+      expect(result).toEqual(Buffer.from([]));
+    });


Use it.each to be more compact

Copilot

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

libs/coin-modules/coin-evm/src/logic/common.test.ts

libs/coin-modules/coin-evm/src/logic/common.ts

francois-guerin-ledger · 2026-03-02T14:49:28Z

libs/coin-modules/coin-evm/src/logic/common.test.ts

+          ),
+        },
+      } as unknown as TransactionIntent<MemoNotSupported, BufferTxData>;
+      const expectedResult = Buffer.from(intent.data.value);


intent.data.value is already a buffer. Also you are still using the input as output expectation

francois-guerin-ledger · 2026-03-02T14:52:09Z

libs/coin-modules/coin-evm/src/logic/common.ts

+    intent.amount === undefined ||
+    intent.amount === null


intent.amount is a bigint, how can it be undefined or null ? Your previous version was good, because it covers 0n.

Moustafa-Koterba · 2026-03-02T20:55:58Z

E2E tests

Copilot

Pull request overview

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

Copilot · 2026-03-02T20:58:45Z

libs/coin-modules/coin-evm/src/logic/validateIntent.test.ts

+        (computeGasLimitModule.computeIntrinsicGasLimit as jest.Mock).mockImplementation(
+          jest.requireActual("./computeGasLimit").computeIntrinsicGasLimit,
+        );
+        // ... other setup


The comment // ... other setup appears to be a placeholder left in from a template or draft. It should be removed before merging.

Suggested change

// ... other setup

Copilot · 2026-03-02T20:58:45Z

libs/coin-modules/coin-evm/src/logic/validateIntent.test.ts

  });
  afterEach(() => {
-    jest.restoreAllMocks();
+    jest.clearAllMocks();


Switching from jest.restoreAllMocks() to jest.clearAllMocks() in afterEach is a regression. jest.restoreAllMocks() also restores the original implementation of jest.spyOn mocks (e.g., the ledgerNode.getTransactionCount spy set up in beforeEach), whereas jest.clearAllMocks() only resets call counts and instances but does not restore them. This means spy mocks set up with jest.spyOn will persist across tests in an unrestored state, potentially causing interference between tests.

Suggested change

jest.clearAllMocks();

jest.restoreAllMocks();

Copilot · 2026-03-02T20:58:46Z

libs/coin-modules/coin-evm/src/logic/computeGasLimit.ts

+export function computeIntrinsicGasLimit(defaultGasLimit: bigint, callData: Buffer): bigint {
+  let intrinsicGasLimit = defaultGasLimit;
+  for (let index = 0; index < callData.length; index++) {
+    intrinsicGasLimit += callData[index] === 0 ? 4n : 16n;
+  }
+
+  return intrinsicGasLimit;
+}


The function lacks a docstring. Given that this implements a specific EVM spec formula (intrinsic gas = 21,000 + 4 × zero bytes + 16 × non-zero bytes), a brief comment linking to the spec and describing the parameters and return value would improve maintainability.

sonarqubecloud · 2026-03-02T21:16:22Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

github-actions bot assigned Moustafa-Koterba Feb 25, 2026

live-github-bot bot added the coin-modules label Feb 25, 2026

feat(evm): compute intrinsic gas limit for send on evm

888dea6

Moustafa-Koterba force-pushed the feature/LIVE-26166 branch from dc1e556 to 888dea6 Compare February 26, 2026 15:52

Moustafa-Koterba marked this pull request as ready for review February 26, 2026 16:51

Moustafa-Koterba requested a review from a team as a code owner February 26, 2026 16:51

Copilot AI review requested due to automatic review settings February 26, 2026 16:51

Copilot started reviewing on behalf of Moustafa-Koterba February 26, 2026 16:52 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

libs/coin-modules/coin-evm/src/logic/validateIntent.test.ts Show resolved Hide resolved

francois-guerin-ledger previously approved these changes Feb 26, 2026

View reviewed changes

feat(evm): add unit test for common

748c3ed

Moustafa-Koterba dismissed francois-guerin-ledger’s stale review via 748c3ed February 27, 2026 16:09

francois-guerin-ledger requested changes Mar 1, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 2, 2026 11:54

Copilot started reviewing on behalf of Moustafa-Koterba March 2, 2026 11:55 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

libs/coin-modules/coin-evm/src/logic/common.test.ts Outdated Show resolved Hide resolved

libs/coin-modules/coin-evm/src/logic/common.ts Outdated Show resolved Hide resolved

Moustafa-Koterba force-pushed the feature/LIVE-26166 branch from d688df6 to a380206 Compare March 2, 2026 13:57

francois-guerin-ledger requested changes Mar 2, 2026

View reviewed changes

feat(evm): pr reviews

6fc7a62

Copilot AI review requested due to automatic review settings March 2, 2026 20:54

Moustafa-Koterba force-pushed the feature/LIVE-26166 branch from a380206 to 6fc7a62 Compare March 2, 2026 20:54

Copilot AI reviewed Mar 2, 2026

View reviewed changes

Conversation

Moustafa-Koterba commented Feb 25, 2026

✅ Checklist

📝 Description

❓ Context

🧐 Checklist for the PR Reviewers

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

⚠️ E2E tests are required

🖥️ Desktop

📱 Mobile

Uh oh!

francois-guerin-ledger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Moustafa-Koterba commented Mar 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Mar 2, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Feb 27, 2026 •

edited

Loading