Skip to content

refactor(coin-evm): remove some dead code#14854

Open
francois-guerin-ledger wants to merge 1 commit intodevelopfrom
refactor/no-issues-remove-dead-code-evm
Open

refactor(coin-evm): remove some dead code#14854
francois-guerin-ledger wants to merge 1 commit intodevelopfrom
refactor/no-issues-remove-dead-code-evm

Conversation

@francois-guerin-ledger
Copy link
Contributor

@francois-guerin-ledger francois-guerin-ledger commented Feb 26, 2026

✅ Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    • coin evm

📝 Description

Remove some dead code on coin evm

❓ Context


🧐 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)

@francois-guerin-ledger francois-guerin-ledger force-pushed the refactor/no-issues-remove-dead-code-evm branch 3 times, most recently from bea316f to 0b38978 Compare February 26, 2026 16:49
);
});

it("should return 0 for invalid transaction", async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already covered by gives 0 additional fees if the transaction is not serializable

);
});

it("should return 0 for invalid transaction", async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already covered by gives 0 additional fees if the transaction is not serializable

Comment on lines +110 to +111
getOptimismAdditionalFees: (currency: CryptoCurrency, transaction: string) => Promise<BigNumber>;
getScrollAdditionalFees: (currency: CryptoCurrency, transaction: string) => Promise<BigNumber>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

transaction is only passed as string in production code

@francois-guerin-ledger francois-guerin-ledger marked this pull request as ready for review February 26, 2026 21:08
@francois-guerin-ledger francois-guerin-ledger requested a review from a team as a code owner February 26, 2026 21:08
Copilot AI review requested due to automatic review settings February 26, 2026 21:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the coin-evm module by removing unused/publicly unreferenced code paths and updating Layer2 additional-fee computation to work with serialized transactions, with corresponding test and docs cleanup.

Changes:

  • Remove dead exports/types/helpers (ERC20 tuple type, ethers adapter, transaction serialization/data helpers, various logic helpers/errors).
  • Update Optimism/Scroll additional-fee APIs to accept a serialized transaction string and adjust tests accordingly.
  • Prune/update documentation to match the reduced public surface.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
libs/coin-modules/coin-evm/src/types/ledger.ts Removes unused ledger explorer block type.
libs/coin-modules/coin-evm/src/types/index.ts Stops re-exporting the erc20 types module.
libs/coin-modules/coin-evm/src/types/erc20.ts Deletes the legacy ERC20Token tuple type.
libs/coin-modules/coin-evm/src/transaction.ts Removes smart-contract calldata + tx serialization helpers.
libs/coin-modules/coin-evm/src/transaction.test.ts Drops tests tied to removed transaction helpers/adapters.
libs/coin-modules/coin-evm/src/network/node/types.ts Narrows additional-fee APIs to accept serialized tx strings only.
libs/coin-modules/coin-evm/src/network/node/rpc.test.ts Updates tests to pass serialized ethers transactions.
libs/coin-modules/coin-evm/src/network/node/rpc.common.ts Removes internal serialization path; uses serialized tx string directly.
libs/coin-modules/coin-evm/src/network/node/ledger.ts Removes internal serialization path; uses serialized tx string directly.
libs/coin-modules/coin-evm/src/network/node/ledger.test.ts Updates tests to pass serialized ethers transactions.
libs/coin-modules/coin-evm/src/logic/estimateFees.test.ts Renames a test description to “serializable”.
libs/coin-modules/coin-evm/src/logic.ts Removes several unused helpers and narrows additional-fee helper signature.
libs/coin-modules/coin-evm/src/logic.test.ts Removes tests for deleted helpers.
libs/coin-modules/coin-evm/src/errors.ts Removes unused custom error classes.
libs/coin-modules/coin-evm/src/adapters/index.ts Stops exporting the ethers adapter module.
libs/coin-modules/coin-evm/src/adapters/ethers.ts Deletes the ethers transaction adapter.
libs/coin-modules/coin-evm/src/adapters/ethers.test.ts Deletes tests for the removed ethers adapter.
libs/coin-modules/coin-evm/docs/transaction.md Removes docs for deleted transaction helpers.
libs/coin-modules/coin-evm/docs/logic.md Removes docs for deleted logic helpers; minor formatting alignment.
libs/coin-modules/coin-evm/docs/adapters.md Removes docs for deleted ethers adapter.
.changeset/lazy-mayflies-itch.md Adds a patch changeset for the refactor.
Comments suppressed due to low confidence (1)

libs/coin-modules/coin-evm/src/transaction.ts:206

  • This module removes previously exported helpers (getTransactionData, getSerializedTransaction). Even if currently unused in this repo, they were part of the public @ledgerhq/coin-evm/transaction API, so this is a breaking change for any downstream consumer. Consider keeping deprecated wrappers for one release (or ensure the changeset/semver bump matches a breaking change).
  return txRaw as EvmTransactionRaw;
};

/**
 * Returns a transaction with the correct type and entries depending
 * on the network compatiblity.
 */
export const getTypedTransaction = (
  _transaction: EvmTransaction,

@francois-guerin-ledger francois-guerin-ledger force-pushed the refactor/no-issues-remove-dead-code-evm branch from 0b38978 to fa38757 Compare February 27, 2026 08:40
@github-actions
Copy link
Contributor

github-actions bot commented Feb 27, 2026

⚠️ 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: refactor/no-issues-remove-dead-code-evm
  • Device: nanoSP or stax

📱 Mobile

-> Run Mobile E2E

  • Select "Run workflow"
  • Branch: refactor/no-issues-remove-dead-code-evm
  • Device: nanoX

Affected coins modules: evm

Copy link
Contributor

@qperrot qperrot left a comment

Choose a reason for hiding this comment

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

Do not forget to run e2e tests

@francois-guerin-ledger
Copy link
Contributor Author

francois-guerin-ledger commented Mar 2, 2026

Copilot AI review requested due to automatic review settings March 2, 2026 13:45
@francois-guerin-ledger francois-guerin-ledger force-pushed the refactor/no-issues-remove-dead-code-evm branch from fa38757 to a74569e Compare March 2, 2026 13:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 2, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
2 New Code Smells (required ≤ 1)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@francois-guerin-ledger
Copy link
Contributor Author

francois-guerin-ledger commented Mar 2, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants