feat: make invalid piggyback cause unchallenged piggyback event when …

…it's past SLA margin(#1493)

README.md

@@ -125,6 +125,6 @@ Branch `gh-pages` is totally diseparated from other development branches and con
 
 See [gh-pages README](https://github.com/omisego/elixir-omg/tree/gh-pages) for more details.
 
-# More details about the design and arhitecture
+# More details about the design and architecture
 
-Details about the repository, code, arhitecture and design decisions are available **[here](docs/details.md)**.
+Details about the repository, code, architecture and design decisions are available **[here](docs/details.md)**.

apps/omg_watcher/lib/omg_watcher/event.ex

@@ -28,9 +28,11 @@ defmodule OMG.Watcher.Event do
           | OMG.Watcher.Event.InvalidExit.t()
           | OMG.Watcher.Event.UnchallengedExit.t()
           | OMG.Watcher.Event.NonCanonicalIFE.t()
+          | OMG.Watcher.Event.UnchallengedNonCanonicalIFE.t()
           | OMG.Watcher.Event.InvalidIFEChallenge.t()
           | OMG.Watcher.Event.PiggybackAvailable.t()
           | OMG.Watcher.Event.InvalidPiggyback.t()
+          | OMG.Watcher.Event.UnchallengedPiggyback.t()
 
   @type t ::
           OMG.Watcher.Event.AddressReceived.t()
@@ -43,9 +45,11 @@ defmodule OMG.Watcher.Event do
           | OMG.Watcher.Event.InvalidExit
           | OMG.Watcher.Event.UnchallengedExit
           | OMG.Watcher.Event.NonCanonicalIFE
+          | OMG.Watcher.Event.UnchallengedNonCanonicalIFE.t()
           | OMG.Watcher.Event.InvalidIFEChallenge
           | OMG.Watcher.Event.PiggybackAvailable
           | OMG.Watcher.Event.InvalidPiggyback
+          | OMG.Watcher.Event.UnchallengedPiggyback
           | OMG.Watcher.Event.AddressReceived
           | OMG.Watcher.Event.ExitFinalized
   #  TODO The reason why events have name as String and byzantine events as atom is that
@@ -195,9 +199,24 @@ defmodule OMG.Watcher.Event do
           }
   end
 
+  defmodule UnchallengedNonCanonicalIFE do
+    @moduledoc """
+    Notifies about an in-flight exit which has a competitor but is dangerously close to finalization.
+
+    It is a prompt to exit
+    """
+
+    defstruct [:txbytes, name: :unchallenged_non_canonical_ife]
+
+    @type t :: %__MODULE__{
+            txbytes: binary(),
+            name: atom()
+          }
+  end
+
   defmodule InvalidIFEChallenge do
     @moduledoc """
-    Notifies about an in-flight exit which has a competitor
+    Notifies that a canonical in-flight exit has been challenged. The challenge should be responded to.
     """
 
     defstruct [:txbytes, name: :invalid_ife_challenge]
@@ -242,4 +261,21 @@ defmodule OMG.Watcher.Event do
             name: atom()
           }
   end
+
+  defmodule UnchallengedPiggyback do
+    @moduledoc """
+    Notifies about invalid piggyback, that is dangerously approaching finalization, without being challenged
+
+    It is a prompt to exit
+    """
+
+    defstruct [:txbytes, :inputs, :outputs, name: :unchallenged_piggyback]
+
+    @type t :: %__MODULE__{
+            txbytes: binary(),
+            inputs: [non_neg_integer()],
+            outputs: [non_neg_integer()],
+            name: atom()
+          }
+  end
 end

apps/omg_watcher/lib/omg_watcher/exit_processor/canonicity.ex

@@ -65,22 +65,45 @@ defmodule OMG.Watcher.ExitProcessor.Canonicity do
           in_flight_proof: binary()
         }
 
