-
Notifications
You must be signed in to change notification settings - Fork 7
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
OIP-draft add exists
to UtilityTokenInterface
#5
base: master
Are you sure you want to change the base?
Conversation
TODO; write specification for cogateway |
exists
to UtilityTokenInterface
exists
to UtilityTokenInterfaceexists
to UtilityTokenInterface
implemented in mosaic-contracts 0.10.0 |
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
We should prevent deadlocks as early as possible and this seems to be the place. It is the first time where the target chain knows about the mint intention and the intended recipient.
In my opinion the "Simple Summary" could be made even more "simple".
"Motivation" could be more verbose, maybe.
@@ -0,0 +1,64 @@ | |||
--- | |||
oip: <to be assigned> | |||
title: <Staking Beneficiary Exists> |
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.
remove brackets when adding actual values that replace the placeholders.
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.
It might be helpful to explicitly state that, in the base case, UT.exists returns true.
<!--A short (~200 word) description of the technical issue being addressed.--> | ||
By default all addresses exist in the Ethereum specification. For internal | ||
token economies addresses that are not internal should return `false` | ||
for `UT.exists`. By including this check on `cogateway.confirmStakingIntent`, |
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.
confirmStakingIntent
has been renamed to confirmStakeIntent
No description provided.