Ensure there is never more validation nodes than storage nodes (arche…

…thic-foundation#722)

* Ensure there is never more validation nodes than storage nodes

and remove an obsolete sort

* Add tests to check validation nodes <= storage nodes

* Move the logic directly in the election function

* lint

* clean tests' tags

lib/archethic/election.ex

@@ -107,8 +107,7 @@ defmodule Archethic.Election do
       [
         %Node{last_public_key: "node6", geo_patch: "ECA"},
         %Node{last_public_key: "node5", geo_patch: "F10"},
-        %Node{last_public_key: "node3", geo_patch: "AA0"},
-        %Node{last_public_key: "node2", geo_patch: "DEF"}
+        %Node{last_public_key: "node3", geo_patch: "AA0"}
       ]
   """
   @spec validation_nodes(
@@ -132,7 +131,10 @@ defmodule Archethic.Election do
     start = System.monotonic_time()
 
     # Evaluate validation constraints
-    nb_validations = validation_number_fun.(tx, length(authorized_nodes))
+    # Ensure there is never more validation nodes than storage nodes
+    nb_validations =
+      min(length(storage_nodes), validation_number_fun.(tx, length(authorized_nodes)))
+
     min_geo_patch = min_geo_patch_fun.()
 
     nodes =
@@ -154,7 +156,7 @@ defmodule Archethic.Election do
       %{
         duration: System.monotonic_time() - start
       },
-      %{nb_nodes: length(authorized_nodes)}
+      %{nb_nodes: length(nodes)}
     )
 
     nodes
@@ -217,7 +219,7 @@ defmodule Archethic.Election do
   end
 
   defp validation_constraints_satisfied?(nb_validations, min_geo_patch, nb_nodes, zones) do
-    MapSet.size(zones) > min_geo_patch and nb_nodes > nb_validations
+    MapSet.size(zones) >= min_geo_patch and nb_nodes >= nb_validations
   end
 
   defp refine_necessary_nodes(selected_nodes, authorized_nodes, nb_validations) do
@@ -271,7 +273,6 @@ defmodule Archethic.Election do
 
     storage_nodes =
       nodes
-      |> Enum.sort_by(&Map.get(&1, :authorized?), :desc)
       |> sort_storage_nodes_by_key_rotation(address, storage_nonce)
       |> Enum.reduce_while(
         %{

test/archethic/election_test.exs

@@ -9,6 +9,9 @@ defmodule Archethic.ElectionTest do
 
   alias Archethic.TransactionChain.Transaction
   alias Archethic.TransactionChain.TransactionData
+  alias Archethic.TransactionChain.TransactionData.Ledger
+  alias Archethic.TransactionChain.TransactionData.UCOLedger
+  alias Archethic.TransactionChain.TransactionData.UCOLedger.Transfer
 
   doctest Election
 
@@ -131,6 +134,93 @@ defmodule Archethic.ElectionTest do
       assert Enum.map(first_election, & &1.last_public_key) !=
                Enum.map(second_election, & &1.last_public_key)
     end
+
+    test "should never return more validation nodes than storages nodes" do
+      authorized_nodes = [
+        %Node{
+          first_public_key: "Node0",
+          last_public_key: "Node0",
+          available?: true,
+          geo_patch: "AAA"
+        },
+        %Node{
+          first_public_key: "Node1",
+          last_public_key: "Node1",
+          available?: true,
+          geo_patch: "BBB"
+        },
+        %Node{
+          first_public_key: "Node2",
+          last_public_key: "Node2",
+          available?: true,
+          geo_patch: "CCC"
+        },
+        %Node{
+          first_public_key: "Node3",
+          last_public_key: "Node3",
+          available?: true,
+          geo_patch: "DDD"
+        }
+      ]
+
+      storage_nodes = [
+        %Node{
+          first_public_key: "Node10",
+          last_public_key: "Node10",
+          available?: true,
+          geo_patch: random_patch()
+        },
+        %Node{
+          first_public_key: "Node11",
+          last_public_key: "Node11",
+          available?: true,
+          geo_patch: random_patch()
+        },
+        %Node{
+          first_public_key: "Node12",
+          last_public_key: "Node12",
+          available?: true,
+          geo_patch: random_patch()
+        }
+      ]
+
+      tx1 = %Transaction{
+        address:
+          <<0, 120, 195, 32, 77, 84, 215, 196, 116, 215, 56, 141, 40, 54, 226, 48, 66, 254, 119,
+            11, 73, 77, 243, 125, 62, 94, 133, 67, 9, 253, 45, 134, 89>>,
+        type: :transfer,
+        data: %TransactionData{
+          ledger: %Ledger{
+            # 1_000_000_000 will require 1 more validator than the min (1 + 3 = 4)
+            uco: %UCOLedger{transfers: [%Transfer{to: <<>>, amount: 1_000_000_000}]}
+          }
+        },
+        previous_public_key:
+          <<0, 239, 240, 90, 182, 66, 190, 68, 20, 250, 131, 83, 190, 29, 184, 177, 52, 166, 207,
+            80, 193, 110, 57, 6, 199, 152, 184, 24, 178, 179, 11, 164, 150>>,
+        previous_signature:
+          <<200, 70, 0, 25, 105, 111, 15, 161, 146, 188, 100, 234, 147, 62, 127, 8, 152, 60, 66,
+            169, 113, 255, 51, 112, 59, 200, 61, 63, 128, 228, 111, 104, 47, 15, 81, 185, 179, 36,
+            59, 86, 171, 7, 138, 199, 203, 252, 50, 87, 160, 107, 119, 131, 121, 11, 239, 169, 99,
+            203, 76, 159, 158, 243, 133, 133>>,
+        origin_signature:
+          <<162, 223, 100, 72, 17, 56, 99, 212, 78, 132, 166, 81, 127, 91, 214, 143, 221, 32, 106,
+            189, 247, 64, 183, 27, 55, 142, 254, 72, 47, 215, 34, 108, 233, 55, 35, 94, 49, 165,
+            180, 248, 229, 160, 229, 220, 191, 35, 80, 127, 213, 240, 195, 185, 165, 89, 172, 97,
+            170, 217, 57, 254, 125, 127, 62, 169>>
+      }
+
+      assert 3 ==
+               length(
+                 Election.validation_nodes(
+                   tx1,
+                   "sorting_seed",
+                   authorized_nodes,
+                   storage_nodes,
+                   ValidationConstraints.new()
+                 )
+               )
+    end
   end
 
   describe "storage_nodes/1" do

test/archethic_test.exs

@@ -269,7 +269,6 @@ defmodule ArchethicTest do
   end
 
   describe "get_transaction_inputs/1" do
-    @tag :toto1
     test "should request the storages nodes to fetch the inputs remotely, this is latest tx" do
       address1 = <<0::8, 0::8, :crypto.strong_rand_bytes(32)::binary>>