Omniscia AllianceBlock Audit

MultiWithDefaultProviderSelector Code Style Findings

MultiWithDefaultProviderSelector Code Style Findings

MWD-01C: Inefficient Configuration of Default Provider

Description:

A likely scenario involving the configuration of the default provider is swapping between already introduced providers.

In such a case, the system will inefficiently "re-add" the newly configured default provider even if they are already authorized.

Example:

contracts/MultiWithDefaultProviderSelector.sol
72/**
73 * @dev Configures the providers info.
74 * @param configData The configuration data.
75 * - It should be encoded using the abi.encode function.
76 * - The data should be a struct of type ConfigCallParamsV2.
77 * - The struct should contain the following fields:
78 * - - action: The action to be performed. It can be AddProvider, RemoveProvider or UpdateDefaultProvider.
79 * - - providerAddress: The address of the provider.
80 * @notice The function can only be called by the teleport contract.
81 */
82function config(bytes calldata configData) external override onlyTeleport {
83 ConfigCallParamsV2 memory params = _decodeConfigParams(configData);
84
85 if (params.action == Action.AddProvider) {
86 _addProvider(params.providerAddress);
87 } else if (params.action == Action.RemoveProvider) {
88 _removeProvider(params.providerAddress);
89 } else if (params.action == Action.SetDefaultProvider) {
90 // Set the new provider.
91 // We specifically don't check if the provider is the zero address, as we want to be able to remove the default provider.
92 // The previous default provider will still be kept as a valid provider, to remove it, a separate call with a removeProvider is needed.
93 _currentProvider = params.providerAddress;
94 if (params.providerAddress != address(0)) {
95 _addProvider(params.providerAddress);
96 }
97 }
98}

Recommendation:

We advise the code to invoke the MultiWithDefaultProviderSelector::_addProvider function solely when the new params.providerAddress is not a validProviders entry (i.e. MultiWithDefaultProviderSelector::isValidProvider yields false).

Alleviation (e6f704512a03e960f6cd0802fb70aa284591fe37):

The code was updated per our recommendation, ensuring that a default provided is newly introduced solely when it did not exist in the first place.

MWD-02C: Inefficient Performance of External Call

Description:

The referenced statement will invoke the local MultiWithDefaultProviderSelector::isValidProvider function externally due to utilizing the this syntax.

Example:

contracts/MultiWithDefaultProviderSelector.sol
116if (!this.isValidProvider(provider)) revert ProviderNotFound();

Recommendation:

We advise either the MultiWithDefaultProviderSelector::isValidProvider function to be made public and thus possible to invoke locally, or the validProviders data entry to be consulted directly, either of which we consider an adequate resolution to this exhibit.

Alleviation (e6f704512a03e960f6cd0802fb70aa284591fe37):

The referenced statement directly utilizes the validProviders data entry per the latter of our two recommendations, addressing this exhibit.