ethers-io / ethers.js

Complete Ethereum library and wallet implementation in JavaScript.
https://ethers.org/
MIT License
7.95k stars 1.85k forks source link

cannot override "chainId" When i used contract.populateTransaction.METHOD_NAME( ...args [ , overrides ] ) #2440

Closed HikariShine closed 2 months ago

HikariShine commented 2 years ago

Describe the bug When i use the code above

let override = {
    gasLimit: 5000000,
    gasPrice: 20000000000,
    nonce: nonce++,
    chainId: 56
};
let now = new Date().getTime();
let unsignedAttackTx = await contract.populateTransaction.setTimestamp(now, override);

I got a message: cannot override "chainId"

Reproduction steps abi

[{
      "inputs": [
        {
          "internalType": "uint256",
          "name": "timestamp",
          "type": "uint256"
        }
      ],
      "name": "setTimestamp",
      "outputs": [],
      "stateMutability": "nonpayable",
      "type": "function"
    }]

Environment: Node etherjs 5.5.2

Search Terms cannot override "chainId"

HikariShine commented 2 years ago

I have read the source code in this file: https://github.com/ethers-io/ethers.js/blob/master/packages/contracts/src.ts/index.ts and i found the chainId field is in the PopulatedTransaction struct, and in populateTransaction method, it return the object type is PopulatedTransaction. It looks like will support chainId override. But why the code havn't override chainId field and also not delete the chainId in override?

ethermachine commented 2 years ago

+1

right now i have to manually override "chainId" in the transaction because the native ether override for chainId can't be applied in the populateTransaction method.

My solution:

let unsigned = await contract.populateTransaction.zzzzz
unsigned.chainId = XXX;
ethermachine commented 2 years ago

Apparently this was partially fixed in https://github.com/ethers-io/ethers.js/releases/tag/v5.6.0

Partially because now at least the promise of populateTx returns the chainId of the provider (or signer?) and there is no need to override it anymore.

However not possible to change to other chain Id, so would be great to finalise the fix for this issue

ricmoo commented 2 months ago

Closing older issues. But this was addressed some time ago in both v5 and v6. :)

Thanks! :)