Fix tests of transactions on non-consensus blocks

Tests for BlockTransactionController mistakenly simulated transactions
on non-consensus blocks by making the `transaction.block` `consensus`
`false`, but this is unrealistic - the importing only sets the consensus
block to `transaction.block` and non-consensus blocks show up as
`transaction.transaction_forks` `transaction_fork.uncle`.  To prevent
this from happening again, `Explorer.Factory.with_block` now checks the
`block` has `consensus: true`.  With this setup fixed, the tests needed
to be updated to show that *no* _transactions_ would be shown for
non-consensus blocks (because there are only transactions forks) and
whether the non-consensus blocks showed as "above tip" only had to do
with the number being above the consensus number and not to do with it
being non-consensus.

Floki selectors are used to improve error messages when messages are
unexpected.

apps/block_scout_web/lib/block_scout_web/controllers/block_transaction_controller.ex

@@ -57,7 +57,7 @@ defmodule BlockScoutWeb.BlockTransactionController do
         |> render(
           "404.html",
           block: nil,
-          block_above_tip: block_above_tip?(formatted_block_hash_or_number)
+          block_above_tip: block_above_tip(formatted_block_hash_or_number)
         )
     end
   end
@@ -79,10 +79,16 @@ defmodule BlockScoutWeb.BlockTransactionController do
     end
   end
 
-  defp block_above_tip?("0x" <> _), do: nil
+  defp block_above_tip("0x" <> _), do: {:error, :hash}
 
-  defp block_above_tip?(block_hash_or_number) when is_binary(block_hash_or_number) do
+  defp block_above_tip(block_hash_or_number) when is_binary(block_hash_or_number) do
-    {block_number, ""} = Integer.parse(block_hash_or_number)
+    case Chain.max_consensus_block_number() do
-    block_number > Chain.block_height()
+      {:ok, max_consensus_block_number} ->
+        {block_number, _} = Integer.parse(block_hash_or_number)
+        {:ok, block_number > max_consensus_block_number}
+
+      {:error, :not_found} ->
+        {:ok, true}
+    end
   end
 end

apps/block_scout_web/lib/block_scout_web/templates/block_transaction/404.html.eex

@@ -6,7 +6,7 @@
           <h1 class="card-title" data-test="detail_type">
             <%= gettext("Block Details") %>
           </h1>
-          <div class="tile tile-muted text-center">
+          <div class="tile tile-muted text-center" data-selector="block-not-found-message">
             <%= block_not_found_message(@block_above_tip) %>
           </div>
         </div>

apps/block_scout_web/lib/block_scout_web/views/block_transaction_view.ex

@@ -3,16 +3,15 @@ defmodule BlockScoutWeb.BlockTransactionView do
 
   import BlockScoutWeb.Gettext, only: [gettext: 1]
 
-  def block_not_found_message(block_above_tip) do
+  def block_not_found_message({:ok, true}) do
-    case block_above_tip do
+    gettext("Easy Cowboy! This block does not exist yet!")
-      true ->
+  end
-        gettext("Easy Cowboy! This block does not exist yet!")
 
-      false ->
+  def block_not_found_message({:ok, false}) do
-        gettext("This block has not been processed yet.")
+    gettext("This block has not been processed yet.")
+  end
 
-      _ ->
+  def block_not_found_message({:error, :hash}) do
-        gettext("Block not found, please try again later.")
+    gettext("Block not found, please try again later.")
-    end
   end
 end

apps/block_scout_web/priv/gettext/default.pot

@@ -1254,12 +1254,12 @@ msgid "Block not found, please try again later."
 msgstr ""
 
 #, elixir-format
-#: lib/block_scout_web/views/block_transaction_view.ex:9
+#: lib/block_scout_web/views/block_transaction_view.ex:7
 msgid "Easy Cowboy! This block does not exist yet!"
 msgstr ""
 
 #, elixir-format
-#: lib/block_scout_web/views/block_transaction_view.ex:12
+#: lib/block_scout_web/views/block_transaction_view.ex:11
 msgid "This block has not been processed yet."
 msgstr ""
 

apps/block_scout_web/priv/gettext/en/LC_MESSAGES/default.po

@@ -1254,12 +1254,12 @@ msgid "Block not found, please try again later."
 msgstr ""
 
 #, elixir-format
-#: lib/block_scout_web/views/block_transaction_view.ex:9
+#: lib/block_scout_web/views/block_transaction_view.ex:7
 msgid "Easy Cowboy! This block does not exist yet!"
 msgstr ""
 
 #, elixir-format
-#: lib/block_scout_web/views/block_transaction_view.ex:12
+#: lib/block_scout_web/views/block_transaction_view.ex:11
 msgid "This block has not been processed yet."
 msgstr ""
 

