-
Notifications
You must be signed in to change notification settings - Fork 45
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
Implement LIP-9: Service Registry #223
Conversation
* @dev Stores service URI endpoint for the caller that can be used to send requests to the caller off-chain | ||
* @param _serviceURI Service URI endpoint for the caller | ||
*/ | ||
function setServiceURI(string _serviceURI) external { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any potential negative externalities for the rest of the network if a bunch of non-transcoders set their service URI records here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm since a broadcaster client would only care about the service URI record for the transcoder address that was assigned to the broadcaster's transcode job if non-transcoder users set their own service URI record they would pay the tx cost, but at least in the current proposed networking architecture those records will never be used in practice unless the non-transcoder user becomes a transcoder in the future. Perhaps something to be wary about though is if a transcoder's key is compromised such that an attacker can change the service URI record right before a broadcaster looks it up to send requests to the transcoder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, keeping the transcoder's keys keys cold would entail something like the discussion in livepeer/LIPs#7 which probably should be done separately from this change.
I was mostly wondering if filling the record mapping with unneeded entries could lead to higher costs for the network down the road, eg during a migration or with any enactment of 'data rents', but that's all fairly speculative. LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one question, but looks really great otherwise.
… assignment integration tests
*/ | ||
contract ServiceRegistry is ManagerProxyTarget { | ||
// Store service metadata | ||
struct Record { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we changed the Record struct in the future, the storage layout will be screwy right? If that's the case, I prefer to have the mapping from address
to string
, that way this potential mistake doesn't happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding additional fields to the Record
struct in an upgrade is safe as long as 1) we do not remove/replace existing fields 2) the Record
struct is not used in an array (it is only used in a mapping right now)
Implementation for LIP-9: https://github.com/livepeer/LIPs/blob/master/LIPs/LIP-9.md