From 682ed26351d2d84f7370642df66a3534a86a3959 Mon Sep 17 00:00:00 2001 From: Bastien CHAMAGNE Date: Mon, 28 Nov 2022 14:55:53 +0100 Subject: [PATCH 1/5] Ensure there is never more validation nodes than storage nodes and remove an obsolete sort --- lib/archethic/election.ex | 1 - lib/archethic/mining/distributed_workflow.ex | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/archethic/election.ex b/lib/archethic/election.ex index 05dc33c72..206a98c0a 100755 --- a/lib/archethic/election.ex +++ b/lib/archethic/election.ex @@ -271,7 +271,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( %{ diff --git a/lib/archethic/mining/distributed_workflow.ex b/lib/archethic/mining/distributed_workflow.ex index 6f7a9b3d4..f708c64c2 100644 --- a/lib/archethic/mining/distributed_workflow.ex +++ b/lib/archethic/mining/distributed_workflow.ex @@ -193,7 +193,9 @@ defmodule Archethic.Mining.DistributedWorkflow do |> Election.io_storage_nodes(authorized_nodes) end - [coordinator_node | cross_validation_nodes] = validation_nodes + # ensure there is never more validation than storage nodes + [coordinator_node | cross_validation_nodes] = + Enum.take(validation_nodes, length(chain_storage_nodes)) context = ValidationContext.new( From e20541c0edcf1e5f53bc507289590e163dd2aa07 Mon Sep 17 00:00:00 2001 From: Bastien CHAMAGNE Date: Mon, 28 Nov 2022 15:48:31 +0100 Subject: [PATCH 2/5] Add tests to check validation nodes <= storage nodes --- .../mining/distributed_workflow_test.exs | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/test/archethic/mining/distributed_workflow_test.exs b/test/archethic/mining/distributed_workflow_test.exs index cda4712cc..548b464e8 100644 --- a/test/archethic/mining/distributed_workflow_test.exs +++ b/test/archethic/mining/distributed_workflow_test.exs @@ -20,6 +20,7 @@ defmodule Archethic.Mining.DistributedWorkflowTest do alias Archethic.P2P.Message.CrossValidate alias Archethic.P2P.Message.CrossValidationDone alias Archethic.P2P.Message.GetTransaction + alias Archethic.P2P.Message.GetTransactionSummary alias Archethic.P2P.Message.GetUnspentOutputs alias Archethic.P2P.Message.NotFound alias Archethic.P2P.Message.Ok @@ -148,6 +149,74 @@ defmodule Archethic.Mining.DistributedWorkflowTest do end end + test "should never have more validation nodes than storage", %{ + tx: tx, + sorting_seed: sorting_seed + } do + validation_nodes = + Election.validation_nodes( + tx, + sorting_seed, + P2P.authorized_and_available_nodes(), + Election.chain_storage_nodes_with_type( + tx.address, + tx.type, + P2P.authorized_and_available_nodes() + ) + ) + + welcome_node = %Node{ + ip: {80, 10, 20, 102}, + port: 3005, + http_port: 4000, + first_public_key: "key1", + last_public_key: "key1", + reward_address: <<0::8, 0::8, :crypto.strong_rand_bytes(32)::binary>>, + geo_patch: "AAA", + network_patch: "AAA" + } + + P2P.add_and_connect_node(welcome_node) + + MockClient + |> stub(:send_message, fn + _, %Ping{}, _ -> + {:ok, %Ok{}} + + _, %GetTransaction{}, _ -> + {:ok, %Transaction{}} + + _, %GetUnspentOutputs{}, _ -> + {:ok, %UnspentOutputList{unspent_outputs: []}} + + _, %GetTransactionSummary{}, _ -> + {:error, %NotFound{}} + end) + + # Fake (3*2=6) validation nodes + validation_nodes = Enum.concat([validation_nodes, validation_nodes, validation_nodes]) + assert 6 == length(validation_nodes) + + {:ok, coordinator_pid} = + Workflow.start_link( + transaction: tx, + welcome_node: welcome_node, + validation_nodes: validation_nodes, + node_public_key: List.first(validation_nodes).last_public_key + ) + + {_, + %{ + context: %ValidationContext{ + cross_validation_nodes: cross_validation_nodes, + chain_storage_nodes: chain_storage_nodes + } + }} = :sys.get_state(coordinator_pid) + + # 1 is the coordinator node + assert length(chain_storage_nodes) >= 1 + length(cross_validation_nodes) + end + describe "add_mining_context/6" do test "should aggregate context and wait enough confirmed validation nodes context building", %{tx: tx, sorting_seed: sorting_seed} do From b770c91092401d5458c020cb05d2c44c18a44782 Mon Sep 17 00:00:00 2001 From: Bastien CHAMAGNE Date: Mon, 28 Nov 2022 17:55:35 +0100 Subject: [PATCH 3/5] Move the logic directly in the election function --- lib/archethic/election.ex | 12 ++- lib/archethic/mining/distributed_workflow.ex | 4 +- test/archethic/election_test.exs | 91 +++++++++++++++++++ .../mining/distributed_workflow_test.exs | 68 -------------- 4 files changed, 99 insertions(+), 76 deletions(-) diff --git a/lib/archethic/election.ex b/lib/archethic/election.ex index 206a98c0a..7ca38a3d6 100755 --- a/lib/archethic/election.ex +++ b/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 diff --git a/lib/archethic/mining/distributed_workflow.ex b/lib/archethic/mining/distributed_workflow.ex index f708c64c2..6f7a9b3d4 100644 --- a/lib/archethic/mining/distributed_workflow.ex +++ b/lib/archethic/mining/distributed_workflow.ex @@ -193,9 +193,7 @@ defmodule Archethic.Mining.DistributedWorkflow do |> Election.io_storage_nodes(authorized_nodes) end - # ensure there is never more validation than storage nodes - [coordinator_node | cross_validation_nodes] = - Enum.take(validation_nodes, length(chain_storage_nodes)) + [coordinator_node | cross_validation_nodes] = validation_nodes context = ValidationContext.new( diff --git a/test/archethic/election_test.exs b/test/archethic/election_test.exs index 7b144d2f8..ed17e9f6e 100644 --- a/test/archethic/election_test.exs +++ b/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,94 @@ defmodule Archethic.ElectionTest do assert Enum.map(first_election, & &1.last_public_key) != Enum.map(second_election, & &1.last_public_key) end + + @tag :toto + 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 diff --git a/test/archethic/mining/distributed_workflow_test.exs b/test/archethic/mining/distributed_workflow_test.exs index 548b464e8..a7a9611cc 100644 --- a/test/archethic/mining/distributed_workflow_test.exs +++ b/test/archethic/mining/distributed_workflow_test.exs @@ -149,74 +149,6 @@ defmodule Archethic.Mining.DistributedWorkflowTest do end end - test "should never have more validation nodes than storage", %{ - tx: tx, - sorting_seed: sorting_seed - } do - validation_nodes = - Election.validation_nodes( - tx, - sorting_seed, - P2P.authorized_and_available_nodes(), - Election.chain_storage_nodes_with_type( - tx.address, - tx.type, - P2P.authorized_and_available_nodes() - ) - ) - - welcome_node = %Node{ - ip: {80, 10, 20, 102}, - port: 3005, - http_port: 4000, - first_public_key: "key1", - last_public_key: "key1", - reward_address: <<0::8, 0::8, :crypto.strong_rand_bytes(32)::binary>>, - geo_patch: "AAA", - network_patch: "AAA" - } - - P2P.add_and_connect_node(welcome_node) - - MockClient - |> stub(:send_message, fn - _, %Ping{}, _ -> - {:ok, %Ok{}} - - _, %GetTransaction{}, _ -> - {:ok, %Transaction{}} - - _, %GetUnspentOutputs{}, _ -> - {:ok, %UnspentOutputList{unspent_outputs: []}} - - _, %GetTransactionSummary{}, _ -> - {:error, %NotFound{}} - end) - - # Fake (3*2=6) validation nodes - validation_nodes = Enum.concat([validation_nodes, validation_nodes, validation_nodes]) - assert 6 == length(validation_nodes) - - {:ok, coordinator_pid} = - Workflow.start_link( - transaction: tx, - welcome_node: welcome_node, - validation_nodes: validation_nodes, - node_public_key: List.first(validation_nodes).last_public_key - ) - - {_, - %{ - context: %ValidationContext{ - cross_validation_nodes: cross_validation_nodes, - chain_storage_nodes: chain_storage_nodes - } - }} = :sys.get_state(coordinator_pid) - - # 1 is the coordinator node - assert length(chain_storage_nodes) >= 1 + length(cross_validation_nodes) - end - describe "add_mining_context/6" do test "should aggregate context and wait enough confirmed validation nodes context building", %{tx: tx, sorting_seed: sorting_seed} do From 8db47cb9ac3c31b247ab42a50f194f15f5ddf0eb Mon Sep 17 00:00:00 2001 From: Bastien CHAMAGNE Date: Mon, 28 Nov 2022 18:05:23 +0100 Subject: [PATCH 4/5] lint --- test/archethic/mining/distributed_workflow_test.exs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/archethic/mining/distributed_workflow_test.exs b/test/archethic/mining/distributed_workflow_test.exs index a7a9611cc..cda4712cc 100644 --- a/test/archethic/mining/distributed_workflow_test.exs +++ b/test/archethic/mining/distributed_workflow_test.exs @@ -20,7 +20,6 @@ defmodule Archethic.Mining.DistributedWorkflowTest do alias Archethic.P2P.Message.CrossValidate alias Archethic.P2P.Message.CrossValidationDone alias Archethic.P2P.Message.GetTransaction - alias Archethic.P2P.Message.GetTransactionSummary alias Archethic.P2P.Message.GetUnspentOutputs alias Archethic.P2P.Message.NotFound alias Archethic.P2P.Message.Ok From 5646e40c1f4c97896123ece7d4587ceec3c76e5c Mon Sep 17 00:00:00 2001 From: Bastien CHAMAGNE Date: Wed, 30 Nov 2022 09:31:39 +0100 Subject: [PATCH 5/5] clean tests' tags --- test/archethic/election_test.exs | 1 - test/archethic_test.exs | 1 - 2 files changed, 2 deletions(-) diff --git a/test/archethic/election_test.exs b/test/archethic/election_test.exs index ed17e9f6e..fa9c689fd 100644 --- a/test/archethic/election_test.exs +++ b/test/archethic/election_test.exs @@ -135,7 +135,6 @@ defmodule Archethic.ElectionTest do Enum.map(second_election, & &1.last_public_key) end - @tag :toto test "should never return more validation nodes than storages nodes" do authorized_nodes = [ %Node{ diff --git a/test/archethic_test.exs b/test/archethic_test.exs index 0c71152c2..c404eedec 100644 --- a/test/archethic_test.exs +++ b/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>>