-  # Gets the list of open IFEs that have the competitors _somewhere_
-  @spec get_ife_txs_with_competitors(Core.t(), KnownTx.known_txs_by_input_t()) :: list(Event.NonCanonicalIFE.t())
-  def get_ife_txs_with_competitors(%Core{in_flight_exits: ifes}, known_txs_by_input) do
-    ifes
-    |> Map.values()
-    |> Stream.map(fn ife -> {ife, DoubleSpend.find_competitor(known_txs_by_input, ife.tx)} end)
-    |> Stream.filter(fn {_ife, maybe_competitor} -> !is_nil(maybe_competitor) end)
-    |> Stream.filter(fn {ife, %DoubleSpend{known_tx: %KnownTx{utxo_pos: utxo_pos}}} ->
-      InFlightExitInfo.is_viable_competitor?(ife, utxo_pos)
-    end)
-    |> Stream.map(fn {ife, _double_spend} -> Transaction.raw_txbytes(ife.tx) end)
-    |> Enum.uniq()
-    |> Enum.map(fn txbytes -> %Event.NonCanonicalIFE{txbytes: txbytes} end)
+  @doc """
+  Returns a tuple with byzantine events: first element is a list of events for ifes with competitor
+  and the second is the same list filtered for late ifes past sla margin
+  """
+  @spec get_ife_txs_with_competitors(Core.t(), KnownTx.known_txs_by_input_t(), pos_integer()) ::
+          {list(Event.NonCanonicalIFE.t()), list(Event.UnchallengedNonCanonicalIFE.t())}
+  def get_ife_txs_with_competitors(state, known_txs_by_input, eth_height_now) do
+    non_canonical_ifes =
+      state.in_flight_exits
+      |> Map.values()
+      |> Stream.map(fn ife -> {ife, DoubleSpend.find_competitor(known_txs_by_input, ife.tx)} end)
+      |> Stream.filter(fn {_ife, maybe_competitor} -> !is_nil(maybe_competitor) end)
+      |> Stream.filter(fn {ife, %DoubleSpend{known_tx: %KnownTx{utxo_pos: utxo_pos}}} ->
+        InFlightExitInfo.is_viable_competitor?(ife, utxo_pos)
+      end)
+
+    non_canonical_ife_events =
+      non_canonical_ifes
+      |> Stream.map(fn {ife, _double_spend} -> Transaction.raw_txbytes(ife.tx) end)
+      |> Enum.uniq()
+      |> Enum.map(fn txbytes -> %Event.NonCanonicalIFE{txbytes: txbytes} end)
+
+    past_sla_margin = fn {ife, _double_spend} ->
+      ife.eth_height + state.sla_margin <= eth_height_now
+    end
+
+    late_non_canonical_ife_events =
+      non_canonical_ifes
+      |> Stream.filter(past_sla_margin)
+      |> Stream.map(fn {ife, _double_spend} -> Transaction.raw_txbytes(ife.tx) end)
+      |> Enum.uniq()
+      |> Enum.map(fn txbytes -> %Event.UnchallengedNonCanonicalIFE{txbytes: txbytes} end)
+
+    {non_canonical_ife_events, late_non_canonical_ife_events}
   end
 
-  # Gets the list of open IFEs that have the competitors _somewhere_
+  @doc """
+  Returns byzantine events for open IFEs that were challenged with an invalid challenge
+  """
   @spec get_invalid_ife_challenges(Core.t()) :: list(Event.InvalidIFEChallenge.t())
   def get_invalid_ife_challenges(%Core{in_flight_exits: ifes}) do
     ifes

apps/omg_watcher/lib/omg_watcher/exit_processor/core.ex

@@ -459,28 +459,33 @@ defmodule OMG.Watcher.ExitProcessor.Core do
 
     known_txs_by_input = KnownTx.get_all_from_blocks_appendix(blocks, state)
 
