Usage of strings increases gas cost#

Informational

Throughout the code base, there are multiple cases where strings are used to choose what action we want to perform. However, these strings can use more memory compared to other types and must be hashed to compare. This adds unnecessary gas cost, when you could use a small uint, bytes or an enum.

Functions that use strings: changeProtocolParameter, changeOwnership, setVrfResult, customizeToken

Example#

In Core.sol there is changeProtocolParameter function that takes string parameter. It uses this string to choose which parameter should be changed.

function changeProtocolParameter(
    address msgSender,
    string memory parameter,
    address addressVal,
    uint256 numberVal,
    string memory stringVal
  ) external {
    Schema.Storage storage ds = canvasStorage();
    if (msgSender != ds.contractInfo.daoAddress) revert NoPermission();

    if (keccak256(bytes(parameter)) == keccak256(bytes('vrf'))) {
      ds.contractInfo.vrfAddress = addressVal;
      emit ContractUpdate();
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('keeper'))) {
      ds.contractInfo.keeperAddress = addressVal;
      emit ContractUpdate();
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('tokenRegistryAddress'))) {
      ds.contractInfo.tokenRegistryAddress = addressVal;
      emit ContractUpdate();
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('licenseRegistryAddress'))) {
      ds.contractInfo.licenseRegistryAddress = addressVal;
      emit ContractUpdate();
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('openCanvas'))) {
      ds.contractInfo.openCanvas = numberVal == 0 ? false : true;
      emit ContractUpdate();
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('openCanvasOne'))) {
      ds.contractInfo.openCanvasOne = numberVal == 0 ? false : true;
      emit ContractUpdate();
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('protocolFeeRecipient'))) {
      ds.contractInfo.protocolFeeRecipient = payable(addressVal);
      emit ContractUpdate();
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('protocolFeeBps'))) {
      if (numberVal > 3000) revert InvalidFee(); // Maximum 30% protocol fee possible
      ds.contractInfo.protocolFeeBps = numberVal;
      emit ContractUpdate();
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('protocolDutchRefundFeeBps'))) {
      if (numberVal > 3000) revert InvalidFee(); // Maximum 30% protocol fee possible
      ds.contractInfo.protocolDutchRefundFeeBps = numberVal;
      emit ContractUpdate();
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('unlockCanvas'))) {
      if (ds.canvas[numberVal].unlockRequest) ds.canvas[numberVal].isImmutable = false;
      emit CanvasUpdate(numberVal);
    } else if (keccak256(bytes(parameter)) == keccak256(bytes('externalBaseUrl'))) {
      ds.externalBaseUrl = stringVal;
    }
}

Recommendation#

Change string to uint or enum. Also consider splitting the function like described in Usage of same functions for different actions.

enum ProtocolParameter{
  vrf,
  keeper,
  tokenRegistryAddress,
  licenseRegistryAddress,
  openCanvas,
  openCanvasOne,
  protocolFeeRecipient,
  protocolFeeBps,
  protocolDutchRefundFeeBps,
  unlockCanvas,
  externalBaseUrl
  }

function changeProtocolParameter(
    address msgSender,
    ProtocolParameter parameter,
    address addressVal,
    uint256 numberVal,
    string memory stringVal
  ) external {
    Schema.Storage storage ds = canvasStorage();
    if (msgSender != ds.contractInfo.daoAddress) revert NoPermission();

    if (parameter == ProtocolParameter.vrf) {
      ds.contractInfo.vrfAddress = addressVal;
      emit ContractUpdate();
    } else if (parameter ==  ProtocolParameter.keeper) {
      ds.contractInfo.keeperAddress = addressVal;
      emit ContractUpdate();
    } else if (parameter ==  ProtocolParameter.tokenRegistryAddress) {
      ds.contractInfo.tokenRegistryAddress = addressVal;
      emit ContractUpdate();
    } else if (parameter == ProtocolParameter.licenseRegistryAddress) {
      ds.contractInfo.licenseRegistryAddress = addressVal;
      emit ContractUpdate();
    } else if (parameter == ProtocolParameter.openCanvas)) {
      ds.contractInfo.openCanvas = numberVal == 0 ? false : true;
      emit ContractUpdate();
    } else if (parameter == ProtocolParameter.openCanvasOne) {
      ds.contractInfo.openCanvasOne = numberVal == 0 ? false : true;
      emit ContractUpdate();
    } else if (parameter == ProtocolParameter.protocolFeeRecipient) {
      ds.contractInfo.protocolFeeRecipient = payable(addressVal);
      emit ContractUpdate();
    } else if (parameter == ProtocolParameter.protocolFeeBps) {
      if (numberVal > 3000) revert InvalidFee(); // Maximum 30% protocol fee possible
      ds.contractInfo.protocolFeeBps = numberVal;
      emit ContractUpdate();
    } else if (parameter == ProtocolParameter.protocolDutchRefundFeeBps) {
      if (numberVal > 3000) revert InvalidFee(); // Maximum 30% protocol fee possible
      ds.contractInfo.protocolDutchRefundFeeBps = numberVal;
      emit ContractUpdate();
    } else if (parameter == ProtocolParameter.unlockCanvas) {
      if (ds.canvas[numberVal].unlockRequest) ds.canvas[numberVal].isImmutable = false;
      emit CanvasUpdate(numberVal);
    } else if (parameter == ProtocolParameter.externalBaseUrl) {
      ds.externalBaseUrl = stringVal;
    }
}

* Notice: When using enums, the values correspond to uint8 values 0, 1, 2 and so on in the order they are defined in. In case that you call the function with an out of range value, a panic exception 0x21 will be generated, like it is described in the documentation.