Skip to content

Commit

Permalink
[cantina finding #9] added gasLimit to relayMessage
Browse files Browse the repository at this point in the history
  • Loading branch information
voith committed Feb 23, 2025
1 parent 7bd6554 commit 02d486a
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 9 deletions.
15 changes: 14 additions & 1 deletion contracts/ccip/GovernanceCCIPRelay.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ contract GovernanceCCIPRelay is IGovernanceCCIPRelay {
mapping(uint64 => address) public destinationReceivers;

uint64 private CCIPMainnetChainSelector = 5009297550715157269;
uint256 constant MIN_GAS_LIMIT = 50_000;
uint256 constant MAX_GAS_LIMIT = 10_000_000;

/// @dev Modifier to restrict access to the Timelock contract.
modifier onlyTimeLock() {
Expand Down Expand Up @@ -103,11 +105,17 @@ contract GovernanceCCIPRelay is IGovernanceCCIPRelay {
/// @inheritdoc IGovernanceCCIPRelay
function relayMessage(
uint64 destinationChainSelector,
uint256 gasLimit,
address target,
bytes calldata payload
) external payable onlyTimeLock returns (bytes32 messageId) {
require(target != address(0), AddressCannotBeZero());
require(payload.length != 0, PayloadCannotBeEmpty());
require(gasLimit >= MIN_GAS_LIMIT, GasLimitTooLow(gasLimit, MIN_GAS_LIMIT));
require(
gasLimit <= MAX_GAS_LIMIT,
GasLimitTooHigh(gasLimit, MAX_GAS_LIMIT)
);

address destinationReceiver = destinationReceivers[
destinationChainSelector
Expand All @@ -121,7 +129,12 @@ contract GovernanceCCIPRelay is IGovernanceCCIPRelay {
receiver: abi.encode(destinationReceiver),
data: abi.encode(target, payload),
tokenAmounts: new Client.EVMTokenAmount[](0),
extraArgs: "",
extraArgs: Client._argsToBytes(
Client.EVMExtraArgsV2({
gasLimit: gasLimit,
allowOutOfOrderExecution: true
})
),
feeToken: address(0)
});

Expand Down
12 changes: 12 additions & 0 deletions contracts/ccip/interfaces/IGovernanceCCIPRelay.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ interface IGovernanceCCIPRelay {
/// @dev Error thrown when a provided address is the zero address.
error AddressCannotBeZero();

/// @dev Thrown when the specified gas limit is too low to execute the transaction.
/// @param gasLimit The provided gas limit.
/// @param minGasLimit The minimum required gas limit.
error GasLimitTooLow(uint256 gasLimit, uint256 minGasLimit);

/// @dev Thrown when the specified gas limit exceeds the maximum allowed threshold.
/// @param gasLimit The provided gas limit.
/// @param maxGasLimit The maximum allowed gas limit.
error GasLimitTooHigh(uint256 gasLimit, uint256 maxGasLimit);

/// @dev Error thrown when payload is empty.
error PayloadCannotBeEmpty();

Expand Down Expand Up @@ -86,11 +96,13 @@ interface IGovernanceCCIPRelay {

/// @notice Relays a governance message to the destination chain.
/// @param destinationChainSelector The ccip chain selector of the destination chain for the message.
/// @param gasLimit the maximum amount of gas CCIP can consume to execute ccipReceive()
/// @param target The target address to execute the message on.
/// @param payload The calldata payload to execute.
/// @return messageId The unique identifier of the sent message.
function relayMessage(
uint64 destinationChainSelector,
uint256 gasLimit,
address target,
bytes calldata payload
) external payable returns (bytes32 messageId);
Expand Down
11 changes: 9 additions & 2 deletions test/ccip/GovernanceCCIPIntegrationTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ contract GovernanceCCIPIntegrationTest is Test {
vm.prank(address(timelock));
bytes32 messageId = governanceRelay.relayMessage{value: fee}(
polygonMainnetChainSelector,
200_000,
target,
payload
);
Expand Down Expand Up @@ -276,6 +277,7 @@ contract GovernanceCCIPIntegrationTest is Test {
vm.prank(address(timelock));
bytes32 messageId = governanceRelay.relayMessage{value: fee}(
polygonMainnetChainSelector,
200_000,
target,
payload
);
Expand Down Expand Up @@ -303,8 +305,13 @@ contract GovernanceCCIPIntegrationTest is Test {

targets[0] = address(governanceRelay);
values[0] = 1 ether;
signatures[0] = "relayMessage(uint64,address,bytes)";
calldatas[0] = abi.encode(polygonMainnetChainSelector, target, payload);
signatures[0] = "relayMessage(uint64,uint256,address,bytes)";
calldatas[0] = abi.encode(
polygonMainnetChainSelector,
200_000,
target,
payload
);
assertEq(numberUpdater.number(), 0, "Initial number should be 0");

uint256 fee = 0.1 ether;
Expand Down
59 changes: 53 additions & 6 deletions test/ccip/GovernanceCCIPRelayTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ contract GovernanceCCIPRelayTest is Test {

vm.deal(address(timelock), fee); // Fund this contract
vm.prank(timelock);
relay.relayMessage{value: fee}(destinationChainSelector, target, payload);
relay.relayMessage{value: fee}(
destinationChainSelector,
200_000,
target,
payload
);
}

/// @notice Test relayMessage reverts when called by non-timelock
Expand All @@ -123,7 +128,7 @@ contract GovernanceCCIPRelayTest is Test {
attacker
)
);
relay.relayMessage(destinationChainSelector, target, payload);
relay.relayMessage(destinationChainSelector, 200_000, target, payload);
}

/// @notice Test relayMessage returns excess ether
Expand All @@ -144,6 +149,7 @@ contract GovernanceCCIPRelayTest is Test {
vm.prank(timelock);
relay.relayMessage{value: fee + excess}(
destinationChainSelector,
200_000,
target,
payload
);
Expand Down Expand Up @@ -173,7 +179,12 @@ contract GovernanceCCIPRelayTest is Test {
vm.prank(timelock);
vm.expectEmit(true, true, true, true);
emit IGovernanceCCIPRelay.MessageRelayed(bytes32(""), target, payload);
relay.relayMessage{value: fee}(destinationChainSelector, target, payload);
relay.relayMessage{value: fee}(
destinationChainSelector,
200_000,
target,
payload
);
}

/// @notice Test relayMessage raises InsufficientFee error when value is less than fee
Expand All @@ -200,6 +211,7 @@ contract GovernanceCCIPRelayTest is Test {
);
relay.relayMessage{value: fee - 0.1 ether}(
destinationChainSelector,
200_000,
target,
payload
);
Expand Down Expand Up @@ -227,7 +239,12 @@ contract GovernanceCCIPRelayTest is Test {
unsupportedChainSelector
)
);
relay.relayMessage{value: fee}(unsupportedChainSelector, target, payload);
relay.relayMessage{value: fee}(
unsupportedChainSelector,
200_000,
target,
payload
);
}

function testAddDestinationChainsByTimelock() public {
Expand Down Expand Up @@ -373,15 +390,45 @@ contract GovernanceCCIPRelayTest is Test {
vm.deal(address(timelock), 1 ether);
vm.startPrank(timelock);
vm.expectRevert(IGovernanceCCIPRelay.AddressCannotBeZero.selector);
relay.relayMessage{value: 1}(1, address(0), "0x1234");
relay.relayMessage{value: 1}(1, 200_000, address(0), "0x1234");
vm.stopPrank();
}

function testRelayMessage_RevertsIfPayloadIsEmpty() public {
vm.deal(address(timelock), 1 ether);
vm.startPrank(timelock);
vm.expectRevert(IGovernanceCCIPRelay.PayloadCannotBeEmpty.selector);
relay.relayMessage{value: 1}(1, address(0xabc), "");
relay.relayMessage{value: 1}(1, 200_000, address(0xabc), "");
vm.stopPrank();
}

function testRelayMessage_GasLimitTooLow() public {
vm.deal(address(timelock), 1 ether);
vm.prank(timelock);
vm.expectRevert(
abi.encodeWithSelector(
IGovernanceCCIPRelay.GasLimitTooLow.selector,
1,
50_000
)
);
relay.relayMessage(destinationChainSelector, 1, address(0x4), "payload");
}

function testRelayMessage_GasLimitTooHigh() public {
vm.prank(timelock); // Simulate timelock calling
vm.expectRevert(
abi.encodeWithSelector(
IGovernanceCCIPRelay.GasLimitTooHigh.selector,
10_000_000 + 1,
10_000_000
)
);
relay.relayMessage(
destinationChainSelector,
10_000_000 + 1,
address(0x4),
"payload"
);
}
}

0 comments on commit 02d486a

Please sign in to comment.