infinite_looping_in_forward_transaction_#931

lib/archethic.ex

@@ -51,56 +51,79 @@ defmodule Archethic do
   @doc """
   Send a new transaction in the network to be mined. The current node will act as welcome node
   """
-  @spec send_new_transaction(Transaction.t()) :: :ok | {:error, :network_issue}
+  @spec send_new_transaction(
+          transaction :: Transaction.t(),
+          welcome_node_key :: Crypto.key(),
+          bad_nodes :: list(Crypto.key())
+        ) ::
+          :ok | {:error, :network_issue}
   def send_new_transaction(
-        tx = %Transaction{},
-        welcome_node_key \\ Crypto.first_node_public_key()
-      ) do
+        tx,
+        welcome_node_key \\ Crypto.first_node_public_key(),
+        bad_nodes \\ []
+      )
+
+  def send_new_transaction(tx = %Transaction{}, welcome_node_key, bad_nodes) do
     if P2P.authorized_and_available_node?() do
       case SharedSecrets.verify_synchronization() do
         :ok ->
           do_send_transaction(tx, welcome_node_key)
 
         :error ->
-          forward_transaction(tx, welcome_node_key)
+          forward_transaction(tx, welcome_node_key, [Crypto.first_node_public_key() | bad_nodes])
 
         {:error, last_address_to_sync} ->
           SelfRepair.resync(last_address_to_sync)
-          forward_transaction(tx, welcome_node_key)
+          forward_transaction(tx, welcome_node_key, [Crypto.first_node_public_key() | bad_nodes])
       end
     else
-      # node not authorized
-      forward_transaction(tx, welcome_node_key)
+      # node not authorized . bad nodes is not synced and not authorized is excluded by default
+      forward_transaction(tx, welcome_node_key, bad_nodes)
     end
   end
 
-  defp forward_transaction(
-         tx,
-         welcome_node_key,
-         nodes \\ P2P.authorized_and_available_nodes()
-         |> Enum.filter(&Node.locally_available?/1)
-         |> P2P.nearest_nodes()
-       )
+  defp forward_transaction(tx, welcome_node_key, bad_nodes) do
+    nodes =
+      P2P.authorized_and_available_nodes()
+      |> Enum.filter(&Node.locally_available?/1)
+      |> P2P.nearest_nodes()
+      |> Enum.reject(fn node -> node.first_public_key in bad_nodes end)
 
-  defp forward_transaction(tx, welcome_node_key, [node | rest]) do
-    case P2P.send_message(node, %NewTransaction{transaction: tx, welcome_node: welcome_node_key}) do
+    message = %NewTransaction{
+      transaction: tx,
+      welcome_node: welcome_node_key,
+      not_synced: bad_nodes
+    }
+
+    notify_nodes(message, nodes)
+  end
+
+  defp notify_nodes(message, [node | rest]) do
+    case P2P.send_message(node, message) do
       {:ok, %Ok{}} ->
         :ok
 
       {:error, _} ->
-        forward_transaction(tx, welcome_node_key, rest)
+        message = Map.put(message, :not_synced, [node.first_public_key | message.not_synced])
+        notify_nodes(message, rest)
     end
   end
 
-  defp forward_transaction(_, _, []), do: {:error, :network_issue}
+  defp notify_nodes(_, []), do: {:error, :network_issue}
 
   defp do_send_transaction(tx = %Transaction{type: tx_type}, welcome_node_key) do
     current_date = DateTime.utc_now()
     sorting_seed = Election.validation_nodes_election_seed_sorting(tx, current_date)
 
     # We are selecting only the authorized nodes the current date of the transaction
     # If new nodes have been authorized, they only will be selected at the application date
-    node_list = P2P.authorized_and_available_nodes(current_date)
+    node_list =
+      current_date
+      |> P2P.authorized_and_available_nodes()
+
+    # not including bad nodes m it will harm the elections
+    # will handle in node responsivity
+    # |> Enum.reject(fn node -> node.first_public_key in bad_nodes end)
 
     storage_nodes = Election.chain_storage_nodes_with_type(tx.address, tx.type, node_list)
 

lib/archethic/bootstrap/transaction_handler.ex

