Skip to content

Commit

Permalink
Cleanup caller / global owner confusion
Browse files Browse the repository at this point in the history
Add a sensible minimum value for dst gas limit
  • Loading branch information
Dominator008 committed Sep 12, 2023
1 parent 09e4eee commit 41cedde
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 46 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();
}
33 changes: 8 additions & 25 deletions test/unit-tests/GAC.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -159,29 +159,12 @@ contract GACTest is Setup {
gac.setGlobalMsgDeliveryGasLimit(420000);
}

/// @dev sets message expiry time
function test_set_msg_expiry_time() public {
/// @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);

gac.setMsgExpiryTime(42000);

assertEq(gac.getMsgExpiryTime(), 42000);
}

/// @dev only owner can set message expiry time
function test_set_msg_expiry_time_only_owner() public {
vm.startPrank(caller);

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

/// @dev cannot set message expiry time to zero
function test_set_msg_expiry_time_zero() public {
vm.startPrank(owner);

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

/// @dev sets refund address
Expand Down Expand Up @@ -209,10 +192,10 @@ contract GACTest is Setup {
gac.setRefundAddress(address(0));
}

/// @dev only owner is privileged caller
function test_is_privileged_caller() public {
assertTrue(gac.isPrivilegedCaller(owner));
assertFalse(gac.isPrivilegedCaller(caller));
/// @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
Expand Down

0 comments on commit 41cedde

Please sign in to comment.