web3.js icon indicating copy to clipboard operation
web3.js copied to clipboard

Update `signTransaction` to Consistently Return `SignedTransactionInfoAPI`

Open spacesailor24 opened this issue 3 years ago • 2 comments

Before this PR, web3Eth.signTransaction was returning whatever the user passed as the transaction parameter for respnose.tx when the RPC client was returning only the RLP encoded transaction as the RPC response (in the below code example, Ganache was only returning the value labeled as raw, while Geth was returning the entire object (i.e. { raw: ..., tx: ... })). This resulted in varying returns values based on which client the request was submitted to:

If the connected client was Ganache:

{
    raw: '0xf86580843b9aca018252089400000000000000000000000000000000000000000180820a95a02bd3ccac498541156149591091ff2af043f0be204a7d723701a9e1366dcc6f5fa02a138f18dcafebcf48d9df2c7b3d7fa5f0741b2921d6f42fdc219f34a7ddd97e',
    tx: {
      from: '0x6e599da0bff7a6598ac1224e4985430bf16458a4',
      nonce: 0n,
      to: '0x0000000000000000000000000000000000000000',
      value: 1n,
      gas: 21000n,
      gasPrice: 1000000001n
    }
}

If the connected client was Geth:

{
   raw: '0xf86580843b9aca018252089400000000000000000000000000000000000000000180820a95a003d7879059608586185d1f1b662a73f2701026403b44ab639b5558b043ca375ca0024054e057f5cc1e1f3d2c7aabf162616caf95bbcfb8215376210ff410d91f32',
   tx: {
      type: 0n,
      nonce: 0n,
      gasPrice: 1000000001n,
      gas: 21000n,
      value: 1n,
      v: 2709n,
      r: '0x03d7879059608586185d1f1b662a73f2701026403b44ab639b5558b043ca375c',
      s: '0x024054e057f5cc1e1f3d2c7aabf162616caf95bbcfb8215376210ff410d91f32',
      to: '0x0000000000000000000000000000000000000000',
      data: '0x'
    }
}

After this PR:

If the connected client was Ganache:

{
    raw: '0xf86580843b9aca018252089400000000000000000000000000000000000000000180820a95a02bd3ccac498541156149591091ff2af043f0be204a7d723701a9e1366dcc6f5fa02a138f18dcafebcf48d9df2c7b3d7fa5f0741b2921d6f42fdc219f34a7ddd97e',
    tx: {
      nonce: 0n,
      gasPrice: 1000000001n,
      gasLimit: 21000n,
      to: '0x0000000000000000000000000000000000000000',
      value: 1n,
      data: '0x',
      v: 2709n,
      r: '0x2bd3ccac498541156149591091ff2af043f0be204a7d723701a9e1366dcc6f5f',
      s: '0x2a138f18dcafebcf48d9df2c7b3d7fa5f0741b2921d6f42fdc219f34a7ddd97e',
      type: 0n
    }
}

If the connected client was Geth:

{
    raw: '0xf86580843b9aca018252089400000000000000000000000000000000000000000180820a95a0e7e229dcf3b6930ec7a090164f3bafe42fe4b343e2ac89f4e7077992e4cae7a9a0368c25b3275b8f8d8c9ea7b4fd7715b474843f4a6f36c9088d1c9e3f0d61dc27',
    tx: {
      type: 0n,
      nonce: 0n,
      gasPrice: 1000000001n,
      gas: 21000n,
      value: 1n,
      v: 2709n,
      r: '0xe7e229dcf3b6930ec7a090164f3bafe42fe4b343e2ac89f4e7077992e4cae7a9',
      s: '0x368c25b3275b8f8d8c9ea7b4fd7715b474843f4a6f36c9088d1c9e3f0d61dc27',
      to: '0x0000000000000000000000000000000000000000',
      data: '0x'
    }
}

One point of discussion that remains: In the last two above code examples, the response from Ganache returns the gasLimit property because the raw encoded transaction is being parsed by @ethereumjs/tx's TransactionFactory. However, the response from Geth returns gasLimit as gas which is what's being done in 1.x. The decision to be made it whether we will continue to use gas to describe gasLimit in 4.x

  • Both the old and new ETH RPC spec use gas for eth_sendTransaction
  • Ethers.js uses gasLimit

closes #5077

spacesailor24 avatar Aug 04 '22 04:08 spacesailor24

Your Render PR Server URL is https://web3js-docs-pr-5312.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-cbll3vpa6gdhl7bpba8g.

render[bot] avatar Aug 04 '22 04:08 render[bot]

The decision to be made it whether we will continue to use gas to describe gasLimit in 4.x Both the old and new ETH RPC spec use gas for eth_sendTransaction Ethers.js uses gasLimit

Yellow paper specifies gasLimit instead of gas, but EL specs is using gas. https://github.com/ethereum/execution-apis/issues/283 will see its response.

IMO we should keep gas field for now. and in future should add support of gasLimit field as well

jdevcs avatar Aug 09 '22 15:08 jdevcs

Is it good to have both gasLimit and gas. Where both of them will have the exact same value? This would mean that the library user can read any of them. And this means, for the user, for example, an easier migration from ethers.js.

Muhammad-Altabba avatar Aug 16 '22 08:08 Muhammad-Altabba

I added code to replace gasLimit with gas when calling formatTransaction thinking this would be a good way to enforce the library only returning gas instead of gasLimit or gas, however this broke a lot of unit tests and wasn't a complete solution as there are other instances where transaction objects are formatted that don't use formatTransaction such as getTransaction. So, I propose we make a separate issue to decide globally how the library should handle gasLimit vs gas as a lot of code would need to be updated that's not directly related to this PR


#5387 was created to track this

spacesailor24 avatar Aug 26 '22 02:08 spacesailor24