From 4a48914010ec7328ce087fba2f943774585b23d5 Mon Sep 17 00:00:00 2001 From: Alisina Bahadori Date: Mon, 5 Feb 2024 12:35:48 -0500 Subject: [PATCH] Refactor transaction signature functionality (#82) --- lib/ethers/signer/local.ex | 9 +- lib/ethers/transaction.ex | 152 +++++++++++++++---------------- test/ethers/transaction_test.exs | 25 ++--- 3 files changed, 92 insertions(+), 94 deletions(-) diff --git a/lib/ethers/signer/local.ex b/lib/ethers/signer/local.ex index 21759b2..452b991 100644 --- a/lib/ethers/signer/local.ex +++ b/lib/ethers/signer/local.ex @@ -36,8 +36,15 @@ defmodule Ethers.Signer.Local do Transaction.encode(tx) |> keccak_module().hash_256() |> secp256k1_module().sign(private_key) do + y_parity_or_v = Transaction.calculate_y_parity_or_v(tx, recovery_id) + signed = - %{tx | signature_r: r, signature_s: s, signature_recovery_id: recovery_id} + %Ethers.Transaction{ + tx + | signature_r: r, + signature_s: s, + signature_y_parity_or_v: y_parity_or_v + } |> Transaction.encode() |> Utils.hex_encode() diff --git a/lib/ethers/transaction.ex b/lib/ethers/transaction.ex index 36cbc32..147bbf8 100644 --- a/lib/ethers/transaction.ex +++ b/lib/ethers/transaction.ex @@ -9,53 +9,52 @@ defmodule Ethers.Transaction do @enforce_keys [:type] defstruct [ :type, + access_list: [], + block_hash: nil, + block_number: nil, chain_id: nil, - nonce: nil, - gas: nil, - from: nil, - to: nil, - value: "0x0", data: "", + from: nil, + gas: nil, gas_price: nil, + hash: nil, max_fee_per_gas: nil, max_priority_fee_per_gas: "0x0", - access_list: [], + nonce: nil, signature_r: nil, signature_s: nil, - signature_v: nil, - signature_recovery_id: nil, - signature_y_parity: nil, - block_hash: nil, - block_number: nil, - hash: nil, - transaction_index: nil + signature_y_parity_or_v: nil, + to: nil, + transaction_index: nil, + value: "0x0" ] @type t_transaction_type :: :legacy | :eip1559 | :eip2930 | :eip4844 @type t :: %__MODULE__{ - type: t_transaction_type(), + access_list: [{binary(), [binary()]}], + block_hash: binary() | nil, + block_number: binary() | nil, chain_id: binary() | nil, - nonce: binary() | nil, - gas: binary() | nil, - from: Types.t_address() | nil, - to: Types.t_address() | nil, - value: binary(), data: binary(), + from: Types.t_address() | nil, + gas: binary() | nil, gas_price: binary() | nil, + hash: binary() | nil, max_fee_per_gas: binary() | nil, max_priority_fee_per_gas: binary(), - access_list: [{binary(), [binary()]}], + nonce: binary() | nil, signature_r: binary() | nil, signature_s: binary() | nil, - signature_v: binary() | non_neg_integer() | nil, - signature_y_parity: binary() | non_neg_integer() | nil, - signature_recovery_id: binary() | 0 | 1 | nil, - block_hash: binary() | nil, - block_number: binary() | nil, - hash: binary() | nil, - transaction_index: binary() | nil + signature_y_parity_or_v: binary() | non_neg_integer() | nil, + to: Types.t_address() | nil, + transaction_index: binary() | nil, + type: t_transaction_type(), + value: binary() } + @transaction_envelope_types %{eip1559: <<2>>, legacy: <<>>} + @legacy_parity_magic_number 27 + @legacy_parity_with_chain_magic_number 35 @common_fillable_params [:chain_id, :nonce] @type_fillable_params %{ legacy: [:gas_price], @@ -69,9 +68,7 @@ defmodule Ethers.Transaction do :max_fee_per_gas, :max_priority_fee_per_gas, :nonce, - :signature_recovery_id, - :signature_y_parity, - :signature_v, + :signature_y_parity_or_v, :transaction_index, :value ] @@ -104,40 +101,13 @@ defmodule Ethers.Transaction do end end - def encode(%{type: :legacy} = tx) do - [ - tx.nonce, - tx.gas_price, - tx.gas, - tx.to, - tx.value, - tx.data - ] - |> maybe_add_signature(tx) + def encode(%__MODULE__{type: type} = transaction) do + transaction + |> to_rlp_list() + |> maybe_append_signature(transaction) |> convert_to_binary() |> ExRLP.encode() - end - - def encode(%{type: :eip1559} = tx) do - [ - tx.chain_id, - tx.nonce, - tx.max_priority_fee_per_gas, - tx.max_fee_per_gas, - tx.gas, - tx.to, - tx.value, - tx.data, - tx.access_list - ] - |> maybe_add_signature(tx) - |> convert_to_binary() - |> ExRLP.encode() - |> then(&(<<2>> <> &1)) - end - - def encode(%{type: type}) do - raise "Ethers does not support encoding of #{inspect(type)} transactions" + |> prepend_type_envelope(type) end def from_map(tx) do @@ -158,9 +128,7 @@ defmodule Ethers.Transaction do nonce: from_map_value(tx, :nonce), signature_r: from_map_value(tx, :r), signature_s: from_map_value(tx, :s), - signature_v: from_map_value(tx, :v), - signature_recovery_id: from_map_value(tx, :v), - signature_y_parity: from_map_value(tx, :yParity), + signature_y_parity_or_v: from_map_value(tx, :yParity) || from_map_value(tx, :v), to: from_map_value(tx, :to), transaction_index: from_map_value(tx, :transactionIndex), value: from_map_value(tx, :value) @@ -212,10 +180,11 @@ defmodule Ethers.Transaction do end) end - defp maybe_add_signature(tx_list, tx) do + defp maybe_append_signature(tx_list, tx) do case tx do - %{signature_r: r, signature_s: s} when has_value(r) and has_value(s) -> - tx_list ++ [get_y_parity(tx), trim_leading(r), trim_leading(s)] + %{signature_r: r, signature_s: s, signature_y_parity_or_v: y_parity} + when has_value(r) and has_value(s) and has_value(y_parity) -> + tx_list ++ [y_parity, trim_leading(r), trim_leading(s)] %{type: :legacy, chain_id: chain_id} when not is_nil(chain_id) -> # EIP-155 encoding for signature mitigation intra-chain replay attack @@ -226,6 +195,39 @@ defmodule Ethers.Transaction do end end + defp to_rlp_list(%{type: :eip1559} = tx) do + [ + tx.chain_id, + tx.nonce, + tx.max_priority_fee_per_gas, + tx.max_fee_per_gas, + tx.gas, + tx.to, + tx.value, + tx.data, + tx.access_list || [] + ] + end + + defp to_rlp_list(%{type: :legacy} = tx) do + [ + tx.nonce, + tx.gas_price, + tx.gas, + tx.to, + tx.value, + tx.data + ] + end + + defp to_rlp_list(%{type: type}) do + raise "Ethers does not support encoding of #{inspect(type)} transactions" + end + + defp prepend_type_envelope(tx_data, type) do + Map.fetch!(@transaction_envelope_types, type) <> tx_data + end + defp fill_action(:chain_id, _tx), do: :chain_id defp fill_action(:nonce, tx), do: {:get_transaction_count, [tx.from, "latest"]} defp fill_action(:max_fee_per_gas, _tx), do: :gas_price @@ -273,29 +275,23 @@ defmodule Ethers.Transaction do end) end - defp get_y_parity(%{signature_y_parity: y_parity}) when has_value(y_parity) do - y_parity - end - - defp get_y_parity(%{signature_recovery_id: rec_id} = tx) when has_value(rec_id) do + def calculate_y_parity_or_v(tx, recovery_id) when has_value(recovery_id) do case tx do %{type: :legacy, chain_id: chain_id} when has_value(chain_id) -> # EIP-155 chain_id = Utils.hex_to_integer!(chain_id) - rec_id + 35 + chain_id * 2 + recovery_id + @legacy_parity_with_chain_magic_number + chain_id * 2 %{type: :legacy} -> # EIP-155 - rec_id + 27 + recovery_id + @legacy_parity_magic_number _ -> # EIP-1559 - rec_id + recovery_id end end - defp get_y_parity(%{type: :legacy, signature_v: v}) when has_value(v), do: v - defp trim_leading(<<0, rest::binary>>), do: trim_leading(rest) defp trim_leading(<>), do: bin diff --git a/test/ethers/transaction_test.exs b/test/ethers/transaction_test.exs index 74fe986..beec5f0 100644 --- a/test/ethers/transaction_test.exs +++ b/test/ethers/transaction_test.exs @@ -19,9 +19,7 @@ defmodule Ethers.TransactionTest do access_list: [], signature_r: "0x639e5b615f34498f3e5a03f4831e4b7a2a1d5b61ed1388181ef7689c01466fc3", signature_s: "0x34a9311fae88125c4f9df5d0ed61f8e37bbaf62681f3ce96d03899114df8997", - signature_recovery_id: "0x1", - signature_y_parity: "0x1", - signature_v: "0x1", + signature_y_parity_or_v: "0x1", block_hash: "0xa2b720a9653afd26411e9bc94283cc496cd3d763378a67fd645bf1a4e332f37d", block_number: "0x595", hash: "0xdc78c7e7ea3a5980f732e466daf1fdc4f009e973530d7e84f0b2012f1ff2cfc7", @@ -48,22 +46,19 @@ defmodule Ethers.TransactionTest do transaction_index: 0, max_priority_fee_per_gas: 0, access_list: [], - signature_recovery_id: 1, - signature_y_parity: 1, - signature_v: 1 - } = decoded - - assert is_binary(decoded.data) - assert is_binary(decoded.signature_r) - assert is_binary(decoded.signature_s) + signature_y_parity_or_v: 1, + signature_r: Ethers.Utils.hex_decode!(@transaction_fixture.signature_r), + signature_s: Ethers.Utils.hex_decode!(@transaction_fixture.signature_s), + data: Ethers.Utils.hex_decode!(@transaction_fixture.data) + } == decoded end test "does not fail with missing values" do - assert %{signature_recovery_id: nil} = - Transaction.decode_values(%{@transaction_fixture | signature_recovery_id: nil}) + assert %{signature_y_parity_or_v: nil} = + Transaction.decode_values(%{@transaction_fixture | signature_y_parity_or_v: nil}) - assert %{signature_recovery_id: nil} = - Transaction.decode_values(%{@transaction_fixture | signature_recovery_id: ""}) + assert %{signature_y_parity_or_v: nil} = + Transaction.decode_values(%{@transaction_fixture | signature_y_parity_or_v: ""}) end end end