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

There is an error in the byte length when toBeHex converts the address address. #4427

Open
kakui-lau opened this issue Oct 17, 2023 · 7 comments
Assignees
Labels
investigate Under investigation and may be a bug. v6 Issues regarding v6

Comments

@kakui-lau
Copy link

Ethers Version

6.7.1

Search Terms

parseTransaction,address toBeHex

Describe the Problem

This transaction is an Ethereum mainnet transaction, https://cn.etherscan.com/tx/0x683167e4a06a66c64f982bccdd7bcf401096d6be755e9c79fc7b335e03f0d4b9

 const contractInterface = new Interface(abis.ERC20Abi);
    const parsedData = contractInterface.parseTransaction({ data:"0xa9059cbb00000000000000000000017cc8538c41ab04558d9a2994b98765a6d2f6d13500000000000000000000000000000000000000000000000000000000001dcd6500" });
    console.log(parsedData, '===parsedData');
    return parsedData;

Code Snippet

No response

Contract ABI

[
  {
    "anonymous": false,
    "inputs": [
      {
        "indexed": true,
        "internalType": "address",
        "name": "owner",
        "type": "address"
      },
      {
        "indexed": true,
        "internalType": "address",
        "name": "spender",
        "type": "address"
      },
      {
        "indexed": false,
        "internalType": "uint256",
        "name": "value",
        "type": "uint256"
      }
    ],
    "name": "Approval",
    "type": "event"
  },
  {
    "anonymous": false,
    "inputs": [
      {
        "indexed": true,
        "internalType": "address",
        "name": "from",
        "type": "address"
      },
      {
        "indexed": true,
        "internalType": "address",
        "name": "to",
        "type": "address"
      },
      {
        "indexed": false,
        "internalType": "uint256",
        "name": "value",
        "type": "uint256"
      }
    ],
    "name": "Transfer",
    "type": "event"
  },
  {
    "inputs": [
      {
        "internalType": "address",
        "name": "owner",
        "type": "address"
      },
      {
        "internalType": "address",
        "name": "spender",
        "type": "address"
      }
    ],
    "name": "allowance",
    "outputs": [
      {
        "internalType": "uint256",
        "name": "",
        "type": "uint256"
      }
    ],
    "stateMutability": "view",
    "type": "function"
  },
  {
    "inputs": [
      {
        "internalType": "address",
        "name": "spender",
        "type": "address"
      },
      {
        "internalType": "uint256",
        "name": "amount",
        "type": "uint256"
      }
    ],
    "name": "approve",
    "outputs": [
      {
        "internalType": "bool",
        "name": "",
        "type": "bool"
      }
    ],
    "stateMutability": "nonpayable",
    "type": "function"
  },
  {
    "inputs": [
      {
        "internalType": "address",
        "name": "account",
        "type": "address"
      }
    ],
    "name": "balanceOf",
    "outputs": [
      {
        "internalType": "uint256",
        "name": "",
        "type": "uint256"
      }
    ],
    "stateMutability": "view",
    "type": "function"
  },
  {
    "inputs": [],
    "name": "decimals",
    "outputs": [
      {
        "internalType": "uint8",
        "name": "",
        "type": "uint8"
      }
    ],
    "stateMutability": "view",
    "type": "function"
  },
  {
    "inputs": [],
    "name": "name",
    "outputs": [
      {
        "internalType": "string",
        "name": "",
        "type": "string"
      }
    ],
    "stateMutability": "view",
    "type": "function"
  },
  {
    "inputs": [],
    "name": "symbol",
    "outputs": [
      {
        "internalType": "string",
        "name": "",
        "type": "string"
      }
    ],
    "stateMutability": "view",
    "type": "function"
  },
  {
    "inputs": [],
    "name": "totalSupply",
    "outputs": [
      {
        "internalType": "uint256",
        "name": "",
        "type": "uint256"
      }
    ],
    "stateMutability": "view",
    "type": "function"
  },
  {
    "inputs": [
      {
        "internalType": "address",
        "name": "recipient",
        "type": "address"
      },
      {
        "internalType": "uint256",
        "name": "amount",
        "type": "uint256"
      }
    ],
    "name": "transfer",
    "outputs": [
      {
        "internalType": "bool",
        "name": "",
        "type": "bool"
      }
    ],
    "stateMutability": "nonpayable",
    "type": "function"
  },
  {
    "inputs": [
      {
        "internalType": "address",
        "name": "sender",
        "type": "address"
      },
      {
        "internalType": "address",
        "name": "recipient",
        "type": "address"
      },
      {
        "internalType": "uint256",
        "name": "amount",
        "type": "uint256"
      }
    ],
    "name": "transferFrom",
    "outputs": [
      {
        "internalType": "bool",
        "name": "",
        "type": "bool"
      }
    ],
    "stateMutability": "nonpayable",
    "type": "function"
  }
]

