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.

Sign up for GitHub

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

[Functions] Minor contract fixes #10511

Merged
merged 6 commits into from
Sep 8, 2023
Merged

[Functions] Minor contract fixes #10511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3a9eb5
(fix): Functions Subscriptions add consumer checks if sub consumers g…
justinkaseman Sep 6, 2023
5bc5371
(fix): Functions Router during fulfillment do not revert on invalid c…
justinkaseman Sep 6, 2023
8e32f5b
Functions Coordinator oracleWithdrawAll checks for 0 balances
justinkaseman Sep 6, 2023
b9d57de
(test): Add unit tests for changes
justinkaseman Sep 7, 2023
bfa35fa
Update gas snapshot
justinkaseman Sep 7, 2023
7fd0b5c
Changes after rebase
justinkaseman Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions contracts/gas-snapshots/functions.gas-snapshot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
FunctionsBilling_EstimateCost:test_EstimateCost_RevertsIfGasPriceAboveCeiling() (gas: 32391)
FunctionsBilling_EstimateCost:test_EstimateCost_Success() (gas: 52979)
FunctionsBilling_OracleWithdrawAll:test_OracleWithdrawAll_RevertIfNotOwner() (gas: 13274)
FunctionsBilling_OracleWithdrawAll:test_OracleWithdrawAll_SuccessPaysTransmittersWithBalance() (gas: 147058)
FunctionsOracle_sendRequest:testEmptyRequestDataReverts() (gas: 13452)
FunctionsOracle_setDONPublicKey:testEmptyPublicKeyReverts() (gas: 10974)
FunctionsOracle_setDONPublicKey:testOnlyOwnerReverts() (gas: 11255)
Expand All @@ -11,16 +13,17 @@ FunctionsOracle_setRegistry:testSetRegistrySuccess() (gas: 35791)
FunctionsOracle_setRegistry:testSetRegistry_gas() (gas: 31987)
FunctionsOracle_typeAndVersion:testTypeAndVersionSuccess() (gas: 6905)
FunctionsRouter_Constructor:test_Constructor_Success() (gas: 12073)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedCostExceedsCommitment() (gas: 48299)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInsufficientGas() (gas: 38830)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInvalidCommitment() (gas: 36239)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedCostExceedsCommitment() (gas: 48277)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInsufficientGas() (gas: 38808)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInvalidCommitment() (gas: 36217)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInvalidRequestId() (gas: 35161)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedSubscriptionBalanceInvariant() (gas: 165)
FunctionsRouter_Fulfill:test_Fulfill_RevertIfNotCommittedCoordinator() (gas: 28037)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedSubscriptionBalanceInvariant() (gas: 210)
FunctionsRouter_Fulfill:test_Fulfill_RevertIfNotCommittedCoordinator() (gas: 28015)
FunctionsRouter_Fulfill:test_Fulfill_RevertIfPaused() (gas: 33206)
FunctionsRouter_Fulfill:test_Fulfill_SuccessFulfilled() (gas: 103393)
FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackReverts() (gas: 1762917)
FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackRunsOutOfGas() (gas: 205990)
FunctionsRouter_Fulfill:test_Fulfill_SuccessClientNoLongerExists() (gas: 93612)
FunctionsRouter_Fulfill:test_Fulfill_SuccessFulfilled() (gas: 103408)
FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackReverts() (gas: 1762929)
FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackRunsOutOfGas() (gas: 206005)
FunctionsRouter_GetAdminFee:test_GetAdminFee_Success() (gas: 17842)
FunctionsRouter_GetAllowListId:test_GetAllowListId_Success() (gas: 12883)
FunctionsRouter_GetConfig:test_GetConfig_Success() (gas: 31332)
Expand All @@ -44,7 +47,7 @@ FunctionsRouter_SendRequest:test_SendRequest_RevertIfConsumerNotAllowed() (gas:
FunctionsRouter_SendRequest:test_SendRequest_RevertIfDuplicateRequestId() (gas: 192424)
FunctionsRouter_SendRequest:test_SendRequest_RevertIfEmptyData() (gas: 29382)
FunctionsRouter_SendRequest:test_SendRequest_RevertIfIncorrectDonId() (gas: 57929)
FunctionsRouter_SendRequest:test_SendRequest_RevertIfInsufficientSubscriptionBalance() (gas: 186036)
FunctionsRouter_SendRequest:test_SendRequest_RevertIfInsufficientSubscriptionBalance() (gas: 186033)
FunctionsRouter_SendRequest:test_SendRequest_RevertIfInvalidCallbackGasLimit() (gas: 48353)
FunctionsRouter_SendRequest:test_SendRequest_RevertIfInvalidDonId() (gas: 25038)
FunctionsRouter_SendRequest:test_SendRequest_RevertIfNoSubscription() (gas: 29088)
Expand All @@ -58,7 +61,7 @@ FunctionsRouter_SendRequestToProposed:test_SendRequestToProposed_RevertIfInvalid
FunctionsRouter_SendRequestToProposed:test_SendRequestToProposed_RevertIfNoSubscription() (gas: 35718)
FunctionsRouter_SendRequestToProposed:test_SendRequestToProposed_RevertIfPaused() (gas: 40788)
FunctionsRouter_SendRequestToProposed:test_SendRequestToProposed_Success() (gas: 204484)
FunctionsRouter_SendRequestToProposed:test_SendRequest_RevertIfInsufficientSubscriptionBalance() (gas: 192600)
FunctionsRouter_SendRequestToProposed:test_SendRequest_RevertIfInsufficientSubscriptionBalance() (gas: 192597)
FunctionsRouter_SetAllowListId:test_SetAllowListId_Success() (gas: 30666)
FunctionsRouter_SetAllowListId:test_UpdateConfig_RevertIfNotOwner() (gas: 13402)
FunctionsRouter_Unpause:test_Unpause_RevertIfNotOwner() (gas: 13294)
Expand All @@ -72,12 +75,13 @@ FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOw
FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOwnerTransfer_RevertIfSenderBecomesBlocked() (gas: 94703)
FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOwnerTransfer_RevertIfSenderIsNotNewOwner() (gas: 62713)
FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOwnerTransfer_Success() (gas: 59611)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfMaximumConsumers() (gas: 137842)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfNoSubscription() (gas: 12914)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfMaximumConsumers() (gas: 137833)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfMaximumConsumersAfterConfigUpdate() (gas: 161337)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfNoSubscription() (gas: 12926)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfNotAllowedSender() (gas: 57789)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfNotSubscriptionOwner() (gas: 87166)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfPaused() (gas: 18006)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_Success() (gas: 95394)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfPaused() (gas: 18051)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_Success() (gas: 95391)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfNoSubscription() (gas: 15041)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfNotAllowedSender() (gas: 57863)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfNotSubscriptionOwner() (gas: 89296)
Expand Down
6 changes: 4 additions & 2 deletions contracts/src/v0.8/functions/dev/1_0_0/FunctionsBilling.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,10 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
// Bounded by "maxNumOracles" on OCR2Abstract.sol
for (uint256 i = 0; i < transmitters.length; ++i) {
uint96 balance = s_withdrawableTokens[transmitters[i]];
s_withdrawableTokens[transmitters[i]] = 0;
IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance);
if (balance > 0) {
s_withdrawableTokens[transmitters[i]] = 0;
IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance);
}
}
}