apps/block_scout_web/test/block_scout_web/controllers/block_transaction_controller_test.exs

@@ -45,16 +45,29 @@ defmodule BlockScoutWeb.BlockTransactionControllerTest do
       assert 2 == Enum.count(conn.assigns.transactions)
     end
 
-    test "does not return transactions for non-consensus block number", %{conn: conn} do
+    test "non-consensus block number without consensus blocks is treated as consensus number above tip", %{conn: conn} do
       block = insert(:block, consensus: false)
 
-      :transaction
+      transaction = insert(:transaction)
-      |> insert()
+      insert(:transaction_fork, hash: transaction.hash, uncle_hash: block.hash)
-      |> with_block(block)
 
       conn = get(conn, block_transaction_path(conn, :index, block.number))
 
-      assert html_response(conn, 404) =~ "This block has not been processed yet."
+      assert_block_above_tip(conn)
+    end
+
+    test "non-consensus block number above consensus block number is treated as consensus number above tip", %{
+      conn: conn
+    } do
+      consensus_block = insert(:block, consensus: true, number: 1)
+      block = insert(:block, consensus: false, number: consensus_block.number + 1)
+
+      transaction = insert(:transaction)
+      insert(:transaction_fork, hash: transaction.hash, uncle_hash: block.hash)
+
+      conn = get(conn, block_transaction_path(conn, :index, block.number))
+
+      assert_block_above_tip(conn)
     end
 
     test "returns transactions for consensus block hash", %{conn: conn} do
@@ -70,17 +83,16 @@ defmodule BlockScoutWeb.BlockTransactionControllerTest do
       assert Enum.count(conn.assigns.transactions) == 1
     end
 
-    test "returns transactions for non-consensus block hash", %{conn: conn} do
+    test "does not return transactions for non-consensus block hash", %{conn: conn} do
       block = insert(:block, consensus: false)
 
-      :transaction
+      transaction = insert(:transaction)
-      |> insert()
+      insert(:transaction_fork, hash: transaction.hash, uncle_hash: block.hash)
-      |> with_block(block)
 
       conn = get(conn, block_transaction_path(conn, :index, block.hash))
 
       assert html_response(conn, 200)
-      assert Enum.count(conn.assigns.transactions) == 1
+      assert Enum.count(conn.assigns.transactions) == 0
     end
 
     test "does not return transactions for invalid block hash", %{conn: conn} do
@@ -149,4 +161,12 @@ defmodule BlockScoutWeb.BlockTransactionControllerTest do
       assert html_response(conn, 200) =~ miner_name
     end
   end
+
+  defp assert_block_above_tip(conn) do
+    assert conn
+           |> html_response(404)
+           |> Floki.find(~S|[data-selector="block-not-found-message"|)
+           |> Floki.text()
+           |> String.trim() == "Easy Cowboy! This block does not exist yet!"
+  end
 end

apps/block_scout_web/test/block_scout_web/features/viewing_blocks_test.exs

@@ -149,9 +149,8 @@ defmodule BlockScoutWeb.ViewingBlocksTest do
           uncle = insert(:block, consensus: false)
           insert(:block_second_degree_relation, uncle_hash: uncle.hash)
 
-          :transaction
+          transaction = insert(:transaction)
-          |> insert()
+          insert(:transaction_fork, hash: transaction.hash, uncle_hash: uncle.hash)
-          |> with_block(uncle)
 
           uncle
         end

apps/explorer/lib/explorer/chain/block.ex

@@ -88,6 +88,7 @@ defmodule Explorer.Chain.Block do
     has_many(:uncles, through: [:uncle_relations, :uncle])
 
     has_many(:transactions, Transaction)
+    has_many(:transaction_forks, Transaction.Fork, foreign_key: :uncle_hash)
 
     has_many(:rewards, Reward, foreign_key: :block_hash)
   end

apps/explorer/test/support/factory.ex

@@ -176,7 +176,9 @@ defmodule Explorer.Factory do
     with_block(transaction, block, [])
   end
 
-  def with_block(transactions, %Block{} = block) when is_list(transactions) do
+  # The `transaction.block` must be consensus.  Non-consensus blocks can only be associated with the
+  # `transaction_forks`.
+  def with_block(transactions, %Block{consensus: true} = block) when is_list(transactions) do
     Enum.map(transactions, &with_block(&1, block))
   end
 
@@ -187,7 +189,9 @@ defmodule Explorer.Factory do
 
   def with_block(
         %Transaction{index: nil} = transaction,
-        %Block{hash: block_hash, number: block_number},
+        # The `transaction.block` must be consensus.  Non-consensus blocks can only be associated with the
+        # `transaction_forks`.
+        %Block{consensus: true, hash: block_hash, number: block_number},
         collated_params
       )
       when is_list(collated_params) do