Skip to content

feat: electra get_attesting_indices changes + helper functions from predicates/misc. #1419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LeanSerra merged 19 commits into from
Apr 10, 2025
+77 −18
Merged

feat: electra get_attesting_indices changes + helper functions from predicates/misc. #1419

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ed74379
feat EpochProcessing.process_slashings electra changes
LeanSerra Apr 3, 2025
71b3674
feat compute_proposer_index changes
LeanSerra Apr 4, 2025
9aa4202
feat compute_sync_committees Electra changes
LeanSerra Apr 4, 2025
f09b71b
fix fork_choice.ex tests were flaky because env var cleanup was not done
LeanSerra Apr 4, 2025
df45ee4
feat eligible_for_activation_queue Electra changes
LeanSerra Apr 7, 2025
8fb4d63
feat new compounding_withdrawal_credential? Electra function
LeanSerra Apr 7, 2025
6e34108
feat new has_compounding_withdrawal_credential Electra function
LeanSerra Apr 7, 2025
620e700
feat new has_execution_withdrawal_credential Electra function
LeanSerra Apr 7, 2025
d904593
feat fully_withdrawable_validator? Electra changes
LeanSerra Apr 7, 2025
5445583
feat new get_max_effective_balance Electra function
LeanSerra Apr 7, 2025
5cbc5af
feat partially_withdrawable_validator? Electra changes
LeanSerra Apr 7, 2025
753badb
fix typo in struct field in has_compounding_withdrawal_credential
LeanSerra Apr 7, 2025
29bf08a
feat new get_committee_indices Electra function
LeanSerra Apr 7, 2025
17bf479
feat get_attesting_indices Electra changes
LeanSerra Apr 7, 2025
42b0bfb
lint remove unnecessary parentheses in if
LeanSerra Apr 7, 2025
c084913
Merge branch 'electra-support' into electra_predicates
LeanSerra Apr 8, 2025
44de40b
refactor replace hardcoded eth1_address_withdrawal_prefix for constan…
LeanSerra Apr 9, 2025
bfeea12
refactor get_committee_indices closer to spec from code review
LeanSerra Apr 10, 2025
82f22f4
refactor correct error handling in get_attesting_indices from code re…
LeanSerra Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 32 additions & 11 deletions lib/lambda_ethereum_consensus/state_transition/accessors.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ defmodule LambdaEthereumConsensus.StateTransition.Accessors do
{:ok, IndexedAttestation.t()} | {:error, String.t()}
def get_indexed_attestation(%BeaconState{} = state, attestation) do
with {:ok, indices} <-
get_attesting_indices(state, attestation.data, attestation.aggregation_bits) do
get_attesting_indices(state, attestation) do
%IndexedAttestation{
attesting_indices: Enum.sort(indices),
data: attestation.data,
Expand Down Expand Up @@ -585,17 +585,28 @@ defmodule LambdaEthereumConsensus.StateTransition.Accessors do
that slot) and then filters the ones that actually participated. It returns an unordered MapSet,
which is useful for checking inclusion, but should be ordered if used to validate an attestation.
"""
@spec get_attesting_indices(BeaconState.t(), Types.AttestationData.t(), Types.bitlist()) ::
@spec get_attesting_indices(BeaconState.t(), Types.Attestation.t()) ::
{:ok, MapSet.t()} | {:error, String.t()}
def get_attesting_indices(%BeaconState{} = state, data, bits) do
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
def get_attesting_indices(%BeaconState{} = state, %Attestation{
data: data,
aggregation_bits: aggregation_bits,
committee_bits: committee_bits
}) do
committee_bits
|> get_committee_indices()
|> Enum.reduce({MapSet.new(), 0}, fn index, {old_set, offset} ->
with {:ok, committee} <- get_beacon_committee(state, data.slot, index) do
committee
|> Stream.with_index()
|> Stream.filter(fn {_value, index} ->
participated?(aggregation_bits, offset + index)
end)
|> Stream.map(fn {value, _index} -> value end)
|> MapSet.new()
|> then(&{MapSet.union(&1, old_set), offset + length(committee)})
end
end)
|> then(fn {map, _offset} -> {:ok, map} end)
Copy link
Collaborator

@rodrigo-o rodrigo-o Apr 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have an issue here, before when the get_beacon_committee/3 returned an error we just bubbled it up, now if this happens the reduce will fail because it will try to parse an {:error, error} as the {old_set, offset} tuple. We could go with a reduce_while, instead (BTW made some changes for readability):

  committee_bits
  |> get_committee_indices()
  |> Enum.reduce_while({MapSet.new(), 0}, fn committee_index, {attesters, offset} ->
    case get_beacon_committee(state, data.slot, committee_index) do
      {:ok, committee} ->
        # Process this committee's attesters
        committee_attesters = 
          committee
          |> Stream.with_index(offset)
          |> Stream.filter(fn {_validator, pos} -> participated?(aggregation_bits, pos) end)
          |> Stream.map(fn {validator, _} -> validator end)
          |> MapSet.new()
        
        {:cont, {MapSet.union(attesters, committee_attesters), offset + length(committee)}}
      
      error -> 
        # Depending on the specs we might just want to skip this error with something like: `{attesters, offset}`
        {:halt, error}
    end
  end)
  |> case do
    {:error, error} -> {:error, error}
    {attesters, _offset} -> {:ok, attesters}
  end

Also, I'm not 100% sure about the usefulness of Streams here, but given that it was already implemented that way I wouldn't change it to Enum for now.

Copy link
Contributor Author

@LeanSerra LeanSerra Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops I didn't know with behaved that way. I implemented the changes here. I also tried both {:halt, error} and {:cont, {attesters, offset} and the amount of tests passing is the same so for now I think we can leave it as {:halt, error}.

end

@spec get_committee_attesting_indices([Types.validator_index()], Types.bitlist()) ::
Expand Down Expand Up @@ -629,4 +640,14 @@ defmodule LambdaEthereumConsensus.StateTransition.Accessors do

max(ChainSpec.get("EFFECTIVE_BALANCE_INCREMENT"), total_balance)
end

@spec get_committee_indices(Types.bitvector()) :: Enumerable.t(Types.commitee_index())
def get_committee_indices(committee_bits) do
committee_bits
|> :binary.bin_to_list()
|> Enum.reverse()
|> Enum.with_index()
|> Enum.filter(fn {bit, _index} -> bit == 1 end)
|> Enum.map(fn {_bit, index} -> index end)
end
end
4 changes: 2 additions & 2 deletions lib/lambda_ethereum_consensus/state_transition/predicates.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ defmodule LambdaEthereumConsensus.StateTransition.Predicates do
@spec eligible_for_activation_queue?(Validator.t()) :: boolean
def eligible_for_activation_queue?(%Validator{} = validator) do
far_future_epoch = Constants.far_future_epoch()
max_effective_balance = ChainSpec.get("MAX_EFFECTIVE_BALANCE")
min_effective_balance = ChainSpec.get("MIN_ACTIVATION_BALANCE")

validator.activation_eligibility_epoch == far_future_epoch &&
validator.effective_balance == max_effective_balance
validator.effective_balance >= min_effective_balance
end

@doc """
Expand Down
46 changes: 40 additions & 6 deletions lib/types/beacon_chain/validator.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ defmodule Types.Validator do
"""
use LambdaEthereumConsensus.Container

@eth1_address_withdrawal_prefix <<0x01>>

fields = [
:pubkey,
:withdrawal_credentials,
Expand Down Expand Up @@ -38,7 +36,7 @@ defmodule Types.Validator do
@spec has_eth1_withdrawal_credential(t()) :: boolean
def has_eth1_withdrawal_credential(%{withdrawal_credentials: withdrawal_credentials}) do
<<first_byte_of_withdrawal_credentials::binary-size(1), _::binary>> = withdrawal_credentials
first_byte_of_withdrawal_credentials == @eth1_address_withdrawal_prefix
first_byte_of_withdrawal_credentials == Constants.eth1_address_withdrawal_prefix()
end

@doc """
Expand All @@ -51,7 +49,7 @@ defmodule Types.Validator do
balance,
epoch
) do
has_eth1_withdrawal_credential(validator) && withdrawable_epoch <= epoch && balance > 0
has_execution_withdrawal_credential(validator) && withdrawable_epoch <= epoch && balance > 0
end

@doc """
Expand All @@ -62,10 +60,12 @@ defmodule Types.Validator do
%{effective_balance: effective_balance} = validator,
balance
) do
max_effective_balance = ChainSpec.get("MAX_EFFECTIVE_BALANCE")
max_effective_balance = get_max_effective_balance(validator)
has_max_effective_balance = effective_balance == max_effective_balance
has_excess_balance = balance > max_effective_balance
has_eth1_withdrawal_credential(validator) && has_max_effective_balance && has_excess_balance

has_execution_withdrawal_credential(validator) && has_max_effective_balance &&
has_excess_balance
end

@impl LambdaEthereumConsensus.Container
Expand All @@ -81,4 +81,38 @@ defmodule Types.Validator do
{:withdrawable_epoch, TypeAliases.epoch()}
]
end

@spec compounding_withdrawal_credential?(Types.bytes32()) :: boolean()
def compounding_withdrawal_credential?(withdrawal_credentials) do
<<first_byte::binary-size(1), _::binary>> = withdrawal_credentials
first_byte == Constants.compounding_withdrawal_prefix()
end

@doc """
Check if ``validator`` has an 0x02 prefixed "compounding" withdrawal credential.
"""
@spec has_compounding_withdrawal_credential(t()) :: boolean()
def has_compounding_withdrawal_credential(validator) do
compounding_withdrawal_credential?(validator.withdrawal_credentials)
end

@doc """
Check if ``validator`` has a 0x01 or 0x02 prefixed withdrawal credential.
"""
@spec has_execution_withdrawal_credential(t()) :: boolean()
def has_execution_withdrawal_credential(validator) do
has_compounding_withdrawal_credential(validator) || has_eth1_withdrawal_credential(validator)
end

@doc """
Get max effective balance for ``validator``.
"""
@spec get_max_effective_balance(t()) :: Types.gwei()
def get_max_effective_balance(validator) do
if has_compounding_withdrawal_credential(validator) do
ChainSpec.get("MAX_EFFECTIVE_BALANCE_ELECTRA")
else
ChainSpec.get("MIN_ACTIVATION_BALANCE")
end
end
end
Loading