-    ifes_with_competitors_events = ExitProcessor.Canonicity.get_ife_txs_with_competitors(state, known_txs_by_input)
+    {non_canonical_ife_events, late_non_canonical_ife_events} =
+      ExitProcessor.Canonicity.get_ife_txs_with_competitors(state, known_txs_by_input, eth_height_now)
+
     invalid_ife_challenges_events = ExitProcessor.Canonicity.get_invalid_ife_challenges(state)
 
-    invalid_piggybacks = ExitProcessor.Piggyback.get_invalid_piggybacks_events(state, known_txs_by_input)
-    has_no_late_invalid_exits = Enum.empty?(late_invalid_exits)
+    {invalid_piggybacks_events, late_invalid_piggybacks_events} =
+      ExitProcessor.Piggyback.get_invalid_piggybacks_events(state, known_txs_by_input, eth_height_now)
 
     available_piggybacks_events =
-      get_ifes_to_piggyback(state)
+      state
+      |> get_ifes_to_piggyback()
       |> Enum.flat_map(&prepare_available_piggyback/1)
 
+    unchallenged_exit_events =
+      late_non_canonical_ife_events ++ late_invalid_exits_events ++ late_invalid_piggybacks_events
+
+    chain_validity = if Enum.empty?(unchallenged_exit_events), do: :ok, else: {:error, :unchallenged_exit}
+
     events =
-      [
-        late_invalid_exits_events,
+      Enum.concat([
+        unchallenged_exit_events,
         invalid_exit_events,
-        invalid_piggybacks,
-        ifes_with_competitors_events,
+        invalid_piggybacks_events,
+        non_canonical_ife_events,
         invalid_ife_challenges_events,
         available_piggybacks_events
-      ]
-      |> Enum.concat()
-
-    chain_validity = if has_no_late_invalid_exits, do: :ok, else: {:error, :unchallenged_exit}
+      ])
 
     {chain_validity, events}
   end

apps/omg_watcher/lib/omg_watcher/exit_processor/piggyback.ex

@@ -86,39 +86,70 @@ defmodule OMG.Watcher.ExitProcessor.Piggyback do
     end
   end
 
-  @spec get_invalid_piggybacks_events(Core.t(), KnownTx.known_txs_by_input_t()) ::
-          list(Event.InvalidPiggyback.t())
-  def get_invalid_piggybacks_events(%Core{in_flight_exits: ifes}, known_txs_by_input) do
-    ifes
-    |> Map.values()
-    |> all_invalid_piggybacks_by_ife(known_txs_by_input)
-    |> group_by_txbytes()
-    |> materials_to_events()
+  @doc """
+  Returns a tuple of ivalid piggybacks and invalid piggybacks that are past SLA margin.
+  This is inclusive, invalid piggybacks past SLA margin are included in the invalid piggybacks list.
+  """
+  @spec get_invalid_piggybacks_events(Core.t(), KnownTx.known_txs_by_input_t(), pos_integer()) ::
+          {list(Event.InvalidPiggyback.t()), list(Event.UnchallengedPiggyback.t())}
+  def get_invalid_piggybacks_events(
+        %Core{sla_margin: sla_margin, in_flight_exits: ifes},
+        known_txs_by_input,
+        eth_height_now
+      ) do
+    invalid_piggybacks_by_ife =
+      ifes
+      |> Map.values()
+      |> all_invalid_piggybacks_by_ife(known_txs_by_input)
+
+    invalid_piggybacks_events = to_events(invalid_piggybacks_by_ife, &to_invalid_piggyback_event/1)
+
+    past_sla_margin = fn {ife, _type, _materials} ->
+      ife.eth_height + sla_margin <= eth_height_now
+    end
+
+    unchallenged_piggybacks_events =
+      invalid_piggybacks_by_ife
+      |> Enum.filter(past_sla_margin)
+      |> to_events(&to_unchallenged_piggyback_event/1)
+
+    {invalid_piggybacks_events, unchallenged_piggybacks_events}
   end
 
   defp all_invalid_piggybacks_by_ife(ifes_values, known_txs_by_input) do
     [:input, :output]
     |> Enum.flat_map(fn pb_type -> invalid_piggybacks_by_ife(known_txs_by_input, pb_type, ifes_values) end)
   end
 
