Add RichErrors for Authorizable

This commit is contained in:
Alex Towle 2019-08-01 13:59:19 -07:00
parent 89d8df3385
commit 6fc38292f2
11 changed files with 257 additions and 75 deletions

View File

@ -32,6 +32,7 @@
"src/Ownable.sol", "src/Ownable.sol",
"src/ReentrancyGuard.sol", "src/ReentrancyGuard.sol",
"src/SafeMath.sol", "src/SafeMath.sol",
"src/interfaces/IAuthorizable.sol",
"src/interfaces/IOwnable.sol", "src/interfaces/IOwnable.sol",
"test/TestConstants.sol", "test/TestConstants.sol",
"test/TestLibAddress.sol", "test/TestLibAddress.sol",

View File

@ -18,8 +18,10 @@
pragma solidity ^0.5.9; pragma solidity ^0.5.9;
import "./Ownable.sol";
import "./interfaces/IAuthorizable.sol"; import "./interfaces/IAuthorizable.sol";
import "./LibAuthorizableRichErrors.sol";
import "./LibRichErrors.sol";
import "./Ownable.sol";
contract Authorizable is contract Authorizable is
@ -28,10 +30,9 @@ contract Authorizable is
{ {
/// @dev Only authorized addresses can invoke functions with this modifier. /// @dev Only authorized addresses can invoke functions with this modifier.
modifier onlyAuthorized { modifier onlyAuthorized {
require( if (!authorized[msg.sender]) {
authorized[msg.sender], LibRichErrors._rrevert(LibAuthorizableRichErrors.SenderNotAuthorizedError(msg.sender));
"SENDER_NOT_AUTHORIZED" }
);
_; _;
} }
@ -44,14 +45,15 @@ contract Authorizable is
external external
onlyOwner onlyOwner
{ {
require( // Ensure that the target is not the zero address.
target != address(0), if (target == address(0)) {
"ZERO_CANT_BE_AUTHORIZED" LibRichErrors._rrevert(LibAuthorizableRichErrors.ZeroCantBeAuthorizedError());
); }
require(
!authorized[target], // Ensure that the target is not already authorized.
"TARGET_ALREADY_AUTHORIZED" if (authorized[target]) {
); LibRichErrors._rrevert(LibAuthorizableRichErrors.TargetAlreadyAuthorizedError(target));
}
authorized[target] = true; authorized[target] = true;
authorities.push(target); authorities.push(target);
@ -64,10 +66,9 @@ contract Authorizable is
external external
onlyOwner onlyOwner
{ {
require( if (!authorized[target]) {
authorized[target], LibRichErrors._rrevert(LibAuthorizableRichErrors.TargetNotAuthorizedError(target));
"TARGET_NOT_AUTHORIZED" }
);
delete authorized[target]; delete authorized[target];
for (uint256 i = 0; i < authorities.length; i++) { for (uint256 i = 0; i < authorities.length; i++) {
@ -90,18 +91,21 @@ contract Authorizable is
external external
onlyOwner onlyOwner
{ {
require( if (!authorized[target]) {
authorized[target], LibRichErrors._rrevert(LibAuthorizableRichErrors.TargetNotAuthorizedError(target));
"TARGET_NOT_AUTHORIZED" }
); if (index >= authorities.length) {
require( LibRichErrors._rrevert(LibAuthorizableRichErrors.IndexOutOfBoundsError(
index < authorities.length, index,
"INDEX_OUT_OF_BOUNDS" authorities.length
); ));
require( }
authorities[index] == target, if (authorities[index] != target) {
"AUTHORIZED_ADDRESS_MISMATCH" LibRichErrors._rrevert(LibAuthorizableRichErrors.AuthorizedAddressMismatchError(
); authorities[index],
target
));
}
delete authorized[target]; delete authorized[target];
authorities[index] = authorities[authorities.length - 1]; authorities[index] = authorities[authorities.length - 1];

View File

@ -0,0 +1,119 @@
/*
Copyright 2019 ZeroEx Intl.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
pragma solidity ^0.5.9;
library LibAuthorizableRichErrors {
// bytes4(keccak256("AuthorizedAddressMismatchError(address,address)"))
bytes4 internal constant AUTHORIZED_ADDRESS_MISMATCH_ERROR_SELECTOR =
0x140a84db;
// bytes4(keccak256("IndexOutOfBoundsError(uint256,uint256)"))
bytes4 internal constant INDEX_OUT_OF_BOUNDS_ERROR_SELECTOR =
0xe9f83771;
// bytes4(keccak256("SenderNotAuthorizedError(address)"))
bytes4 internal constant SENDER_NOT_AUTHORIZED_ERROR_SELECTOR =
0xb65a25b9;
// bytes4(keccak256("TargetAlreadyAuthorizedError(address)"))
bytes4 internal constant TARGET_ALREADY_AUTHORIZED_ERROR_SELECTOR =
0xde16f1a0;
// bytes4(keccak256("TargetNotAuthorizedError(address)"))
bytes4 internal constant TARGET_NOT_AUTHORIZED_ERROR_SELECTOR =
0xeb5108a2;
// bytes4(keccak256("ZeroCantBeAuthorizedError()"))
bytes internal constant ZERO_CANT_BE_AUTHORIZED_ERROR_BYTES =
hex"57654fe4";
// solhint-disable func-name-mixedcase
function AuthorizedAddressMismatchError(
address authorized,
address target
)
internal
pure
returns (bytes memory)
{
return abi.encodeWithSelector(
AUTHORIZED_ADDRESS_MISMATCH_ERROR_SELECTOR,
authorized,
target
);
}
function IndexOutOfBoundsError(
uint256 index,
uint256 length
)
internal
pure
returns (bytes memory)
{
return abi.encodeWithSelector(
INDEX_OUT_OF_BOUNDS_ERROR_SELECTOR,
index,
length
);
}
function SenderNotAuthorizedError(address sender)
internal
pure
returns (bytes memory)
{
return abi.encodeWithSelector(
SENDER_NOT_AUTHORIZED_ERROR_SELECTOR,
sender
);
}
function TargetAlreadyAuthorizedError(address target)
internal
pure
returns (bytes memory)
{
return abi.encodeWithSelector(
TARGET_ALREADY_AUTHORIZED_ERROR_SELECTOR,
target
);
}
function TargetNotAuthorizedError(address target)
internal
pure
returns (bytes memory)
{
return abi.encodeWithSelector(
TARGET_NOT_AUTHORIZED_ERROR_SELECTOR,
target
);
}
function ZeroCantBeAuthorizedError()
internal
pure
returns (bytes memory)
{
return ZERO_CANT_BE_AUTHORIZED_ERROR_BYTES;
}
}

View File

@ -34,7 +34,7 @@
"lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol" "lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol"
}, },
"config": { "config": {
"abis": "./generated-artifacts/@(Authorizable|IOwnable|LibAddress|LibBytes|LibEIP1271|LibEIP712|Ownable|ReentrancyGuard|SafeMath|TestConstants|TestLibAddress|TestLibAddressArray|TestLibBytes|TestLibEIP712|TestLibRichErrors|TestOwnable|TestReentrancyGuard|TestSafeMath).json", "abis": "./generated-artifacts/@(Authorizable|IAuthorizable|IOwnable|LibAddress|LibBytes|LibEIP1271|LibEIP712|Ownable|ReentrancyGuard|SafeMath|TestConstants|TestLibAddress|TestLibAddressArray|TestLibBytes|TestLibEIP712|TestLibRichErrors|TestOwnable|TestReentrancyGuard|TestSafeMath).json",
"abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually." "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
}, },
"repository": { "repository": {

View File

@ -6,6 +6,7 @@
import { ContractArtifact } from 'ethereum-types'; import { ContractArtifact } from 'ethereum-types';
import * as Authorizable from '../generated-artifacts/Authorizable.json'; import * as Authorizable from '../generated-artifacts/Authorizable.json';
import * as IAuthorizable from '../generated-artifacts/IAuthorizable.json';
import * as IOwnable from '../generated-artifacts/IOwnable.json'; import * as IOwnable from '../generated-artifacts/IOwnable.json';
import * as LibAddress from '../generated-artifacts/LibAddress.json'; import * as LibAddress from '../generated-artifacts/LibAddress.json';
import * as LibBytes from '../generated-artifacts/LibBytes.json'; import * as LibBytes from '../generated-artifacts/LibBytes.json';
@ -32,6 +33,7 @@ export const artifacts = {
Ownable: Ownable as ContractArtifact, Ownable: Ownable as ContractArtifact,
ReentrancyGuard: ReentrancyGuard as ContractArtifact, ReentrancyGuard: ReentrancyGuard as ContractArtifact,
SafeMath: SafeMath as ContractArtifact, SafeMath: SafeMath as ContractArtifact,
IAuthorizable: IAuthorizable as ContractArtifact,
IOwnable: IOwnable as ContractArtifact, IOwnable: IOwnable as ContractArtifact,
TestConstants: TestConstants as ContractArtifact, TestConstants: TestConstants as ContractArtifact,
TestLibAddress: TestLibAddress as ContractArtifact, TestLibAddress: TestLibAddress as ContractArtifact,

View File

@ -4,6 +4,7 @@
* ----------------------------------------------------------------------------- * -----------------------------------------------------------------------------
*/ */
export * from '../generated-wrappers/authorizable'; export * from '../generated-wrappers/authorizable';
export * from '../generated-wrappers/i_authorizable';
export * from '../generated-wrappers/i_ownable'; export * from '../generated-wrappers/i_ownable';
export * from '../generated-wrappers/lib_address'; export * from '../generated-wrappers/lib_address';
export * from '../generated-wrappers/lib_bytes'; export * from '../generated-wrappers/lib_bytes';

View File

@ -1,14 +1,12 @@
import { import {
chaiSetup, chaiSetup,
constants, constants,
expectTransactionFailedAsync,
provider, provider,
txDefaults, txDefaults,
web3Wrapper, web3Wrapper,
} from '@0x/contracts-test-utils'; } from '@0x/contracts-test-utils';
import { BlockchainLifecycle } from '@0x/dev-utils'; import { BlockchainLifecycle } from '@0x/dev-utils';
import { RevertReason } from '@0x/types'; import { AuthorizableRevertErrors, BigNumber, OwnableRevertErrors } from '@0x/utils';
import { BigNumber, OwnableRevertErrors } from '@0x/utils';
import * as chai from 'chai'; import * as chai from 'chai';
import * as _ from 'lodash'; import * as _ from 'lodash';
@ -51,7 +49,7 @@ describe('Authorizable', () => {
}); });
describe('addAuthorizedAddress', () => { describe('addAuthorizedAddress', () => {
it('should throw if not called by owner', async () => { it('should revert if not called by owner', async () => {
const expectedError = new OwnableRevertErrors.OnlyOwnerError(notOwner, owner); const expectedError = new OwnableRevertErrors.OnlyOwnerError(notOwner, owner);
const tx = authorizable.addAuthorizedAddress.sendTransactionAsync(notOwner, { from: notOwner }); const tx = authorizable.addAuthorizedAddress.sendTransactionAsync(notOwner, { from: notOwner });
return expect(tx).to.revertWith(expectedError); return expect(tx).to.revertWith(expectedError);
@ -67,28 +65,26 @@ describe('Authorizable', () => {
expect(isAuthorized).to.be.true(); expect(isAuthorized).to.be.true();
}); });
it('should throw if owner attempts to authorize the zero address', async () => { it('should revert if owner attempts to authorize the zero address', async () => {
return expectTransactionFailedAsync( const expectedError = new AuthorizableRevertErrors.ZeroCantBeAuthorizedError();
authorizable.addAuthorizedAddress.sendTransactionAsync(constants.NULL_ADDRESS, { from: owner }), const tx = authorizable.addAuthorizedAddress.sendTransactionAsync(constants.NULL_ADDRESS, { from: owner });
RevertReason.ZeroCantBeAuthorized, return expect(tx).to.revertWith(expectedError);
);
}); });
it('should throw if owner attempts to authorize a duplicate address', async () => { it('should revert if owner attempts to authorize a duplicate address', async () => {
await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync( await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync(
address, address,
{ from: owner }, { from: owner },
constants.AWAIT_TRANSACTION_MINED_MS, constants.AWAIT_TRANSACTION_MINED_MS,
); );
return expectTransactionFailedAsync( const expectedError = new AuthorizableRevertErrors.TargetAlreadyAuthorizedError(address);
authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), const tx = authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner });
RevertReason.TargetAlreadyAuthorized, return expect(tx).to.revertWith(expectedError);
);
}); });
}); });
describe('removeAuthorizedAddress', () => { describe('removeAuthorizedAddress', () => {
it('should throw if not called by owner', async () => { it('should revert if not called by owner', async () => {
await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync( await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync(
address, address,
{ from: owner }, { from: owner },
@ -114,18 +110,15 @@ describe('Authorizable', () => {
expect(isAuthorized).to.be.false(); expect(isAuthorized).to.be.false();
}); });
it('should throw if owner attempts to remove an address that is not authorized', async () => { it('should revert if owner attempts to remove an address that is not authorized', async () => {
return expectTransactionFailedAsync( const expectedError = new AuthorizableRevertErrors.TargetNotAuthorizedError(address);
authorizable.removeAuthorizedAddress.sendTransactionAsync(address, { const tx = authorizable.removeAuthorizedAddress.sendTransactionAsync(address, { from: owner });
from: owner, return expect(tx).to.revertWith(expectedError);
}),
RevertReason.TargetNotAuthorized,
);
}); });
}); });
describe('removeAuthorizedAddressAtIndex', () => { describe('removeAuthorizedAddressAtIndex', () => {
it('should throw if not called by owner', async () => { it('should revert if not called by owner', async () => {
await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync( await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync(
address, address,
{ from: owner }, { from: owner },
@ -139,32 +132,26 @@ describe('Authorizable', () => {
return expect(tx).to.revertWith(expectedError); return expect(tx).to.revertWith(expectedError);
}); });
it('should throw if index is >= authorities.length', async () => { it('should revert if index is >= authorities.length', async () => {
await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync( await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync(
address, address,
{ from: owner }, { from: owner },
constants.AWAIT_TRANSACTION_MINED_MS, constants.AWAIT_TRANSACTION_MINED_MS,
); );
const index = new BigNumber(1); const index = new BigNumber(1);
return expectTransactionFailedAsync( const expectedError = new AuthorizableRevertErrors.IndexOutOfBoundsError(index, constants.ZERO_AMOUNT);
authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { const tx = authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { from: owner });
from: owner, return expect(tx).to.revertWith(expectedError);
}),
RevertReason.IndexOutOfBounds,
);
}); });
it('should throw if owner attempts to remove an address that is not authorized', async () => { it('should revert if owner attempts to remove an address that is not authorized', async () => {
const index = new BigNumber(0); const index = new BigNumber(0);
return expectTransactionFailedAsync( const expectedError = new AuthorizableRevertErrors.TargetNotAuthorizedError(address);
authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { const tx = authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { from: owner });
from: owner, return expect(tx).to.revertWith(expectedError);
}),
RevertReason.TargetNotAuthorized,
);
}); });
it('should throw if address at index does not match target', async () => { it('should revert if address at index does not match target', async () => {
const address1 = address; const address1 = address;
const address2 = notOwner; const address2 = notOwner;
await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync( await authorizable.addAuthorizedAddress.awaitTransactionSuccessAsync(
@ -178,12 +165,9 @@ describe('Authorizable', () => {
constants.AWAIT_TRANSACTION_MINED_MS, constants.AWAIT_TRANSACTION_MINED_MS,
); );
const address1Index = new BigNumber(0); const address1Index = new BigNumber(0);
return expectTransactionFailedAsync( const expectedError = new AuthorizableRevertErrors.AuthorizedAddressMismatchError(address1, address2);
authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address2, address1Index, { const tx = authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address2, address1Index, { from: owner });
from: owner, return expect(tx).to.revertWith(expectedError);
}),
RevertReason.AuthorizedAddressMismatch,
);
}); });
it('should allow owner to remove an authorized address', async () => { it('should allow owner to remove an authorized address', async () => {

View File

@ -4,6 +4,7 @@
"include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"], "include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"],
"files": [ "files": [
"generated-artifacts/Authorizable.json", "generated-artifacts/Authorizable.json",
"generated-artifacts/IAuthorizable.json",
"generated-artifacts/IOwnable.json", "generated-artifacts/IOwnable.json",
"generated-artifacts/LibAddress.json", "generated-artifacts/LibAddress.json",
"generated-artifacts/LibBytes.json", "generated-artifacts/LibBytes.json",

View File

@ -268,7 +268,6 @@ export enum RevertReason {
SenderNotAuthorized = 'SENDER_NOT_AUTHORIZED', SenderNotAuthorized = 'SENDER_NOT_AUTHORIZED',
TargetNotAuthorized = 'TARGET_NOT_AUTHORIZED', TargetNotAuthorized = 'TARGET_NOT_AUTHORIZED',
TargetAlreadyAuthorized = 'TARGET_ALREADY_AUTHORIZED', TargetAlreadyAuthorized = 'TARGET_ALREADY_AUTHORIZED',
ZeroCantBeAuthorized = 'ZERO_CANT_BE_AUTHORIZED',
IndexOutOfBounds = 'INDEX_OUT_OF_BOUNDS', IndexOutOfBounds = 'INDEX_OUT_OF_BOUNDS',
AuthorizedAddressMismatch = 'AUTHORIZED_ADDRESS_MISMATCH', AuthorizedAddressMismatch = 'AUTHORIZED_ADDRESS_MISMATCH',
OnlyContractOwner = 'ONLY_CONTRACT_OWNER', OnlyContractOwner = 'ONLY_CONTRACT_OWNER',

View File

@ -0,0 +1,69 @@
import { BigNumber } from './configured_bignumber';
import { RevertError } from './revert_error';
// tslint:disable:max-classes-per-file
export class AuthorizedAddressMismatchError extends RevertError {
constructor(authorized?: string, target?: string) {
super(
'AuthorizedAddressMismatchError',
'AuthorizedAddressMismatchError(address authorized, address target)',
{ authorized, target },
);
}
}
export class IndexOutOfBoundsError extends RevertError {
constructor(index?: BigNumber, length?: BigNumber) {
super(
'IndexOutOfBoundsError',
'IndexOutOfBoundsError(uint256 index, uint256 length)',
{ index, length },
);
}
}
export class SenderNotAuthorizedError extends RevertError {
constructor(sender?: string) {
super('SenderNotAuthorizedError', 'SenderNotAuthorizedError()', { sender });
}
}
export class TargetAlreadyAuthorizedError extends RevertError {
constructor(target?: string) {
super(
'TargetAlreadyAuthorizedError',
'TargetAlreadyAuthorizedError(address target)',
{ target },
);
}
}
export class TargetNotAuthorizedError extends RevertError {
constructor(target?: string) {
super(
'TargetNotAuthorizedError',
'TargetNotAuthorizedError(address target)',
{ target },
);
}
}
export class ZeroCantBeAuthorizedError extends RevertError {
constructor() {
super('ZeroCantBeAuthorizedError', 'ZeroCantBeAuthorizedError()', {});
}
}
const types = [
AuthorizedAddressMismatchError,
IndexOutOfBoundsError,
SenderNotAuthorizedError,
TargetAlreadyAuthorizedError,
TargetNotAuthorizedError,
ZeroCantBeAuthorizedError,
];
// Register the types we've defined.
for (const type of types) {
RevertError.registerType(type);
}

View File

@ -1,3 +1,4 @@
import * as AuthorizableRevertErrors from './authorizable_revert_errors';
import * as LibAddressArrayRevertErrors from './lib_address_array_revert_errors'; import * as LibAddressArrayRevertErrors from './lib_address_array_revert_errors';
import * as LibBytesRevertErrors from './lib_bytes_revert_errors'; import * as LibBytesRevertErrors from './lib_bytes_revert_errors';
import * as OwnableRevertErrors from './ownable_revert_errors'; import * as OwnableRevertErrors from './ownable_revert_errors';
@ -32,6 +33,7 @@ export {
} from './revert_error'; } from './revert_error';
export { export {
AuthorizableRevertErrors,
LibAddressArrayRevertErrors, LibAddressArrayRevertErrors,
LibBytesRevertErrors, LibBytesRevertErrors,
OwnableRevertErrors, OwnableRevertErrors,