remove admin functionality from RBAC.sol #836
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shrugs
force-pushed
the
fix/rbac-separation-of-concerns-#802
branch
from
479c7ff
to
c2f2099
Compare
1 task
frangio
requested changes
Apr 3, 2018
test/Bounty.test.js
Outdated
| @@ -26,7 +26,7 @@ contract('Bounty', function (accounts) { | |||
| let bounty = await SecureTargetBounty.new(); | |||
| sendReward(owner, bounty.address, reward); | |||
|
|
|||
| assert.equal(reward, web3.eth.getBalance(bounty.address).toNumber()); | |||
| assert.equal(reward, await web3.eth.getBalance(bounty.address).toNumber()); | |||
There was a problem hiding this comment.
I don't think toNumber returns a promise, so the await isn't necessary.
Additionally, we shouldn't be converting to Number. See #204. The Crowdsale tests do the correct thing:
| @@ -99,16 +99,16 @@ contract('SampleCrowdsale', function ([owner, wallet, investor]) { | |||
| }); | |||
|
|
|||
| it('should allow refunds if the goal is not reached', async function () { | |||
| const balanceBeforeInvestment = web3.eth.getBalance(investor); | |||
| // const balanceBeforeInvestment = web3.eth.getBalance(investor); | |||
There was a problem hiding this comment.
Should these commented out lines be part of the PR?
|
thanks for the second pair of eyes!! the commented out functions are a very odd bug on my side; the last transaction in SampleCrowdsale.test.js is complaining about the wrong number of solidity function arguments, but it's a 0-argument function. The travis tests seem to pass just fine. I've reverted all of those changes and force-pushed this branch |
shrugs
force-pushed
the
fix/rbac-separation-of-concerns-#802
branch
from
7983b1c
to
485034a
Compare
shrugs
force-pushed
the
fix/rbac-separation-of-concerns-#802
branch
from
485034a
to
997a004
Compare
frangio
pushed a commit
to frangio/openzeppelin-contracts
that referenced
this pull request
Apr 5, 2018
…penZeppelin#836) - splits the admin part of RBAC.sol into RBACWithAdmin.sol
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #802
🚀 Description
separates the rbac admin functionality into a second contract. this is a breaking change.
npm run lint:all:fix).