Fix p2 p view validation

lib/archethic/beacon_chain.ex

@@ -144,6 +144,9 @@ defmodule Archethic.BeaconChain do
       !SlotValidation.valid_end_of_node_sync?(slot) ->
         {:error, :invalid_end_of_node_sync}
 
+      !SlotValidation.valid_p2p_view?(slot) ->
+        {:error, :invalid_p2p_view}
+
       true ->
         :ok
     end

lib/archethic/beacon_chain/network_coordinates.ex

@@ -267,14 +267,23 @@ defmodule Archethic.BeaconChain.NetworkCoordinates do
     )
     |> Stream.filter(fn
       {:ok, {:ok, %NetworkStats{stats: stats}}} when map_size(stats) > 0 ->
-        true
+        valid_stats?(stats)
 
       _ ->
         false
     end)
     |> Stream.map(fn {:ok, {:ok, %NetworkStats{stats: stats}}} -> stats end)
   end
 
+  defp valid_stats?(stats) do
+    Enum.all?(stats, fn {subset, nodes_stats} ->
+      expected_stats_length = P2PSampling.list_nodes_to_sample(subset) |> length()
+
+      Enum.map(nodes_stats, fn {_node, stats} -> length(stats) end)
+      |> Enum.all?(&(&1 == expected_stats_length))
+    end)
+  end
+
   defp aggregate_stats_per_subset(stats) do
     stats
     |> Enum.flat_map(& &1)

lib/archethic/beacon_chain/slot/validation.ex

@@ -4,6 +4,7 @@ defmodule Archethic.BeaconChain.Slot.Validation do
   alias Archethic.BeaconChain.ReplicationAttestation
   alias Archethic.BeaconChain.Slot
   alias Archethic.BeaconChain.Slot.EndOfNodeSync
+  alias Archethic.BeaconChain.Subset.P2PSampling
 
   alias Archethic.P2P
   alias Archethic.P2P.Node
@@ -60,4 +61,18 @@ defmodule Archethic.BeaconChain.Slot.Validation do
       match?({:ok, %Node{first_public_key: ^key}}, P2P.get_node_info(key))
     end)
   end
+
+  @doc """
+  Validate the p2p view to ensure it correspond to the node list of the subset
+  """
+  @spec valid_p2p_view?(slot :: Slot.t()) :: boolean
+  def valid_p2p_view?(%Slot{
+        subset: subset,
+        p2p_view: %{availabilities: availabilities_bin, network_stats: network_stats}
+      }) do
+    subset_nodes_length = P2PSampling.list_nodes_to_sample(subset) |> length()
+    availabilities = for <<availability_time::16 <- availabilities_bin>>, do: availability_time
+
+    length(availabilities) == subset_nodes_length and length(network_stats) == subset_nodes_length
+  end
 end

lib/archethic/beacon_chain/summary_aggregate.ex

@@ -71,8 +71,8 @@ defmodule Archethic.BeaconChain.SummaryAggregate do
       )
       |> Map.update!(:availability_adding_time, &[availability_adding_time | &1])
 
-    if bit_size(node_availabilities) > 0 or length(node_average_availabilities) > 0 or
-         length(end_of_node_synchronizations) > 0 do
+    if node_availabilities != <<>> or not Enum.empty?(node_average_availabilities) or
+         not Enum.empty?(end_of_node_synchronizations) or not Enum.empty?(network_patches) do
       update_in(
         agg,
         [
@@ -85,22 +85,13 @@ defmodule Archethic.BeaconChain.SummaryAggregate do
           })
         ],
         fn prev ->
-          prev
-          |> Map.update!(
-            :node_availabilities,
-            &Enum.concat(&1, [Utils.bitstring_to_integer_list(node_availabilities)])
-          )
-          |> Map.update!(
-            :node_average_availabilities,
-            &Enum.concat(&1, [node_average_availabilities])
-          )
-          |> Map.update!(
-            :end_of_node_synchronizations,
-            &Enum.concat(&1, end_of_node_synchronizations)
-          )
-          |> Map.update!(
-            :network_patches,
-            &Enum.concat(&1, [network_patches])
+          add_p2p_availabilities(
+            subset,
+            prev,
+            node_availabilities,
+            node_average_availabilities,
+            end_of_node_synchronizations,
+            network_patches
           )
         end
       )
@@ -109,6 +100,49 @@ defmodule Archethic.BeaconChain.SummaryAggregate do
     end
   end
 
+  defp add_p2p_availabilities(
+         subset,
+         map,
+         node_availabilities,
+         node_average_availabilities,
+         end_of_node_synchronizations,
+         network_patches
+       ) do
+    map =
+      map
+      |> Map.update!(
+        :end_of_node_synchronizations,
+        &Enum.concat(&1, end_of_node_synchronizations)
+      )
+      |> Map.update!(
+        :network_patches,
+        &Enum.concat(&1, [network_patches])
+      )
+
+    expected_subset_length = P2PSampling.list_nodes_to_sample(subset) |> Enum.count()
+
+    map =
+      if bit_size(node_availabilities) == expected_subset_length do
+        map
+        |> Map.update!(
+          :node_availabilities,
+          &Enum.concat(&1, [Utils.bitstring_to_integer_list(node_availabilities)])
+        )
+      else
+        map
+      end
+
+    if Enum.count(node_average_availabilities) == expected_subset_length do
+      map
+      |> Map.update!(
+        :node_average_availabilities,
+        &Enum.concat(&1, [node_average_availabilities])
+      )
+    else
+      map
+    end
+  end
+
   @doc """
   Aggregate summaries batch
 

test/archethic/beacon_chain/network_coordinates_test.exs

@@ -13,7 +13,7 @@ defmodule Archethic.BeaconChain.NetworkCoordinatesTest do
   import Mox
 
   describe "fetch_network_stats/1" do
-    test "should retrieve the stats for a given summary time" do
+    setup do
       beacon_nodes =
         Enum.map(0..2, fn i ->
           %Node{
@@ -40,9 +40,11 @@ defmodule Archethic.BeaconChain.NetworkCoordinatesTest do
 
       Enum.each(beacon_nodes, &P2P.add_and_connect_node/1)
       Enum.each(sampled_nodes, &P2P.add_and_connect_node/1)
+    end
 
+    test "should retrieve the stats for a given summary time" do
       MockClient
-      |> stub(:send_message, fn
+      |> expect(:send_message, 3, fn
         _, %GetNetworkStats{subsets: _}, _ ->
           {:ok,
            %NetworkStats{
@@ -77,5 +79,62 @@ defmodule Archethic.BeaconChain.NetworkCoordinatesTest do
                [90, 105, 90, 0, 0, 0]
              ]) == NetworkCoordinates.fetch_network_stats(DateTime.utc_now())
     end
+
+    test "should filter stats that are different from expected nodes for a subset" do
+      ok_stats_1 = %NetworkStats{
+        stats: %{
+          <<0>> => %{
+            <<0::8, 0::8, 1::8, "key_b0">> => [%{latency: 100}, %{latency: 100}, %{latency: 100}],
+            <<0::8, 0::8, 1::8, "key_b1">> => [%{latency: 100}, %{latency: 100}, %{latency: 100}],
+            <<0::8, 0::8, 1::8, "key_b2">> => [%{latency: 100}, %{latency: 100}, %{latency: 100}]
+          }
+        }
+      }
+
+      ok_stats_2 = %NetworkStats{
+        stats: %{
+          <<0>> => %{
+            <<0::8, 0::8, 1::8, "key_b0">> => [%{latency: 200}, %{latency: 200}, %{latency: 200}],
+            <<0::8, 0::8, 1::8, "key_b1">> => [%{latency: 200}, %{latency: 200}, %{latency: 200}],
+            <<0::8, 0::8, 1::8, "key_b2">> => [%{latency: 200}, %{latency: 200}, %{latency: 200}]
+          }
+        }
+      }
+
+      wrong_stats = %NetworkStats{
+        stats: %{
+          <<0>> => %{
+            <<0::8, 0::8, 1::8, "key_b0">> => [%{latency: 100}, %{latency: 200}],
+            <<0::8, 0::8, 1::8, "key_b1">> => [%{latency: 100}, %{latency: 105}, %{latency: 90}],
+            <<0::8, 0::8, 1::8, "key_b2">> => [%{latency: 90}, %{latency: 105}, %{latency: 90}]
+          }
+        }
+      }
+
+      wrong_node = P2P.get_node_info!(<<0::8, 0::8, 1::8, "key_b0">>)
+      ok_node_1 = P2P.get_node_info!(<<0::8, 0::8, 1::8, "key_b1">>)
+      ok_node_2 = P2P.get_node_info!(<<0::8, 0::8, 1::8, "key_b2">>)
+
+      MockClient
+      |> expect(:send_message, 3, fn
+        ^wrong_node, %GetNetworkStats{subsets: _}, _ ->
+          {:ok, wrong_stats}
+
+        ^ok_node_1, %GetNetworkStats{subsets: _}, _ ->
+          {:ok, ok_stats_1}
+
+        ^ok_node_2, %GetNetworkStats{subsets: _}, _ ->
+          {:ok, ok_stats_2}
+      end)
+
+      assert [
+               [0, 0, 0, 150, 150, 150],
+               [0, 0, 0, 150, 150, 150],
+               [0, 0, 0, 150, 150, 150],
+               [150, 150, 150, 0, 0, 0],
+               [150, 150, 150, 0, 0, 0],
+               [150, 150, 150, 0, 0, 0]
+             ] == NetworkCoordinates.fetch_network_stats(DateTime.utc_now()) |> Nx.to_list()
+    end
   end
 end

test/archethic/beacon_chain/slot/validation_test.exs

@@ -0,0 +1,152 @@
+defmodule Archethic.BeaconChain.Slot.ValidationTest do
+  use ArchethicCase
+
+  alias Archethic.BeaconChain.ReplicationAttestation
+  alias Archethic.BeaconChain.Slot
+  alias Archethic.BeaconChain.Slot.EndOfNodeSync
+  alias Archethic.BeaconChain.Slot.Validation, as: SlotValidation
+
+  alias Archethic.P2P
+  alias Archethic.P2P.Node
+
+  alias Archethic.TransactionChain.TransactionSummary
+
+  alias Archethic.TransactionFactory
+
+  import Mock
+
+  describe "valid_transaction_attestations?/1" do
+    setup_with_mocks([
+      {ReplicationAttestation, [],
+       validate: fn %ReplicationAttestation{transaction_summary: %TransactionSummary{type: type}} ->
+         if type == :transfer, do: :ok, else: {:error, :invalid_confirmations_signatures}
+       end}
+    ]) do
+      :ok
+    end
+
+    test "should return true if all attestation are valid" do
+      tx1 =
+        TransactionFactory.create_valid_transaction([], seed: "abc")
+        |> TransactionSummary.from_transaction()
+
+      tx2 =
+        TransactionFactory.create_valid_transaction([], seed: "123")
+        |> TransactionSummary.from_transaction()
+
+      attestation1 = %ReplicationAttestation{transaction_summary: tx1, confirmations: []}
+      attestation2 = %ReplicationAttestation{transaction_summary: tx2, confirmations: []}
+
+      slot = %Slot{transaction_attestations: [attestation1, attestation2]}
+
+      assert SlotValidation.valid_transaction_attestations?(slot)
+    end
+
+    test "should return false if at least one attestation is invalid" do
+      tx1 =
+        TransactionFactory.create_valid_transaction([], seed: "abc")
+        |> TransactionSummary.from_transaction()
+
+      tx2 =
+        TransactionFactory.create_valid_transaction([], seed: "123", type: :node)
+        |> TransactionSummary.from_transaction()
+
+      attestation1 = %ReplicationAttestation{transaction_summary: tx1, confirmations: []}
+      attestation2 = %ReplicationAttestation{transaction_summary: tx2, confirmations: []}
+
+      slot = %Slot{transaction_attestations: [attestation1, attestation2]}
+
+      refute SlotValidation.valid_transaction_attestations?(slot)
+    end
+  end
+
+  describe "valid_end_of_node_sync?/1" do
+    setup do
+      P2P.add_and_connect_node(%Node{
+        ip: {127, 0, 0, 1},
+        port: 3000,
+        first_public_key: "key1",
+        last_public_key: "key1",
+        geo_patch: "AAA",
+        network_patch: "AAA"
+      })
+
+      P2P.add_and_connect_node(%Node{
+        ip: {127, 0, 0, 1},
+        port: 3000,
+        first_public_key: "key2",
+        last_public_key: "key2",
+        geo_patch: "AAA",
+        network_patch: "AAA"
+      })
+    end
+
+    test "should return true if node exists" do
+      slot = %Slot{
+        end_of_node_synchronizations: [
+          %EndOfNodeSync{public_key: "key1", timestamp: DateTime.utc_now()},
+          %EndOfNodeSync{public_key: "key2", timestamp: DateTime.utc_now()}
+        ]
+      }
+
+      assert SlotValidation.valid_end_of_node_sync?(slot)
+    end
+
+    test "should return false if at least one node doesn't exist" do
+      slot = %Slot{
+        end_of_node_synchronizations: [
+          %EndOfNodeSync{public_key: "key1", timestamp: DateTime.utc_now()},
+          %EndOfNodeSync{public_key: "key3", timestamp: DateTime.utc_now()}
+        ]
+      }
+
+      refute SlotValidation.valid_end_of_node_sync?(slot)
+    end
+  end
+
+  describe "valid_p2p_view?/1" do
+    setup do
+      P2P.add_and_connect_node(%Node{
+        ip: {127, 0, 0, 1},
+        port: 3000,
+        first_public_key: <<0::24, :crypto.strong_rand_bytes(31)::binary>>,
+        last_public_key: <<0::24, :crypto.strong_rand_bytes(31)::binary>>,
+        geo_patch: "AAA",
+        network_patch: "AAA"
+      })
+
+      P2P.add_and_connect_node(%Node{
+        ip: {127, 0, 0, 1},
+        port: 3000,
+        first_public_key: <<0::24, :crypto.strong_rand_bytes(31)::binary>>,
+        last_public_key: <<0::24, :crypto.strong_rand_bytes(31)::binary>>,
+        geo_patch: "AAA",
+        network_patch: "AAA"
+      })
+    end
+
+    test "should return true if p2p view length correspond to node subset" do
+      slot = %Slot{
+        subset: <<0>>,
+        p2p_view: %{
+          availabilities: <<600::16, 600::16>>,
+          network_stats: [%{latency: 10}, %{latency: 10}]
+        }
+      }
+
+      assert SlotValidation.valid_p2p_view?(slot)
+    end
+
+    test "should return false if p2p view length does not correspond to node subset" do
+      slot = %Slot{
+        subset: <<0>>,
+        p2p_view: %{
+          availabilities: <<600::16>>,
+          network_stats: [%{latency: 10}]
+        }
+      }
+
+      refute SlotValidation.valid_p2p_view?(slot)
+    end
+  end
+end