Skip to content
This repository has been archived by the owner on May 9, 2024. It is now read-only.

Reduce Deploy Cost of All Contracts #167

Merged
merged 11 commits into from
Jun 26, 2020
32 changes: 15 additions & 17 deletions contracts/Bridge.sol
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
pragma solidity 0.6.4;
pragma experimental ABIEncoderV2;

import "@openzeppelin/contracts/math/SafeMath.sol";
import "@openzeppelin/contracts/utils/Pausable.sol";
import "@openzeppelin/contracts/access/AccessControl.sol";
import "./utils/Pausable.sol";
import "./utils/SafeMath.sol";
import "./interfaces/IDepositExecute.sol";
import "./interfaces/IBridge.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/access/AccessControl.sol";
import "./interfaces/IERCHandler.sol";
import "./interfaces/IGenericHandler.sol";

/**
@title Facilitates deposits, creation and votiing of deposit proposals, and deposit executions.
@author ChainSafe Systems.
*/
contract Bridge is Pausable, AccessControl {
using SafeMath for uint;
contract Bridge is Pausable, AccessControl, SafeMath {

uint8 public _chainID;
uint256 public _relayerThreshold;
Expand Down Expand Up @@ -92,15 +90,15 @@ contract Bridge is Pausable, AccessControl {

function _onlyAdminOrRelayer() private {
require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender) || hasRole(RELAYER_ROLE, msg.sender),
"sender does not have admin role");
"sender is not relayer or admin");
}

function _onlyAdmin() private {
require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender does not have admin role");
require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender doesn't have admin role");
}

function _onlyRelayers() private {
require(hasRole(RELAYER_ROLE, msg.sender), "sender does not have relayer role");
require(hasRole(RELAYER_ROLE, msg.sender), "sender doesn't have relayer role");
}

/**
Expand Down Expand Up @@ -264,7 +262,7 @@ contract Bridge is Pausable, AccessControl {
@param newFee Value {_fee} will be updated to.
*/
function adminChangeFee(uint newFee) external onlyAdmin {
require(_fee != newFee, "Current fee is equal to proposed new fee");
require(_fee != newFee, "Current fee is equal to new fee");
_fee = newFee;
}