Errors

TransactionDescription {
  fragment: FunctionFragment {
    type: 'function',
    inputs: [ [ParamType], [ParamType] ],
    name: 'transfer',
    constant: false,
    outputs: [ [ParamType] ],
    stateMutability: 'nonpayable',
    payable: false,
    gas: null
  },
  name: 'transfer',
  args: Result(2) [
    recipient: value exceeds width (20 bits) (operation="toBeHex", fault="overflow", value=556514283519825059651953496263113110841818824652032, code=NUMERIC_FAULT, version=6.7.1)
        at makeError (/Users/kakui/projects/client-monorepo/node_modules/ethers6/src.ts/utils/errors.ts:682:21)
        at assert (/Users/kakui/projects/client-monorepo/node_modules/ethers6/src.ts/utils/errors.ts:702:25)
        at toBeHex (/Users/kakui/projects/client-monorepo/node_modules/ethers6/src.ts/utils/maths.ts:194:15)
        at AddressCoder.decode (/Users/kakui/projects/client-monorepo/node_modules/ethers6/src.ts/abi/coders/address.ts:34:34)
        at /Users/kakui/projects/client-monorepo/node_modules/ethers6/src.ts/abi/coders/array.ts:108:31
        at Array.forEach (<anonymous>)
        at unpack (/Users/kakui/projects/client-monorepo/node_modules/ethers6/src.ts/abi/coders/array.ts:86:12)
        at TupleCoder.decode (/Users/kakui/projects/client-monorepo/node_modules/ethers6/src.ts/abi/coders/tuple.ts:66:22)
        at AbiCoder.decode (/Users/kakui/projects/client-monorepo/node_modules/ethers6/src.ts/abi/abi-coder.ts:209:22)
        at Interface.parseTransaction (/Users/kakui/projects/client-monorepo/node_modules/ethers6/src.ts/abi/interface.ts:1201:37) {
      code: 'NUMERIC_FAULT',
      operation: 'toBeHex',
      fault: 'overflow',
      value: 556514283519825059651953496263113110841818824652032n,
      baseType: 'address',
      type: 'address'
    },
    500000000n
  ],
  signature: 'transfer(address,uint256)',
  selector: '0xa9059cbb',
  value: 0n
}

Environment

node.js (v12 or newer)

Environment (Other)

No response

@kakui-lau kakui-lau added investigate Under investigation and may be a bug. v6 Issues regarding v6 labels Oct 17, 2023
@kakui-lau kakui-lau changed the title Add Bug Title Here There is an error in the byte length when toBeHex converts the address address. Oct 17, 2023
@kakui-lau
Copy link
Author

@kakui-lau
Copy link
Author

@ricmoo

@ricmoo
Copy link
Member

ricmoo commented Oct 24, 2023

There is some sort of weird bug in whatever generated that calldata. Here is the data broken down:

// 0xa9059cbb00000000000000000000017cc8538c41ab04558d9a2994b98765a6d2f6d13500000000000000000000000000000000000000000000000000000000001dcd6500

// Selector; i.e. Transfer(address,uint256)
selector = 0xa9059cbb

// next 32 bytes; discussed below (this is too big, requiring 23 bytes to encode, while addresses must fit in 20 bytes)
a = 00000000000000000000017cc8538c41ab04558d9a2994b98765a6d2f6d13500

