Skip to content
This repository has been archived by the owner on Aug 31, 2021. It is now read-only.

Enable contractWatcher without prior headerSync #156

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

rmulhol
Copy link
Contributor

@rmulhol rmulhol commented Oct 4, 2019

  • Previous setup would fail if there were no headers in the db. This
    makes sense because we need headers that haven't been checked for
    logs to exist so that we can fetch logs for those headers. But it
    also prevents us from kicking off the headerSync and contractWatcher
    processes concurrently. These changes enable kicking off both
    processes at the same time with the idea that we will have unchecked
    headers upon transformer execution.

Resolves #155

@i-norden let me know if I'm missing unforeseen risks to enabling concurrent execution - ran into the issue while working on a docker deploy that execute both processes

Copy link
Contributor

@elizabethengelman elizabethengelman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

At first it felt like we would miss something at the beginning before headers are populated... but I think that those headers will be returned in the next MissingHeadersForAll call.

- Previous setup would fail if there were no headers in the db. This
  makes sense because we need headers that haven't been checked for
  logs to exist so that we can fetch logs for those headers. But it
  also prevents us from kicking off the headerSync and contractWatcher
  processes concurrently. These changes enable kicking off both
  processes at the same time with the idea that we will have unchecked
  headers upon transformer execution.
Copy link
Collaborator

@i-norden i-norden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT and I can't spot any issues with this, MissingHeadersForAll will just return empty slice of headers until some can be found in the db and tr.Start won't update until it has processed headers. Thanks for catching this!

@rmulhol rmulhol merged commit 7be070f into staging Oct 9, 2019
@rmulhol rmulhol deleted the contract-watcher-init branch October 9, 2019 23:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

contractWatcher requires previous header sync
3 participants