Skip to content

fix: add clear signing selector#475

Draft
gantunesr wants to merge 2 commits intomainfrom
gar/fix/clear-sign-flags
Draft

fix: add clear signing selector#475
gantunesr wants to merge 2 commits intomainfrom
gar/fix/clear-sign-flags

Conversation

@gantunesr
Copy link
Copy Markdown
Member

@gantunesr gantunesr commented Mar 19, 2026

Examples

  • Reason for the change: When signing swaps or other ERC-20 flows (e.g. USDC approve) on Base, Polygon, and similar chains with a Ledger device, the device could show “Review transaction to manage NFT allowance” even though the call was a normal token approval, not an NFT one.
  • Root cause: Ledger clear-sign resolution in @ledgerhq/hw-app-eth is driven by the call’s 4-byte selector. In @ledgerhq/evm-tools, ERC-20 and ERC-721 share the same selector for approve() (0x095ea7b3). Passing nft: true unconditionally enables NFT-related resolution for any call with that selector, so the device can receive NFT plugin data and show NFT allowance wording for ERC-20 contracts.
  • Fix in this codebase: We derive the nft flag from the serialized transaction instead of always passing nft: true. The helper shouldUseNftLedgerClearSign RLP-decodes the raw tx with @ethereumjs/tx’s TransactionFactory.fromSerializedData, reads the first four bytes of calldata, and returns true only for selectors we treat as NFT-only for Ledger resolution: setApprovalForAll (0xa22cb465), ERC-721 / ERC-1155 safeTransferFrom variants (0x42842e0e, 0xb88d4fde, 0xf242432a), and safeBatchTransferFrom (0x2eb2c2d6). For the shared approve (0x095ea7b3) and transferFrom (0x23b872dd) selectors—and everything else—we use nft: false, so ERC-20 approves and transfers get ERC-20 clear-sign text while real NFT transfers and approvals still get NFT clear signing. LedgerSignTransactionParams also accepts an optional nft override, forwarded through the iframe bridges, for callers that need to force the flag; the mobile bridge uses nft ?? shouldUseNftLedgerClearSign(tx) when the override is omitted.

Technical Details

// Source: https://github.com/openzeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/IERC20.sol

interface IERC20 {
  // Partial snippet from the interface

  function transfer(address to, uint256 value) external returns (bool);

  function approve(address spender, uint256 value) external returns (bool);

  function transferFrom(address from, address to, uint256 value) external returns (bool);
}
// Source: https://github.com/openzeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/IERC721.sol 

interface IERC721 is IERC165 {
  // Partial snippet from the interface

  function safeTransferFrom(address from, address to, uint256 tokenId, bytes calldata data) external;

  function safeTransferFrom(address from, address to, uint256 tokenId) external;

  function transferFrom(address from, address to, uint256 tokenId) external;

  function approve(address to, uint256 tokenId) external;

  function setApprovalForAll(address operator, bool approved) external;
}
// https://github.com/openzeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155/IERC1155.sol

interface IERC1155 is IERC165 {
  // Partial snippet from the interface

  function setApprovalForAll(address operator, bool approved) external;

  function safeTransferFrom(address from, address to, uint256 id, uint256 value, bytes calldata data) external;

  function safeBatchTransferFrom(
    address from,
    address to,
    uint256[] calldata ids,
    uint256[] calldata values,
    bytes calldata data
  ) external;
}
Selector ERC-20 ERC-721 ERC-1155
0x23b872dd
0x095ea7b3
0xa22cb465
0xa9059cbb
0x42842e0e
0xb88d4fde
0xf242432a
0x2eb2c2d6

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant