MintableToken using Roles #1236

nventuro · 2018-08-23T17:33:28Z

[note that this is going into the rbac-migration branch, which will be merged into master once #1146 is considered solved by it]

Depends on #1235: the diff will be a bit messy until that PR is merged and this one rebased.

MintableToken no longer uses RBAC, and instead is implemented with a single Role. This MinterRole contract provides initial assignment, all necessary public methods (query and transfer), an access modifier, and internal methods for extensions (i.e. derived contracts may add or remove minters), while preventing direct access to the Role struct itself. This 'boilerplate' will likely be repeated for every single role that we implement (BurnerRole, PauserRole, etc.): ideally, we'd autogenerate those contracts from a template.

* Changed .eq to .equal * Changed equal(bool) to .to.be.bool * Changed be.bool to equal(bool), disallowed unused expressions.

@frangio

* Add ERC165Query library * Address PR Comments * Add tests and mocks from OpenZeppelin#1024 and refactor code slightly * Fix javascript and solidity linting errors * Split supportsInterface into three methods as discussed in OpenZeppelin#1086 * Change InterfaceId_ERC165 comment to match style in the rest of the repo * Fix max-len lint issue on ERC165Checker.sol * Conditionally ignore the asserts during solidity-coverage test * Switch to abi.encodeWithSelector and add test for account addresses * Switch to supportsInterfaces API as suggested by @frangio * Adding ERC165InterfacesSupported.sol * Fix style issues * Add test for supportsInterfaces returning false * Add ERC165Checker.sol newline * feat: fix coverage implementation * fix: solidity linting error * fix: revert to using boolean tests instead of require statements * fix: make supportsERC165Interface private again * rename SupportsInterfaceWithLookupMock to avoid name clashing

shrugs · 2018-08-23T20:41:21Z

contracts/access/rbac/MinterRole.sol

+    minters.remove(_account);
+  }
+
+  function transferMinterRole(address _account) public {


this function is public, which is probably fine, but should be noted that it allows the existing minters to change who has the permission

That's intentional - we've discussed this with @frangio and see no reason why people wouldn't be able to transfer their roles.

Also, this allows to easily setup scenarios like this one:

const token = await MintableToken.new([initialMinter]); const crowdsale = await Crowdsale.new(token.address); await token.transferMinterRole(crowdsale.address, { from: initialMinter });

contracts/access/rbac/MinterRole.sol

shrugs · 2018-08-23T20:42:36Z

contracts/examples/SampleCrowdsale.sol

@@ -17,6 +17,8 @@ contract SampleCrowdsaleToken is MintableToken {
  string public constant symbol = "SCT";
  uint8 public constant decimals = 18;

+  constructor(address[] _minters) MintableToken(_minters) {


nit: newline for the inheritable constructor

constructor(address[] _minters) MintableToken(_minters) public { }

shrugs · 2018-08-23T20:42:52Z

contracts/mocks/FinalizableCrowdsaleImpl.sol

@@ -11,7 +11,7 @@ contract FinalizableCrowdsaleImpl is FinalizableCrowdsale {
    uint256 _closingTime,
    uint256 _rate,
    address _wallet,
-    MintableToken _token
+    ERC20 _token


shrugs · 2018-08-23T20:43:16Z

contracts/token/ERC20/MintableToken.sol

  event Mint(address indexed to, uint256 amount);
  event MintFinished();

  bool public mintingFinished = false;

+  constructor(address[] _minters) MinterRole(_minters) {


nit: newline here as well, plus visibility modifier

shrugs

added some comments

nventuro · 2018-08-24T09:52:49Z

Addressed those, thanks!

* ERC721 pausable token * Reuse of ERC721 Basic behavior for Pausable, split view checks in paused state & style fixes * [~] paused token behavior

* add note about pulling upstream changes to release branch * add comment about upstream changes in merging section

shrugs · 2018-08-24T18:00:50Z

@nventuro can you merge master? looks like there are some conflicts

nventuro · 2018-08-24T18:22:33Z

#1235 needs to be merged first, could you review it?

frangio · 2018-08-24T20:52:06Z

@nventuro This PR shouldn't be targetting master right?

nventuro · 2018-08-24T21:29:07Z

Gah you're right, thanks!

frangio

Looking good.

frangio · 2018-08-24T21:24:59Z

test/crowdsale/FinalizableCrowdsale.test.js

@@ -26,11 +26,10 @@ contract('FinalizableCrowdsale', function ([_, owner, wallet, thirdparty]) {
    this.closingTime = this.openingTime + duration.weeks(1);
    this.afterClosingTime = this.closingTime + duration.seconds(1);

-    this.token = await MintableToken.new();
+    this.token = await StandardToken.new([]);


What is the [] argument?

Probably the empty minters list from before I changed that token to a StandardToken, I find it worrying that truffle didn't complain about that.

frangio · 2018-08-24T21:32:33Z

contracts/token/ERC20/CappedToken.sol

@@ -11,7 +11,7 @@ contract CappedToken is MintableToken {

  uint256 public cap;

-  constructor(uint256 _cap) public {
+  constructor(uint256 _cap, address[] _minters) public MintableToken(_minters) {


Just to be explicit: the convention will be to have all role bearer arrays at the end of the argument list.

This should probably be in the ownership mini readme.

frangio · 2018-08-24T21:35:19Z

test/token/ERC20/MintableToken.behavior.js

        it('returns false', async function () {
          (await this.token.mintingFinished()).should.be.false;
        });
      });

-      describe('when the token is minting finished', function () {
+      describe('when the token minting is finished', function () {


Remove the for consistency.

* Fixed a SplitPayment test * Deleted unnecessary function. * Improved PostDeliveryCrowdsale tests. * Improved RefundableCrowdsale tests. * Improved MintedCrowdsale tests. * Improved IncreasingPriceCrowdsale tests. * Fixed a CappedCrowdsale test. * Improved TimedCrowdsale tests. * Improved descriptions of added tests.

* fix: refactor sign.js and related tests * fix: remove unused dep * fix: update package.json correctly

nventuro · 2018-08-29T16:28:00Z

I've decided to add some extra bits to this PR (namely MinterRole tests) - we shouldn't merge yet.

nventuro · 2018-08-30T17:19:39Z

The test behavior should be reusable by all publicly-exposed roles (which I very poorly called PublicRole): the method names are inferred from the role name. A mock has to be created however, exposing the addRole and removeRole methods.

nventuro · 2018-08-31T15:00:35Z

@shrugs @frangio ready for review

shrugs · 2018-08-31T16:54:16Z

contracts/token/ERC20/CappedToken.sol

-  constructor(uint256 _cap) public {
+  constructor(uint256 _cap, address[] _minters)
+    public
+    MintableToken(_minters)


nit: does solium complain that the public modifier has to be above MintableToken here? Convention is the inherited contract instantiation and then the visibility modifier

It doesn't complain either if I place it first or last. We don't seem to follow that convention though, in some of our contracts visibility is indeed first, we should get a Solium rule and fix that.

interesting. I'd support inheritances before visibility for constructors, since that seems to be the convention in contracts I've seen.

* rename ERC20 to IERC20 * move ERC20.sol to IERC20.sol * rename StandardToken to ERC20 * rename StandardTokenMock to ERC20Mock * move StandardToken.sol to ERC20.sol, likewise test and mock files * rename MintableToken to ERC20Mintable * move MintableToken.sol to ERC20Mintable.sol, likewise test and mock files * rename BurnableToken to ERC20Burnable * move BurnableToken.sol to ERC20Burnable.sol, likewise for related files * rename CappedToken to ERC20Capped * move CappedToken.sol to ERC20Capped.sol, likewise for related files * rename PausableToken to ERC20Pausable * move PausableToken.sol to ERC20Pausable.sol, likewise for related files * rename DetailedERC20 to ERC20Detailed * move DetailedERC20.sol to ERC20Detailed.sol, likewise for related files * rename ERC721 to IERC721, and likewise for other related interfaces * move ERC721.sol to IERC721.sol, likewise for other 721 interfaces * rename ERC721Token to ERC721 * move ERC721Token.sol to ERC721.sol, likewise for related files * rename ERC721BasicToken to ERC721Basic * move ERC721BasicToken.sol to ERC721Basic.sol, likewise for related files * rename ERC721PausableToken to ERC721Pausable * move ERC721PausableToken.sol to ERC721Pausable.sol * rename ERC165 to IERC165 * move ERC165.sol to IERC165.sol * amend comment that ERC20 is based on FirstBlood * fix comments mentioning IERC721Receiver

* Remove underscores from event parameters. Fixes OpenZeppelin#1175 * Add comment about ERC

* Move contracts to subdirectories Fixes OpenZeppelin#1177. This Change also removes the LimitBalance contract. * fix import * move MerkleProof to cryptography * Fix import

…elin#1254) * remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner * remove unused ERC223TokenMock * remove Contactable * remove TokenDestructible * remove DeprecatedERC721 * inline Destructible#destroy in Bounty * remove Destructible

frangio · 2018-09-03T20:16:37Z

contracts/mocks/FinalizableCrowdsaleImpl.sol

@@ -11,7 +11,7 @@ contract FinalizableCrowdsaleImpl is FinalizableCrowdsale {
    uint256 _closingTime,
    uint256 _rate,
    address _wallet,
-    MintableToken _token
+    ERC20 _token


This is IERC20 now in master. It won't be a merge error and the tests will pass, so we should keep an eye on it when we merge.

Good catch, I expected the merge errors to cover most of these.

frangio · 2018-09-03T20:34:55Z

test/access/rbac/PublicRole.behavior.js

+
+    describe('add', function () {
+      it('adds role to a new account', async function () {
+        await this.contract[`add${rolename}`](authorized);


Shouldn't this be adding the role to anyone?

Yes, looks like I forgot to update those tests after I added the preconditions :/

frangio · 2018-09-03T20:42:07Z

test/access/rbac/PublicRole.behavior.js

+
+      it('adds role to an already-assigned account', async function () {
+        await this.contract[`add${rolename}`](authorized);
+        await this.contract[`add${rolename}`](authorized);


If we have a precondition that authorized is authorized, is it necessary to run add twice?

frangio · 2018-09-03T20:42:40Z

test/access/rbac/PublicRole.behavior.js

+    });
+
+    describe('remove', function () {
+      it('removes role from an already assgined account', async function () {


Typo in assigned.

test/access/rbac/PublicRole.behavior.js

…elin#1257) * Add a leading underscore to internal and private functions. Fixes OpenZeppelin#1176 * Remove super * update the ERC721 changes * add missing underscore after merge * Fix mock

…OpenZeppelin#1265) * Improve encapsulation on Whitelist * remove only * update whitelisted crowdsale test * Improve encapsulation on SignatureBouncer * fix missing test * Improve encapsulation on RBAC example * Improve encapsulation on RBAC example * Remove extra visibility * Improve encapsulation on ERC20 Mintable * Improve encapsulation on Superuser * fix lint * add missing constant

frangio

LGTM!

@frangio

* Role tests (#1228) * Moved RBAC tests to access. * Added Roles.addMany and tests. * Fixed linter error. * Now using uint256 indexes. * Removed RBAC tokens (#1229) * Deleted RBACCappedTokenMock. * Removed RBACMintableToken. * Removed RBACMintableToken from the MintedCrowdsale tests. * Roles can now be transfered. (#1235) * Roles can now be transfered. * Now explicitly checking support for the null address. * Now rejecting transfer to a role-haver. * Added renounce, roles can no longer be transfered to 0. * Fixed linter errors. * Fixed a Roles test. * True Ownership (#1247) * Added barebones Secondary. * Added transferPrimary * Escrow is now Secondary instead of Ownable. * Now reverting on transfers to 0. * The Secondary's primary is now private. * MintableToken using Roles (#1236) * Minor test style improvements (#1219) * Changed .eq to .equal * Changed equal(bool) to .to.be.bool * Changed be.bool to equal(bool), disallowed unused expressions. * Add ERC165Query library (#1086) * Add ERC165Query library * Address PR Comments * Add tests and mocks from #1024 and refactor code slightly * Fix javascript and solidity linting errors * Split supportsInterface into three methods as discussed in #1086 * Change InterfaceId_ERC165 comment to match style in the rest of the repo * Fix max-len lint issue on ERC165Checker.sol * Conditionally ignore the asserts during solidity-coverage test * Switch to abi.encodeWithSelector and add test for account addresses * Switch to supportsInterfaces API as suggested by @frangio * Adding ERC165InterfacesSupported.sol * Fix style issues * Add test for supportsInterfaces returning false * Add ERC165Checker.sol newline * feat: fix coverage implementation * fix: solidity linting error * fix: revert to using boolean tests instead of require statements * fix: make supportsERC165Interface private again * rename SupportsInterfaceWithLookupMock to avoid name clashing * Added mint and burn tests for zero amounts. (#1230) * Changed .eq to .equal. (#1231) * ERC721 pausable token (#1154) * ERC721 pausable token * Reuse of ERC721 Basic behavior for Pausable, split view checks in paused state & style fixes * [~] paused token behavior * Add some detail to releasing steps (#1190) * add note about pulling upstream changes to release branch * add comment about upstream changes in merging section * Increase test coverage (#1237) * Fixed a SplitPayment test * Deleted unnecessary function. * Improved PostDeliveryCrowdsale tests. * Improved RefundableCrowdsale tests. * Improved MintedCrowdsale tests. * Improved IncreasingPriceCrowdsale tests. * Fixed a CappedCrowdsale test. * Improved TimedCrowdsale tests. * Improved descriptions of added tests. * ci: trigger docs update on tag (#1186) * MintableToken now uses Roles. * Fixed FinalizableCrowdsale test. * Roles can now be transfered. * Fixed tests related to MintableToken. * Removed Roles.check. * Renamed transferMintPermission. * Moved MinterRole * Fixed RBAC. * Adressed review comments. * Addressed review comments * Fixed linter errors. * Added Events tests of Pausable contract (#1207) * Fixed roles tests. * Rename events to past-tense (#1181) * fix: refactor sign.js and related tests (#1243) * fix: refactor sign.js and related tests * fix: remove unused dep * fix: update package.json correctly * Added "_" sufix to internal variables (#1171) * Added PublicRole test. * Fixed crowdsale tests. * Rename ERC interfaces to I prefix (#1252) * rename ERC20 to IERC20 * move ERC20.sol to IERC20.sol * rename StandardToken to ERC20 * rename StandardTokenMock to ERC20Mock * move StandardToken.sol to ERC20.sol, likewise test and mock files * rename MintableToken to ERC20Mintable * move MintableToken.sol to ERC20Mintable.sol, likewise test and mock files * rename BurnableToken to ERC20Burnable * move BurnableToken.sol to ERC20Burnable.sol, likewise for related files * rename CappedToken to ERC20Capped * move CappedToken.sol to ERC20Capped.sol, likewise for related files * rename PausableToken to ERC20Pausable * move PausableToken.sol to ERC20Pausable.sol, likewise for related files * rename DetailedERC20 to ERC20Detailed * move DetailedERC20.sol to ERC20Detailed.sol, likewise for related files * rename ERC721 to IERC721, and likewise for other related interfaces * move ERC721.sol to IERC721.sol, likewise for other 721 interfaces * rename ERC721Token to ERC721 * move ERC721Token.sol to ERC721.sol, likewise for related files * rename ERC721BasicToken to ERC721Basic * move ERC721BasicToken.sol to ERC721Basic.sol, likewise for related files * rename ERC721PausableToken to ERC721Pausable * move ERC721PausableToken.sol to ERC721Pausable.sol * rename ERC165 to IERC165 * move ERC165.sol to IERC165.sol * amend comment that ERC20 is based on FirstBlood * fix comments mentioning IERC721Receiver * added explicit visibility (#1261) * Remove underscores from event parameters. (#1258) * Remove underscores from event parameters. Fixes #1175 * Add comment about ERC * Move contracts to subdirectories (#1253) * Move contracts to subdirectories Fixes #1177. This Change also removes the LimitBalance contract. * fix import * move MerkleProof to cryptography * Fix import * Remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner (#1254) * remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner * remove unused ERC223TokenMock * remove Contactable * remove TokenDestructible * remove DeprecatedERC721 * inline Destructible#destroy in Bounty * remove Destructible * Functions in interfaces changed to "external" (#1263) * Add a leading underscore to internal and private functions. (#1257) * Add a leading underscore to internal and private functions. Fixes #1176 * Remove super * update the ERC721 changes * add missing underscore after merge * Fix mock * Improve encapsulation on SignatureBouncer, Whitelist and RBAC example (#1265) * Improve encapsulation on Whitelist * remove only * update whitelisted crowdsale test * Improve encapsulation on SignatureBouncer * fix missing test * Improve encapsulation on RBAC example * Improve encapsulation on RBAC example * Remove extra visibility * Improve encapsulation on ERC20 Mintable * Improve encapsulation on Superuser * fix lint * add missing constant * Addressed review comments. * Fixed build error. * Improved Roles API. (#1280) * Improved Roles API. * fix linter error * Added PauserRole. (#1283) * Remove Claimable, DelayedClaimable, Heritable (#1274) * remove Claimable, DelayedClaimable, Heritable * remove SimpleSavingsWallet example which used Heritable (cherry picked from commit 0dc7117) * Role behavior tests (#1285) * Added role tests. * Added PauserRole tests to contracts that have that role. * Added MinterRole tests to contracts that have that role. * Fixed linter errors. * Migrate Ownable to Roles (#1287) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Remove RBAC, SignatureBouncer refactor (#1289) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Moved SignatureBouncer tests. * Deleted RBAC and Superuser. * Deleted rbac directory. * Updated readme. * SignatureBouncer now uses SignerRole, renamed bouncer to signer. * feat: implement ERC721Mintable and ERC721Burnable (#1276) * feat: implement ERC721Mintable and ERC721Burnable * fix: linting errors * fix: remove unused mintable mock for ERC721BasicMock * fix: add finishMinting tests * fix: catch MintFinished typo * inline ERC721Full behavior * undo pretty formatting * fix lint errors * rename canMint to onlyBeforeMintingFinished for consistency with ERC20Mintable * Fix the merge with the privatization branch * remove duplicate CapperRole test

@frangio

…meters (#1282) * Role tests (#1228) * Moved RBAC tests to access. * Added Roles.addMany and tests. * Fixed linter error. * Now using uint256 indexes. * Removed RBAC tokens (#1229) * Deleted RBACCappedTokenMock. * Removed RBACMintableToken. * Removed RBACMintableToken from the MintedCrowdsale tests. * Roles can now be transfered. (#1235) * Roles can now be transfered. * Now explicitly checking support for the null address. * Now rejecting transfer to a role-haver. * Added renounce, roles can no longer be transfered to 0. * Fixed linter errors. * Fixed a Roles test. * True Ownership (#1247) * Added barebones Secondary. * Added transferPrimary * Escrow is now Secondary instead of Ownable. * Now reverting on transfers to 0. * The Secondary's primary is now private. * Improve encapsulation on ERC165 * Improve encapsulation on ERC20 * Improve encapsulation on ERC721 * Add tests, use standard getters * fix tests * Fix lint * MintableToken using Roles (#1236) * Minor test style improvements (#1219) * Changed .eq to .equal * Changed equal(bool) to .to.be.bool * Changed be.bool to equal(bool), disallowed unused expressions. * Add ERC165Query library (#1086) * Add ERC165Query library * Address PR Comments * Add tests and mocks from #1024 and refactor code slightly * Fix javascript and solidity linting errors * Split supportsInterface into three methods as discussed in #1086 * Change InterfaceId_ERC165 comment to match style in the rest of the repo * Fix max-len lint issue on ERC165Checker.sol * Conditionally ignore the asserts during solidity-coverage test * Switch to abi.encodeWithSelector and add test for account addresses * Switch to supportsInterfaces API as suggested by @frangio * Adding ERC165InterfacesSupported.sol * Fix style issues * Add test for supportsInterfaces returning false * Add ERC165Checker.sol newline * feat: fix coverage implementation * fix: solidity linting error * fix: revert to using boolean tests instead of require statements * fix: make supportsERC165Interface private again * rename SupportsInterfaceWithLookupMock to avoid name clashing * Added mint and burn tests for zero amounts. (#1230) * Changed .eq to .equal. (#1231) * ERC721 pausable token (#1154) * ERC721 pausable token * Reuse of ERC721 Basic behavior for Pausable, split view checks in paused state & style fixes * [~] paused token behavior * Add some detail to releasing steps (#1190) * add note about pulling upstream changes to release branch * add comment about upstream changes in merging section * Increase test coverage (#1237) * Fixed a SplitPayment test * Deleted unnecessary function. * Improved PostDeliveryCrowdsale tests. * Improved RefundableCrowdsale tests. * Improved MintedCrowdsale tests. * Improved IncreasingPriceCrowdsale tests. * Fixed a CappedCrowdsale test. * Improved TimedCrowdsale tests. * Improved descriptions of added tests. * ci: trigger docs update on tag (#1186) * MintableToken now uses Roles. * Fixed FinalizableCrowdsale test. * Roles can now be transfered. * Fixed tests related to MintableToken. * Removed Roles.check. * Renamed transferMintPermission. * Moved MinterRole * Fixed RBAC. * Adressed review comments. * Addressed review comments * Fixed linter errors. * Added Events tests of Pausable contract (#1207) * Fixed roles tests. * Rename events to past-tense (#1181) * fix: refactor sign.js and related tests (#1243) * fix: refactor sign.js and related tests * fix: remove unused dep * fix: update package.json correctly * Added "_" sufix to internal variables (#1171) * Added PublicRole test. * Fixed crowdsale tests. * Rename ERC interfaces to I prefix (#1252) * rename ERC20 to IERC20 * move ERC20.sol to IERC20.sol * rename StandardToken to ERC20 * rename StandardTokenMock to ERC20Mock * move StandardToken.sol to ERC20.sol, likewise test and mock files * rename MintableToken to ERC20Mintable * move MintableToken.sol to ERC20Mintable.sol, likewise test and mock files * rename BurnableToken to ERC20Burnable * move BurnableToken.sol to ERC20Burnable.sol, likewise for related files * rename CappedToken to ERC20Capped * move CappedToken.sol to ERC20Capped.sol, likewise for related files * rename PausableToken to ERC20Pausable * move PausableToken.sol to ERC20Pausable.sol, likewise for related files * rename DetailedERC20 to ERC20Detailed * move DetailedERC20.sol to ERC20Detailed.sol, likewise for related files * rename ERC721 to IERC721, and likewise for other related interfaces * move ERC721.sol to IERC721.sol, likewise for other 721 interfaces * rename ERC721Token to ERC721 * move ERC721Token.sol to ERC721.sol, likewise for related files * rename ERC721BasicToken to ERC721Basic * move ERC721BasicToken.sol to ERC721Basic.sol, likewise for related files * rename ERC721PausableToken to ERC721Pausable * move ERC721PausableToken.sol to ERC721Pausable.sol * rename ERC165 to IERC165 * move ERC165.sol to IERC165.sol * amend comment that ERC20 is based on FirstBlood * fix comments mentioning IERC721Receiver * added explicit visibility (#1261) * Remove underscores from event parameters. (#1258) * Remove underscores from event parameters. Fixes #1175 * Add comment about ERC * Move contracts to subdirectories (#1253) * Move contracts to subdirectories Fixes #1177. This Change also removes the LimitBalance contract. * fix import * move MerkleProof to cryptography * Fix import * Remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner (#1254) * remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner * remove unused ERC223TokenMock * remove Contactable * remove TokenDestructible * remove DeprecatedERC721 * inline Destructible#destroy in Bounty * remove Destructible * Functions in interfaces changed to "external" (#1263) * Add a leading underscore to internal and private functions. (#1257) * Add a leading underscore to internal and private functions. Fixes #1176 * Remove super * update the ERC721 changes * add missing underscore after merge * Fix mock * Improve encapsulation on SignatureBouncer, Whitelist and RBAC example (#1265) * Improve encapsulation on Whitelist * remove only * update whitelisted crowdsale test * Improve encapsulation on SignatureBouncer * fix missing test * Improve encapsulation on RBAC example * Improve encapsulation on RBAC example * Remove extra visibility * Improve encapsulation on ERC20 Mintable * Improve encapsulation on Superuser * fix lint * add missing constant * Addressed review comments. * Fixed build error. * move interface ids to implementation contracts * Do not prefix getters * Improve encapsulation on Crowdsales * add missing tests * remove only * Improve encapsulation on Pausable * add the underscore * Improve encapsulation on ownership * fix rebase * fix ownership * Improve encapsulation on payments * Add missing tests * add missing test * Do not prefix getters * Do not prefix getters * Fix tests. * Update modifiers to call public view functions. Fixes #1179. * Improve encapsulation on BreakInvariantBounty * Make researchers private * Improve encapsulation on Crowdsales * add missing tests * remove only * Improve encapsulation on Pausable * add the underscore * Improve encapsulation on ownership * fix rebase * fix ownership * Improve encapsulation on payments * Add missing tests * add missing test * Do not prefix getters * Do not prefix getters * Do not prefix getters * Fix tests. * tmp * remove isMinter * fix is owner call * fix isOpen * Fix merge * tmp * Improve encapsulation on TimedCrowdsale * Add missing parentheses * Use prefix underscore for state variables and no underscore for parameters * Improved Roles API. (#1280) * Improved Roles API. * fix linter error * Added PauserRole. (#1283) * remove duplicate function definition * Remove Claimable, DelayedClaimable, Heritable (#1274) * remove Claimable, DelayedClaimable, Heritable * remove SimpleSavingsWallet example which used Heritable (cherry picked from commit 0dc7117) * Role behavior tests (#1285) * Added role tests. * Added PauserRole tests to contracts that have that role. * Added MinterRole tests to contracts that have that role. * Fixed linter errors. * Migrate Ownable to Roles (#1287) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Remove RBAC, SignatureBouncer refactor (#1289) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Moved SignatureBouncer tests. * Deleted RBAC and Superuser. * Deleted rbac directory. * Updated readme. * SignatureBouncer now uses SignerRole, renamed bouncer to signer. * feat: implement ERC721Mintable and ERC721Burnable (#1276) * feat: implement ERC721Mintable and ERC721Burnable * fix: linting errors * fix: remove unused mintable mock for ERC721BasicMock * fix: add finishMinting tests * fix: catch MintFinished typo * inline ERC721Full behavior * undo pretty formatting * fix lint errors * rename canMint to onlyBeforeMintingFinished for consistency with ERC20Mintable * Fix the merge with the privatization branch * fix lint * Remove underscore * Delete CapperRole.test.js * fix increaseApproval

nventuro and others added 3 commits August 22, 2018 16:05

Minor test style improvements (OpenZeppelin#1219)

4fb9bb7

* Changed .eq to .equal * Changed equal(bool) to .to.be.bool * Changed be.bool to equal(bool), disallowed unused expressions.

Added mint and burn tests for zero amounts. (OpenZeppelin#1230)

d58fac8

nventuro added feature New contracts, functions, or helpers. contracts Smart contract code. breaking change Changes that break backwards compatibility of the public API. labels Aug 23, 2018

nventuro added this to the v2.0 milestone Aug 23, 2018

nventuro self-assigned this Aug 23, 2018

nventuro requested review from frangio and shrugs August 23, 2018 17:33

shrugs reviewed Aug 23, 2018

View reviewed changes

contracts/access/rbac/MinterRole.sol Outdated Show resolved Hide resolved

shrugs reviewed Aug 23, 2018

View reviewed changes

Changed .eq to .equal. (OpenZeppelin#1231)

48fe7b8

urvalla and others added 2 commits August 24, 2018 07:02

ERC721 pausable token (OpenZeppelin#1154)

9e6307f

* ERC721 pausable token * Reuse of ERC721 Basic behavior for Pausable, split view checks in paused state & style fixes * [~] paused token behavior

Add some detail to releasing steps (OpenZeppelin#1190)

a466e76

* add note about pulling upstream changes to release branch * add comment about upstream changes in merging section

nventuro changed the base branch from master to rbac-migration August 24, 2018 21:28

frangio reviewed Aug 24, 2018

View reviewed changes

nventuro and others added 4 commits August 24, 2018 19:23

ci: trigger docs update on tag (OpenZeppelin#1186)

524a276

Roles can now be transfered.

4b3b6b6

MintableToken now uses Roles.

6ea040f

shrugs and others added 2 commits August 29, 2018 06:35

fix: refactor sign.js and related tests (OpenZeppelin#1243)

1c57637

* fix: refactor sign.js and related tests * fix: remove unused dep * fix: update package.json correctly

Added "_" sufix to internal variables (OpenZeppelin#1171)

b694326

nventuro added the status:in-progress label Aug 29, 2018

shrugs changed the title ~~MintableToken using Roles~~ WIP: MintableToken using Roles Aug 29, 2018

Added PublicRole test.

c54681a

Fixed crowdsale tests.

894fbed

nventuro removed the status:in-progress label Aug 31, 2018

nventuro changed the title ~~WIP: MintableToken using Roles~~ MintableToken using Roles Aug 31, 2018

shrugs reviewed Aug 31, 2018

View reviewed changes

frangio and others added 6 commits August 31, 2018 16:13

added explicit visibility (OpenZeppelin#1261)

4385fd5

Remove underscores from event parameters. (OpenZeppelin#1258)

964bc40

* Remove underscores from event parameters. Fixes OpenZeppelin#1175 * Add comment about ERC

Move contracts to subdirectories (OpenZeppelin#1253)

2441fd7

* Move contracts to subdirectories Fixes OpenZeppelin#1177. This Change also removes the LimitBalance contract. * fix import * move MerkleProof to cryptography * Fix import

Functions in interfaces changed to "external" (OpenZeppelin#1263)

fbfd1fe

frangio reviewed Sep 3, 2018

View reviewed changes

Leo Arias and others added 6 commits September 3, 2018 18:08

Add a leading underscore to internal and private functions. (OpenZepp…

b0f20d4

…elin#1257) * Add a leading underscore to internal and private functions. Fixes OpenZeppelin#1176 * Remove super * update the ERC721 changes * add missing underscore after merge * Fix mock

Merge branch 'master' into rbac-to-roles

69f7f49

Addressed review comments.

41d0c08

Merge branch 'rbac-migration' into rbac-to-roles

2136be9

Fixed build error.

b2c48b6

frangio approved these changes Sep 4, 2018

View reviewed changes

nventuro merged commit d8e7c25 into OpenZeppelin:rbac-migration Sep 4, 2018

nventuro deleted the rbac-to-roles branch September 4, 2018 17:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MintableToken using Roles #1236

MintableToken using Roles #1236

nventuro commented Aug 23, 2018 •

edited

Loading

shrugs Aug 23, 2018

nventuro Aug 24, 2018 •

edited

Loading

shrugs Aug 23, 2018

shrugs Aug 23, 2018

shrugs Aug 23, 2018

shrugs left a comment

nventuro commented Aug 24, 2018

shrugs commented Aug 24, 2018

nventuro commented Aug 24, 2018

frangio commented Aug 24, 2018

nventuro commented Aug 24, 2018

frangio left a comment

frangio Aug 24, 2018

nventuro Aug 24, 2018

frangio Aug 24, 2018

nventuro Aug 28, 2018

frangio Aug 24, 2018

nventuro commented Aug 29, 2018

nventuro commented Aug 30, 2018

nventuro commented Aug 31, 2018

shrugs Aug 31, 2018

nventuro Sep 3, 2018

shrugs Sep 3, 2018

frangio Sep 3, 2018

nventuro Sep 4, 2018

frangio Sep 3, 2018

nventuro Sep 4, 2018

frangio Sep 3, 2018

frangio Sep 3, 2018

frangio left a comment

MintableToken using Roles #1236

MintableToken using Roles #1236

Conversation

nventuro commented Aug 23, 2018 • edited Loading

Choose a reason for hiding this comment

nventuro Aug 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shrugs left a comment

Choose a reason for hiding this comment

nventuro commented Aug 24, 2018

shrugs commented Aug 24, 2018

nventuro commented Aug 24, 2018

frangio commented Aug 24, 2018

nventuro commented Aug 24, 2018

frangio left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nventuro commented Aug 29, 2018

nventuro commented Aug 30, 2018

nventuro commented Aug 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frangio left a comment

Choose a reason for hiding this comment

nventuro commented Aug 23, 2018 •

edited

Loading

nventuro Aug 24, 2018 •

edited

Loading