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.

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

(feat): Add ability for Functions ToS migrations #11827

Merged
merged 1 commit into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 25 additions & 25 deletions contracts/gas-snapshots/functions.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumGoerli() (gas: 14859450)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumMainnet() (gas: 14859428)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumSepolia() (gas: 14859444)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseGoerli() (gas: 14870866)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseMainnet() (gas: 14870843)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseSepolia() (gas: 14870815)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismGoerli() (gas: 14870766)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismMainnet() (gas: 14870755)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismSepolia() (gas: 14870799)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumGoerli() (gas: 14860808)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumMainnet() (gas: 14860786)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumSepolia() (gas: 14860802)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseGoerli() (gas: 14872224)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseMainnet() (gas: 14872201)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseSepolia() (gas: 14872173)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismGoerli() (gas: 14872124)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismMainnet() (gas: 14872113)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismSepolia() (gas: 14872157)
FunctionsBilling_Constructor:test_Constructor_Success() (gas: 14812)
FunctionsBilling_DeleteCommitment:test_DeleteCommitment_RevertIfNotRouter() (gas: 13282)
FunctionsBilling_DeleteCommitment:test_DeleteCommitment_Success() (gas: 15897)
Expand Down Expand Up @@ -50,17 +50,17 @@ FunctionsRequest_DEFAULT_BUFFER_SIZE:test_DEFAULT_BUFFER_SIZE() (gas: 246)
FunctionsRequest_EncodeCBOR:test_EncodeCBOR_Success() (gas: 223)
FunctionsRequest_REQUEST_DATA_VERSION:test_REQUEST_DATA_VERSION() (gas: 225)
FunctionsRouter_Constructor:test_Constructor_Success() (gas: 12007)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedCostExceedsCommitment() (gas: 169845)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInsufficientGas() (gas: 160176)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedCostExceedsCommitment() (gas: 169829)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInsufficientGas() (gas: 160160)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInvalidCommitment() (gas: 38115)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInvalidRequestId() (gas: 35238)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedSubscriptionBalanceInvariant() (gas: 178321)
FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedSubscriptionBalanceInvariant() (gas: 178305)
FunctionsRouter_Fulfill:test_Fulfill_RevertIfNotCommittedCoordinator() (gas: 28086)
FunctionsRouter_Fulfill:test_Fulfill_RevertIfPaused() (gas: 153883)
FunctionsRouter_Fulfill:test_Fulfill_SuccessClientNoLongerExists() (gas: 321333)
FunctionsRouter_Fulfill:test_Fulfill_SuccessFulfilled() (gas: 334954)
FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackReverts() (gas: 2510380)
FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackRunsOutOfGas() (gas: 540815)
FunctionsRouter_Fulfill:test_Fulfill_RevertIfPaused() (gas: 153867)
FunctionsRouter_Fulfill:test_Fulfill_SuccessClientNoLongerExists() (gas: 321317)
FunctionsRouter_Fulfill:test_Fulfill_SuccessFulfilled() (gas: 334938)
FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackReverts() (gas: 2510364)
FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackRunsOutOfGas() (gas: 540803)
FunctionsRouter_GetAdminFee:test_GetAdminFee_Success() (gas: 17983)
FunctionsRouter_GetAllowListId:test_GetAllowListId_Success() (gas: 12904)
FunctionsRouter_GetConfig:test_GetConfig_Success() (gas: 37159)
Expand Down Expand Up @@ -165,7 +165,7 @@ FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_SuccessIfRecipientAddres
FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_SuccessPaysRecipient() (gas: 54413)
FunctionsSubscriptions_OwnerWithdraw:test_OwnerWithdraw_SuccessSetsBalanceToZero() (gas: 37790)
FunctionsSubscriptions_PendingRequestExists:test_PendingRequestExists_SuccessFalseIfNoPendingRequests() (gas: 14981)
FunctionsSubscriptions_PendingRequestExists:test_PendingRequestExists_SuccessTrueIfPendingRequests() (gas: 176494)
FunctionsSubscriptions_PendingRequestExists:test_PendingRequestExists_SuccessTrueIfPendingRequests() (gas: 176478)
FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscriptionOwnerTransfer_RevertIfEmptyNewOwner() (gas: 27655)
FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscriptionOwnerTransfer_RevertIfInvalidNewOwner() (gas: 57797)
FunctionsSubscriptions_ProposeSubscriptionOwnerTransfer:test_ProposeSubscriptionOwnerTransfer_RevertIfNoSubscription() (gas: 15001)
Expand All @@ -181,7 +181,7 @@ FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfNoSubscription
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfNotAllowedSender() (gas: 102439)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfNotSubscriptionOwner() (gas: 87245)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfPaused() (gas: 18049)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfPendingRequests() (gas: 191902)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfPendingRequests() (gas: 191894)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_Success() (gas: 42023)
FunctionsSubscriptions_SetFlags:test_SetFlags_RevertIfNoSubscription() (gas: 12891)
FunctionsSubscriptions_SetFlags:test_SetFlags_RevertIfNotOwner() (gas: 15684)
Expand All @@ -193,7 +193,7 @@ FunctionsSubscriptions_TimeoutRequests:test_TimeoutRequests_Success() (gas: 5775
FunctionsSubscriptions_createSubscription:test_CreateSubscription_RevertIfNotAllowedSender() (gas: 26434)
FunctionsSubscriptions_createSubscription:test_CreateSubscription_RevertIfPaused() (gas: 15759)
FunctionsSubscriptions_createSubscription:test_CreateSubscription_Success() (gas: 152708)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:testAcceptTermsOfService_InvalidSigner_vuln() (gas: 94864)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:testAcceptTermsOfService_InvalidSigner_vuln() (gas: 94913)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfAcceptorIsNotSender() (gas: 25859)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfBlockedSender() (gas: 88990)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfInvalidSigner() (gas: 23619)
Expand All @@ -206,25 +206,25 @@ FunctionsTermsOfServiceAllowList_BlockSender:test_BlockSender_Success() (gas: 96
FunctionsTermsOfServiceAllowList_Constructor:test_Constructor_Success() (gas: 12253)
FunctionsTermsOfServiceAllowList_GetAllAllowedSenders:test_GetAllAllowedSenders_Success() (gas: 19199)
FunctionsTermsOfServiceAllowList_GetAllowedSendersCount:test_GetAllowedSendersCount_Success() (gas: 12995)
FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfAllowedSendersIsEmpty() (gas: 12237753)
FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfAllowedSendersIsEmpty() (gas: 12239111)
FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfEndIsAfterLastAllowedSender() (gas: 16571)
FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfStartIsAfterEnd() (gas: 13301)
FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_Success() (gas: 20448)
FunctionsTermsOfServiceAllowList_GetBlockedSendersCount:test_GetBlockedSendersCount_Success() (gas: 12931)
FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_RevertIfAllowedSendersIsEmpty() (gas: 12237757)
FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_RevertIfAllowedSendersIsEmpty() (gas: 12239115)
FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_RevertIfEndIsAfterLastAllowedSender() (gas: 16549)
FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_RevertIfStartIsAfterEnd() (gas: 13367)
FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_Success() (gas: 18493)
FunctionsTermsOfServiceAllowList_GetConfig:test_GetConfig_Success() (gas: 15751)
FunctionsTermsOfServiceAllowList_GetMessage:test_GetMessage_Success() (gas: 11593)
FunctionsTermsOfServiceAllowList_HasAccess:test_HasAccess_FalseWhenEnabled() (gas: 15969)
FunctionsTermsOfServiceAllowList_HasAccess:test_HasAccess_TrueWhenDisabled() (gas: 23496)
FunctionsTermsOfServiceAllowList_HasAccess:test_HasAccess_TrueWhenDisabled() (gas: 23560)
FunctionsTermsOfServiceAllowList_IsBlockedSender:test_IsBlockedSender_SuccessFalse() (gas: 15445)
FunctionsTermsOfServiceAllowList_IsBlockedSender:test_IsBlockedSender_SuccessTrue() (gas: 86643)
FunctionsTermsOfServiceAllowList_UnblockSender:test_UnblockSender_RevertIfNotOwner() (gas: 13502)
FunctionsTermsOfServiceAllowList_UnblockSender:test_UnblockSender_Success() (gas: 96216)
FunctionsTermsOfServiceAllowList_UpdateConfig:test_UpdateConfig_RevertIfNotOwner() (gas: 13749)
FunctionsTermsOfServiceAllowList_UpdateConfig:test_UpdateConfig_Success() (gas: 22073)
FunctionsTermsOfServiceAllowList_UpdateConfig:test_UpdateConfig_RevertIfNotOwner() (gas: 13824)
FunctionsTermsOfServiceAllowList_UpdateConfig:test_UpdateConfig_Success() (gas: 22183)
Gas_AcceptTermsOfService:test_AcceptTermsOfService_Gas() (gas: 84702)
Gas_AddConsumer:test_AddConsumer_Gas() (gas: 79131)
Gas_CreateSubscription:test_CreateSubscription_Gas() (gas: 73419)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,24 @@ contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController,
// | Initialization |
// ================================================================

constructor(TermsOfServiceAllowListConfig memory config) ConfirmedOwner(msg.sender) {
constructor(
TermsOfServiceAllowListConfig memory config,
address[] memory initialAllowedSenders,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we going to run into gas issues if the allowlist gets too large?

Also, what will the EngOps process be for this contract upgrade?

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if a "admin add" feature is a better option where the contract owner can manually add people to the allowlist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be able to just automate all of the process with tooling.

As for size, I was thinking that too. Let me add a unit test to see how many we can reasonably add. I agree that another method will be needed if we need to batch add people.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Repeating from slack thread:
Tested up to 500k addresses via a unit test.
Leaving the unit test out because it does slow down the test suite by 20-30 seconds.

address[] memory initialBlockedSenders
) ConfirmedOwner(msg.sender) {
updateConfig(config);

for (uint256 i = 0; i < initialAllowedSenders.length; ++i) {
s_allowedSenders.add(initialAllowedSenders[i]);
}

for (uint256 j = 0; j < initialBlockedSenders.length; ++j) {
if (s_allowedSenders.contains(initialBlockedSenders[j])) {
// Allowed senders cannot also be blocked
revert InvalidCalldata();
}
s_blockedSenders.add(initialBlockedSenders[j]);
}
}

// ================================================================
Expand Down
8 changes: 7 additions & 1 deletion contracts/src/v0.8/functions/tests/v1_X/Setup.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ contract FunctionsRouterSetup is BaseTest {
getCoordinatorConfig(),
address(s_linkEthFeed)
);
s_termsOfServiceAllowList = new TermsOfServiceAllowList(getTermsOfServiceConfig());
address[] memory initialAllowedSenders;
address[] memory initialBlockedSenders;
s_termsOfServiceAllowList = new TermsOfServiceAllowList(
getTermsOfServiceConfig(),
initialAllowedSenders,
initialBlockedSenders
);
}

function getRouterConfig() public view returns (FunctionsRouter.Config memory) {
Expand Down
4 changes: 3 additions & 1 deletion contracts/test/v0.8/functions/v1/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,11 @@ export function getSetupFactory(): () => {
.connect(roles.defaultAccount)
.deploy(router.address, coordinatorConfig, mockLinkEth.address)

const initialAllowedSenders: string[] = []
const initialBlockedSenders: string[] = []
const accessControl = await factories.accessControlFactory
.connect(roles.defaultAccount)
.deploy(accessControlConfig)
.deploy(accessControlConfig, initialAllowedSenders, initialBlockedSenders)

const client = await factories.clientTestHelperFactory
.connect(roles.consumer)
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
GETH_VERSION: 1.13.8
functions: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsRequest.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsRequest.bin 3c972870b0afeb6d73a29ebb182f24956a2cebb127b21c4f867d1ecf19a762db
functions_allow_list: ../../../contracts/solc/v0.8.19/functions/v1_X/TermsOfServiceAllowList.abi ../../../contracts/solc/v0.8.19/functions/v1_X/TermsOfServiceAllowList.bin 586696a0cacc0e5112bdd6c99535748a3fbf08c9319360aee868831d38a96d7b
functions_allow_list: ../../../contracts/solc/v0.8.19/functions/v1_X/TermsOfServiceAllowList.abi ../../../contracts/solc/v0.8.19/functions/v1_X/TermsOfServiceAllowList.bin 0c2156289e11f884ca6e92bf851192d3917c9094a0a301bcefa61266678d0e57
functions_billing_registry_events_mock: ../../../contracts/solc/v0.8.6/functions/v0_0_0/FunctionsBillingRegistryEventsMock.abi ../../../contracts/solc/v0.8.6/functions/v0_0_0/FunctionsBillingRegistryEventsMock.bin 50deeb883bd9c3729702be335c0388f9d8553bab4be5e26ecacac496a89e2b77
functions_client: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsClient.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsClient.bin 2368f537a04489c720a46733f8596c4fc88a31062ecfa966d05f25dd98608aca
functions_client_example: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsClientExample.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsClientExample.bin abf32e69f268f40e8530eb8d8e96bf310b798a4c0049a58022d9d2fb527b601b
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,9 @@ func StartNewChainWithContracts(t *testing.T, nClients int) (*bind.TransactOpts,
Enabled: false, // TODO: true
SignerPublicKey: proofSignerPublicKey,
}
allowListAddress, _, allowListContract, err := functions_allow_list.DeployTermsOfServiceAllowList(owner, b, allowListConfig)
var initialAllowedSenders []common.Address
var initialBlockedSenders []common.Address
allowListAddress, _, allowListContract, err := functions_allow_list.DeployTermsOfServiceAllowList(owner, b, allowListConfig, initialAllowedSenders, initialBlockedSenders)
require.NoError(t, err)

// Deploy Coordinator contract (matches updateConfig() in FunctionsBilling.sol)
Expand Down
Loading