From ff76ce5a4e42ca434ad46de8264569ddeff97bf2 Mon Sep 17 00:00:00 2001
From: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date: Wed, 2 Oct 2024 15:52:35 -0300
Subject: [PATCH] Taking into account when the store is in the past for future
 slot calculations

---
 .../fork_choice/fork_choice.ex                | 33 ++++++++++---------
 .../p2p/gossip/beacon_block.ex                |  4 +--
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/lib/lambda_ethereum_consensus/fork_choice/fork_choice.ex b/lib/lambda_ethereum_consensus/fork_choice/fork_choice.ex
index 4eb2e448a..9caa8ef77 100644
--- a/lib/lambda_ethereum_consensus/fork_choice/fork_choice.ex
+++ b/lib/lambda_ethereum_consensus/fork_choice/fork_choice.ex
@@ -115,32 +115,33 @@ defmodule LambdaEthereumConsensus.ForkChoice do
   def get_current_slot(%Types.Store{} = store),
     do: compute_current_slot(store.time, store.genesis_time)
 
-  # TODO: Some parts of the node calculate the current slot using the previous function given a time
-  # specifically from the store (this was previously in the Store type module). The following function
-  # calculates it using the system time, we might need to make sure that each case makes sense.
   @doc """
   Get the current chain slot based on the system time.
+
+  TODO: There are just 2 uses of this function outside this module:
+   - At the begining of SyncBlocks.run/1 function, to get the head slot
+   - In the Helpers.block_root_by_block_id/1 function
   """
   @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
+  def get_current_chain_slot(genesis_time \\ StoreDb.fetch_genesis_time!()),
+    do: compute_current_slot(:os.system_time(:second), genesis_time)
 
   @doc """
   Check if a slot is in the future with respect to the systems time.
   """
   @spec future_slot?(Types.Store.t(), Types.slot()) :: boolean()
   def future_slot?(%Types.Store{} = store, slot) do
-    # Due to ticks being every 1000ms, we need to use the system time instead of
-    # the store time to account for the 500ms of MAXIMUM_GOSSIP_CLOCK_DISPARITY.
-    # the time in the store will always be a whole second.
-    time_ms = :os.system_time(:millisecond)
-
-    time_ms
-    |> compute_currents_slots_within_disparity(store.genesis_time)
-    |> Enum.all?(fn possible_slot -> possible_slot < slot end)
+    if get_current_slot(store) < get_current_chain_slot(store.genesis_time) do
+      # If the store store slot is in the past, we can safely assume that MAXIMUM_GOSSIP_CLOCK_DISPARITY
+      # will not make a difference, store time is updated once every second and disparity is just 500ms.
+      get_current_slot(store) < slot
+    else
+      # If the store slot is not in the past we need to take the actual system time in milliseconds
+      # to calculate the current slot, having in mind the MAXIMUM_GOSSIP_CLOCK_DISPARITY.
+      :os.system_time(:millisecond)
+      |> compute_currents_slots_within_disparity(store.genesis_time)
+      |> Enum.all?(fn possible_slot -> possible_slot < slot end)
+    end
   end
 
   @spec get_finalized_checkpoint() :: Types.Checkpoint.t()
diff --git a/lib/lambda_ethereum_consensus/p2p/gossip/beacon_block.ex b/lib/lambda_ethereum_consensus/p2p/gossip/beacon_block.ex
index 3ce2b8ab3..ee30d7422 100644
--- a/lib/lambda_ethereum_consensus/p2p/gossip/beacon_block.ex
+++ b/lib/lambda_ethereum_consensus/p2p/gossip/beacon_block.ex
@@ -63,7 +63,7 @@ defmodule LambdaEthereumConsensus.P2P.Gossip.BeaconBlock do
 
   @spec validate(SignedBeaconBlock.t(), Types.Store.t()) :: :ok | {:ignore, String.t()}
   defp validate(%SignedBeaconBlock{message: block}, store) do
-    current_slot = ForkChoice.get_current_chain_slot()
+    current_slot = ForkChoice.get_current_slot(store)
     min_slot = current_slot - ChainSpec.get("SLOTS_PER_EPOCH")
 
     cond do
@@ -74,7 +74,7 @@ defmodule LambdaEthereumConsensus.P2P.Gossip.BeaconBlock do
 
       ForkChoice.future_slot?(store, block.slot) ->
         {:ignore,
-         "Block is from the future: block.slot=#{block.slot}. Current slot: #{current_slot}."}
+         "Block is from the future: block.slot=#{block.slot}. Current store calculated slot: #{current_slot}."}
 
       true ->
         :ok