root chain coordinator and ethereum event listeners simplification

[InoMurko]

EthereumEventListener's have a feature where they're allowed to cache events before they're being processed, which seems like it's adding a fair amount of complexity without additional value. The way it works is that if a certain ethereum event listener is allowed to sync, but not processes, it would store ethereum events (eth_getLogs) into the cache field of it's internal struct:

@type t() :: %__MODULE__{
          synced_height_update_key: atom(),
          service_name: atom(),
          cached: %{
            data: list(event),
            request_max_size: pos_integer(),
            events_upper_bound: non_neg_integer()
          },
          ethereum_events_check_interval_ms: non_neg_integer()
        }

The suggested simplification would drop cached completely and only ask for as much events as it's allowed to process.

Cache gets filled when setup like this is used in Root Chain Coordinator setup:

exit_processor: [waits_for: :depositor, finality_margin: 0],

Exit Processor would fill cached with the logs difference between depositor height and rootchain height (ethereum height).

[InoMurko]

In Root Chain Coordinator we have these two available setup properties called finality_margin and waits_for. For example: https://github.com/omgnetwork/elixir-omg/blob/master/apps/omg_watcher/lib/omg_watcher/coordinator_setup.ex#L53-L58

These two have a relationship that's not well understood. For example, a caller with finality_margin: 0 would ask Root Chain Coordinator get_synced_info/2 that would reply with:

%OMG.RootChainCoordinator.SyncGuide{           
  root_chain_height: 6797783,
  sync_height: 6797772
}

when the current ethereum height is 6797784.

Ethereum Event Listener would take this here https://github.com/omgnetwork/elixir-omg/blob/361c925f7a19ed45a686103bc8efb03c03048b70/apps/omg/lib/omg/ethereum_event_listener/core.ex#L113-L115

and apply the following:

    next_upper_bound =
      root_chain_height
      |> min(old_upper_bound + request_max_size)
      |> max(sync_height)

which means the next_upper_bound is going to be closer to root_chain_height rather than sync_height.

Later, the get_events: https://github.com/omgnetwork/elixir-omg/blob/361c925f7a19ed45a686103bc8efb03c03048b70/apps/omg/lib/omg/ethereum_event_listener/core.ex#L146 would take events UP TO sync_height and put everything above sync_height into cached. As said, this has questionable value.

[unnawut]

The suggested simplification would drop cached completely and only ask for as much events as it's allowed to process.

+1. Imho this caching is too close & too tied to the business logic. Also given the current eth_getLogs rate I'm not sure if this caching is used that much to worth the added complexity to the sync.

[pnowosie]

My guess is that this feature was invented for Watcher to speed up the resync from scratch. I'm happy seeing this is going out 👍