+  defp to_events(piggybacks_by_ife, to_event) do
+    piggybacks_by_ife
+    |> group_by_txbytes()
+    |> Enum.map(to_event)
+  end
+
+  defp to_invalid_piggyback_event({txbytes, type_materials_pairs}) do
+    %Event.InvalidPiggyback{
+      txbytes: txbytes,
+      inputs: invalid_piggyback_indices(type_materials_pairs, :input),
+      outputs: invalid_piggyback_indices(type_materials_pairs, :output)
+    }
+  end
+
+  defp to_unchallenged_piggyback_event({txbytes, type_materials_pairs}) do
+    %Event.UnchallengedPiggyback{
+      txbytes: txbytes,
+      inputs: invalid_piggyback_indices(type_materials_pairs, :input),
+      outputs: invalid_piggyback_indices(type_materials_pairs, :output)
+    }
+  end
+
   # we need to produce only one event per IFE, with both piggybacks on inputs and outputs
   defp group_by_txbytes(invalid_piggybacks) do
     invalid_piggybacks
     |> Enum.map(fn {ife, type, materials} -> {Transaction.raw_txbytes(ife.tx), type, materials} end)
     |> Enum.group_by(&elem(&1, 0), fn {_, type, materials} -> {type, materials} end)
   end
 
-  defp materials_to_events(invalid_piggybacks_by_txbytes) do
-    invalid_piggybacks_by_txbytes
-    |> Enum.map(fn {txbytes, type_materials_pairs} ->
-      %Event.InvalidPiggyback{
-        txbytes: txbytes,
-        inputs: invalid_piggyback_indices(type_materials_pairs, :input),
-        outputs: invalid_piggyback_indices(type_materials_pairs, :output)
-      }
-    end)
-  end
-
   defp invalid_piggyback_indices(type_materials_pairs, pb_type) do
     # here we need to additionally group the materials found by type input/output
     # then we gut just the list of indices to present to the user in the event

apps/omg_watcher/test/omg_watcher/exit_processor/canonicity_test.exs

@@ -323,6 +323,24 @@ defmodule OMG.Watcher.ExitProcessor.CanonicityTest do
       assert {:ok, %{}} = request |> Core.get_competitor_for_ife(processor, txbytes)
     end
 
+    test "detects that non-canonical ife becomes unchallenged exit when sla period passes",
+         %{processor_filled: processor, transactions: [tx1 | _], competing_tx: comp} do
+      txbytes = txbytes(tx1)
+      other_blknum = 3000
+
+      request = %ExitProcessor.Request{
+        blknum_now: 5000,
+        eth_height_now: 5 + processor.sla_margin,
+        blocks_result: [Block.hashed_txs_at([comp, tx1], other_blknum)],
+        ife_input_spending_blocks_result: [Block.hashed_txs_at([comp, tx1], other_blknum)]
+      }
+
+      processor = processor |> Core.find_ifes_in_blocks(request)
+
+      assert {{:error, :unchallenged_exit}, [%Event.UnchallengedNonCanonicalIFE{txbytes: ^txbytes}]} =
+               request |> check_validity_filtered(processor, only: [Event.UnchallengedNonCanonicalIFE])
+    end
+
     test "show competitors, if IFE tx is included but not the oldest - distinct blocks",
          %{processor_filled: processor, transactions: [tx1 | _], competing_tx: comp} do
       txbytes = txbytes(tx1)

apps/omg_watcher/test/omg_watcher/exit_processor/piggyback_test.exs

@@ -429,6 +429,31 @@ defmodule OMG.Watcher.ExitProcessor.PiggybackTest do
       assert_proof_sound(proof_bytes)
     end
 
+    test "detects that invalid piggyback becomes unchalleneged exit when sla period passes",
+         %{alice: alice, processor_filled: state, transactions: [tx | _], ife_tx_hashes: [ife_id | _]} do
+      # 1. transaction which is, ife'd, output piggybacked, and included in a block
+      txbytes = txbytes(tx)
+      tx_blknum = 3000
+
+      # 2. transaction which spends that piggybacked output
+      comp = TestHelper.create_recovered([{tx_blknum, 0, 0, alice}], [{alice, @eth, 1}])
+
+      request = %ExitProcessor.Request{
+        blknum_now: 5000,
+        eth_height_now: 5 + state.sla_margin,
+        ife_input_spending_blocks_result: [Block.hashed_txs_at([tx], tx_blknum)]
+      }
+
+      # 3. stuff happens in the contract
+      state =
+        state |> start_ife_from(comp) |> piggyback_ife_from(ife_id, 0, :output) |> Core.find_ifes_in_blocks(request)
+
+      assert {{:error, :unchallenged_exit},
+              [
+                %Event.UnchallengedPiggyback{txbytes: ^txbytes, inputs: [], outputs: [0]}
+              ]} = check_validity_filtered(request, state, only: [Event.UnchallengedPiggyback])
+    end
+
     test "detects double-spend of an output, found in a IFE, even if finalized",
          %{alice: alice, processor_filled: state, transactions: [tx | _], ife_tx_hashes: [tx_hash | _]} do
       txbytes = txbytes(tx)

apps/omg_watcher/test/omg_watcher/integration/in_flight_exit_test.exs

@@ -198,8 +198,12 @@ defmodule OMG.Watcher.Integration.InFlightExitTest do
     DevHelper.wait_for_root_chain_block(response_eth_height + exit_finality_margin + 1)
 
     # dissapearing of `invalid_ife_challenge` event
-    assert %{"byzantine_events" => [%{"event" => "non_canonical_ife"}, %{"event" => "piggyback_available"}]} =
-             WatcherHelper.success?("/status.get")
+    assert %{
+             "byzantine_events" => [
+               %{"event" => "non_canonical_ife"},
+               %{"event" => "piggyback_available"}
+             ]
+           } = WatcherHelper.success?("/status.get")
   end
 
   @tag fixtures: [:in_beam_watcher, :alice, :bob, :mix_based_child_chain, :token, :alice_deposits]
@@ -257,7 +261,9 @@ defmodule OMG.Watcher.Integration.InFlightExitTest do
            end) =~ "Invalid in-flight exit finalization"
 
     assert %{"in_flight_exits" => [_], "byzantine_events" => byzantine_events} = WatcherHelper.success?("/status.get")
-    assert [%{"event" => "invalid_piggyback"}] = Enum.filter(byzantine_events, &(&1["event"] != "piggyback_available"))
+    # invalid piggyback is past sla margin, unchallenged_piggyback event is emitted
+    assert [%{"event" => "unchallenged_piggyback"}, %{"event" => "invalid_piggyback"}] =
+             Enum.filter(byzantine_events, &(&1["event"] != "piggyback_available"))
   end
 
   defp exit_in_flight(%Transaction.Signed{} = tx, exiting_user) do

config/test.exs

@@ -136,9 +136,9 @@ config :omg_watcher, child_chain_url: "http://localhost:9657"
 
 config :omg_watcher,
   block_getter_loops_interval_ms: 50,
-  # NOTE `exit_processor_sla_margin` can't be made shorter. At 3 it sometimes
-  # causes :unchallenged_exit because `geth --dev` is too fast
-  exit_processor_sla_margin: 5,
+  # NOTE `exit_processor_sla_margin` can't be made shorter. At 8 it sometimes
+  # causes unchallenged exits events because `geth --dev` is too fast
+  exit_processor_sla_margin: 10,
   # this means we allow the `sla_margin` above be larger than the `min_exit_period`
   exit_processor_sla_margin_forced: true,
   # NOTE: `maximum_block_withholding_time_ms` must be here - one of our integration tests