@@ -38,7 +38,8 @@ defmodule Archethic.Bootstrap.TransactionHandler do
   defp do_send_transaction([node | rest], tx) do
     case P2P.send_message(node, %NewTransaction{
            transaction: tx,
-           welcome_node: node.first_public_key
+           welcome_node: node.first_public_key,
+           not_synced: []
          }) do
       {:ok, %Ok{}} ->
         Logger.info("Waiting transaction validation",

lib/archethic/p2p/message/new_transaction.ex

@@ -4,20 +4,21 @@ defmodule Archethic.P2P.Message.NewTransaction do
 
   This message is used locally within a node during the bootstrap
   """
-  @enforce_keys [:transaction, :welcome_node]
-  defstruct [:transaction, :welcome_node]
+  @enforce_keys [:transaction, :welcome_node, :not_synced]
+  defstruct [:transaction, :welcome_node, not_synced: []]
 
   alias Archethic.{TransactionChain.Transaction, Crypto, Utils, P2P.Message}
   alias Message.{Ok, Error}
 
   @type t :: %__MODULE__{
           transaction: Transaction.t(),
-          welcome_node: Crypto.key()
+          welcome_node: Crypto.key(),
+          not_synced: list(Crypto.key())
         }
 
   @spec process(__MODULE__.t(), Crypto.key()) :: Ok.t() | Error.t()
-  def process(%__MODULE__{transaction: tx, welcome_node: node_pbkey}, _) do
-    case Archethic.send_new_transaction(tx, node_pbkey) do
+  def process(%__MODULE__{transaction: tx, welcome_node: welcome_node, not_synced: bad_nodes}, _) do
+    case Archethic.send_new_transaction(tx, welcome_node, bad_nodes) do
       :ok ->
         %Ok{}
 
@@ -27,14 +28,23 @@ defmodule Archethic.P2P.Message.NewTransaction do
   end
 
   @spec serialize(t()) :: bitstring()
-  def serialize(%__MODULE__{transaction: tx, welcome_node: node_pbkey}) do
-    <<Transaction.serialize(tx)::bitstring, node_pbkey::binary>>
+  def serialize(%__MODULE__{
+        transaction: tx,
+        welcome_node: welcome_node_key,
+        not_synced: bad_nodes
+      }) do
+    nodes_list = [welcome_node_key | bad_nodes]
+
+    <<Transaction.serialize(tx)::bitstring, length(nodes_list)::8,
+      :erlang.list_to_binary(nodes_list)::binary>>
   end
 
   @spec deserialize(bitstring()) :: {t(), bitstring}
   def deserialize(<<rest::bitstring>>) do
-    {tx, rest} = Transaction.deserialize(rest)
-    {node_pbkey, rest} = Utils.deserialize_public_key(rest)
-    {%__MODULE__{transaction: tx, welcome_node: node_pbkey}, rest}
+    {tx, <<nb_nodes::8, rest::bitstring>>} = Transaction.deserialize(rest)
+
+    {[welcome_node | bad_nodes], rest} = Utils.deserialize_public_key_list(rest, nb_nodes, [])
+
+    {%__MODULE__{transaction: tx, welcome_node: welcome_node, not_synced: bad_nodes}, rest}
   end
 end

test/archethic/p2p/messages_test.exs

@@ -145,8 +145,16 @@ defmodule Archethic.P2P.MessageTest do
     test "NewTransaction message" do
       tx = Transaction.new(:transfer, %TransactionData{}, "seed", 0)
 
-      assert %NewTransaction{transaction: tx, welcome_node: Crypto.first_node_public_key()} ==
-               %NewTransaction{transaction: tx, welcome_node: Crypto.first_node_public_key()}
+      assert %NewTransaction{
+               transaction: tx,
+               welcome_node: Crypto.first_node_public_key(),
+               not_synced: []
+             } ==
+               %NewTransaction{
+                 transaction: tx,
+                 welcome_node: Crypto.first_node_public_key(),
+                 not_synced: []
+               }
                |> Message.encode()
                |> Message.decode()
                |> elem(0)

test/archethic_test.exs

@@ -58,7 +58,7 @@ defmodule ArchethicTest do
       assert :ok = Archethic.send_new_transaction(tx)
     end
 
-    test "When NOT synced should forward the tx and start repair " do
+    test "should return error when node not synced,and try to forward the tx to itself " do
       nss_genesis_address = "nss_genesis_address"
       nss_last_address = "nss_last_address"
       :persistent_term.put(:node_shared_secrets_gen_addr, nss_genesis_address)
@@ -101,7 +101,7 @@ defmodule ArchethicTest do
       )
 
       MockClient
-      |> expect(:send_message, 4, fn
+      |> expect(:send_message, 3, fn
         # validate nss chain from network
         # anticippated to be failed
         _, %GetLastTransactionAddress{}, _ ->
@@ -121,6 +121,96 @@ defmodule ArchethicTest do
       # last address is d/f it returns last address from quorum
       assert {:error, "willnotmatchaddress"} = SharedSecrets.verify_synchronization()
 
+      # trying to ssend a tx when NSS chain not synced
+      tx = Transaction.new(:transfer, %TransactionData{}, "seed", 0)
+      assert {:error, :network_issue} = Archethic.send_new_transaction(tx)
+
+      # start repair and should bottleneck requests
+      pid = SelfRepair.repair_in_progress?(nss_genesis_address)
+      Process.sleep(150)
+      assert pid != nil
+    end
+
+    test "should forward tx to other nodes " do
+      nss_genesis_address = "nss_genesis_address"
+      nss_last_address = "nss_last_address"
+      :persistent_term.put(:node_shared_secrets_gen_addr, nss_genesis_address)
+      first_pb_key = Crypto.first_node_public_key()
+
+      P2P.add_and_connect_node(%Node{
+        ip: {127, 0, 0, 1},
+        port: 3000,
+        first_public_key: Crypto.first_node_public_key(),
+        last_public_key: Crypto.first_node_public_key(),
+        network_patch: "AAA",
+        geo_patch: "AAA",
+        available?: true,
+        authorized?: true,
+        authorization_date: DateTime.utc_now() |> DateTime.add(-20_000)
+      })
+
+      node2_fpbk = "node2"
+
+      P2P.add_and_connect_node(%Node{
+        ip: {127, 0, 0, 1},
+        port: 3000,
+        first_public_key: node2_fpbk,
+        last_public_key: node2_fpbk,
+        network_patch: "AAA",
+        geo_patch: "AAA",
+        available?: true,
+        authorized?: true,
+        authorization_date: DateTime.utc_now() |> DateTime.add(-20_000)
+      })
+
+      start_supervised!({SummaryTimer, Application.get_env(:archethic, SummaryTimer)})
+
+      MockDB
+      |> stub(:get_last_chain_address, fn ^nss_genesis_address ->
+        {nss_last_address, DateTime.utc_now()}
+      end)
+      |> stub(
+        :get_transaction,
+        fn
+          ^nss_last_address, [validation_stamp: [:timestamp]], :chain ->
+            {:ok,
+             %Transaction{
+               validation_stamp: %{
+                 __struct__: :ValidationStamp,
+                 # fail mathematical check with irregular timestamp
+                 # causes validate_scheduling_time() to fail
+                 timestamp: DateTime.utc_now() |> DateTime.add(-86_400)
+               }
+             }}
+
+          _, _, _ ->
+            {:error, :transaction_not_exists}
+        end
+      )
+
+      MockClient
+      |> expect(:send_message, 5, fn
+        # validate nss chain from network
+        # anticippated to be failed
+        _, %GetLastTransactionAddress{}, _ ->
+          {:ok, %LastTransactionAddress{address: "willnotmatchaddress"}}
+
+        %Node{first_public_key: ^node2_fpbk},
+        %NewTransaction{transaction: _, welcome_node: ^first_pb_key},
+        _ ->
+          # forward the tx
+          {:ok, %Ok{}}
+
+        _, %GetTransaction{address: _}, _ ->
+          {:ok, %NotFound{}}
+
+        _, _, _ ->
+          {:ok, %Ok{}}
+      end)
+
+      # last address is d/f it returns last address from quorum
+      assert {:error, "willnotmatchaddress"} = SharedSecrets.verify_synchronization()
+
       # trying to ssend a tx when NSS chain not synced
       tx = Transaction.new(:transfer, %TransactionData{}, "seed", 0)
       assert :ok = Archethic.send_new_transaction(tx)