// If we break this into the top 12 bytes and bottom 20 bytes, we get:
aTop = a[0:12] = 00000000000000000000017c
aBottom = a[12:] c8538c41ab04558d9a2994b98765a6d2f6d13500

The problem is the top bytes (aTop) of an address slot must be zero, but we see the value 0x17c (i.e. 380) is being prepended here, which means the encoded slot included junk in the top, which isn't permitted.


// next 32 bytes; i.e. 500000000 (this is fine)
v = 000000000000000000000000000000000000000000000000000000001dcd6500

There is a bug in the error message, it should indicate "20 bytes", not "20 bits". I've made that change locally and will push it out soon.

@kakui-lau
Copy link
Author

There is some sort of weird bug in whatever generated that calldata. Here is the data broken down:

// 0xa9059cbb00000000000000000000017cc8538c41ab04558d9a2994b98765a6d2f6d13500000000000000000000000000000000000000000000000000000000001dcd6500

// Selector; i.e. Transfer(address,uint256)
selector = 0xa9059cbb

// next 32 bytes; discussed below (this is too big, requiring 23 bytes to encode, while addresses must fit in 20 bytes)
a = 00000000000000000000017cc8538c41ab04558d9a2994b98765a6d2f6d13500

// If we break this into the top 12 bytes and bottom 20 bytes, we get:
aTop = a[0:12] = 00000000000000000000017c
aBottom = a[12:] c8538c41ab04558d9a2994b98765a6d2f6d13500

The problem is the top bytes (aTop) of an address slot must be zero, but we see the value 0x17c (i.e. 380) is being prepended here, which means the encoded slot included junk in the top, which isn't permitted.


// next 32 bytes; i.e. 500000000 (this is fine)
v = 000000000000000000000000000000000000000000000000000000001dcd6500

There is a bug in the error message, it should indicate "20 bytes", not "20 bits". I've made that change locally and will push it out soon.

Yes, 17c of 00000000000000000000017c is not allowed, but it seems that this situation may be entered by mistake when sending, but the underlying protocol actually supports and allows parsing this method...

Will there be a fix for this situation? Instead of fixing the "20 bytes" instead of the "20 bits" name

@ricmoo
Copy link
Member

ricmoo commented Nov 1, 2023

The first part of this (the incorrect error message) has been fixed in v6.8.1.

It requires a larger discussion on what the correct thing to do with invalid data with dirty data in the padding range.

It is usually indicative of a more serious underlying issue, so I don't know that I simply want to ignore it. As a first step though, it may make sense to expose a "deferred error", so that the value can be recovered easily. This is something I want to do for uint/int types too. For example, if a value is a uint8, but contains the value 1000 (which is outside the range), it should throw an error; it should not just accept the dirty bytes and allow the 1000 through for a uint8.

But the "deferred error API", could allow something similar to:

try {
  console.log(result. recipient);
} catch (e) {
  console.log(e.error.recovered);
}

It should likely expose the entire bytes that were present (.bytes) and the best-guess at what was intended (.recovered) for each type. It can be added to the string case too, where .recovered uses the replace strategy, which seems mildly safer than the ignore strategy for invalid UTF-8 strings.

@ricmoo
Copy link
Member

ricmoo commented Nov 1, 2023

As an aside, a similar issue occurred once before, where due to a bug in Solidity, methods that were marked as external incorrectly padded event data (events emitted from public methods worked correctly).

This is what led to the loose argument. If these issues are cropping up because of a compiler bug, a similar situation may arise. Which is also why I'm curious about how this is happening; is it a one-off misstep of someone writing some faulty assembly in a single contract, or is it a compiler bug or use of a widely adopted library.

@sharifzadesina
Copy link

@ricmoo I got this error also for this data:

a9059cbb
0000000000000000000000417196a564824974842dd03cde3e3020af201baaa4
00000000000000000000000000000000000000000000000000000000001e8480

It is generated from USDT contract in the TRON network.
The interesting thing is, it is a valid transaction in TRON network.

This is the TXID:
7e45745226085334dd91ce42a3beb76dcb3b6f164f5ec558a49abac8ba1885a0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigate Under investigation and may be a bug. v6 Issues regarding v6
Projects
None yet
Development

No branches or pull requests

3 participants