Skip to content
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

cmd, eth: orchestrator activated/deactivated watcher #1173

Merged
merged 5 commits into from
Nov 13, 2019

Conversation

kyriediculous
Copy link
Contributor

What does this pull request do? Explain your changes. (required)
This PR adds an implementation for an OrchestratorWatcher that watches for on-chain emitted TranscoderActivated and TranscoderDeactivated events and updates orchestrators stored in the database accordingly.

Specific updates (required)

  • Added TranscoderActivated and TranscoderDeactivated to eth/watchers/topics.go
  • Added an OrchestratorStore interface to eth/watchers/types to be able to stub out common.DB for easier unit testing
  • Added an OrchestratorWatcher type
  • Start an OrchestratorWatcher in livepeer.go
  • Add a StubOrchestratorStore and helpers for creating stubbed out TranscoderActivated/TranscoderDeactivated events to eth/watchers/stub.go
  • Added ActivationRound and DeactivationRound fields to types.Transcoder
  • Make sure LivepeerEthClient.GetTranscoder() now also returns ActivationRound and DeactivationRound

How did you test each of these updates (required)
Ran unit tests

Does this pull request close any open issues?
Fixes #1150

Checklist:

  • README and other documentation updated
  • Node runs in OSX and devenv
  • All tests in ./test.sh pass

@yondonfu yondonfu changed the base branch from master to nv/db-orchestrator-updates November 6, 2019 19:17
@yondonfu
Copy link
Member

yondonfu commented Nov 6, 2019

Changed the target branch to nv/db-orchestrator-updates since this PR builds on top of that branch to make it easier to review the commits specific to this PR.

eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/types.go Outdated Show resolved Hide resolved
eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/stub.go Outdated Show resolved Hide resolved
eth/watchers/stub.go Outdated Show resolved Hide resolved
@kyriediculous
Copy link
Contributor Author

2ebd7fc fixes TranscoderDeactivated test for removed logs (stubClient.Orch was missing an Address field) and use NewDBOrch() instead of literal.

@kyriediculous
Copy link
Contributor Author

9d860c1 properly sets an O's deactivationRound to the maximum possible value for an int64 upon a TranscoderActivated event (similar to how we set it to 2^256-1 in BondingManager), when a TranscoderActivated event is removed now also setting the deactivationRound retrieved from the RPC call on O.

Otherwise deactivationRound would be 0, but currentRound < deactivationRound would always return false, and thus an O would always be inactive.

eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
eth/watchers/orchestratorwatcher.go Outdated Show resolved Hide resolved
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

Changes look good - let's rebase

@kyriediculous
Copy link
Contributor Author

kyriediculous commented Nov 13, 2019 via email

@yondonfu
Copy link
Member

dc7b318 looks like a duplicate commit (already in #1171). Can we drop it?

@kyriediculous
Copy link
Contributor Author

done !

Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

@kyriediculous kyriediculous merged commit 53388fa into nv/db-orchestrator-updates Nov 13, 2019
@kyriediculous kyriediculous deleted the nv/orch-watcher branch November 13, 2019 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants