Skip to content

Commit

Permalink
Add unit tests for GAC (#56)
Browse files Browse the repository at this point in the history
* Add unit tests for GAC

Part of #45.

* Cleanup caller / global owner confusion

Add a sensible minimum value for dst gas limit
  • Loading branch information
Dominator008 authored Sep 12, 2023
1 parent dc99b0f commit 7e25a04
Show file tree
Hide file tree
Showing 10 changed files with 241 additions and 21 deletions.
6 changes: 3 additions & 3 deletions src/adapters/BaseSenderAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ abstract contract BaseSenderAdapter is IBridgeSenderAdapter {
_;
}

modifier onlyCaller() {
if (!gac.isPrivilegedCaller(msg.sender)) {
modifier onlyGlobalOwner() {
if (!gac.isGlobalOwner(msg.sender)) {
revert Error.INVALID_PRIVILEGED_CALLER();
}
_;
Expand All @@ -53,7 +53,7 @@ abstract contract BaseSenderAdapter is IBridgeSenderAdapter {
function updateReceiverAdapter(uint256[] calldata _dstChainIds, address[] calldata _receiverAdapters)
external
override
onlyCaller
onlyGlobalOwner
{
uint256 arrLength = _dstChainIds.length;

Expand Down
8 changes: 4 additions & 4 deletions src/adapters/Celer/CelerReceiverAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ contract CelerReceiverAdapter is IBridgeReceiverAdapter, IMessageReceiverApp {
/*/////////////////////////////////////////////////////////////////
MODIFIER
////////////////////////////////////////////////////////////////*/
modifier onlyCaller() {
if (!gac.isPrivilegedCaller(msg.sender)) {
revert Error.INVALID_PRIVILEGED_CALLER();
modifier onlyGlobalOwner() {
if (!gac.isGlobalOwner(msg.sender)) {
revert Error.CALLER_NOT_OWNER();
}
_;
}
Expand All @@ -73,7 +73,7 @@ contract CelerReceiverAdapter is IBridgeReceiverAdapter, IMessageReceiverApp {
////////////////////////////////////////////////////////////////*/

/// @inheritdoc IBridgeReceiverAdapter
function updateSenderAdapter(bytes memory _senderChain, address _senderAdapter) external override onlyCaller {
function updateSenderAdapter(bytes memory _senderChain, address _senderAdapter) external override onlyGlobalOwner {
uint64 _senderChainDecoded = abi.decode(_senderChain, (uint64));

if (_senderChainDecoded == 0) {
Expand Down
12 changes: 6 additions & 6 deletions src/adapters/Wormhole/WormholeReceiverAdapter.sol
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// SPDX-License-Identifier: GPL-3.0-only
/// SPDX-License-Identifier: GPL-3.0-only
pragma solidity >=0.8.9;

/// library imports
Expand Down Expand Up @@ -36,9 +36,9 @@ contract WormholeReceiverAdapter is IBridgeReceiverAdapter, IWormholeReceiver {
/*/////////////////////////////////////////////////////////////////
MODIFIER
////////////////////////////////////////////////////////////////*/
modifier onlyCaller() {
if (!gac.isPrivilegedCaller(msg.sender)) {
revert Error.INVALID_PRIVILEGED_CALLER();
modifier onlyGlobalOwner() {
if (!gac.isGlobalOwner(msg.sender)) {
revert Error.CALLER_NOT_OWNER();
}
_;
}
Expand Down Expand Up @@ -67,7 +67,7 @@ contract WormholeReceiverAdapter is IBridgeReceiverAdapter, IWormholeReceiver {
////////////////////////////////////////////////////////////////*/

/// @inheritdoc IBridgeReceiverAdapter
function updateSenderAdapter(bytes memory _senderChain, address _senderAdapter) external override onlyCaller {
function updateSenderAdapter(bytes memory _senderChain, address _senderAdapter) external override onlyGlobalOwner {
uint16 _senderChainDecoded = abi.decode(_senderChain, (uint16));

if (_senderChainDecoded == 0) {
Expand All @@ -88,7 +88,7 @@ contract WormholeReceiverAdapter is IBridgeReceiverAdapter, IWormholeReceiver {
/// @dev maps the MMA chain id to bridge specific chain id
/// @dev _origIds is the chain's native chain id
/// @dev _whIds are the bridge allocated chain id
function setChainIdMap(uint256[] calldata _origIds, uint16[] calldata _whIds) external onlyCaller {
function setChainIdMap(uint256[] calldata _origIds, uint16[] calldata _whIds) external onlyGlobalOwner {
uint256 arrLength = _origIds.length;

if (arrLength != _whIds.length) {
Expand Down
2 changes: 1 addition & 1 deletion src/adapters/Wormhole/WormholeSenderAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ contract WormholeSenderAdapter is BaseSenderAdapter {
/// @dev maps the MMA chain id to bridge specific chain id
/// @dev _origIds is the chain's native chain id
/// @dev _whIds are the bridge allocated chain id
function setChainIdMap(uint256[] calldata _origIds, uint16[] calldata _whIds) external onlyCaller {
function setChainIdMap(uint256[] calldata _origIds, uint16[] calldata _whIds) external onlyGlobalOwner {
uint256 arrLength = _origIds.length;

if (arrLength != _whIds.length) {
Expand Down
8 changes: 4 additions & 4 deletions src/adapters/axelar/AxelarReceiverAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ contract AxelarReceiverAdapter is IAxelarExecutable, IBridgeReceiverAdapter {
/*/////////////////////////////////////////////////////////////////
MODIFIER
////////////////////////////////////////////////////////////////*/
modifier onlyCaller() {
if (!gac.isPrivilegedCaller(msg.sender)) {
revert Error.INVALID_PRIVILEGED_CALLER();
modifier onlyGlobalOwner() {
if (!gac.isGlobalOwner(msg.sender)) {
revert Error.CALLER_NOT_OWNER();
}
_;
}
Expand All @@ -56,7 +56,7 @@ contract AxelarReceiverAdapter is IAxelarExecutable, IBridgeReceiverAdapter {
////////////////////////////////////////////////////////////////*/

/// @inheritdoc IBridgeReceiverAdapter
function updateSenderAdapter(bytes memory _senderChain, address _senderAdapter) external override onlyCaller {
function updateSenderAdapter(bytes memory _senderChain, address _senderAdapter) external override onlyGlobalOwner {
string memory _senderChainDecoded = abi.decode(_senderChain, (string));

if (keccak256(abi.encode(_senderChainDecoded)) == keccak256(abi.encode(""))) {
Expand Down
2 changes: 1 addition & 1 deletion src/adapters/axelar/AxelarSenderAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ contract AxelarSenderAdapter is BaseSenderAdapter {
/// @dev maps the MMA chain id to bridge specific chain id
/// @dev _origIds is the chain's native chain id
/// @dev _axlIds are the bridge allocated chain id
function setChainIdMap(uint256[] calldata _origIds, string[] calldata _axlIds) external onlyCaller {
function setChainIdMap(uint256[] calldata _origIds, string[] calldata _axlIds) external onlyGlobalOwner {
uint256 arrLength = _origIds.length;

if (arrLength != _axlIds.length) {
Expand Down
11 changes: 10 additions & 1 deletion src/controllers/GAC.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ import {Error} from "../libraries/Error.sol";

/// @dev is the global access control contract for bridge adapters
contract GAC is IGAC, Ownable {
/*///////////////////////////////////////////////////////////////
CONSTANTS
//////////////////////////////////////////////////////////////*/
uint256 public constant MINIMUM_DST_GAS_LIMIT = 50000;

/*///////////////////////////////////////////////////////////////
STATE VARIABLES
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -76,6 +81,10 @@ contract GAC is IGAC, Ownable {

/// @inheritdoc IGAC
function setGlobalMsgDeliveryGasLimit(uint256 _gasLimit) external override onlyOwner {
if (_gasLimit < MINIMUM_DST_GAS_LIMIT) {
revert Error.INVALID_DST_GAS_LIMIT_MIN();
}

uint256 oldLimit = dstGasLimit;
dstGasLimit = _gasLimit;

Expand All @@ -96,7 +105,7 @@ contract GAC is IGAC, Ownable {
//////////////////////////////////////////////////////////////*/

/// @inheritdoc IGAC
function isPrivilegedCaller(address _caller) external view override returns (bool) {
function isGlobalOwner(address _caller) external view override returns (bool) {
if (_caller == owner()) {
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IGAC.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ interface IGAC {
/// @dev returns `true` if the caller is global access controller
/// @param _caller is the msg.sender to validate
/// @return boolean indicating the validity of the caller
function isPrivilegedCaller(address _caller) external view returns (bool);
function isGlobalOwner(address _caller) external view returns (bool);

/// @dev returns the global owner address
/// @return _owner is the global owner address
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/Error.sol
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,10 @@ library Error {

/// @dev is thrown if transaction is expired
error TX_EXPIRED();

/*/////////////////////////////////////////////////////////////////
GAC ERRORS
////////////////////////////////////////////////////////////////*/
/// @dev is thrown if the gas limit is less than minimum
error INVALID_DST_GAS_LIMIT_MIN();
}
205 changes: 205 additions & 0 deletions test/unit-tests/GAC.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
// SPDX-License-Identifier: GPL-3.0-only
pragma solidity >=0.8.9;

/// library imports
import {Vm} from "forge-std/Test.sol";

import "openzeppelin-contracts/contracts/access/Ownable.sol";

/// local imports
import "../Setup.t.sol";
import "../contracts-mock/FailingSenderAdapter.sol";
import "../contracts-mock/ZeroAddressReceiverGac.sol";
import "src/interfaces/IBridgeSenderAdapter.sol";
import "src/interfaces/IMultiMessageReceiver.sol";
import "src/interfaces/IGAC.sol";
import "src/libraries/Error.sol";
import "src/libraries/Message.sol";
import {MultiMessageSender} from "src/MultiMessageSender.sol";

contract GACTest is Setup {
event DstGasLimitUpdated(uint256 oldLimit, uint256 newLimit);
event MultiMessageCallerUpdated(address indexed mmaCaller);
event MultiMessageSenderUpdated(address indexed mmaSender);
event MultiMessageReceiverUpdated(uint256 chainId, address indexed mmaReceiver);

uint256 constant SRC_CHAIN_ID = 1;
uint256 constant DST_CHAIN_ID = 137;

address senderAddr;
address receiverAddr;
IGAC gac;

/// @dev initializes the setup
function setUp() public override {
super.setUp();

vm.selectFork(fork[SRC_CHAIN_ID]);
senderAddr = contractAddress[SRC_CHAIN_ID]["MMA_SENDER"];
receiverAddr = contractAddress[DST_CHAIN_ID]["MMA_RECEIVER"];
gac = IGAC(contractAddress[SRC_CHAIN_ID]["GAC"]);
}

/// @dev constructor
function test_constructor() public {
// checks existing setup
assertEq(address(Ownable(address(gac)).owner()), owner);
}

/// @dev sets multi message sender
function test_set_multi_message_sender() public {
vm.startPrank(owner);

vm.expectEmit(true, true, true, true, address(gac));
emit MultiMessageSenderUpdated(address(42));

gac.setMultiMessageSender(address(42));

assertEq(gac.getMultiMessageSender(), address(42));
}

/// @dev only owner can set multi message sender
function test_set_multi_message_sender_only_owner() public {
vm.startPrank(caller);

vm.expectRevert("Ownable: caller is not the owner");
gac.setMultiMessageSender(address(42));
}

/// @dev cannot set multi message sender to zero address
function test_set_multi_message_sender_zero_address() public {
vm.startPrank(owner);

vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector);
gac.setMultiMessageSender(address(0));
}

/// @dev sets multi message caller
function test_set_multi_message_caller() public {
vm.startPrank(owner);

vm.expectEmit(true, true, true, true, address(gac));
emit MultiMessageCallerUpdated(address(42));

gac.setMultiMessageCaller(address(42));

assertEq(gac.getMultiMessageCaller(), address(42));
}

/// @dev only owner can set multi message caller
function test_set_multi_message_caller_only_owner() public {
vm.startPrank(caller);

vm.expectRevert("Ownable: caller is not the owner");
gac.setMultiMessageCaller(address(42));
}

/// @dev cannot set multi message caller to zero address
function test_set_multi_message_caller_zero_address() public {
vm.startPrank(owner);

vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector);
gac.setMultiMessageCaller(address(0));
}

/// @dev sets multi message receiver
function test_set_multi_message_receiver() public {
vm.startPrank(owner);

vm.expectEmit(true, true, true, true, address(gac));
emit MultiMessageReceiverUpdated(DST_CHAIN_ID, address(42));

gac.setMultiMessageReceiver(DST_CHAIN_ID, address(42));

assertEq(gac.getMultiMessageReceiver(DST_CHAIN_ID), address(42));
}

/// @dev only owner can set multi message receiver
function test_set_multi_message_receiver_only_owner() public {
vm.startPrank(caller);

vm.expectRevert("Ownable: caller is not the owner");
gac.setMultiMessageReceiver(DST_CHAIN_ID, address(42));
}

/// @dev cannot set multi message receiver to zero address
function test_set_multi_message_receiver_zero_address() public {
vm.startPrank(owner);

vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector);
gac.setMultiMessageReceiver(DST_CHAIN_ID, address(0));
}

/// @dev cannot set multi message receiver on zero chain ID
function test_set_multi_message_receiver_zero_chain_id() public {
vm.startPrank(owner);

vm.expectRevert(Error.ZERO_CHAIN_ID.selector);
gac.setMultiMessageReceiver(0, address(42));
}

/// @dev sets global message delivery gas limit
function test_set_global_msg_delivery_gas_limit() public {
vm.startPrank(owner);

uint256 oldLimit = gac.getGlobalMsgDeliveryGasLimit();
vm.expectEmit(true, true, true, true, address(gac));
emit DstGasLimitUpdated(oldLimit, 420000);

gac.setGlobalMsgDeliveryGasLimit(420000);

assertEq(gac.getGlobalMsgDeliveryGasLimit(), 420000);
}

/// @dev only owner can set global message delivery gas limit
function test_set_global_msg_delivery_gas_limit_only_owner() public {
vm.startPrank(caller);

vm.expectRevert("Ownable: caller is not the owner");
gac.setGlobalMsgDeliveryGasLimit(420000);
}

/// @dev cannot set a gas limit lower than the minimum
function test_set_global_msg_delivery_gas_limit_lower_than_min() public {
vm.startPrank(owner);

vm.expectRevert(Error.INVALID_DST_GAS_LIMIT_MIN.selector);
gac.setGlobalMsgDeliveryGasLimit(30000);
}

/// @dev sets refund address
function test_set_refund_address() public {
vm.startPrank(owner);

gac.setRefundAddress(address(42));

assertEq(gac.getRefundAddress(), address(42));
}

/// @dev only owner can set refund address
function test_set_refund_address_only_owner() public {
vm.startPrank(caller);

vm.expectRevert("Ownable: caller is not the owner");
gac.setRefundAddress(address(42));
}

/// @dev cannot set refund address to zero
function test_set_refund_address_zero() public {
vm.startPrank(owner);

vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector);
gac.setRefundAddress(address(0));
}

/// @dev checks if address is the global owner
function test_is_global_owner() public {
assertTrue(gac.isGlobalOwner(owner));
assertFalse(gac.isGlobalOwner(caller));
}

/// @dev gets the global owner
function test_get_global_owner() public {
assertEq(gac.getGlobalOwner(), owner);
}
}

0 comments on commit 7e25a04

Please sign in to comment.