diff --git a/contracts/gas-snapshots/functions.gas-snapshot b/contracts/gas-snapshots/functions.gas-snapshot index 9e88da0d044..9b63bbc7e88 100644 --- a/contracts/gas-snapshots/functions.gas-snapshot +++ b/contracts/gas-snapshots/functions.gas-snapshot @@ -83,7 +83,7 @@ FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOw FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOwnerTransfer_RevertIfPaused() (gas: 60962) FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOwnerTransfer_RevertIfSenderBecomesBlocked() (gas: 94675) FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOwnerTransfer_RevertIfSenderIsNotNewOwner() (gas: 62691) -FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOwnerTransfer_Success() (gas: 59679) +FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer:test_AcceptSubscriptionOwnerTransfer_Success() (gas: 214258) FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfMaximumConsumers() (gas: 137833) FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfMaximumConsumersAfterConfigUpdate() (gas: 164777) FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfNoSubscription() (gas: 12926) @@ -102,20 +102,21 @@ FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_SuccessRecieve FunctionsSubscriptions_Constructor:test_Constructor_Success() (gas: 7654) FunctionsSubscriptions_CreateSubscriptionWithConsumer:test_CreateSubscriptionWithConsumer_RevertIfNotAllowedSender() (gas: 28637) FunctionsSubscriptions_CreateSubscriptionWithConsumer:test_CreateSubscriptionWithConsumer_RevertIfPaused() (gas: 17948) -FunctionsSubscriptions_CreateSubscriptionWithConsumer:test_CreateSubscriptionWithConsumer_Success() (gas: 371980) +FunctionsSubscriptions_CreateSubscriptionWithConsumer:test_CreateSubscriptionWithConsumer_Success() (gas: 351723) FunctionsSubscriptions_GetConsumer:test_GetConsumer_Success() (gas: 16225) -FunctionsSubscriptions_GetFlags:test_GetFlags_Success() (gas: 40858) +FunctionsSubscriptions_GetFlags:test_GetFlags_SuccessInvalidSubscription() (gas: 13100) +FunctionsSubscriptions_GetFlags:test_GetFlags_SuccessValidSubscription() (gas: 40858) FunctionsSubscriptions_GetSubscription:test_GetSubscription_Success() (gas: 30959) FunctionsSubscriptions_GetSubscriptionCount:test_GetSubscriptionCount_Success() (gas: 12967) FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_RevertIfEndIsAfterLastSubscription() (gas: 16523) FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_RevertIfStartIsAfterEnd() (gas: 13436) FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_Success() (gas: 59568) FunctionsSubscriptions_GetTotalBalance:test_GetTotalBalance_Success() (gas: 15032) -FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoCalldata() (gas: 27594) -FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoSubscription() (gas: 30071) -FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNotLink() (gas: 13402) -FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfPaused() (gas: 35000) -FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_Success() (gas: 56279) +FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoCalldata(uint96) (runs: 256, μ: 28401, ~: 28401) +FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoSubscription(uint96) (runs: 256, μ: 30913, ~: 30913) +FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNotLink(uint96) (runs: 256, μ: 14248, ~: 14248) +FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfPaused(uint96) (runs: 256, μ: 35870, ~: 35870) +FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_Success(uint96) (runs: 256, μ: 59685, ~: 59685) FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfAmountMoreThanBalance() (gas: 20745) FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfBalanceInvariant() (gas: 189) FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfNoAmount() (gas: 15638) @@ -129,10 +130,11 @@ FunctionsSubscriptions_OwnerCancelSubscription:test_OwnerCancelSubscription_Succ FunctionsSubscriptions_OwnerCancelSubscription:test_OwnerCancelSubscription_SuccessSubOwnerRefunded() (gas: 50896) FunctionsSubscriptions_OwnerCancelSubscription:test_OwnerCancelSubscription_SuccessWhenRequestInFlight() (gas: 164045) FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_RevertIfAmountMoreThanBalance() (gas: 17924) -FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_RevertIfBalanceInvariant() (gas: 165) -FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_RevertIfNotOwner() (gas: 15555) +FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_RevertIfBalanceInvariant() (gas: 210) +FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_RevertIfNotOwner() (gas: 15533) FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_SuccessIfNoAmount() (gas: 37396) -FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_SuccessPaysRecipient() (gas: 54435) +FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_SuccessIfRecipientAddressZero() (gas: 52130) +FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_SuccessPaysRecipient() (gas: 54413) FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_SuccessSetsBalanceToZero() (gas: 37790) FunctionsSubscriptions_PendingRequestExists:test_PendingRequestExists_SuccessFalseIfNoPendingRequests() (gas: 15025) FunctionsSubscriptions_PendingRequestExists:test_PendingRequestExists_SuccessTrueIfPendingRequests() (gas: 175579) @@ -142,9 +144,10 @@ FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscription FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscriptionOwnerTransfer_RevertIfNotAllowedSender() (gas: 75130) FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscriptionOwnerTransfer_RevertIfNotSubscriptionOwner() (gas: 17959) FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscriptionOwnerTransfer_RevertIfPaused() (gas: 20104) -FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscriptionOwnerTransfer_Success() (gas: 68216) +FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscriptionOwnerTransfer_Success() (gas: 68217) +FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscriptionOwnerTransfer_SuccessChangeProposedOwner() (gas: 82791) FunctionsSubscriptions_RecoverFunds:test_OwnerCancelSubscription_RevertIfNotOwner() (gas: 15532) -FunctionsSubscriptions_RecoverFunds:test_RecoverFunds_Success() (gas: 41093) +FunctionsSubscriptions_RecoverFunds:test_RecoverFunds_Success(uint64) (runs: 256, μ: 41699, ~: 41704) FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfInvalidConsumer() (gas: 30238) FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfNoSubscription() (gas: 14997) FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfNotAllowedSender() (gas: 57778) @@ -162,15 +165,16 @@ FunctionsSubscriptions_TimeoutRequests:test_TimeoutRequests_Success() (gas: 5775 FunctionsSubscriptions_createSubscription:test_CreateSubscription_RevertIfNotAllowedSender() (gas: 26368) FunctionsSubscriptions_createSubscription:test_CreateSubscription_RevertIfPaused() (gas: 15714) FunctionsSubscriptions_createSubscription:test_CreateSubscription_Success() (gas: 152510) +FunctionsTermsOfServiceAllowList_AcceptTermsOfService:testAcceptTermsOfService_InvalidSigner_vuln() (gas: 94815) FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfAcceptorIsNotSender() (gas: 25837) FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfBlockedSender() (gas: 44348) FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfInvalidSigner() (gas: 23597) FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfRecipientContractIsNotSender() (gas: 1866530) FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfRecipientIsNotSender() (gas: 26003) -FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_SuccessIfAcceptingForContract() (gas: 1946649) -FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_SuccessIfAcceptingForSelf() (gas: 94684) +FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_SuccessIfAcceptingForContract() (gas: 1946591) +FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_SuccessIfAcceptingForSelf() (gas: 104851) FunctionsTermsOfServiceAllowList_BlockSender:test_BlockSender_RevertIfNotOwner() (gas: 15469) -FunctionsTermsOfServiceAllowList_BlockSender:test_BlockSender_Success() (gas: 50421) +FunctionsTermsOfServiceAllowList_BlockSender:test_BlockSender_Success() (gas: 51794) FunctionsTermsOfServiceAllowList_Constructor:test_Constructor_Success() (gas: 12187) FunctionsTermsOfServiceAllowList_GetAllAllowedSenders:test_GetAllAllowedSenders_Success() (gas: 19243) FunctionsTermsOfServiceAllowList_GetConfig:test_GetConfig_Success() (gas: 15773) diff --git a/contracts/src/v0.8/functions/tests/v1_X/FunctionsSubscriptions.t.sol b/contracts/src/v0.8/functions/tests/v1_X/FunctionsSubscriptions.t.sol index 93203b02be8..8046bf7d939 100644 --- a/contracts/src/v0.8/functions/tests/v1_X/FunctionsSubscriptions.t.sol +++ b/contracts/src/v0.8/functions/tests/v1_X/FunctionsSubscriptions.t.sol @@ -95,7 +95,7 @@ contract FunctionsSubscriptions_OwnerCancelSubscription is FunctionsSubscription string[] memory args = new string[](0); bytes[] memory bytesArgs = new bytes[](0); - s_functionsClient.sendRequest(s_donId, sourceCode, secrets, args, bytesArgs, s_subscriptionId, 5000); + s_functionsClient.sendRequest(s_donId, sourceCode, secrets, args, bytesArgs, s_subscriptionId, 5500); s_functionsRouter.ownerCancelSubscription(s_subscriptionId); } @@ -130,8 +130,12 @@ contract FunctionsSubscriptions_OwnerCancelSubscription is FunctionsSubscription contract FunctionsSubscriptions_RecoverFunds is FunctionsRouterSetup { event FundsRecovered(address to, uint256 amount); - function test_RecoverFunds_Success() public { - uint256 fundsTransferred = 1 * 1e18; // 1 LINK + function test_RecoverFunds_Success(uint64 fundsTransferred) public { + //amount must be less than LINK total supply + vm.assume(fundsTransferred < 1_000_000_000 * 1e18); + vm.assume(fundsTransferred > 0); + + // uint256 fundsTransferred = 1 * 1e18; // 1 LINK s_linkToken.transfer(address(s_functionsRouter), fundsTransferred); // topic0 (function signature, always checked), NOT topic1 (false), NOT topic2 (false), NOT topic3 (false), and data (true). @@ -268,6 +272,14 @@ contract FunctionsSubscriptions_OwnerWithdraw is FunctionsFulfillmentSetup { // s_functionsRouter.ownerWithdraw(OWNER_ADDRESS, amountToWithdraw); } + function test_OwnerWithdraw_SuccessIfRecipientAddressZero() public { + uint256 balanceBefore = s_linkToken.balanceOf(address(0)); + uint96 amountToWithdraw = s_fulfillmentRouterOwnerBalance; + s_functionsRouter.ownerWithdraw(address(0), amountToWithdraw); + uint256 balanceAfter = s_linkToken.balanceOf(address(0)); + assertEq(balanceBefore + s_fulfillmentRouterOwnerBalance, balanceAfter); + } + function test_OwnerWithdraw_SuccessIfNoAmount() public { uint256 balanceBefore = s_linkToken.balanceOf(OWNER_ADDRESS); uint96 amountToWithdraw = 0; @@ -298,35 +310,53 @@ contract FunctionsSubscriptions_OwnerWithdraw is FunctionsFulfillmentSetup { /// @notice #onTokenTransfer contract FunctionsSubscriptions_OnTokenTransfer is FunctionsSubscriptionSetup { - function test_OnTokenTransfer_RevertIfPaused() public { + function test_OnTokenTransfer_RevertIfPaused(uint96 fundingAmount) public { + // Funding amount must be less than LINK total supply + vm.assume(fundingAmount < 1_000_000_000 * 1e18); + vm.assume(fundingAmount > 0); + s_functionsRouter.pause(); vm.expectRevert("Pausable: paused"); - uint96 fundingAmount = 100; s_linkToken.transferAndCall(address(s_functionsRouter), fundingAmount, abi.encode(s_subscriptionId)); } - function test_OnTokenTransfer_RevertIfCallerIsNotLink() public { + function test_OnTokenTransfer_RevertIfCallerIsNotLink(uint96 fundingAmount) public { + // Funding amount must be less than LINK total supply + vm.assume(fundingAmount < 1_000_000_000 * 1e18); + vm.assume(fundingAmount > 0); + vm.expectRevert(FunctionsSubscriptions.OnlyCallableFromLink.selector); - uint96 fundingAmount = 100; s_functionsRouter.onTokenTransfer(address(s_functionsRouter), fundingAmount, abi.encode(s_subscriptionId)); } - function test_OnTokenTransfer_RevertIfCallerIsNoCalldata() public { + function test_OnTokenTransfer_RevertIfCallerIsNoCalldata(uint96 fundingAmount) public { + // Funding amount must be less than LINK total supply + vm.assume(fundingAmount < 1_000_000_000 * 1e18); + vm.assume(fundingAmount > 0); + vm.expectRevert(FunctionsSubscriptions.InvalidCalldata.selector); - uint96 fundingAmount = 100; s_linkToken.transferAndCall(address(s_functionsRouter), fundingAmount, new bytes(0)); } - function test_OnTokenTransfer_RevertIfCallerIsNoSubscription() public { + function test_OnTokenTransfer_RevertIfCallerIsNoSubscription(uint96 fundingAmount) public { + // Funding amount must be less than LINK total supply + vm.assume(fundingAmount < 1_000_000_000 * 1e18); + vm.assume(fundingAmount > 0); + vm.expectRevert(FunctionsSubscriptions.InvalidSubscription.selector); - uint96 fundingAmount = 100; uint64 invalidSubscriptionId = 123456789; s_linkToken.transferAndCall(address(s_functionsRouter), fundingAmount, abi.encode(invalidSubscriptionId)); } - function test_OnTokenTransfer_Success() public { - uint96 fundingAmount = 100; + function test_OnTokenTransfer_Success(uint96 fundingAmount) public { uint96 subscriptionBalanceBefore = s_functionsRouter.getSubscription(s_subscriptionId).balance; + + // Funding amount must be less than LINK total supply + uint96 TOTAL_LINK = 1_000_000_000 * 1e18; + // Some of the total supply is already in the subscription account + vm.assume(fundingAmount < TOTAL_LINK - subscriptionBalanceBefore); + vm.assume(fundingAmount > 0); + s_linkToken.transferAndCall(address(s_functionsRouter), fundingAmount, abi.encode(s_subscriptionId)); uint96 subscriptionBalanceAfter = s_functionsRouter.getSubscription(s_subscriptionId).balance; assertEq(subscriptionBalanceBefore + fundingAmount, subscriptionBalanceAfter); @@ -541,13 +571,14 @@ contract FunctionsSubscriptions_CreateSubscriptionWithConsumer is FunctionsClien assertEq(firstCallSubscriptionId, 1); assertEq(s_functionsRouter.getSubscription(firstCallSubscriptionId).consumers[0], address(s_functionsClient)); + // Consumer can be address(0) vm.expectEmit(checkTopic1, checkTopic2, checkTopic3, checkData); emit SubscriptionCreated(2, OWNER_ADDRESS); vm.expectEmit(checkTopic1, checkTopic2, checkTopic3, checkData); - emit SubscriptionConsumerAdded(2, address(s_functionsClient)); - uint64 secondCallSubscriptionId = s_functionsRouter.createSubscriptionWithConsumer(address(s_functionsClient)); + emit SubscriptionConsumerAdded(2, address(0)); + uint64 secondCallSubscriptionId = s_functionsRouter.createSubscriptionWithConsumer(address(0)); assertEq(secondCallSubscriptionId, 2); - assertEq(s_functionsRouter.getSubscription(secondCallSubscriptionId).consumers[0], address(s_functionsClient)); + assertEq(s_functionsRouter.getSubscription(secondCallSubscriptionId).consumers[0], address(0)); vm.expectEmit(checkTopic1, checkTopic2, checkTopic3, checkData); emit SubscriptionCreated(3, OWNER_ADDRESS); @@ -579,13 +610,15 @@ contract FunctionsSubscriptions_CreateSubscriptionWithConsumer is FunctionsClien contract FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer is FunctionsSubscriptionSetup { uint256 internal NEW_OWNER_PRIVATE_KEY_WITH_TOS = 0x3; address internal NEW_OWNER_ADDRESS_WITH_TOS = vm.addr(NEW_OWNER_PRIVATE_KEY_WITH_TOS); - uint256 internal NEW_OWNER_PRIVATE_KEY_WITHOUT_TOS = 0x4; + uint256 internal NEW_OWNER_PRIVATE_KEY_WITH_TOS2 = 0x4; + address internal NEW_OWNER_ADDRESS_WITH_TOS2 = vm.addr(NEW_OWNER_PRIVATE_KEY_WITH_TOS2); + uint256 internal NEW_OWNER_PRIVATE_KEY_WITHOUT_TOS = 0x5; address internal NEW_OWNER_ADDRESS_WITHOUT_TOS = vm.addr(NEW_OWNER_PRIVATE_KEY_WITHOUT_TOS); function setUp() public virtual override { FunctionsSubscriptionSetup.setUp(); - // Accept ToS as new owner + // Accept ToS as new owner #1 vm.stopPrank(); vm.startPrank(NEW_OWNER_ADDRESS_WITH_TOS); bytes32 message = s_termsOfServiceAllowList.getMessage(NEW_OWNER_ADDRESS_WITH_TOS, NEW_OWNER_ADDRESS_WITH_TOS); @@ -593,6 +626,20 @@ contract FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer is FunctionsSub (uint8 v, bytes32 r, bytes32 s) = vm.sign(TOS_SIGNER_PRIVATE_KEY, prefixedMessage); s_termsOfServiceAllowList.acceptTermsOfService(NEW_OWNER_ADDRESS_WITH_TOS, NEW_OWNER_ADDRESS_WITH_TOS, r, s, v); + // Accept ToS as new owner #2 + vm.stopPrank(); + vm.startPrank(NEW_OWNER_ADDRESS_WITH_TOS2); + bytes32 message2 = s_termsOfServiceAllowList.getMessage(NEW_OWNER_ADDRESS_WITH_TOS2, NEW_OWNER_ADDRESS_WITH_TOS2); + bytes32 prefixedMessage2 = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", message2)); + (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(TOS_SIGNER_PRIVATE_KEY, prefixedMessage2); + s_termsOfServiceAllowList.acceptTermsOfService( + NEW_OWNER_ADDRESS_WITH_TOS2, + NEW_OWNER_ADDRESS_WITH_TOS2, + r2, + s2, + v2 + ); + vm.stopPrank(); vm.startPrank(OWNER_ADDRESS); } @@ -653,6 +700,24 @@ contract FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer is FunctionsSub s_functionsRouter.proposeSubscriptionOwnerTransfer(s_subscriptionId, NEW_OWNER_ADDRESS_WITH_TOS); assertEq(s_functionsRouter.getSubscription(s_subscriptionId).proposedOwner, NEW_OWNER_ADDRESS_WITH_TOS); } + + function test_ProposeSubscriptionOwnerTransfer_SuccessChangeProposedOwner() public { + // topic0 (function signature, always checked), topic1 (true), NOT topic2 (false), NOT topic3 (false), and data (true). + bool checkTopic1 = true; + bool checkTopic2 = false; + bool checkTopic3 = false; + bool checkData = true; + + vm.expectEmit(checkTopic1, checkTopic2, checkTopic3, checkData); + emit SubscriptionOwnerTransferRequested(s_subscriptionId, OWNER_ADDRESS, NEW_OWNER_ADDRESS_WITH_TOS); + s_functionsRouter.proposeSubscriptionOwnerTransfer(s_subscriptionId, NEW_OWNER_ADDRESS_WITH_TOS); + assertEq(s_functionsRouter.getSubscription(s_subscriptionId).proposedOwner, NEW_OWNER_ADDRESS_WITH_TOS); + + vm.expectEmit(checkTopic1, checkTopic2, checkTopic3, checkData); + emit SubscriptionOwnerTransferRequested(s_subscriptionId, OWNER_ADDRESS, NEW_OWNER_ADDRESS_WITH_TOS2); + s_functionsRouter.proposeSubscriptionOwnerTransfer(s_subscriptionId, NEW_OWNER_ADDRESS_WITH_TOS2); + assertEq(s_functionsRouter.getSubscription(s_subscriptionId).proposedOwner, NEW_OWNER_ADDRESS_WITH_TOS2); + } } /// @notice #acceptSubscriptionOwnerTransfer @@ -735,6 +800,13 @@ contract FunctionsSubscriptions_AcceptSubscriptionOwnerTransfer is FunctionsSubs event SubscriptionOwnerTransferred(uint64 indexed subscriptionId, address from, address to); function test_AcceptSubscriptionOwnerTransfer_Success() public { + // Can transfer ownership with a pending request + string memory sourceCode = "return 'hello world';"; + bytes memory secrets; + string[] memory args = new string[](0); + bytes[] memory bytesArgs = new bytes[](0); + s_functionsClient.sendRequest(s_donId, sourceCode, secrets, args, bytesArgs, s_subscriptionId, 5500); + s_functionsRouter.proposeSubscriptionOwnerTransfer(s_subscriptionId, NEW_OWNER_ADDRESS_WITH_TOS); // Send as new owner, who has accepted Terms of Service @@ -1184,7 +1256,18 @@ contract FunctionsSubscriptions_SetFlags is FunctionsSubscriptionSetup { /// @notice #getFlags contract FunctionsSubscriptions_GetFlags is FunctionsSubscriptionSetup { - function test_GetFlags_Success() public { + function test_GetFlags_SuccessInvalidSubscription() public { + // Send as stranger + vm.stopPrank(); + vm.startPrank(STRANGER_ADDRESS); + + uint64 invalidSubscriptionId = 999999; + + bytes32 flags = s_functionsRouter.getFlags(invalidSubscriptionId); + assertEq(flags, bytes32(0)); + } + + function test_GetFlags_SuccessValidSubscription() public { // Set flags bytes32 flagsToSet = bytes32("1"); s_functionsRouter.setFlags(s_subscriptionId, flagsToSet); diff --git a/contracts/src/v0.8/functions/tests/v1_X/FunctionsTermsOfServiceAllowList.t.sol b/contracts/src/v0.8/functions/tests/v1_X/FunctionsTermsOfServiceAllowList.t.sol index 4ab3daab0aa..450ec48d504 100644 --- a/contracts/src/v0.8/functions/tests/v1_X/FunctionsTermsOfServiceAllowList.t.sol +++ b/contracts/src/v0.8/functions/tests/v1_X/FunctionsTermsOfServiceAllowList.t.sol @@ -154,6 +154,26 @@ contract FunctionsTermsOfServiceAllowList_AcceptTermsOfService is FunctionsRoute s_termsOfServiceAllowList.acceptTermsOfService(STRANGER_ADDRESS, address(s_functionsClientHelper), r, s, v); } + function testAcceptTermsOfService_InvalidSigner_vuln() public { + // Set the signer as the zero address + TermsOfServiceAllowList.Config memory allowListConfig; + allowListConfig.enabled = true; + allowListConfig.signerPublicKey = address(0); + s_termsOfServiceAllowList.updateConfig(allowListConfig); + + // Provide garbage data (v cannot be 29) to generate an invalid signature + uint8 v = 29; + bytes32 r = 0x0101010000000000000000000000000000000000000000000000000000000000; + bytes32 s = 0x0101010000000000000000000000000000000000000000000000000000000000; + + // Expect a revert on invalid signature but the call is successful + vm.stopPrank(); + vm.startPrank(STRANGER_ADDRESS); + // vm.expectRevert(TermsOfServiceAllowList.InvalidSignature.selector); + // TODO: Add validation to setConfig to prevent empty signer + s_termsOfServiceAllowList.acceptTermsOfService(STRANGER_ADDRESS, STRANGER_ADDRESS, r, s, v); + } + event AddedAccess(address user); function test_AcceptTermsOfService_SuccessIfAcceptingForSelf() public { @@ -175,7 +195,14 @@ contract FunctionsTermsOfServiceAllowList_AcceptTermsOfService is FunctionsRoute s_termsOfServiceAllowList.acceptTermsOfService(STRANGER_ADDRESS, STRANGER_ADDRESS, r, s, v); - assertEq(s_termsOfServiceAllowList.hasAccess(STRANGER_ADDRESS, new bytes(0)), true); + assertTrue(s_termsOfServiceAllowList.hasAccess(STRANGER_ADDRESS, new bytes(0))); + + // Event emitted even though adding existing item into EnumerableSet set does nothing + // TODO: handle differently in contract + vm.expectEmit(checkTopic1, checkTopic2, checkTopic3, checkData); + emit AddedAccess(STRANGER_ADDRESS); + s_termsOfServiceAllowList.acceptTermsOfService(STRANGER_ADDRESS, STRANGER_ADDRESS, r, s, v); + assertTrue(s_termsOfServiceAllowList.hasAccess(STRANGER_ADDRESS, new bytes(0))); } function test_AcceptTermsOfService_SuccessIfAcceptingForContract() public { @@ -279,6 +306,8 @@ contract FunctionsTermsOfServiceAllowList_BlockSender is FunctionsRoutesSetup { event BlockedAccess(address user); function test_BlockSender_Success() public { + assertFalse(s_termsOfServiceAllowList.isBlockedSender(STRANGER_ADDRESS)); + // topic0 (function signature, always checked), NOT topic1 (false), NOT topic2 (false), NOT topic3 (false), and data (true). bool checkTopic1 = false; bool checkTopic2 = false; @@ -288,8 +317,8 @@ contract FunctionsTermsOfServiceAllowList_BlockSender is FunctionsRoutesSetup { emit BlockedAccess(STRANGER_ADDRESS); s_termsOfServiceAllowList.blockSender(STRANGER_ADDRESS); - assertEq(s_termsOfServiceAllowList.hasAccess(STRANGER_ADDRESS, new bytes(0)), false); - assertEq(s_termsOfServiceAllowList.isBlockedSender(STRANGER_ADDRESS), true); + assertFalse(s_termsOfServiceAllowList.hasAccess(STRANGER_ADDRESS, new bytes(0))); + assertTrue(s_termsOfServiceAllowList.isBlockedSender(STRANGER_ADDRESS)); // Account can no longer accept Terms of Service bytes32 message = s_termsOfServiceAllowList.getMessage(STRANGER_ADDRESS, STRANGER_ADDRESS);