From 008eb194b52febd71c86edcf6d58804279c45013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 7 Dec 2023 12:50:39 -0300 Subject: [PATCH 1/7] Fix `Mismatched aggregation bits length` error --- .../state_transition/operations.ex | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/lambda_ethereum_consensus/state_transition/operations.ex b/lib/lambda_ethereum_consensus/state_transition/operations.ex index b5bf22ba5..a48af6a61 100644 --- a/lib/lambda_ethereum_consensus/state_transition/operations.ex +++ b/lib/lambda_ethereum_consensus/state_transition/operations.ex @@ -854,22 +854,29 @@ defmodule LambdaEthereumConsensus.StateTransition.Operations do end defp mismatched_aggregation_bits_length?(attestation, beacon_committee) do - length_of_bitstring(attestation.aggregation_bits) - 1 != length(beacon_committee) + length_of_bitlist(attestation.aggregation_bits) != length(beacon_committee) end defp valid_signature?(state, indexed_attestation) do Predicates.is_valid_indexed_attestation(state, indexed_attestation) end - defp length_of_bitstring(binary) when is_binary(binary) do - binary - |> :binary.bin_to_list() - |> Enum.reduce("", fn byte, acc -> - acc <> Integer.to_string(byte, 2) - end) - |> String.length() + defp length_of_bitlist(bitlist) when is_binary(bitlist) do + bit_size = bit_size(bitlist) + <<_::size(bit_size - 8), last_byte>> = bitlist + bit_size - leading_zeros(<>) - 1 end + defp leading_zeros(<<1::1, _::7>>), do: 0 + defp leading_zeros(<<0::1, 1::1, _::6>>), do: 1 + defp leading_zeros(<<0::2, 1::1, _::5>>), do: 2 + defp leading_zeros(<<0::3, 1::1, _::4>>), do: 3 + defp leading_zeros(<<0::4, 1::1, _::3>>), do: 4 + defp leading_zeros(<<0::5, 1::1, _::2>>), do: 5 + defp leading_zeros(<<0::6, 1::1, _::1>>), do: 6 + defp leading_zeros(<<0::7, 1::1>>), do: 7 + defp leading_zeros(<<0::8>>), do: 8 + def process_bls_to_execution_change(state, signed_address_change) do address_change = signed_address_change.message From 79d9267cefcfb4405139d1080c20621e5d407f19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 7 Dec 2023 12:58:04 -0300 Subject: [PATCH 2/7] Invert some checks --- .../state_transition/operations.ex | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/lambda_ethereum_consensus/state_transition/operations.ex b/lib/lambda_ethereum_consensus/state_transition/operations.ex index a48af6a61..cbae6483e 100644 --- a/lib/lambda_ethereum_consensus/state_transition/operations.ex +++ b/lib/lambda_ethereum_consensus/state_transition/operations.ex @@ -744,13 +744,13 @@ defmodule LambdaEthereumConsensus.StateTransition.Operations do with {:ok, beacon_committee} <- Accessors.get_beacon_committee(state, data.slot, data.index), {:ok, indexed_attestation} <- Accessors.get_indexed_attestation(state, attestation) do cond do - invalid_target_epoch?(data, state) -> + not valid_target_epoch?(data, state) -> {:error, "Invalid target epoch"} epoch_mismatch?(data) -> {:error, "Epoch mismatch"} - invalid_slot_range?(data, state) -> + not valid_slot_range?(data, state) -> {:error, "Invalid slot range"} exceeds_committee_count?(data, state) -> @@ -835,18 +835,17 @@ defmodule LambdaEthereumConsensus.StateTransition.Operations do end end - defp invalid_target_epoch?(data, state) do - data.target.epoch < Accessors.get_previous_epoch(state) || - data.target.epoch > Accessors.get_current_epoch(state) + defp valid_target_epoch?(data, state) do + data.target.epoch in [Accessors.get_previous_epoch(state), Accessors.get_current_epoch(state)] end defp epoch_mismatch?(data) do data.target.epoch != Misc.compute_epoch_at_slot(data.slot) end - defp invalid_slot_range?(data, state) do - state.slot < data.slot + ChainSpec.get("MIN_ATTESTATION_INCLUSION_DELAY") || - state.slot > data.slot + ChainSpec.get("SLOTS_PER_EPOCH") + defp valid_slot_range?(data, state) do + data.slot + ChainSpec.get("MIN_ATTESTATION_INCLUSION_DELAY") <= state.slot or + state.slot <= data.slot + ChainSpec.get("SLOTS_PER_EPOCH") end defp exceeds_committee_count?(data, state) do From f895f97c28c6bc4c8b14fa5b20e10b429bf8335f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 7 Dec 2023 19:19:45 -0300 Subject: [PATCH 3/7] Refactor code to use `:ok`/`:error` checks --- .../state_transition/operations.ex | 94 ++++++++++--------- 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/lib/lambda_ethereum_consensus/state_transition/operations.ex b/lib/lambda_ethereum_consensus/state_transition/operations.ex index cbae6483e..a61c26592 100644 --- a/lib/lambda_ethereum_consensus/state_transition/operations.ex +++ b/lib/lambda_ethereum_consensus/state_transition/operations.ex @@ -644,11 +644,18 @@ defmodule LambdaEthereumConsensus.StateTransition.Operations do """ @spec process_attestation(BeaconState.t(), Attestation.t()) :: {:ok, BeaconState.t()} | {:error, binary()} - def process_attestation(state, attestation) do + def process_attestation(state, %Attestation{data: data} = attestation) do # TODO: optimize (takes ~3s) - with :ok <- verify_attestation_for_process(state, attestation) do + with :ok <- check_valid_target_epoch(data, state), + :ok <- check_epoch_matches(data), + :ok <- check_valid_slot_range(data, state), + :ok <- check_committee_count(data, state), + {:ok, beacon_committee} <- Accessors.get_beacon_committee(state, data.slot, data.index), + :ok <- check_matching_aggregation_bits_length(attestation, beacon_committee), + {:ok, indexed_attestation} <- Accessors.get_indexed_attestation(state, attestation), + :ok <- check_valid_signature(state, indexed_attestation) do # TODO: optimize (takes ~1s) - process_attestation(state, attestation.data, attestation.aggregation_bits) + process_attestation(state, data, attestation.aggregation_bits) end end @@ -740,34 +747,6 @@ defmodule LambdaEthereumConsensus.StateTransition.Operations do defp update_state(state, false, updated_epoch_participation), do: %{state | previous_epoch_participation: updated_epoch_participation} - def verify_attestation_for_process(state, %Attestation{data: data} = attestation) do - with {:ok, beacon_committee} <- Accessors.get_beacon_committee(state, data.slot, data.index), - {:ok, indexed_attestation} <- Accessors.get_indexed_attestation(state, attestation) do - cond do - not valid_target_epoch?(data, state) -> - {:error, "Invalid target epoch"} - - epoch_mismatch?(data) -> - {:error, "Epoch mismatch"} - - not valid_slot_range?(data, state) -> - {:error, "Invalid slot range"} - - exceeds_committee_count?(data, state) -> - {:error, "Index exceeds committee count"} - - mismatched_aggregation_bits_length?(attestation, beacon_committee) -> - {:error, "Mismatched aggregation bits length"} - - not valid_signature?(state, indexed_attestation) -> - {:error, "Invalid signature"} - - true -> - :ok - end - end - end - @doc """ Provide randomness to the operation of the beacon chain. """ @@ -835,29 +814,56 @@ defmodule LambdaEthereumConsensus.StateTransition.Operations do end end - defp valid_target_epoch?(data, state) do - data.target.epoch in [Accessors.get_previous_epoch(state), Accessors.get_current_epoch(state)] + defp check_valid_target_epoch(data, state) do + if data.target.epoch in [ + Accessors.get_previous_epoch(state), + Accessors.get_current_epoch(state) + ] do + :ok + else + {:error, "Invalid target epoch"} + end end - defp epoch_mismatch?(data) do - data.target.epoch != Misc.compute_epoch_at_slot(data.slot) + defp check_epoch_matches(data) do + if data.target.epoch == Misc.compute_epoch_at_slot(data.slot) do + :ok + else + {:error, "Epoch mismatch"} + end end - defp valid_slot_range?(data, state) do - data.slot + ChainSpec.get("MIN_ATTESTATION_INCLUSION_DELAY") <= state.slot or - state.slot <= data.slot + ChainSpec.get("SLOTS_PER_EPOCH") + defp check_valid_slot_range(data, state) do + if data.slot + ChainSpec.get("MIN_ATTESTATION_INCLUSION_DELAY") <= state.slot or + state.slot <= data.slot + ChainSpec.get("SLOTS_PER_EPOCH") do + :ok + else + {:error, "Invalid slot range"} + end end - defp exceeds_committee_count?(data, state) do - data.index >= Accessors.get_committee_count_per_slot(state, data.target.epoch) + defp check_committee_count(data, state) do + if data.index >= Accessors.get_committee_count_per_slot(state, data.target.epoch) do + {:error, "Index exceeds committee count"} + else + :ok + end end - defp mismatched_aggregation_bits_length?(attestation, beacon_committee) do - length_of_bitlist(attestation.aggregation_bits) != length(beacon_committee) + defp check_matching_aggregation_bits_length(attestation, beacon_committee) do + if length_of_bitlist(attestation.aggregation_bits) == length(beacon_committee) do + :ok + else + {:error, "Mismatched aggregation bits length"} + end end - defp valid_signature?(state, indexed_attestation) do - Predicates.is_valid_indexed_attestation(state, indexed_attestation) + defp check_valid_signature(state, indexed_attestation) do + if Predicates.is_valid_indexed_attestation(state, indexed_attestation) do + :ok + else + {:error, "Invalid signature"} + end end defp length_of_bitlist(bitlist) when is_binary(bitlist) do From b635b7ce7a50383a876b10ffc7291f07bb7e0e0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 7 Dec 2023 19:29:48 -0300 Subject: [PATCH 4/7] Rename `indexed_attestation_mainnet.ex` --- .../{indexed_attestation_mainnet.ex => indexed_attestation.ex} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename lib/ssz_types/beacon_chain/{indexed_attestation_mainnet.ex => indexed_attestation.ex} (100%) diff --git a/lib/ssz_types/beacon_chain/indexed_attestation_mainnet.ex b/lib/ssz_types/beacon_chain/indexed_attestation.ex similarity index 100% rename from lib/ssz_types/beacon_chain/indexed_attestation_mainnet.ex rename to lib/ssz_types/beacon_chain/indexed_attestation.ex From 87fc797a0ab0fe48cbb5e2567eb50cf8fd01ecba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 8 Dec 2023 15:13:40 -0300 Subject: [PATCH 5/7] Fix: was checking incorrect index --- .../state_transition/accessors.ex | 60 +++++++------------ 1 file changed, 21 insertions(+), 39 deletions(-) diff --git a/lib/lambda_ethereum_consensus/state_transition/accessors.ex b/lib/lambda_ethereum_consensus/state_transition/accessors.ex index 1399d7283..c189d05a8 100644 --- a/lib/lambda_ethereum_consensus/state_transition/accessors.ex +++ b/lib/lambda_ethereum_consensus/state_transition/accessors.ex @@ -434,21 +434,14 @@ defmodule LambdaEthereumConsensus.StateTransition.Accessors do @spec get_indexed_attestation(BeaconState.t(), Attestation.t()) :: {:ok, IndexedAttestation.t()} | {:error, binary()} def get_indexed_attestation(%BeaconState{} = state, attestation) do - case get_attesting_indices(state, attestation.data, attestation.aggregation_bits) do - {:ok, indices} -> - attesting_indices = indices - sorted_attesting_indices = Enum.sort(attesting_indices) - - res = %IndexedAttestation{ - attesting_indices: sorted_attesting_indices, - data: attestation.data, - signature: attestation.signature - } - - {:ok, res} - - {:error, reason} -> - {:error, reason} + with {:ok, indices} <- + get_attesting_indices(state, attestation.data, attestation.aggregation_bits) do + %IndexedAttestation{ + attesting_indices: Enum.sort(indices), + data: attestation.data, + signature: attestation.signature + } + |> then(&{:ok, &1}) end end @@ -458,33 +451,22 @@ defmodule LambdaEthereumConsensus.StateTransition.Accessors do @spec get_attesting_indices(BeaconState.t(), SszTypes.AttestationData.t(), SszTypes.bitlist()) :: {:ok, MapSet.t()} | {:error, binary()} def get_attesting_indices(%BeaconState{} = state, data, bits) do - case get_beacon_committee(state, data.slot, data.index) do - {:ok, committee} -> - bit_list = bitstring_to_list(bits) - - res = - committee - |> Stream.with_index() - |> Stream.filter(fn {_value, index} -> Enum.at(bit_list, index) == "1" end) - |> Stream.map(fn {value, _index} -> value end) - |> MapSet.new() - - {:ok, res} - - {:error, reason} -> - {:error, reason} + with {:ok, committee} <- get_beacon_committee(state, data.slot, data.index) do + committee + |> Stream.with_index() + |> Stream.filter(fn {_value, index} -> participated?(bits, index) end) + |> Stream.map(fn {value, _index} -> value end) + |> MapSet.new() + |> then(&{:ok, &1}) end end - def bitstring_to_list(binary) when is_binary(binary) do - binary - |> :binary.bin_to_list() - |> Enum.reduce("", fn byte, acc -> - acc <> Integer.to_string(byte, 2) - end) - # Exclude last bit - |> String.slice(0..-2) - |> String.graphemes() + defp participated?(bits, index) do + # The bit order inside the byte is reversed (e.g. bits[0] is the 8th bit). + # Here we keep the byte index the same, but reverse the bit index. + bit_index = index + 7 - 2 * rem(index, 8) + <<_::size(bit_index), flag::1, _::bits>> = bits + flag == 1 end @doc """ From 70f93b21da69f64e4c9b495f4d648256c9cfda2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 8 Dec 2023 15:21:43 -0300 Subject: [PATCH 6/7] Some small changes --- lib/lambda_ethereum_consensus/state_transition/misc.ex | 9 +++------ .../state_transition/state_transition.ex | 1 - 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/lambda_ethereum_consensus/state_transition/misc.ex b/lib/lambda_ethereum_consensus/state_transition/misc.ex index dd029d20e..060dab96e 100644 --- a/lib/lambda_ethereum_consensus/state_transition/misc.ex +++ b/lib/lambda_ethereum_consensus/state_transition/misc.ex @@ -247,11 +247,8 @@ defmodule LambdaEthereumConsensus.StateTransition.Misc do Return the signing root for the corresponding signing data. """ @spec compute_signing_root(SszTypes.bytes32(), SszTypes.domain()) :: SszTypes.root() - def compute_signing_root(<<_::256>> = data, domain) do - Ssz.hash_tree_root!(%SszTypes.SigningData{ - object_root: data, - domain: domain - }) + def compute_signing_root(<<_::256>> = root, domain) do + Ssz.hash_tree_root!(%SszTypes.SigningData{object_root: root, domain: domain}) end @spec compute_signing_root(any(), SszTypes.domain()) :: SszTypes.root() @@ -259,7 +256,7 @@ defmodule LambdaEthereumConsensus.StateTransition.Misc do ssz_object |> Ssz.hash_tree_root!() |> compute_signing_root(domain) end - @spec compute_signing_root(any(), module, SszTypes.domain()) :: SszTypes.root() + @spec compute_signing_root(any(), module(), SszTypes.domain()) :: SszTypes.root() def compute_signing_root(ssz_object, schema, domain) do ssz_object |> Ssz.hash_tree_root!(schema) |> compute_signing_root(domain) end diff --git a/lib/lambda_ethereum_consensus/state_transition/state_transition.ex b/lib/lambda_ethereum_consensus/state_transition/state_transition.ex index fcce6062d..16c9446d8 100644 --- a/lib/lambda_ethereum_consensus/state_transition/state_transition.ex +++ b/lib/lambda_ethereum_consensus/state_transition/state_transition.ex @@ -113,7 +113,6 @@ defmodule LambdaEthereumConsensus.StateTransition do Bls.valid?(proposer.pubkey, signing_root, signed_block.signature) end - # TODO: uncomment when implemented def process_block(state, block) do verify_and_notify_new_payload = &Execution.verify_and_notify_new_payload/1 From e538891739032f52ffdf8d76d36257a3a186c75b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 11 Dec 2023 10:36:14 -0300 Subject: [PATCH 7/7] Fix: wrong boolean operator --- lib/lambda_ethereum_consensus/state_transition/operations.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/lambda_ethereum_consensus/state_transition/operations.ex b/lib/lambda_ethereum_consensus/state_transition/operations.ex index a61c26592..76748eef1 100644 --- a/lib/lambda_ethereum_consensus/state_transition/operations.ex +++ b/lib/lambda_ethereum_consensus/state_transition/operations.ex @@ -834,7 +834,7 @@ defmodule LambdaEthereumConsensus.StateTransition.Operations do end defp check_valid_slot_range(data, state) do - if data.slot + ChainSpec.get("MIN_ATTESTATION_INCLUSION_DELAY") <= state.slot or + if data.slot + ChainSpec.get("MIN_ATTESTATION_INCLUSION_DELAY") <= state.slot and state.slot <= data.slot + ChainSpec.get("SLOTS_PER_EPOCH") do :ok else