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

Create and test PausableToken Contract #206

Merged

maraoz merged 1 commit into OpenZeppelin:master from DavidKnott:create-pausable-token

May 3, 2017

Contributor

DavidKnott commented Apr 24, 2017 •

edited

Loading

Creates PausableToken contract that inherits from the StandardTokenContract.
Change Pauable contract to use pause as opposed to emergency
PausableToken has an owner who is able to:

Pause all token transfers
Unpause all token transfers

This contract was created in response to issue #194
It's fully tested in PausableToken.js and all tests are passing.

Contributor

maraoz commented Apr 24, 2017

Amazing work @DavidKnott!!! Thanks a lot.
Do you mind refactoring the code so that it inherits from Pausable?
(https://github.com/OpenZeppelin/zeppelin-solidity/blob/master/contracts/lifecycle/Pausable.sol)

I actually like the modifier and event names better here, so if you can also modify Pausable to adopt this new names that would be great.
What do you think?

BTW, if you haven't already, please join our slack, I'd like to have a chat with you and thank you for your contributions :)
https://slack.openzeppelin.org/

Contributor Author

DavidKnott commented Apr 24, 2017

Thanks for looking it over so quickly @maraoz! I like that idea, I'll integrate it with the Pause contract and change the names to match the PauseToken.

DavidKnott force-pushed the create-pausable-token branch from db16fbf to 5b5fef7 Compare

April 25, 2017 15:43

Contributor Author

DavidKnott commented Apr 25, 2017

@maraoz I changed the Pausable contract like we talked about before. I think the modifier '''canTransfer''' makes sense for allowing transactions only when they're not paused but cannotTransfer seems a bit confusing to me (the functionality is that it allows stuff to occur only when paused). What do you think?

Contributor

maraoz commented Apr 25, 2017

@DavidKnott I think Pausable should have no mentions to transfers, I would call the methods pause, unpause, and the modifiers ifPaused or whenPaused and whenNotPaused

maraoz reviewed

View reviewed changes

contracts/lifecycle/Pausable.sol Outdated

-                  if (stopped) {
-                    throw;
-                  }
+                bool public transfersPaused = false;

Contributor

maraoz Apr 25, 2017

bool public paused = false;?

contracts/lifecycle/Pausable.sol Outdated

+                bool public transfersPaused = false;
+                modifier canTransfer() {
+                  if (transfersPaused) throw;
                   _;
                 }

Contributor

maraoz Apr 25, 2017

whenNotPaused

contracts/lifecycle/Pausable.sol Outdated

-                    throw;
-                  }
+                modifier cannotTransfer {

Contributor

maraoz Apr 25, 2017

whenPaused

contracts/lifecycle/Pausable.sol Outdated

-                function emergencyStop() external onlyOwner {
-                  stopped = true;
+                // called by the owner to pause transfers, triggers stopped state
+                function pauseTransfers() onlyOwner canTransfer returns (bool) {

Contributor

maraoz Apr 25, 2017

pause

contracts/lifecycle/Pausable.sol Outdated

-                function release() external onlyOwner onlyInEmergency {
-                  stopped = false;
+                // called by the owner to allow transfers, returns to normal state
+                function unpauseTransfers() onlyOwner cannotTransfer returns (bool) {

Contributor

maraoz Apr 25, 2017

unpause

DavidKnott force-pushed the create-pausable-token branch 2 times, most recently from 671b869 to 927ef98 Compare

April 25, 2017 17:47


          Create and test PausableToken Contract

b4b6029

DavidKnott force-pushed the create-pausable-token branch from 927ef98 to b4b6029 Compare

April 29, 2017 00:08

Contributor Author

DavidKnott commented Apr 29, 2017

@maraoz thanks for the feedback, I took all mentions of transactions out of the Pausable contract (tried to make it as modular as possible). Do you see anything else that needs to be changed?

Contributor

maraoz commented May 3, 2017

LGTM, many thanks for the contribution!

maraoz merged commit f7a7fc3 into OpenZeppelin:master

ProphetDaniel pushed a commit to classicdelta/Smart-Contracts that referenced this pull request


          Merge pull request OpenZeppelin#206 from DavidKnott/create-pausable-t…

58613dc

…oken

Create and test PausableToken Contract

Woodpile37 mentioned this pull request

[Snyk] Upgrade eslint-plugin-node from 10.0.0 to 11.1.0 Woodpile37/openzeppelin-contracts#65

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet