aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol7
-rw-r--r--packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol5
-rw-r--r--packages/contracts/test/asset_proxy/authorizable.ts69
3 files changed, 79 insertions, 2 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
index 8cb4254c5..37c12f861 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
@@ -69,7 +69,7 @@ contract MixinAuthorizable is
);
delete authorized[target];
- for (uint i = 0; i < authorities.length; i++) {
+ for (uint256 i = 0; i < authorities.length; i++) {
if (authorities[i] == target) {
authorities[i] = authorities[authorities.length - 1];
authorities.length -= 1;
@@ -87,8 +87,13 @@ contract MixinAuthorizable is
uint256 index
)
external
+ onlyOwner
{
require(
+ authorized[target],
+ TARGET_NOT_AUTHORIZED
+ );
+ require(
index < authorities.length,
INDEX_OUT_OF_BOUNDS
);
diff --git a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
index 296c6c856..99b93d0d2 100644
--- a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
+++ b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
@@ -13,6 +13,9 @@ import "./IOwnable.sol";
contract Ownable is IOwnable {
address public owner;
+ // Revert reasons
+ string constant ONLY_CONTRACT_OWNER = "ONLY_CONTRACT_OWNER";
+
constructor ()
public
{
@@ -22,7 +25,7 @@ contract Ownable is IOwnable {
modifier onlyOwner() {
require(
msg.sender == owner,
- "Only contract owner is allowed to call this method."
+ ONLY_CONTRACT_OWNER
);
_;
}
diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts
index ed582b63e..c35dc7882 100644
--- a/packages/contracts/test/asset_proxy/authorizable.ts
+++ b/packages/contracts/test/asset_proxy/authorizable.ts
@@ -1,4 +1,5 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
+import { BigNumber } from '@0xproject/utils';
import * as chai from 'chai';
import { MixinAuthorizableContract } from '../../src/generated_contract_wrappers/mixin_authorizable';
@@ -102,6 +103,74 @@ describe('Authorizable', () => {
});
});
+ describe('removeAuthorizedAddressAtIndex', () => {
+ it('should throw if not called by owner', async () => {
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ const index = new BigNumber(0);
+ return expectRevertOrAlwaysFailingTransactionAsync(
+ authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, {
+ from: notOwner,
+ }),
+ );
+ });
+ it('should throw if index is >= authorities.length', async () => {
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ const index = new BigNumber(1);
+ return expectRevertOrAlwaysFailingTransactionAsync(
+ authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, {
+ from: owner,
+ }),
+ );
+ });
+ it('should throw if owner attempts to remove an address that is not authorized', async () => {
+ const index = new BigNumber(0);
+ return expectRevertOrAlwaysFailingTransactionAsync(
+ authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, {
+ from: owner,
+ }),
+ );
+ });
+ it('should throw if address at index does not match target', async () => {
+ const address1 = address;
+ const address2 = notOwner;
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await authorizable.addAuthorizedAddress.sendTransactionAsync(address1, { from: owner }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await authorizable.addAuthorizedAddress.sendTransactionAsync(address2, { from: owner }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ const address1Index = new BigNumber(0);
+ return expectRevertOrAlwaysFailingTransactionAsync(
+ authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address2, address1Index, {
+ from: owner,
+ }),
+ );
+ });
+ it('should allow owner to remove an authorized address', async () => {
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ const index = new BigNumber(0);
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, {
+ from: owner,
+ }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ const isAuthorized = await authorizable.authorized.callAsync(address);
+ expect(isAuthorized).to.be.false();
+ });
+ });
+
describe('getAuthorizedAddresses', () => {
it('should return all authorized addresses', async () => {
const initial = await authorizable.getAuthorizedAddresses.callAsync();