-
Notifications
You must be signed in to change notification settings - Fork 31
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
Admin initiate and revoke protocol transfer #55
Conversation
… and OpenSTUtility. Addresses #36
contracts/OpenSTUtility.sol
Outdated
* Administrative functions | ||
*/ | ||
function initiateProtocolTransfer( | ||
// including ProtocolVersioned interface explicitly adds to the size |
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.
I'll need to change this comment
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.
Missing explicit return
s.
onlyAdmin | ||
returns (bool) | ||
{ | ||
_token.initiateProtocolTransfer(_proposedProtocol); |
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.
Need to explicitly return, otherwise will always return false. This comment applies to the identically-named function in OpenSTValue.sol
as well.
onlyAdmin | ||
returns (bool) | ||
{ | ||
_token.revokeProtocolTransfer(); |
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.
Need to explicitly return, otherwise will always return false. This comment applies to the matching function in OpenSTValue.sol
as well.
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.
LGTM
Enable a very simple form of protocol transfer to begin with on 0.9.1 OpenSTValue and OpenSTUtility to transfer protocol. Addresses #36