Unified current slot calculation and minor fixes

lib/lambda_ethereum_consensus/fork_choice/fork_choice.ex

@@ -31,7 +31,7 @@ defmodule LambdaEthereumConsensus.ForkChoice do
 
     store = Handlers.on_tick(store, time)
 
-    :telemetry.execute([:sync, :store], %{slot: Store.get_current_slot(store)})
+    :telemetry.execute([:sync, :store], %{slot: get_current_slot(store)})
     :telemetry.execute([:sync, :on_block], %{slot: head_slot})
 
     Metrics.block_status(head_root, head_slot, :transitioned)
@@ -111,13 +111,37 @@ defmodule LambdaEthereumConsensus.ForkChoice do
     |> tap(&StoreDb.persist_store/1)
   end
 
+  @spec get_current_slot(Types.Store.t()) :: Types.slot()
+  def get_current_slot(%Types.Store{} = store),
+    do: compute_current_slot(store.time, store.genesis_time)
+
+  @spec get_current_slot(Types.uint64(), Types.uint64()) :: Types.slot()
+  def get_current_slot(time, genesis_time),
+    do: compute_current_slot(time, genesis_time)
+
+  # TODO: Some parts of the code calculate the current slot using the previous function given a time
+  # specifically from the store (this was previously in the Store type module). This one calculates
+  # it using the system time, we might need to unify.
   @spec get_current_chain_slot() :: Types.slot()
   def get_current_chain_slot() do
     time = :os.system_time(:second)
     genesis_time = StoreDb.fetch_genesis_time!()
     compute_current_slot(time, genesis_time)
   end
 
+  @doc """
+  Check if a slot is in the future with respect to the current time.
+  """
+  @spec future_slot?(Types.slot()) :: boolean()
+  def future_slot?(slot) do
+    time = :os.system_time(:second)
+    genesis_time = StoreDb.fetch_genesis_time!()
+
+    time
+    |> compute_currents_slots_within_disparity(genesis_time)
+    |> Enum.all?(fn possible_slot -> possible_slot < slot end)
+  end
+
   @spec get_finalized_checkpoint() :: Types.Checkpoint.t()
   def get_finalized_checkpoint() do
     %{finalized_checkpoint: finalized} = fetch_store!()
@@ -282,11 +306,7 @@ defmodule LambdaEthereumConsensus.ForkChoice do
 
     Logger.debug("[Fork choice] Updated fork choice cache", slot: slot)
 
-    %{
-      store
-      | head_root: head_root,
-        head_slot: slot
-    }
+    Store.update_head_info(store, slot, head_root)
   end
 
   defp fetch_store!() do
@@ -297,6 +317,13 @@ defmodule LambdaEthereumConsensus.ForkChoice do
   defp compute_current_slot(time, genesis_time),
     do: div(time - genesis_time, ChainSpec.get("SECONDS_PER_SLOT"))
 
+  defp compute_currents_slots_within_disparity(time, genesis_time) do
+    [
+      compute_current_slot(time - ChainSpec.get("MAXIMUM_GOSSIP_CLOCK_DISPARITY"), genesis_time),
+      compute_current_slot(time + ChainSpec.get("MAXIMUM_GOSSIP_CLOCK_DISPARITY"), genesis_time)
+    ]
+  end
+
   defp compute_fork_digest(slot, genesis_validators_root) do
     Misc.compute_epoch_at_slot(slot)
     |> ChainSpec.get_fork_version_for_epoch()

lib/lambda_ethereum_consensus/fork_choice/handlers.ex

@@ -4,6 +4,7 @@ defmodule LambdaEthereumConsensus.ForkChoice.Handlers do
   """
   require Logger
 
+  alias LambdaEthereumConsensus.ForkChoice
   alias LambdaEthereumConsensus.Execution.ExecutionClient
   alias LambdaEthereumConsensus.StateTransition
   alias LambdaEthereumConsensus.StateTransition.Accessors
@@ -38,7 +39,7 @@ defmodule LambdaEthereumConsensus.ForkChoice.Handlers do
     # to ensure that every previous slot is processed with ``on_tick_per_slot``
     seconds_per_slot = ChainSpec.get("SECONDS_PER_SLOT")
     tick_slot = div(time - store.genesis_time, seconds_per_slot)
-    current_slot = Store.get_current_slot(store)
+    current_slot = ForkChoice.get_current_slot(store)
     next_slot_start = (current_slot + 1) * seconds_per_slot
     last_slot_start = tick_slot * seconds_per_slot
 
@@ -69,7 +70,7 @@ defmodule LambdaEthereumConsensus.ForkChoice.Handlers do
 
       # Blocks cannot be in the future. If they are, their
       # consideration must be delayed until they are in the past.
-      Store.get_current_slot(store) < block.slot ->
+      ForkChoice.future_slot?(block.slot) ->
         # TODO: handle this error somehow
         {:error, "block is from the future"}
 
@@ -235,8 +236,8 @@ defmodule LambdaEthereumConsensus.ForkChoice.Handlers do
       )
 
       is_first_block = new_store.proposer_boost_root == <<0::256>>
-      # TODO: store block timeliness data?
-      is_timely = Store.get_current_slot(new_store) == block.slot and is_before_attesting_interval
+      # TODO: store block timeliness data? we might need to take MAXIMUM_GOSSIP_CLOCK_DISPARITY into account
+      is_timely = ForkChoice.get_current_slot(new_store) == block.slot and is_before_attesting_interval
 
       state = new_state_info.beacon_state
 
@@ -283,12 +284,13 @@ defmodule LambdaEthereumConsensus.ForkChoice.Handlers do
   end
 
   defp on_tick_per_slot(%Store{} = store, time) do
-    previous_slot = Store.get_current_slot(store)
+    previous_slot = ForkChoice.get_current_slot(store)
 
     # Update store time
     store = %Store{store | time: time}
 
-    current_slot = Store.get_current_slot(store)
+    # Why is this needed? the previous line shoud be immediate.
+    current_slot = ForkChoice.get_current_slot(store)
 
     store
     # If this is a new slot, reset store.proposer_boost_root
@@ -394,10 +396,10 @@ defmodule LambdaEthereumConsensus.ForkChoice.Handlers do
       target.root != Store.get_checkpoint_block(store, block_root, target.epoch) ->
         {:error, "mismatched head and target blocks"}
 
-      # Attestations can only affect the fork choice of subsequent slots.
+      # Attestations can only affect the fork choice of subsequent slots (that's why the - 1).
       # Delay consideration in the fork choice until their slot is in the past.
       # TODO: delay consideration
-      Store.get_current_slot(store) <= attestation.data.slot ->
+      ForkChoice.future_slot?(attestation.data.slot - 1) ->
         {:error, "attestation is for a future slot"}
 
       true ->

lib/lambda_ethereum_consensus/p2p/gossip/beacon_block.ex

@@ -17,11 +17,9 @@ defmodule LambdaEthereumConsensus.P2P.Gossip.BeaconBlock do
 
   @impl true
   def handle_gossip_message(store, _topic, msg_id, message) do
-    slot = ForkChoice.get_current_chain_slot()
-
     with {:ok, uncompressed} <- :snappyer.decompress(message),
          {:ok, signed_block} <- Ssz.from_ssz(uncompressed, SignedBeaconBlock),
-         :ok <- validate(signed_block, slot) do
+         :ok <- validate(signed_block) do
       Logger.info("[Gossip] Block received, block.slot: #{signed_block.message.slot}.")
       Libp2pPort.validate_message(msg_id, :accept)
       PendingBlocks.add_block(store, signed_block)
@@ -63,8 +61,9 @@ defmodule LambdaEthereumConsensus.P2P.Gossip.BeaconBlock do
   ### Private functions
   ##########################
 
-  @spec validate(SignedBeaconBlock.t(), Types.slot()) :: :ok | {:ignore, String.t()}
-  defp validate(%SignedBeaconBlock{message: block}, current_slot) do
+  @spec validate(SignedBeaconBlock.t()) :: :ok | {:ignore, String.t()}
+  defp validate(%SignedBeaconBlock{message: block}) do
+    current_slot = ForkChoice.get_current_chain_slot()
     min_slot = current_slot - ChainSpec.get("SLOTS_PER_EPOCH")
 
     cond do
@@ -73,7 +72,7 @@ defmodule LambdaEthereumConsensus.P2P.Gossip.BeaconBlock do
         {:ignore,
          "Block too old: block.slot=#{block.slot}. Current slot: #{current_slot}. Minimum expected slot: #{min_slot}"}
 
-      block.slot > current_slot ->
+      ForkChoice.future_slot?(block.slot) ->
         {:ignore,
          "Block is from the future: block.slot=#{block.slot}. Current slot: #{current_slot}."}
 

lib/lambda_ethereum_consensus/p2p/peerbook.ex

@@ -7,7 +7,7 @@ defmodule LambdaEthereumConsensus.P2P.Peerbook do
   alias LambdaEthereumConsensus.Store.KvSchema
 
   @initial_score 100
-  @penalize 2
+  @penalize 5
   @target_peers 128
   @max_prune_size 8
   @prune_percentage 0.05
@@ -43,14 +43,17 @@ defmodule LambdaEthereumConsensus.P2P.Peerbook do
   Get some peer from the peerbook.
   """
   def get_some_peer() do
-    # TODO: use some algorithm to pick a good peer, for now it's random
+    # TODO: This is a very naive implementation of a peer selection algorithm.
     peerbook = fetch_peerbook!()
 
     if peerbook == %{} do
       nil
     else
-      {peer_id, _score} = Enum.random(peerbook)
-      peer_id
+      peerbook
+      |> Enum.sort_by(fn {_peer_id, score} -> score end)
+      |> Enum.take(4)
+      |> Enum.random()
+      |> elem(0)
     end
   end
 

lib/types/store.ex

@@ -3,6 +3,7 @@ defmodule Types.Store do
     The Store struct is used to track information required for the fork choice algorithm.
   """
 
+  alias LambdaEthereumConsensus.ForkChoice
   alias LambdaEthereumConsensus.ForkChoice.Head
   alias LambdaEthereumConsensus.ForkChoice.Simple.Tree
   alias LambdaEthereumConsensus.StateTransition
@@ -110,13 +111,9 @@ defmodule Types.Store do
     end
   end
 
-  def get_current_slot(%__MODULE__{time: time, genesis_time: genesis_time}) do
-    # NOTE: this assumes GENESIS_SLOT == 0
-    div(time - genesis_time, ChainSpec.get("SECONDS_PER_SLOT"))
-  end
-
+  # We probably want to move this to a more appropriate module
   def get_current_epoch(store) do
-    store |> get_current_slot() |> Misc.compute_epoch_at_slot()
+    store |> ForkChoice.get_current_slot() |> Misc.compute_epoch_at_slot()
   end
 
   def get_ancestor(%__MODULE__{} = store, root, slot) do
@@ -245,9 +242,15 @@ defmodule Types.Store do
     end
   end
 
-  defp update_head_info(store) do
+  @spec update_head_info(t()) :: t()
+  def update_head_info(store) do
     {:ok, head_root} = Head.get_head(store)
     %{slot: head_slot} = Blocks.get_block!(head_root)
+   update_head_info(store, head_slot, head_root)
+  end
+
+  @spec update_head_info(t(), Types.slot(), Types.root()) :: t()
+  def update_head_info(store, head_slot, head_root) do
     %{store | head_root: head_root, head_slot: head_slot}
   end