Skip to content

Commit

Permalink
fix: use callbackGasLimit to callback contract (#316)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mattstam
mattstam authored Jan 9, 2024
1 parent 9360fb8 commit bf8ad4e
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 25 deletions.
5 changes: 3 additions & 2 deletions contracts/src/SuccinctGateway.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,9 @@ contract SuccinctGateway is ISuccinctGateway, FunctionRegistry, TimelockedUpgrad

// Execute the callback.
isCallback = true;
(bool status,) =
_callbackAddress.call(abi.encodeWithSelector(_callbackSelector, _output, _context));
(bool status,) = _callbackAddress.call{gas: _callbackGasLimit}(
abi.encodeWithSelector(_callbackSelector, _output, _context)
);
isCallback = false;

// If the callback failed, revert.
Expand Down
87 changes: 69 additions & 18 deletions contracts/test/SuccinctGateway.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ contract SuccinctGatewayTest is Test, ISuccinctGatewayEvents, ISuccinctGatewayEr
bytes internal constant PROOF = hex"";

uint256 internal constant DEFAULT_FEE = 0.1 ether;
uint32 internal constant DEFAULT_GAS_LIMIT = 1_000_000;

address internal timelock;
address internal guardian;
Expand Down Expand Up @@ -107,7 +108,7 @@ contract RequestTest is SuccinctGatewayTest {
bytes32 functionId = TestConsumer(consumer).FUNCTION_ID();
address callbackAddress = consumer;
bytes4 callbackSelector = TestConsumer.handleCallback.selector;
uint32 callbackGasLimit = TestConsumer(consumer).CALLBACK_GAS_LIMIT();
uint32 callbackGasLimit = DEFAULT_GAS_LIMIT;
uint256 fee = DEFAULT_FEE;
bytes memory context = abi.encode(nonce);
bytes memory output = OUTPUT;
Expand All @@ -126,7 +127,7 @@ contract RequestTest is SuccinctGatewayTest {
fee
);
vm.prank(sender);
TestConsumer(consumer).requestCallback{value: fee}();
TestConsumer(consumer).requestCallback{value: fee}(callbackGasLimit);

assertEq(prevNonce + 1, SuccinctGateway(gateway).nonce());
assertEq(TestConsumer(consumer).handledRequests(0), false);
Expand Down Expand Up @@ -269,7 +270,7 @@ contract RequestTest is SuccinctGatewayTest {
bytes32 functionId = TestConsumer(consumer).FUNCTION_ID();
address callbackAddress = consumer;
bytes4 callbackSelector = TestConsumer.handleCallback.selector;
uint32 callbackGasLimit = TestConsumer(consumer).CALLBACK_GAS_LIMIT();
uint32 callbackGasLimit = DEFAULT_GAS_LIMIT;
uint256 fee = 0;
bytes memory context = abi.encode(nonce);
bytes memory output = OUTPUT;
Expand All @@ -288,7 +289,7 @@ contract RequestTest is SuccinctGatewayTest {
fee
);
vm.prank(sender);
TestConsumer(consumer).requestCallback{value: fee}();
TestConsumer(consumer).requestCallback{value: fee}(callbackGasLimit);

assertEq(nonce + 1, SuccinctGateway(gateway).nonce());
assertEq(TestConsumer(consumer).handledRequests(0), false);
Expand Down Expand Up @@ -324,7 +325,7 @@ contract RequestTest is SuccinctGatewayTest {
bytes32 functionId = TestConsumer(consumer).FUNCTION_ID();
address callbackAddress = consumer;
bytes4 callbackSelector = TestConsumer.handleCallback.selector;
uint32 callbackGasLimit = TestConsumer(consumer).CALLBACK_GAS_LIMIT();
uint32 callbackGasLimit = DEFAULT_GAS_LIMIT;
uint256 fee = DEFAULT_FEE;
bytes memory context = abi.encode(nonce);
bytes memory output = OUTPUT;
Expand All @@ -343,7 +344,7 @@ contract RequestTest is SuccinctGatewayTest {
fee
);
vm.prank(sender);
TestConsumer(consumer).requestCallback{value: fee}();
TestConsumer(consumer).requestCallback{value: fee}(callbackGasLimit);

assertEq(prevNonce + 1, SuccinctGateway(gateway).nonce());
assertEq(TestConsumer(consumer).handledRequests(0), false);
Expand All @@ -367,13 +368,62 @@ contract RequestTest is SuccinctGatewayTest {
assertEq(TestConsumer(consumer).handledRequests(0), true);
}

function test_Callback_WhenCallbackGasLimitTooLow() public {
uint32 prevNonce = SuccinctGateway(gateway).nonce();
assertEq(prevNonce, 0);

uint32 nonce = prevNonce;
bytes32 inputHash = INPUT_HASH;
bytes32 functionId = TestConsumer(consumer).FUNCTION_ID();
address callbackAddress = consumer;
bytes4 callbackSelector = TestConsumer.handleCallback.selector;
uint32 callbackGasLimit = 0;
uint256 fee = DEFAULT_FEE;
bytes memory context = abi.encode(nonce);
bytes memory output = OUTPUT;
bytes memory proof = PROOF;

// Request
vm.expectEmit(true, true, true, true, gateway);
emit RequestCallback(
nonce,
functionId,
INPUT,
context,
callbackAddress,
callbackSelector,
callbackGasLimit,
fee
);
vm.prank(sender);
TestConsumer(consumer).requestCallback{value: fee}(callbackGasLimit);

assertEq(prevNonce + 1, SuccinctGateway(gateway).nonce());
assertEq(TestConsumer(consumer).handledRequests(0), false);

// Fulfill
vm.expectRevert();
vm.prank(prover);
SuccinctGateway(gateway).fulfillCallback(
nonce,
functionId,
inputHash,
callbackAddress,
callbackSelector,
callbackGasLimit,
context,
output,
proof
);
}

function test_RevertCallback_WhenNoRequest() public {
uint32 nonce = SuccinctGateway(gateway).nonce();
bytes32 inputHash = INPUT_HASH;
bytes32 functionId = TestConsumer(consumer).FUNCTION_ID();
address callbackAddress = consumer;
bytes4 callbackSelector = TestConsumer.handleCallback.selector;
uint32 callbackGasLimit = TestConsumer(consumer).CALLBACK_GAS_LIMIT();
uint32 callbackGasLimit = DEFAULT_GAS_LIMIT;
bytes memory context = abi.encode(nonce);
bytes memory output = OUTPUT;
bytes memory proof = PROOF;
Expand All @@ -400,14 +450,14 @@ contract RequestTest is SuccinctGatewayTest {
bytes32 functionId = TestConsumer(consumer).FUNCTION_ID();
address callbackAddress = consumer;
bytes4 callbackSelector = TestConsumer.handleCallback.selector;
uint32 callbackGasLimit = TestConsumer(consumer).CALLBACK_GAS_LIMIT();
uint32 callbackGasLimit = DEFAULT_GAS_LIMIT;
uint256 fee = DEFAULT_FEE;
bytes memory context = abi.encode(nonce);
bytes memory output = OUTPUT;
bytes memory proof = PROOF;

// Request
TestConsumer(consumer).requestCallback{value: fee}();
TestConsumer(consumer).requestCallback{value: fee}(callbackGasLimit);

// Fulfill
vm.expectRevert(abi.encodeWithSelector(OnlyProver.selector, functionId, sender));
Expand All @@ -432,13 +482,13 @@ contract RequestTest is SuccinctGatewayTest {
bytes memory proof = PROOF;
address callAddress = consumer;
bytes memory callData = abi.encodeWithSelector(TestConsumer.handleCall.selector);
uint32 callGasLimit = TestConsumer(consumer).CALLBACK_GAS_LIMIT();
uint32 callGasLimit = DEFAULT_GAS_LIMIT;
uint256 fee = DEFAULT_FEE;

// Request
vm.expectEmit(true, true, true, true, gateway);
emit RequestCall(functionId, input, callAddress, callData, callGasLimit, consumer, fee);
TestConsumer(consumer).requestCall{value: fee}();
TestConsumer(consumer).requestCall{value: fee}(callGasLimit);

assertEq(TestConsumer(consumer).handledRequests(0), false);

Expand Down Expand Up @@ -475,13 +525,13 @@ contract RequestTest is SuccinctGatewayTest {
bytes memory proof = PROOF;
address callAddress = consumer;
bytes memory callData = abi.encodeWithSelector(TestConsumer.handleCall.selector);
uint32 callGasLimit = TestConsumer(consumer).CALLBACK_GAS_LIMIT();
uint32 callGasLimit = DEFAULT_GAS_LIMIT;
uint256 fee = 0;

// Request
vm.expectEmit(true, true, true, true, gateway);
emit RequestCall(functionId, input, callAddress, callData, callGasLimit, consumer, fee);
TestConsumer(consumer).requestCall{value: fee}();
TestConsumer(consumer).requestCall{value: fee}(callGasLimit);

assertEq(TestConsumer(consumer).handledRequests(0), false);

Expand All @@ -506,13 +556,13 @@ contract RequestTest is SuccinctGatewayTest {
bytes memory proof = PROOF;
address callAddress = consumer;
bytes memory callData = abi.encodeWithSelector(TestConsumer.handleCall.selector);
uint32 callGasLimit = TestConsumer(consumer).CALLBACK_GAS_LIMIT();
uint32 callGasLimit = DEFAULT_GAS_LIMIT;
uint256 fee = DEFAULT_FEE;

// Request
vm.expectEmit(true, true, true, true, gateway);
emit RequestCall(functionId, input, callAddress, callData, callGasLimit, consumer, fee);
TestConsumer(consumer).requestCall{value: fee}();
TestConsumer(consumer).requestCall{value: fee}(callGasLimit);

assertEq(TestConsumer(consumer).handledRequests(0), false);

Expand All @@ -533,11 +583,12 @@ contract RequestTest is SuccinctGatewayTest {
bytes memory output = OUTPUT;
bytes memory proof = PROOF;
address callAddress = consumer;
uint32 callGasLimit = DEFAULT_GAS_LIMIT;
bytes memory callData = abi.encodeWithSelector(TestConsumer.handleCall.selector);
uint256 fee = DEFAULT_FEE;

// Request
TestConsumer(consumer).requestCall{value: fee}();
TestConsumer(consumer).requestCall{value: fee}(callGasLimit);

// Fulfill
vm.expectRevert(abi.encodeWithSelector(OnlyProver.selector, functionId, sender));
Expand Down Expand Up @@ -1060,7 +1111,7 @@ contract SetFeeVaultTest is SuccinctGatewayTest {
bytes memory input = INPUT;
address callAddress = consumer;
bytes memory callData = abi.encodeWithSelector(TestConsumer.handleCall.selector);
uint32 callGasLimit = TestConsumer(consumer).CALLBACK_GAS_LIMIT();
uint32 callGasLimit = DEFAULT_GAS_LIMIT;
uint256 fee = DEFAULT_FEE;
address newFeeVault = address(new SuccinctFeeVault());

Expand All @@ -1075,7 +1126,7 @@ contract SetFeeVaultTest is SuccinctGatewayTest {
// Request with fee
vm.expectEmit(true, true, true, true, gateway);
emit RequestCall(functionId, input, callAddress, callData, callGasLimit, consumer, fee);
TestConsumer(consumer).requestCall{value: fee}();
TestConsumer(consumer).requestCall{value: fee}(callGasLimit);
}

function test_RevertSetFeeVault_WhenNotGuardian() public {
Expand Down
9 changes: 4 additions & 5 deletions contracts/test/TestUtils.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ contract TestConsumer {
address public immutable SUCCINCT_GATEWAY;
bytes32 public immutable FUNCTION_ID;
bytes public INPUT;
uint32 public constant CALLBACK_GAS_LIMIT = 2000000;

uint32 public nonce;
mapping(uint32 => bool) public handledRequests;
Expand All @@ -47,9 +46,9 @@ contract TestConsumer {
INPUT = _input;
}

function requestCallback() external payable {
function requestCallback(uint32 _callbackGasLimit) external payable {
ISuccinctGateway(SUCCINCT_GATEWAY).requestCallback{value: msg.value}(
FUNCTION_ID, INPUT, abi.encode(nonce), this.handleCallback.selector, CALLBACK_GAS_LIMIT
FUNCTION_ID, INPUT, abi.encode(nonce), this.handleCallback.selector, _callbackGasLimit
);

nonce++;
Expand All @@ -71,13 +70,13 @@ contract TestConsumer {
handledRequests[nonce - 1] = result;
}

function requestCall() external payable {
function requestCall(uint32 _callGasLimit) external payable {
ISuccinctGateway(SUCCINCT_GATEWAY).requestCall{value: msg.value}(
FUNCTION_ID,
INPUT,
address(this),
abi.encodeWithSelector(this.handleCall.selector),
CALLBACK_GAS_LIMIT
_callGasLimit
);
}

Expand Down

0 comments on commit bf8ad4e

Please sign in to comment.