Expand Down
19 changes: 12 additions & 7 deletions contracts/src/v0.8/functions/dev/1_0_0/FunctionsRouter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,18 @@ contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable,
uint32 callbackGasLimit,
address client
) private returns (CallbackResult memory) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get @se3000 or @RensR to review this? I am not confident enough it my assembly skills to feel comfortable approving.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only moves the check up a bit, assembly logic is almost the same so looks good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see unit tests cover this logic before merging this

bool destinationNoLongerExists;
// solhint-disable-next-line no-inline-assembly
assembly {
// solidity calls check that a contract actually exists at the destination, so we do the same
destinationNoLongerExists := iszero(extcodesize(client))
}
if (destinationNoLongerExists) {
// Return without attempting callback
// The subscription will still be charged to reimburse transmitter's gas overhead
return CallbackResult({success: false, gasUsed: 0, returnData: new bytes(0)});
}

bytes memory encodedCallback = abi.encodeWithSelector(
s_config.handleOracleFulfillmentSelector,
requestId,
Expand All @@ -410,13 +422,6 @@ contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable,

// solhint-disable-next-line no-inline-assembly
assembly {
// solidity calls check that a contract actually exists at the destination, so we do the same
// Note we do this check prior to measuring gas so gasForCallExactCheck (our "cushion")
// doesn't need to account for it.
if iszero(extcodesize(client)) {
revert(0, 0)
}

let g := gas()
// Compute g -= gasForCallExactCheck and check for underflow
// The gas actually passed to the callee is _min(gasAmount, 63//64*gas available).
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/functions/dev/1_0_0/FunctionsSubscriptions.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ abstract contract FunctionsSubscriptions is IFunctionsSubscriptions, IERC677Rece

// Already maxed, cannot add any more consumers.
uint16 maximumConsumers = _getMaxConsumers();
if (s_subscriptions[subscriptionId].consumers.length == maximumConsumers) {
if (s_subscriptions[subscriptionId].consumers.length >= maximumConsumers) {
revert TooManyConsumers(maximumConsumers);
}
if (s_consumers[consumer][subscriptionId].allowed) {
Expand Down
43 changes: 41 additions & 2 deletions contracts/src/v0.8/functions/tests/1_0_0/FunctionsCoordinator.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {FunctionsCoordinator} from "../../dev/1_0_0/FunctionsCoordinator.sol";
import {FunctionsBilling} from "../../dev/1_0_0/FunctionsBilling.sol";
import {FunctionsRequest} from "../../dev/1_0_0/libraries/FunctionsRequest.sol";

import {FunctionsSubscriptionSetup} from "./Setup.t.sol";
import {FunctionsSubscriptionSetup, FunctionsMultipleFulfillmentsSetup} from "./Setup.t.sol";

// ================================================================
// | Functions Coordinator |
Expand Down Expand Up @@ -209,8 +209,47 @@ contract FunctionsBilling_OracleWithdraw {
}

/// @notice #oracleWithdrawAll
contract FunctionsBilling_OracleWithdrawAll {
contract FunctionsBilling_OracleWithdrawAll is FunctionsMultipleFulfillmentsSetup {
function setUp() public virtual override {
// Use no DON fee so that a transmitter has a balance of 0
s_donFee = 0;

FunctionsMultipleFulfillmentsSetup.setUp();
}

function test_OracleWithdrawAll_RevertIfNotOwner() public {
// Send as stranger
vm.stopPrank();
bolekk marked this conversation as resolved.
Show resolved Hide resolved
vm.startPrank(STRANGER_ADDRESS);

vm.expectRevert("Only callable by owner");
s_functionsCoordinator.oracleWithdrawAll();
}

function test_OracleWithdrawAll_SuccessPaysTransmittersWithBalance() public {
uint256 transmitter1BalanceBefore = s_linkToken.balanceOf(NOP_TRANSMITTER_ADDRESS_1);
assertEq(transmitter1BalanceBefore, 0);
uint256 transmitter2BalanceBefore = s_linkToken.balanceOf(NOP_TRANSMITTER_ADDRESS_2);
assertEq(transmitter2BalanceBefore, 0);
uint256 transmitter3BalanceBefore = s_linkToken.balanceOf(NOP_TRANSMITTER_ADDRESS_3);
assertEq(transmitter3BalanceBefore, 0);
uint256 transmitter4BalanceBefore = s_linkToken.balanceOf(NOP_TRANSMITTER_ADDRESS_4);
assertEq(transmitter4BalanceBefore, 0);

s_functionsCoordinator.oracleWithdrawAll();

uint96 expectedTransmitterBalance = s_fulfillmentCoordinatorBalance / 3;

uint256 transmitter1BalanceAfter = s_linkToken.balanceOf(NOP_TRANSMITTER_ADDRESS_1);
assertEq(transmitter1BalanceAfter, expectedTransmitterBalance);
uint256 transmitter2BalanceAfter = s_linkToken.balanceOf(NOP_TRANSMITTER_ADDRESS_2);
assertEq(transmitter2BalanceAfter, expectedTransmitterBalance);
uint256 transmitter3BalanceAfter = s_linkToken.balanceOf(NOP_TRANSMITTER_ADDRESS_3);
assertEq(transmitter3BalanceAfter, expectedTransmitterBalance);
// Transmitter 4 has no balance
uint256 transmitter4BalanceAfter = s_linkToken.balanceOf(NOP_TRANSMITTER_ADDRESS_4);
assertEq(transmitter4BalanceAfter, 0);
}
}

/// @notice #_getTransmitters
Expand Down
47 changes: 47 additions & 0 deletions contracts/src/v0.8/functions/tests/1_0_0/FunctionsRouter.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,6 +1191,53 @@ contract FunctionsRouter_Fulfill is FunctionsClientRequestSetup {
assertEq(callbackGasCostJuels, 0);
}

function test_Fulfill_SuccessClientNoLongerExists() public {
// Delete the Client contract in the time between request and fulfillment
vm.etch(address(s_functionsClient), new bytes(0));

// Send as committed Coordinator
vm.stopPrank();
vm.startPrank(address(s_functionsCoordinator));

bytes memory response = bytes("hello world!");
bytes memory err = new bytes(0);
uint96 juelsPerGas = 0;
uint96 costWithoutCallback = 0;
address transmitter = NOP_TRANSMITTER_ADDRESS_1;
FunctionsResponse.Commitment memory commitment = s_requestCommitment;

// topic0 (function signature, always checked), topic1 (true), topic2 (true), NOT topic3 (false), and data (true).
bool checkTopic1RequestId = true;
bool checkTopic2SubscriptionId = true;
bool checkTopic3 = false;
bool checkData = true;
vm.expectEmit(checkTopic1RequestId, checkTopic2SubscriptionId, checkTopic3, checkData);
emit RequestProcessed({
requestId: s_requestId,
subscriptionId: s_subscriptionId,
totalCostJuels: s_adminFee + costWithoutCallback, // NOTE: tx.gasprice is at 0, so no callback gas used
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly confused by this comment... tx.gasprice is zero? I know that we should only be charging the DON fee (which is zero) + admin fee + gas overhead, but why is tx.gasprice zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that's the default in foundry. I'll bump it up next week for all tests

transmitter: transmitter,
resultCode: FunctionsResponse.FulfillResult.USER_CALLBACK_ERROR,
response: response,
err: err,
callbackReturnData: new bytes(0)
});

vm.recordLogs();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we doing vm.recordLogs() here? Do we need to do this if we are already doing vm.expectEmit()? I don't see any subsequent vm.getRecordedLogs() calls

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be here. I'll remove it in the next one on the rebase


(FunctionsResponse.FulfillResult resultCode, uint96 callbackGasCostJuels) = s_functionsRouter.fulfill(
response,
err,
juelsPerGas,
costWithoutCallback,
transmitter,
commitment
);

assertEq(uint(resultCode), uint(FunctionsResponse.FulfillResult.USER_CALLBACK_ERROR));
assertEq(callbackGasCostJuels, 0);
}

function test_Fulfill_SuccessFulfilled() public {
// Send as committed Coordinator
vm.stopPrank();
Expand Down
18 changes: 18 additions & 0 deletions contracts/src/v0.8/functions/tests/1_0_0/FunctionsSubscriptions.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,24 @@ contract FunctionsSubscriptions_AddConsumer is FunctionsSubscriptionSetup {
s_functionsRouter.addConsumer(s_subscriptionId, address(3));
}

function test_AddConsumer_RevertIfMaximumConsumersAfterConfigUpdate() public {
// Fill Consumers to s_maxConsumersPerSubscription
// Already has one from setup
s_functionsRouter.addConsumer(s_subscriptionId, address(1));
s_functionsRouter.addConsumer(s_subscriptionId, address(2));

// Lower maxConsumersPerSubscription
s_maxConsumersPerSubscription = 1;
FunctionsRouter.Config memory newRouterConfig = getRouterConfig();
s_functionsRouter.updateConfig(newRouterConfig);

// .AddConsumer should still revert
vm.expectRevert(
abi.encodeWithSelector(FunctionsSubscriptions.TooManyConsumers.selector, s_maxConsumersPerSubscription)
);
s_functionsRouter.addConsumer(s_subscriptionId, address(3));
}

event SubscriptionConsumerAdded(uint64 indexed subscriptionId, address consumer);

function test_AddConsumer_Success() public {
Expand Down
131 changes: 131 additions & 0 deletions contracts/src/v0.8/functions/tests/1_0_0/Setup.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,134 @@ contract FunctionsFulfillmentSetup is FunctionsClientRequestSetup {
vm.startPrank(OWNER_ADDRESS);
}
}

contract FunctionsMultipleFulfillmentsSetup is FunctionsFulfillmentSetup {
bytes32 s_requestId2;
FunctionsResponse.Commitment s_requestCommitment2;
bytes32 s_requestId3;
FunctionsResponse.Commitment s_requestCommitment3;

function setUp() public virtual override {
FunctionsFulfillmentSetup.setUp();

// Make 2 additional requests (1 already complete)

// *** Request #2 ***
vm.recordLogs();
s_requestId2 = s_functionsClient.sendRequest(
s_donId,
"return 'hello world';",
new bytes(0),
new string[](0),
new bytes[](0),
s_subscriptionId,
5500
);

// Get commitment data from OracleRequest event log
Vm.Log[] memory entriesAfterRequest2 = vm.getRecordedLogs();
(, , , , , , , FunctionsResponse.Commitment memory commitment2) = abi.decode(
entriesAfterRequest2[0].data,
(address, uint64, address, bytes, uint16, bytes32, uint64, FunctionsResponse.Commitment)
);
s_requestCommitment2 = commitment2;

// Transmit as transmitter 2
vm.stopPrank();
vm.startPrank(NOP_TRANSMITTER_ADDRESS_2);

// Build report
bytes32[] memory requestIds2 = new bytes32[](1);
requestIds2[0] = s_requestId2;
bytes[] memory results2 = new bytes[](1);
results2[0] = bytes("hello world!");
bytes[] memory errors2 = new bytes[](1);
// No error
bytes[] memory onchainMetadata2 = new bytes[](1);
onchainMetadata2[0] = abi.encode(s_requestCommitment2);
bytes[] memory offchainMetadata2 = new bytes[](1);
// No offchain metadata
bytes memory report2 = abi.encode(requestIds2, results2, errors2, onchainMetadata2, offchainMetadata2);

// Build signers
address[31] memory signers2;
signers2[0] = NOP_SIGNER_ADDRESS_2;

// Send report
vm.recordLogs();
s_functionsCoordinator.callReportWithSigners(report2, signers2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can that work with only one address in signers2 array? Don't we always need 3? I'm confused.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way this is working is bypassing the OCR validation, because the testhelper calls _report directly, rather than transmit.
I'll go back in and make it run through all that for real next week.


// Get actual cost from RequestProcessed event log
Vm.Log[] memory entriesAfterFulfill2 = vm.getRecordedLogs();
(uint96 totalCostJuels2, , , , , ) = abi.decode(
entriesAfterFulfill2[2].data,
(uint96, address, FunctionsResponse.FulfillResult, bytes, bytes, bytes)
);
// totalCostJuels = costWithoutCallbackJuels + adminFee + callbackGasCostJuels
s_fulfillmentCoordinatorBalance += totalCostJuels2 - s_adminFee;
s_fulfillmentRouterOwnerBalance += s_adminFee;

// Return prank to Owner
vm.stopPrank();
vm.startPrank(OWNER_ADDRESS);

// *** Request #3 ***
vm.recordLogs();
s_requestId3 = s_functionsClient.sendRequest(
s_donId,
"return 'hello world';",
new bytes(0),
new string[](0),
new bytes[](0),
s_subscriptionId,
5500
);

// Get commitment data from OracleRequest event log
Vm.Log[] memory entriesAfterRequest3 = vm.getRecordedLogs();
(, , , , , , , FunctionsResponse.Commitment memory commitment3) = abi.decode(
entriesAfterRequest3[0].data,
(address, uint64, address, bytes, uint16, bytes32, uint64, FunctionsResponse.Commitment)
);
s_requestCommitment3 = commitment3;

// Transmit as transmitter 3
vm.stopPrank();
vm.startPrank(NOP_TRANSMITTER_ADDRESS_3);

// Build report
bytes32[] memory requestIds3 = new bytes32[](1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has a bunch of duplicate code, should be extracted into a helper.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, went for speed. I'll refactor later

requestIds3[0] = s_requestId3;
bytes[] memory results3 = new bytes[](1);
results3[0] = bytes("hello world!");
bytes[] memory errors3 = new bytes[](1);
// No error
bytes[] memory onchainMetadata3 = new bytes[](1);
onchainMetadata3[0] = abi.encode(s_requestCommitment3);
bytes[] memory offchainMetadata3 = new bytes[](1);
// No offchain metadata
bytes memory report3 = abi.encode(requestIds3, results3, errors3, onchainMetadata3, offchainMetadata3);

// Build signers
address[31] memory signers3;
signers3[0] = NOP_SIGNER_ADDRESS_3;

// Send report
vm.recordLogs();
s_functionsCoordinator.callReportWithSigners(report3, signers3);

// Get actual cost from RequestProcessed event log
Vm.Log[] memory entriesAfterFulfill3 = vm.getRecordedLogs();
(uint96 totalCostJuels3, , , , , ) = abi.decode(
entriesAfterFulfill3[2].data,
(uint96, address, FunctionsResponse.FulfillResult, bytes, bytes, bytes)
);

// totalCostJuels = costWithoutCallbackJuels + adminFee + callbackGasCostJuels
s_fulfillmentCoordinatorBalance += totalCostJuels3 - s_adminFee;

// Return prank to Owner
vm.stopPrank();
vm.startPrank(OWNER_ADDRESS);
}
}
2 changes: 1 addition & 1 deletion core/gethwrappers/functions/generated/functions_coordinator/functions_coordinator.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion core/gethwrappers/functions/generated/functions_router/functions_router.go
View file
Open in desktop

Large diffs are not rendered by default.

Loading
Loading