From a3d51f5e1aa0a1416f4e51c2560f6dff40bfb41d Mon Sep 17 00:00:00 2001
From: Luke Imhoff <luke.imhoff@dockyard.com>
Date: Mon, 16 Jul 2018 10:21:16 -0500
Subject: [PATCH] Raise error if `:first` would cause retraversal

---
 apps/indexer/lib/indexer/block_fetcher.ex   |  2 +-
 apps/indexer/lib/indexer/sequence.ex        | 46 ++++++++++++++++-----
 apps/indexer/test/indexer/sequence_test.exs |  5 +++
 3 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/apps/indexer/lib/indexer/block_fetcher.ex b/apps/indexer/lib/indexer/block_fetcher.ex
index ebe105bec7..db5dfaa188 100644
--- a/apps/indexer/lib/indexer/block_fetcher.ex
+++ b/apps/indexer/lib/indexer/block_fetcher.ex
@@ -189,7 +189,7 @@ defmodule Indexer.BlockFetcher do
     debug(fn -> "#{count} missed block ranges between #{latest_block_number} and genesis" end)
 
     {:ok, seq} =
-      Sequence.start_link(prefix: missing_ranges, first: latest_block_number, step: -1 * state.blocks_batch_size)
+      Sequence.start_link(prefix: missing_ranges, first: 0, step: -1 * state.blocks_batch_size)
 
     stream_import(state, seq, max_concurrency: state.blocks_concurrency)
   end
diff --git a/apps/indexer/lib/indexer/sequence.ex b/apps/indexer/lib/indexer/sequence.ex
index 898384701e..8148562f8b 100644
--- a/apps/indexer/lib/indexer/sequence.ex
+++ b/apps/indexer/lib/indexer/sequence.ex
@@ -96,20 +96,19 @@ defmodule Indexer.Sequence do
 
   @impl GenServer
   @spec init(options) :: {:ok, t}
-  def init(named_arguments) when is_list(named_arguments) do
+  def init(options) when is_list(options) do
     Process.flag(:trap_exit, true)
-    step = Keyword.fetch!(named_arguments, :step)
 
-    initial_queue = :queue.new()
-    prefix = Keyword.get(named_arguments, :prefix, [])
-    queue = queue_chunked_ranges(initial_queue, step, prefix)
+    case validate_options(options) do
+      {:ok, %{prefix: prefix, first: first, step: step}} ->
+        initial_queue = :queue.new()
+        queue = queue_chunked_ranges(initial_queue, step, prefix)
 
-    {:ok,
-     %__MODULE__{
-       queue: queue,
-       current: Keyword.fetch!(named_arguments, :first),
-       step: step
-     }}
+        {:ok, %__MODULE__{queue: queue, current: first, step: step}}
+
+      {:error, reason} ->
+        {:stop, reason}
+    end
   end
 
   @impl GenServer
@@ -202,4 +201,29 @@ defmodule Indexer.Sequence do
 
   @spec sign(non_neg_integer()) :: 1
   defp sign(_), do: 1
+
+  defp validate_options(options) do
+    step = Keyword.fetch!(options, :step)
+    prefix = Keyword.get(options, :prefix, [])
+    first = Keyword.fetch!(options, :first)
+
+    validated_options = %{prefix: prefix, first: first, step: step}
+
+    if step < 0 do
+      minimum_in_prefix =
+        prefix
+        |> Stream.map(&Enum.min/1)
+        |> Enum.min(fn -> first end)
+
+      if first > minimum_in_prefix do
+        {:error,
+         ":first (#{first}) is greater than the minimum in `:prefix` (#{minimum_in_prefix}), " <>
+           "which will cause retraversal."}
+      else
+        {:ok, validated_options}
+      end
+    else
+      {:ok, validated_options}
+    end
+  end
 end
diff --git a/apps/indexer/test/indexer/sequence_test.exs b/apps/indexer/test/indexer/sequence_test.exs
index 35061d5777..c8fdf8b8ab 100644
--- a/apps/indexer/test/indexer/sequence_test.exs
+++ b/apps/indexer/test/indexer/sequence_test.exs
@@ -29,6 +29,11 @@ defmodule Indexer.SequenceTest do
       # noproc when the sequence has already died by the time monitor is called
       assert_receive {:DOWN, ^sequence_ref, :process, ^sequence_pid, status} when status in [:normal, :noproc]
     end
+
+    test "with negative :step with :first greater than minimum in :prefix returns error" do
+      assert {:error, ":first (1) is greater than the minimum in `:prefix` (0), which will cause retraversal."} =
+               Sequence.start_link(prefix: [1..0], first: 1, step: -1)
+    end
   end
 
   test "inject_range" do