Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: rearrange core contracts addresses #32

Merged
merged 10 commits into from
Mar 27, 2024
12 changes: 11 additions & 1 deletion src/contracts/Bridge.sol
Original file line number Diff line number Diff line change
@@ -20,8 +20,10 @@ contract Bridge is Pausable, Context {
uint8 public immutable _domainID;

IFeeHandler public _feeHandler;

IAccessControlSegregator public _accessControl;
address public _executorAddress;
address public _routerAddress;


// resourceID => handler address
mapping(bytes32 => address) public _resourceIDToHandlerAddress;
@@ -140,4 +142,12 @@ contract Bridge is Pausable, Context {
IERCHandler handler = IERCHandler(handlerAddress);
handler.withdraw(data);
}

function adminChangeRouterAddress(address routerAddress) external onlyAllowed {
_routerAddress = routerAddress;
}

function adminChangeExecutorAddress(address executorAddress) external onlyAllowed {
_executorAddress = executorAddress;
}
}
9 changes: 3 additions & 6 deletions src/contracts/TestContracts.sol
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ pragma solidity 0.8.11;
import "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol";
import "./handlers/ERCHandlerHelpers.sol";
import "./interfaces/IERC20Plus.sol";
import "./interfaces/IBridge.sol";

contract NoArgument {
event NoArgumentCalled();
@@ -58,13 +59,9 @@ abstract contract HandlerRevert is ERCHandlerHelpers {
uint256 private _totalAmount;

constructor(
address bridgeAddress,
address routerAddress,
address executorAddress
IBridge bridgeAddress
) ERCHandlerHelpers(
bridgeAddress,
routerAddress,
executorAddress
bridgeAddress
) {}

function executeProposal(bytes32, bytes calldata) external view {
48 changes: 40 additions & 8 deletions src/contracts/handlers/ERC20Handler.sol
Original file line number Diff line number Diff line change
@@ -13,18 +13,41 @@ import "../ERC20Safe.sol";
*/
contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe {
/**
@param bridgeAddress Contract address of previously deployed Bridge.
@param bridge Contract address of previously deployed Bridge.
*/
constructor(
address bridgeAddress,
address routerAddress,
address executorAddress
IBridge bridge
) ERCHandlerHelpers(
bridgeAddress,
routerAddress,
executorAddress
bridge
) {}

modifier onlyBridge() {
_onlyBridge();
_;
}

modifier onlyRouter() {
_onlyRouter();
_;
}

modifier onlyExecutor() {
_onlyExecutor();
_;
}

function _onlyBridge() private {
if (msg.sender != address(_bridge)) revert SenderNotBridgeContract();
}

function _onlyExecutor() private {
if (msg.sender != _bridge._executorAddress()) revert SenderNotExecutorContract();
}

function _onlyRouter() private {
if (msg.sender != _bridge._routerAddress()) revert SenderNotRouterContract();
}

/**
@notice A deposit is initiated by making a deposit in the Bridge contract.
@param resourceID ResourceID used to find address of token to be used for deposit.
@@ -114,7 +137,7 @@ contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe {
recipient address bytes 32 - 64
amount uint256 bytes 64 - 96
*/
function withdraw(bytes memory data) external override onlyBridge {
function withdraw(bytes memory data) external onlyBridge {
address tokenAddress;
address recipient;
uint256 amount;
@@ -142,4 +165,13 @@ contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe {
_setDecimals(contractAddress, externalTokenDecimals);
}
}

/**
@notice First verifies {contractAddress} is whitelisted, then sets
{_tokenContractAddressToTokenProperties[contractAddress].isBurnable} to true.
@param contractAddress Address of contract to be used when making or executing deposits.
*/
function setBurnable(address contractAddress) external onlyBridge {
_setBurnable(contractAddress);
}
}
55 changes: 7 additions & 48 deletions src/contracts/handlers/ERCHandlerHelpers.sol
Original file line number Diff line number Diff line change
@@ -3,16 +3,16 @@
pragma solidity 0.8.11;

import "../interfaces/IERCHandler.sol";
import "../interfaces/IBridge.sol";


/**
@title Function used across handler contracts.
@author ChainSafe Systems.
@notice This contract is intended to be used with the Bridge contract.
*/
abstract contract ERCHandlerHelpers is IERCHandler {
address public immutable _bridgeAddress;
address public immutable _routerAddress;
address public immutable _executorAddress;
contract ERCHandlerHelpers {
IBridge public _bridge;

uint8 public constant DEFAULT_DECIMALS = 18;

@@ -40,52 +40,11 @@ abstract contract ERCHandlerHelpers is IERCHandler {
// token contract address => ERCTokenContractProperties
mapping(address => ERCTokenContractProperties) public _tokenContractAddressToTokenProperties;

modifier onlyBridge() {
_onlyBridge();
_;
}

modifier onlyRouter() {
_onlyRouter();
_;
}

modifier onlyExecutor() {
_onlyExecutor();
_;
}

function _onlyBridge() private view {
if (msg.sender != _bridgeAddress) revert SenderNotBridgeContract();
}

function _onlyExecutor() private view {
if (msg.sender != _executorAddress) revert SenderNotExecutorContract();
}

function _onlyRouter() private view {
if (msg.sender != _routerAddress) revert SenderNotRouterContract();
}


/**
@param bridgeAddress Contract address of previously deployed Bridge.
@param routerAddress Contract address of previously deployed Router.
@param executorAddress Contract address of previously deployed Executor.
*/
constructor(address bridgeAddress, address routerAddress, address executorAddress) {
_bridgeAddress = bridgeAddress;
_routerAddress = routerAddress;
_executorAddress = executorAddress;
}

/**
@notice First verifies {contractAddress} is whitelisted, then sets
{_tokenContractAddressToTokenProperties[contractAddress].isBurnable} to true.
@param contractAddress Address of contract to be used when making or executing deposits.
@param bridge Contract address of previously deployed Bridge.
*/
function setBurnable(address contractAddress) external override onlyBridge {
_setBurnable(contractAddress);
constructor(IBridge bridge) {
_bridge = bridge;
}

function _setResource(bytes32 resourceID, address contractAddress) internal {
20 changes: 16 additions & 4 deletions src/contracts/interfaces/IBridge.sol
Original file line number Diff line number Diff line change
@@ -14,19 +14,31 @@ interface IBridge {
@notice Exposing getter for {_domainID} instead of forcing the use of call.
@return uint8 The {_domainID} that is currently set for the Bridge contract.
*/
function _domainID() external returns (uint8);
function _domainID() external view returns (uint8);

/**
@notice Exposing getter for {_feeHandler} instead of forcing the use of call.
@return IFeeHandler The {_feeHandler} that is currently set for the Bridge contract.
*/
function _feeHandler() external returns (IFeeHandler);
function _feeHandler() external view returns (IFeeHandler);

/**
@notice Exposing getter for {_routerAddress} instead of forcing the use of call.
@return address The {_routerAddress} that is currently set for the Bridge contract.
*/
function _routerAddress() external returns (address);
nmlinaric marked this conversation as resolved.
Show resolved Hide resolved

/**
@notice Exposing getter for {_executorAddress} instead of forcing the use of call.
@return address The {_executorAddress} that is currently set for the Bridge contract.
*/
function _executorAddress() external returns (address);

/**
@notice Exposing getter for {_accessControl} instead of forcing the use of call.
@return IAccessControlSegregator The {_accessControl} that is currently set for the Bridge contract.
*/
function _accessControl() external returns (IAccessControlSegregator);
function _accessControl() external view returns (IAccessControlSegregator);

/**IFeeHandler
@notice Exposing getter for {_resourceIDToHandlerAddress}.
@@ -39,5 +51,5 @@ interface IBridge {
@notice Exposing getter for {paused} instead of forcing the use of call.
@return bool The {paused} status that is currently set for the Bridge contract.
*/
function paused() external returns (bool);
function paused() external view returns (bool);
}
32 changes: 24 additions & 8 deletions test/contractBridge/admin.test.ts
Original file line number Diff line number Diff line change
@@ -55,8 +55,6 @@ describe("Bridge - [admin]", () => {
ERC20HandlerContract = await ethers.getContractFactory("ERC20Handler");
ERC20HandlerInstance = await ERC20HandlerContract.deploy(
await bridgeInstance.getAddress(),
await routerInstance.getAddress(),
await executorInstance.getAddress(),
);
});

@@ -117,8 +115,6 @@ describe("Bridge - [admin]", () => {
);
const ERC20HandlerInstance = await ERC20HandlerContract.deploy(
await bridgeInstance.getAddress(),
await routerInstance.getAddress(),
await executorInstance.getAddress(),
);

await expect(
@@ -190,8 +186,6 @@ describe("Bridge - [admin]", () => {
);
const ERC20HandlerInstance = await ERC20HandlerContract.deploy(
await bridgeInstance.getAddress(),
await routerInstance.getAddress(),
await executorInstance.getAddress(),
);

await expect(
@@ -245,8 +239,6 @@ describe("Bridge - [admin]", () => {
);
const ERC20HandlerInstance = await ERC20HandlerContract.deploy(
await bridgeInstance.getAddress(),
await routerInstance.getAddress(),
await executorInstance.getAddress(),
);

await expect(
@@ -389,4 +381,28 @@ describe("Bridge - [admin]", () => {
executorInstance.connect(tokenOwnerAccount).adminChangeSlotIndex(3, 1),
).not.to.be.reverted;
});

it("Should require admin role to change router address", async () => {
await expect(
bridgeInstance
.connect(nonAdminAccount)
.adminChangeRouterAddress("0x388C818CA8B9251b393131C08a736A67ccB19297"),
).to.be.revertedWithCustomError(
routerInstance,
"AccessNotAllowed(address,bytes4)",
);
});

it("Should require admin role to change executor address", async () => {
await expect(
bridgeInstance
.connect(nonAdminAccount)
.adminChangeExecutorAddress(
"0x388C818CA8B9251b393131C08a736A67ccB19297",
),
).to.be.revertedWithCustomError(
routerInstance,
"AccessNotAllowed(address,bytes4)",
);
});
});
10 changes: 4 additions & 6 deletions test/contractBridge/depositERC20.test.ts
Original file line number Diff line number Diff line change
@@ -11,7 +11,6 @@ import type {
ERC20Handler,
ERC20PresetMinterPauser,
ERC20Safe__factory,
Executor,
} from "../../typechain-types";
import {
createERCDepositData,
@@ -33,7 +32,6 @@ describe("Bridge - [deposit - ERC20]", () => {

let bridgeInstance: Bridge;
let routerInstance: Router;
let executorInstance: Executor;
let ERC20MintableInstance1: ERC20PresetMinterPauser;
let ERC20MintableInstance2: ERC20PresetMinterPauser;
let ERC20HandlerInstance: ERC20Handler;
@@ -48,8 +46,10 @@ describe("Bridge - [deposit - ERC20]", () => {
beforeEach(async () => {
[, depositorAccount, recipientAccount] = await ethers.getSigners();

[bridgeInstance, routerInstance, executorInstance] =
await deployBridgeContracts(originDomainID, routerAddress);
[bridgeInstance, routerInstance] = await deployBridgeContracts(
originDomainID,
routerAddress,
);
const ERC20MintableContract = await ethers.getContractFactory(
"ERC20PresetMinterPauser",
);
@@ -65,8 +65,6 @@ describe("Bridge - [deposit - ERC20]", () => {
await ethers.getContractFactory("ERC20Handler");
ERC20HandlerInstance = await ERC20HandlerContract.deploy(
await bridgeInstance.getAddress(),
await routerInstance.getAddress(),
await executorInstance.getAddress(),
);

resourceID1 = createResourceID(
2 changes: 0 additions & 2 deletions test/contractBridge/executeProposalERC20.test.ts
Original file line number Diff line number Diff line change
@@ -75,8 +75,6 @@ describe("Bridge - [execute proposal - ERC20]", () => {
await ethers.getContractFactory("ERC20Handler");
ERC20HandlerInstance = await ERC20HandlerContract.deploy(
await bridgeInstance.getAddress(),
await routerInstance.getAddress(),
await executorInstance.getAddress(),
);

resourceID =
2 changes: 0 additions & 2 deletions test/contractBridge/executeProposals.test.ts
Original file line number Diff line number Diff line change
@@ -70,8 +70,6 @@ describe("Bridge - [execute proposals]", () => {
await ethers.getContractFactory("ERC20Handler");
ERC20HandlerInstance = await ERC20HandlerContract.deploy(
await bridgeInstance.getAddress(),
await routerInstance.getAddress(),
await executorInstance.getAddress(),
);

erc20ResourceID =
4 changes: 0 additions & 4 deletions test/e2e/erc20/decimals/bothChainsNot18Decimals.test.ts
Original file line number Diff line number Diff line change
@@ -117,13 +117,9 @@ describe("E2E ERC20 - Two EVM Chains both with decimal places != 18", () => {
await ethers.getContractFactory("ERC20Handler");
originERC20HandlerInstance = await ERC20HandlerContract.deploy(
await originBridgeInstance.getAddress(),
await originRouterInstance.getAddress(),
await originExecutorInstance.getAddress(),
);
destinationERC20HandlerInstance = await ERC20HandlerContract.deploy(
await destinationBridgeInstance.getAddress(),
await destinationRouterInstance.getAddress(),
await destinationExecutorInstance.getAddress(),
);
originResourceID =
"0x0000000000000000000000000000000000000000000000000000000000000000";
4 changes: 0 additions & 4 deletions test/e2e/erc20/decimals/oneChainNot18Decimals.test.ts
Original file line number Diff line number Diff line change
@@ -128,13 +128,9 @@ describe("E2E ERC20 - Two EVM Chains, one with decimal places == 18, other with
await ethers.getContractFactory("ERC20Handler");
originERC20HandlerInstance = await ERC20HandlerContract.deploy(
await originBridgeInstance.getAddress(),
await originRouterInstance.getAddress(),
await originExecutorInstance.getAddress(),
);
destinationERC20HandlerInstance = await ERC20HandlerContract.deploy(
await destinationBridgeInstance.getAddress(),
await destinationRouterInstance.getAddress(),
await destinationExecutorInstance.getAddress(),
);

originResourceID =
Loading

Unchanged files with check annotations Beta

uint8 destinationDomainID,
bytes32 resourceID,
uint8 securityModel,
bytes calldata depositData,

Check warning on line 98 in src/contracts/handlers/fee/BasicFeeHandler.sol

GitHub Actions / Lint Solidity

Variable "depositData" is unused
bytes calldata feeData

Check warning on line 99 in src/contracts/handlers/fee/BasicFeeHandler.sol

GitHub Actions / Lint Solidity

Variable "feeData" is unused
) external virtual payable onlyRouterOrFeeRouter {
uint256 currentFee = _domainResourceIDSecurityModelToFee[destinationDomainID][resourceID][securityModel];
if (msg.value != currentFee) revert IncorrectFeeSupplied(msg.value);
@return Returns the fee amount.
*/
function calculateFee(
address sender,

Check warning on line 118 in src/contracts/handlers/fee/BasicFeeHandler.sol

GitHub Actions / Lint Solidity

Variable "sender" is unused
uint8 fromDomainID,

Check warning on line 119 in src/contracts/handlers/fee/BasicFeeHandler.sol

GitHub Actions / Lint Solidity

Variable "fromDomainID" is unused
uint8 destinationDomainID,
bytes32 resourceID,
uint8 securityModel,
bytes calldata depositData,

Check warning on line 123 in src/contracts/handlers/fee/BasicFeeHandler.sol

GitHub Actions / Lint Solidity

Variable "depositData" is unused
bytes calldata feeData

Check warning on line 124 in src/contracts/handlers/fee/BasicFeeHandler.sol

GitHub Actions / Lint Solidity

Variable "feeData" is unused
) virtual external view returns(uint256, address) {
return (_domainResourceIDSecurityModelToFee[destinationDomainID][resourceID][securityModel], address(0));
}
}
function _calculateFee(
address sender,

Check warning on line 81 in src/contracts/handlers/fee/PercentageERC20FeeHandlerEVM.sol

GitHub Actions / Lint Solidity

Variable "sender" is unused
uint8 fromDomainID,

Check warning on line 82 in src/contracts/handlers/fee/PercentageERC20FeeHandlerEVM.sol

GitHub Actions / Lint Solidity

Variable "fromDomainID" is unused
uint8 destinationDomainID,
bytes32 resourceID,
uint8 securityModel,
bytes calldata depositData,
bytes calldata feeData

Check warning on line 87 in src/contracts/handlers/fee/PercentageERC20FeeHandlerEVM.sol

GitHub Actions / Lint Solidity

Variable "feeData" is unused
) internal view returns (uint256 fee, address tokenAddress) {
address tokenHandler = IBridge(_bridgeAddress)._resourceIDToHandlerAddress(resourceID);
tokenAddress = IERCHandler(tokenHandler)._resourceIDToTokenContractAddress(resourceID);
After this, the target contract will get the following:
executeFuncSignature(address executionDataDepositor, uint256[] uintArray, address addr)
*/
function deposit(bytes32 resourceID, address depositor, bytes calldata data) external pure returns (bytes memory) {

Check warning on line 121 in src/contracts/handlers/PermissionlessGenericHandler.sol

GitHub Actions / Lint Solidity

Variable "resourceID" is unused
if (data.length < 76) revert IncorrectDataLength(data.length); // 32 + 2 + 1 + 1 + 20 + 20
uint256 maxFee;