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

pm, server: broadcaster deposit & withdrawround self-check #1212

Merged
merged 4 commits into from Jan 28, 2020

Conversation

kyriediculous
Copy link
Contributor

What does this pull request do? Explain your changes. (required)
Currently, a broadcaster in on-chain mode will alert a user that a broadcast session cannot be started if the deposit is 0, we now expand this functionality to also check whether a broadcaster has unlocked it's reserve & deposit and is nearing its WithdrawRound

In addition we now favor using cached values (using SenderWatcher) over RPC calls to a remote ethereum node to do these checks

Specific updates (required)

  • Added a Validate() method to pm.Sender (perhaps "SelfValidate" is a better name?)
  • Call pm.Sender.Validate in LivepeerServer.registerConnection() return an error if self validation fails and prevent from sending segments to O's

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

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

Checklist:

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

Alternatives Considered

  • Move the validation check to pm.Sender.StartSession so we don't have to expand the API surface. We would have to change the signature to return an error when validation logic fails. This will result in an empty sessions list and transcoding will not happen. Harder to propagate the correct error here though.
  • Move the validation check to the constructor pm.NewSender and return when we hit an error, this will however shut down the node and prevents a B from re-locking or re-depositing through the CLI

pm/sender.go Outdated Show resolved Hide resolved
pm/sender.go Outdated Show resolved Hide resolved
pm/sender.go Outdated Show resolved Hide resolved
pm/sender_test.go Outdated Show resolved Hide resolved
pm/sender_test.go Outdated Show resolved Hide resolved
@kyriediculous kyriediculous force-pushed the nv/b-deposit-withdrawround-check branch 2 times, most recently from 11be6ab to 1cec746 Compare January 24, 2020 22:03
pm/sender_test.go Outdated Show resolved Hide resolved
pm/sendermonitor_test.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 kyriediculous force-pushed the nv/b-deposit-withdrawround-check branch from 3b14e74 to 7ab94f6 Compare January 27, 2020 22:11
@kyriediculous
Copy link
Contributor Author

rebased !

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

pm, server: close RTMP connection when sender validation fails
@kyriediculous kyriediculous merged commit 9cb8a3d into master Jan 28, 2020
@kyriediculous kyriediculous deleted the nv/b-deposit-withdrawround-check branch January 28, 2020 12:10
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.

Broadcaster should alert user that stream will not be transcoded if funds are unlocked
2 participants