-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
root chain coordinator and ethereum event listeners simplification #1624
Comments
In Root Chain Coordinator we have these two available setup properties called These two have a relationship that's not well understood. For example, a caller with
when the current ethereum height is 6797784. Ethereum Event Listener would take this here elixir-omg/apps/omg/lib/omg/ethereum_event_listener/core.ex Lines 113 to 115 in 361c925
and apply the following:
which means the Later, the get_events:
would take events UP TO sync_height and put everything above sync_height into cached. As said, this has questionable value.
|
+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. |
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 👍 |
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: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 would fill
cached
with the logs difference between depositor height and rootchain height (ethereum height).The text was updated successfully, but these errors were encountered: