Skip to content

Commit

Permalink
Only allow implementor to change balances (Closes #76) (#80)
Browse files Browse the repository at this point in the history 
* Only allow implementor to change balances (Closes #76)

* Linting

* Set initial implementor to pausable mock contract

* Linting
  • Loading branch information
@truls @peteremiljensen
truls authored and peteremiljensen committed Jan 26, 2019
1 parent 0305bed commit 2afb8ae
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 53 deletions.
9 changes: 7 additions & 2 deletions contracts/mocks/ExternalERC20PausableMock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@ contract ExternalERC20PausableMock is ExternalERC20Pausable, PauserRoleMock {
* @param initialAccount The account that tokens should be minted to
* @param initialBalance The amount of tokens that should be minted
*/
constructor(address initialAccount, uint initialBalance)
ExternalERC20(new ExternalERC20Storage())
constructor(
address initialAccount,
uint initialBalance,
ExternalERC20Storage stor
)
ExternalERC20(stor)
public
{
stor.latchInitialImplementor();
_mint(initialAccount, initialBalance);
}
}
24 changes: 16 additions & 8 deletions contracts/token/ERC20/ExternalERC20Storage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,38 +55,38 @@ contract ExternalERC20Storage is Ownable {
function setBalance(address owner,
uint256 value)
public
onlyImplementorOrOwner
onlyImplementor
{
balances[owner] = value;
}

/**
* @dev Sets new allowance.
* Can only be done by owner or implementor contract.
* Can only be called by implementor contract.
*/
function setAllowed(address owner,
address spender,
uint256 value)
public
onlyImplementorOrOwner
onlyImplementor
{
allowed[owner][spender] = value;
}

/**
* @dev Change totalSupply.
* Can only be done by owner or implementor contract.
* Can only be called by implementor contract.
*/
function setTotalSupply(uint256 value)
public
onlyImplementorOrOwner
onlyImplementor
{
totalSupply = value;
}

/**
* @dev Transfer implementor to new contract
* Can only be done by owner or implementor contract.
* Can only be called by owner or implementor contract.
*/
function transferImplementor(address newImplementor)
public
Expand All @@ -101,15 +101,23 @@ contract ExternalERC20Storage is Ownable {
}

/**
* @dev Asserts if sender is neither owner nor implementor.
* @dev Asserts that sender is either owner or implementor.
*/
modifier onlyImplementorOrOwner() {
require(isImplementor() || isOwner(), "Is not implementor or owner");
_;
}

/**
* @dev Asserts if the given address is the null-address
* @dev Asserts that sender is the implementor.
*/
modifier onlyImplementor() {
require(isImplementor(), "Is not implementor");
_;
}

/**
* @dev Asserts that the given address is not the null-address
*/
modifier requireNonZero(address addr) {
require(addr != address(0), "Expected a non-zero address");
Expand Down
2 changes: 1 addition & 1 deletion test/token/ERC20/ExternalERC20Mintable.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ contract('ExternalERC20Mintable', function ([_, minter, otherMinter, ...otherAcc

it('should return correct address when changed', async function () {
const newAccount = '0x000000000000000000000000000000000000dddd';
await this.token.changeMintingRecipient(newAccount, {from: minter});
await this.token.changeMintingRecipient(newAccount, { from: minter });
(await this.token.getMintingRecipientAccount()).should.be.equal(newAccount);
});
});
Expand Down
6 changes: 5 additions & 1 deletion test/token/ERC20/ExternalERC20Pausable.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@
const { shouldBehaveLikeERC20Pausable } = require('./behaviors/ERC20Pausable.behavior');

const ExternalERC20PausableMock = artifacts.require('ExternalERC20PausableMock');
const ExternalERC20Storage = artifacts.require('ExternalERC20Storage');

contract('ExternalERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherAccount, ...otherAccounts]) {
beforeEach(async function () {
this.token = await ExternalERC20PausableMock.new(pauser, 100, { from: pauser });
const storage = await ExternalERC20Storage.new({ from: pauser });
this.token = await ExternalERC20PausableMock.new(pauser, 100,
storage.address,
{ from: pauser });
});

shouldBehaveLikeERC20Pausable(pauser, otherPauser, recipient, anotherAccount, otherAccounts);
Expand Down
79 changes: 38 additions & 41 deletions test/token/ERC20/ExternalERC20Storage.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,99 +123,96 @@ contract('ExternalERC20Storage', function ([_, owner, implementor, anotherAccoun
});

describe('setBalance', function () {
const commonOwnerImplementorTests = function (ownerOrImplementorAddress) {
it('should allow changing balance', async function () {
const newBalance = 200;

(await this.token.balances(anotherAccount)).should.not.be.bignumber.equal(newBalance);
await this.token.setBalance(anotherAccount, newBalance, { from: ownerOrImplementorAddress });
(await this.token.balances(anotherAccount)).should.be.bignumber.equal(newBalance);
});
};
const newBalance = 200;

describe('when is owner', function () {
commonOwnerImplementorTests(owner);
it('should not allow changing balance', async function () {
await utils.assertRevertsReason(
this.token.setBalance(anotherAccount, newBalance, { from: owner }),
'Is not implementor');
});
});

describe('when is implementor', function () {
commonOwnerImplementorTests(implementor);
it('should allow changing balance', async function () {
(await this.token.balances(anotherAccount)).should.not.be.bignumber.equal(newBalance);
await this.token.setBalance(anotherAccount, newBalance, { from: implementor });
(await this.token.balances(anotherAccount)).should.be.bignumber.equal(newBalance);
});
});

describe('when is not owner or implementor', function () {
it('should not allow changing balance', async function () {
const newBalance = 200;
const oldBalance = await this.token.balances(anotherAccount);

oldBalance.should.not.be.bignumber.equal(newBalance);
await utils.assertRevertsReason(
this.token.setBalance(anotherAccount, newBalance, { from: thirdAccount }),
'Is not implementor or owner');
'Is not implementor');
(await this.token.balances(anotherAccount)).should.be.bignumber.equal(oldBalance);
});
});
});

describe('setAllowed', function () {
const commonOwnerImplementorTests = function (ownerOrImplementorAddress) {
it('should allow changing allowance', async function () {
const newAllowance = 200;

(await this.token.allowed(anotherAccount, thirdAccount)).should.not.be.bignumber.equal(newAllowance);
await this.token.setAllowed(anotherAccount, thirdAccount, newAllowance, { from: ownerOrImplementorAddress });
(await this.token.allowed(anotherAccount, thirdAccount)).should.be.bignumber.equal(newAllowance);
});
};
const newAllowance = 200;

describe('when is owner', function () {
commonOwnerImplementorTests(owner);
it('should not allow changing allowance', async function () {
await utils.assertRevertsReason(
this.token.setAllowed(anotherAccount, thirdAccount, newAllowance, { from: owner }),
'Is not implementor');
});
});

describe('when is implementor', function () {
commonOwnerImplementorTests(implementor);
it('should allow changing allowance', async function () {
(await this.token.allowed(anotherAccount, thirdAccount)).should.not.be.bignumber.equal(newAllowance);
await this.token.setAllowed(anotherAccount, thirdAccount, newAllowance, { from: implementor });
(await this.token.allowed(anotherAccount, thirdAccount)).should.be.bignumber.equal(newAllowance);
});
});

describe('when is not owner or implementor', function () {
it('should not allow changing balance', async function () {
const newAllowance = 200;
it('should not allow changing allowance', async function () {
const oldAllowance = (await this.token.allowed(anotherAccount, thirdAccount));

oldAllowance.should.not.be.bignumber.equal(newAllowance);
await utils.assertRevertsReason(
this.token.setAllowed(anotherAccount, thirdAccount, newAllowance, { from: anotherAccount }),
'Is not implementor or owner');
'Is not implementor');
(await this.token.allowed(anotherAccount, thirdAccount)).should.be.bignumber.equal(oldAllowance);
});
});
});

describe('setTotalSupply', function () {
const commonOwnerImplementorTests = function (ownerOrImplementorAddress) {
it('should allow changing total supply', async function () {
const newTotalSupply = 200;

(await this.token.totalSupply()).should.not.be.bignumber.equal(newTotalSupply);
await this.token.setTotalSupply(newTotalSupply, { from: ownerOrImplementorAddress });
(await this.token.totalSupply()).should.be.bignumber.equal(newTotalSupply);
});
};
const newTotalSupply = 200;

describe('when is owner', function () {
commonOwnerImplementorTests(owner);
it('should not allow changing total supply', async function () {
await utils.assertRevertsReason(
this.token.setTotalSupply(newTotalSupply, { from: owner }),
'Is not implementor');
});
});

describe('when is implementor', function () {
commonOwnerImplementorTests(implementor);
it('should allow changing total supply', async function () {
(await this.token.totalSupply()).should.not.be.bignumber.equal(newTotalSupply);
await this.token.setTotalSupply(newTotalSupply, { from: implementor });
(await this.token.totalSupply()).should.be.bignumber.equal(newTotalSupply);
});
});

describe('when is not owner or implementor', function () {
it('should not allow changing balance', async function () {
const newTotalSupply = 200;
it('should not allow changing total supply', async function () {
const oldTotalSupply = await this.token.totalSupply();

oldTotalSupply.should.not.be.bignumber.equal(newTotalSupply);
await utils.assertRevertsReason(
this.token.setTotalSupply(newTotalSupply, { from: thirdAccount }),
'Is not implementor or owner');
'Is not implementor');
(await this.token.totalSupply()).should.be.bignumber.equal(oldTotalSupply);
});
});
Expand Down

0 comments on commit 2afb8ae

Please sign in to comment.