Expand Down Expand Up @@ -297,7 +295,7 @@ contract Bridge is Pausable, AccessControl {
require(msg.value == _fee, "Incorrect fee supplied");

address handler = _resourceIDToHandlerAddress[resourceID];
require(handler != address(0), "resourceID not mapped to handler address");
require(handler != address(0), "resourceID not mapped to handler");

uint64 depositNonce = ++_depositCounts[destinationChainID];
_depositRecords[depositNonce][destinationChainID] = data;
Expand Down Expand Up @@ -325,8 +323,8 @@ contract Bridge is Pausable, AccessControl {
Proposal storage proposal = _proposals[nonceAndID][dataHash];

require(_resourceIDToHandlerAddress[resourceID] != address(0), "no handler for resourceID");
require(uint(proposal._status) <= 1, "proposal has already been passed, transferred, or cancelled");
require(!_hasVotedOnProposal[nonceAndID][dataHash][msg.sender], "relayer has already voted on proposal");
require(uint(proposal._status) <= 1, "proposal already passed/transferred/cancelled");
require(!_hasVotedOnProposal[nonceAndID][dataHash][msg.sender], "relayer already voted");

if (uint(proposal._status) == 0) {
++_totalProposals;
Expand All @@ -342,7 +340,7 @@ contract Bridge is Pausable, AccessControl {
proposal._yesVotes[0] = msg.sender;
emit ProposalEvent(chainID, depositNonce, ProposalStatus.Active, resourceID, dataHash);
} else {
if ((block.number).sub(proposal._proposedBlock) > _expiry) {
if (sub(block.number, proposal._proposedBlock) > _expiry) {
// if the number of blocks that has passed since this proposal was
// submitted exceeds the expiry threshold set, cancel the proposal
proposal._status = ProposalStatus.Cancelled;
Expand Down Expand Up @@ -384,7 +382,7 @@ contract Bridge is Pausable, AccessControl {
Proposal storage proposal = _proposals[nonceAndID][dataHash];

require(proposal._status != ProposalStatus.Cancelled, "Proposal already cancelled");
require((block.number).sub(proposal._proposedBlock) > _expiry, "Proposal does not meet expiry threshold");
require(sub(block.number, proposal._proposedBlock) > _expiry, "Proposal not at expiry threshold");

proposal._status = ProposalStatus.Cancelled;
emit ProposalEvent(chainID, depositNonce, ProposalStatus.Cancelled, proposal._resourceID, proposal._dataHash);
Expand All @@ -409,8 +407,8 @@ contract Bridge is Pausable, AccessControl {
Proposal storage proposal = _proposals[nonceAndID][dataHash];

require(proposal._status != ProposalStatus.Inactive, "proposal is not active");
require(proposal._status == ProposalStatus.Passed, "proposal was not passed or has already been transferred");
require(dataHash == proposal._dataHash, "provided data does not match proposal's data hash");
require(proposal._status == ProposalStatus.Passed, "proposal already transferred");
require(dataHash == proposal._dataHash, "data doesn't match datahash");

proposal._status = ProposalStatus.Transferred;

Expand Down
2 changes: 1 addition & 1 deletion contracts/handlers/ERC20Handler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract ERC20Handler is IDepositExecute, HandlerHelpers, ERC20Safe {
address[] memory burnableContractAddresses
) public {
require(initialResourceIDs.length == initialContractAddresses.length,
"mismatch length between initialResourceIDs and initialContractAddresses");
"initialResourceIDs and initialContractAddresses len mismatch");

_bridgeAddress = bridgeAddress;

Expand Down
2 changes: 1 addition & 1 deletion contracts/handlers/ERC721Handler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ contract ERC721Handler is IDepositExecute, HandlerHelpers, ERC721Safe {
address[] memory burnableContractAddresses
) public {
require(initialResourceIDs.length == initialContractAddresses.length,
"mismatch length between initialResourceIDs and initialContractAddresses");
"initialResourceIDs and initialContractAddresses len mismatch");

_bridgeAddress = bridgeAddress;

Expand Down
12 changes: 8 additions & 4 deletions contracts/handlers/GenericHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ contract GenericHandler is IGenericHandler {
mapping (address => bool) public _contractWhitelist;

modifier onlyBridge() {
require(msg.sender == _bridgeAddress, "sender must be bridge contract");
_onlyBridge();
_;
}

function _onlyBridge() private {
require(msg.sender == _bridgeAddress, "sender must be bridge contract");
}

/**
@param bridgeAddress Contract address of previously deployed Bridge.
@param initialResourceIDs Resource IDs used to identify a specific contract address.
Expand All @@ -66,13 +70,13 @@ contract GenericHandler is IGenericHandler {
bytes4[] memory initialExecuteFunctionSignatures
) public {
require(initialResourceIDs.length == initialContractAddresses.length,
"mismatch length between initialResourceIDs and initialContractAddresses");
"initialResourceIDs and initialContractAddresses len mismatch");

require(initialContractAddresses.length == initialDepositFunctionSignatures.length,
"mismatch length between provided contract addresses and function signatures");
"provided contract addresses and function signatures len mismatch");

require(initialDepositFunctionSignatures.length == initialExecuteFunctionSignatures.length,
"mismatch length between provided deposit and execute function signatures");
"provided deposit and execute function signatures len mismatch");

_bridgeAddress = bridgeAddress;

Expand Down
96 changes: 96 additions & 0 deletions contracts/utils/Pausable.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.6.0;


/**
* @dev Contract module which allows children to implement an emergency stop
* mechanism that can be triggered by an authorized account.
*
* This is a stripped down version of Open zeppelin's Pausable contract.
* https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/EnumerableSet.sol
*
*/
contract Pausable {
/**
* @dev Emitted when the pause is triggered by `account`.
*/
event Paused(address account);

/**
* @dev Emitted when the pause is lifted by `account`.
*/
event Unpaused(address account);

bool private _paused;

/**
* @dev Initializes the contract in unpaused state.
*/
constructor () internal {
_paused = false;
}

/**
* @dev Returns true if the contract is paused, and false otherwise.
*/
function paused() public view returns (bool) {
return _paused;
}

/**
* @dev Modifier to make a function callable only when the contract is not paused.
*
* Requirements:
*
* - The contract must not be paused.
*/
modifier whenNotPaused() {
_whenNotPaused();
_;
}

function _whenNotPaused() private view {
require(!_paused, "Pausable: paused");
}

/**
* @dev Modifier to make a function callable only when the contract is not paused.
*
* Requirements:
*
* - The contract must not be paused.
*/
modifier whenPaused() {
_whenPaused();
_;
}

function _whenPaused() private view {
require(_paused, "Pausable: not paused");
}

/**
* @dev Triggers stopped state.
*
* Requirements:
*
* - The contract must not be paused.
*/
function _pause() internal virtual whenNotPaused {
_paused = true;
emit Paused(msg.sender);
}

/**
* @dev Returns to normal state.
*
* Requirements:
*
* - The contract must be paused.
*/
function _unpause() internal virtual whenPaused {
_paused = false;
emit Unpaused(msg.sender);
}
}
44 changes: 44 additions & 0 deletions contracts/utils/SafeMath.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.6.0;

/**
* @dev Wrappers over Solidity's arithmetic operations with added overflow
* checks.
*
* note that this is a stripped down version of open zeppelin's safemath
* https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/math/SafeMath.sol
*/

contract SafeMath {

/**
* @dev Returns the subtraction of two unsigned integers, reverting on
* overflow (when the result is negative).
*
* Counterpart to Solidity's `-` operator.
*
* Requirements:
* - Subtraction cannot overflow.
*/
function sub(uint256 a, uint256 b) internal pure returns (uint256) {
return _sub(a, b, "SafeMath: subtraction overflow");
}

/**
* @dev Returns the subtraction of two unsigned integers, reverting with custom message on
* overflow (when the result is negative).
*
* Counterpart to Solidity's `-` operator.
*
* Requirements:
* - Subtraction cannot overflow.
*/
function _sub(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
require(b <= a, errorMessage);
uint256 c = a - b;

return c;
}

}
6 changes: 3 additions & 3 deletions test/contractBridge/cancelDepositProposal.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)

const depositProposal = await BridgeInstance.getProposal(originChainID, expectedDepositNonce, depositDataHash);
assert.deepInclude(Object.assign({}, depositProposal), expectedDepositProposal);
await TruffleAssert.reverts(vote(relayer3Address), "proposal has already been passed, transferred, or cancelled.")
await TruffleAssert.reverts(vote(relayer3Address), "proposal already passed/transferred/cancelled.")

});

Expand All @@ -138,7 +138,7 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)
await TruffleAssert.passes(BridgeInstance.cancelProposal(originChainID, expectedDepositNonce, depositDataHash))
const depositProposal = await BridgeInstance.getProposal(originChainID, expectedDepositNonce, depositDataHash);
assert.deepInclude(Object.assign({}, depositProposal), expectedDepositProposal);
await TruffleAssert.reverts(vote(relayer4Address), "proposal has already been passed, transferred, or cancelled.")
await TruffleAssert.reverts(vote(relayer4Address), "proposal already passed/transferred/cancelled.")

});

Expand All @@ -159,7 +159,7 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)
await TruffleAssert.passes(BridgeInstance.cancelProposal(originChainID, expectedDepositNonce, depositDataHash))
const depositProposal = await BridgeInstance.getProposal(originChainID, expectedDepositNonce, depositDataHash);
assert.deepInclude(Object.assign({}, depositProposal), expectedDepositProposal);
await TruffleAssert.reverts(vote(relayer2Address), "proposal has already been passed, transferred, or cancelled.")
await TruffleAssert.reverts(vote(relayer2Address), "proposal already passed/transferred/cancelled.")

});

