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.

Sign up for GitHub

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

Jump to bottom

Clean up of UtilityToken and UtilityTokenInterface. #540

Merged
merged 21 commits into from
Dec 17, 2018
Merged

Clean up of UtilityToken and UtilityTokenInterface. #540

merged 21 commits into from
Dec 17, 2018

Conversation

deepesh-kn
Copy link
Contributor

@deepesh-kn deepesh-kn commented Dec 11, 2018 •
edited
Loading

This PR includes the following changes

  • Function mint is renamed to increaseSupply .
  • Function burn is renamed to decreaseSupply.
  • Minted and Burnt events are deleted.
  • EIP20CoGateway is updated to incorporate this change.
  • UtilityTokenInterface is updated.
  • UtilityToken is updated.
  • Unit test for function constructor.
  • Unit test for functionsetCoGateway.
  • Unit test for function increaseSupply.
  • Unit test for function decreaseSupply.
  • Related changes in OST Prime.

fixes: #449, #524 and #529

@deepesh-kn deepesh-kn self-assigned this Dec 11, 2018
@deepesh-kn deepesh-kn changed the title Clean up of UtilityToken and UtilityTokenInterface. [Don't Review for now] Clean up of UtilityToken and UtilityTokenInterface. Dec 12, 2018
@deepesh-kn deepesh-kn changed the title [Don't Review for now] Clean up of UtilityToken and UtilityTokenInterface. Clean up of UtilityToken and UtilityTokenInterface. Dec 12, 2018
schemar
schemar approved these changes Dec 13, 2018
View reviewed changes
Copy link
Contributor

@schemar schemar left a comment

Choose a reason for hiding this comment

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

Nice!! 🐻

Nothing critical from my end. See some comments on typos/docs in-line.
Generally, I think we should use the NatSpec modifiers like intended here: Ethereum Natural Specification
I.e. @notice should be for the user and @dev for developers.

contracts/gateway/OSTPrime.sol Outdated Show resolved Hide resolved
contracts/gateway/OSTPrime.sol Outdated Show resolved Hide resolved
contracts/gateway/UtilityToken.sol Outdated Show resolved Hide resolved
contracts/gateway/UtilityToken.sol Outdated Show resolved Hide resolved
contracts/gateway/UtilityToken.sol Show resolved Hide resolved
contracts/test/gateway/MockEIP20CoGateway.sol Outdated Show resolved Hide resolved
test/gateway/ost_prime/constructor.js Outdated Show resolved Hide resolved
test/gateway/ost_prime/decrease_supply.js Outdated Show resolved Hide resolved
test/gateway/ost_prime/decrease_supply.js Outdated Show resolved Hide resolved
test/gateway/utility_token/decrease_supply.js Outdated Show resolved Hide resolved
0xsarvesh
0xsarvesh reviewed Dec 14, 2018
View reviewed changes
Copy link
Contributor

@0xsarvesh 0xsarvesh left a comment

Choose a reason for hiding this comment

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

Nice work 🙌
Some minor feedbacks inline.

contracts/gateway/CoGatewayUtilityTokenInterface.sol Show resolved Hide resolved
contracts/gateway/EIP20CoGateway.sol Outdated Show resolved Hide resolved
contracts/gateway/EIP20CoGateway.sol Show resolved Hide resolved
contracts/gateway/OSTPrime.sol Show resolved Hide resolved
contracts/gateway/OSTPrime.sol Show resolved Hide resolved
contracts/gateway/UtilityToken.sol Outdated Show resolved Hide resolved
contracts/gateway/UtilityToken.sol Show resolved Hide resolved
contracts/gateway/UtilityToken.sol Show resolved Hide resolved
test/gateway/ost_prime/increase_supply.js Show resolved Hide resolved
test/gateway/utility_token/decrease_supply.js Outdated Show resolved Hide resolved
0xsarvesh
0xsarvesh approved these changes Dec 17, 2018
View reviewed changes
Copy link
Contributor

@0xsarvesh 0xsarvesh left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 🙌
Thanks

gulshanvasnani
gulshanvasnani reviewed Dec 17, 2018
View reviewed changes
Copy link
Contributor

@gulshanvasnani gulshanvasnani left a comment

Choose a reason for hiding this comment

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

LGTM 👍

contracts/gateway/UtilityToken.sol Outdated Show resolved Hide resolved
contracts/gateway/UtilityToken.sol Outdated Show resolved Hide resolved
contracts/test/gateway/TestUtilityToken.sol Outdated Show resolved Hide resolved
@gulshanvasnani gulshanvasnani merged commit df24690 into OpenST:develop Dec 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schemar schemar schemar approved these changes

@gulshanvasnani gulshanvasnani gulshanvasnani left review comments

@0xsarvesh 0xsarvesh 0xsarvesh approved these changes

Assignees

@deepesh-kn deepesh-kn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@deepesh-kn @schemar @0xsarvesh @gulshanvasnani @benjaminbollen