[go: up one dir, main page]

EVM: Groom EthereumTransactionCommon type

Context

Next MR: !9555 (merged)

Initially, this MR was supposed to introduce ethereum usage crate to the codebase but after going forth and back I decided to give up this idea, here are reasons:

  • No support for decoding of unsigned legacy transactions, we perhaps want to support it (at least for tests as I can see)

  • Signing is not implemented in the crate, hence, no chance to reuse it

  • They allow transactions without chain_id  we perhaps want only support transactions with specified chain_id, at least according to the discussion in the thread

  • (The most crucial) Encoding/decoding for EIP-2930 & EIP-2718 is defined incorrectly: they wrap sequence of bytes

    0x01 || rlp([chainId, nonce, gasPrice, gasLimit, to, value, data, accessList, signatureYParity, signatureR, signatureS])

    with outer rlp list, like

    rlp(0x01, rlp([chainId, nonce, gasPrice, gasLimit, to, value, data, accessList, signatureYParity, signatureR, signatureS]) which is not correct, it should be raw bytes 0x01 || ... without any wrappers, not RLP

Basically, this MR refactor EthereumTransactionCommon type.

List of things done in this MR:

  • Option<TxSignature> instead of (v, r, s) is used in EthereumTransactionCommon
  • Prohibit unspecified chain_id completely. Fail during decoding if chain_id is not provided.
  • Impossible to create an invalid signature (with r = 0 or s = 0), hence, decoding will fail in this case as well.
  • Types of chain_id and v have been changed to u64
  • Create a module for tx_signature and rename signatures one to tx_common

Manually testing the MR

cargo make test still passes, though, some tests were modified a bit.

Checklist

  • Document the interface of any function added or modified (see the coding guidelines)
  • Document any change to the user interface, including configuration parameters (see node configuration)
  • Provide automatic testing (see the testing guide).
  • For new features and bug fixes, add an item in the appropriate changelog (docs/protocols/alpha.rst for the protocol and the environment, CHANGES.rst at the root of the repository for everything else).
  • Select suitable reviewers using the Reviewers field below.
  • Select as Assignee the next person who should take action on that MR
Edited by Valentin Chaboche

Merge request reports

Loading