Expand Down
6 changes: 3 additions & 3 deletions test/contractBridge/voteDepositProposal.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)

await TruffleAssert.passes(vote(relayer3Address));

await TruffleAssert.reverts(vote(relayer4Address), 'proposal has already been passed, transferred, or cancelled.');
await TruffleAssert.reverts(vote(relayer4Address), 'proposal already passed/transferred/cancelled.');
});

it("depositProposal shouldn't be voted on if it has a Transferred status", async () => {
Expand All @@ -118,14 +118,14 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)

await TruffleAssert.passes(executeProposal(relayer1Address));

await TruffleAssert.reverts(vote(relayer4Address), 'proposal has already been passed, transferred, or cancelled.');
await TruffleAssert.reverts(vote(relayer4Address), 'proposal already passed/transferred/cancelled.');

});

it("relayer shouldn't be able to vote on a depositProposal more than once", async () => {
await TruffleAssert.passes(vote(relayer1Address));

await TruffleAssert.reverts(vote(relayer1Address), 'relayer has already voted on proposal');
await TruffleAssert.reverts(vote(relayer1Address), 'relayer already voted');
});

it("Should be able to create a proposal with a different hash", async () => {
Expand Down
12 changes: 6 additions & 6 deletions test/handlers/generic/constructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,37 +59,37 @@ contract('GenericHandler - [constructor]', async () => {
initialExecuteFunctionSignatures));
});

it('should revert because mismatch length between initialResourceIDs and initialContractAddresses', async () => {
it('should revert because initialResourceIDs and initialContractAddresses len mismatch', async () => {
await TruffleAssert.reverts(
GenericHandlerContract.new(
BridgeInstance.address,
[],
initialContractAddresses,
initialDepositFunctionSignatures,
initialExecuteFunctionSignatures),
"mismatch length between initialResourceIDs and initialContractAddresses");
"initialResourceIDs and initialContractAddresses len mismatch");
});

it('should revert because mismatch length between provided contract addresses and function signatures', async () => {
it('should revert because provided contract addresses and function signatures len mismatch.', async () => {
await TruffleAssert.reverts(
GenericHandlerContract.new(
BridgeInstance.address,
initialResourceIDs,
initialContractAddresses,
[],
initialExecuteFunctionSignatures),
"mismatch length between provided contract addresses and function signatures");
"provided contract addresses and function signatures len mismatch.");
});

it('should revert because mismatch length between provided deposit and execute function signatures', async () => {
it('should revert because provided deposit and execute function signatures len mismatch', async () => {
await TruffleAssert.reverts(
GenericHandlerContract.new(
BridgeInstance.address,
initialResourceIDs,
initialContractAddresses,
initialDepositFunctionSignatures,
[]),
"mismatch length between provided deposit and execute function signatures");
"provided deposit and execute function signatures len mismatch");
});

it('contract mappings were set with expected values', async () => {
Expand Down