Skip to content

Usage of same functions for different actions#

Informational

Throughout the code base there are multiple instances where one function is used to perform different actions. This makes the execution more expensive, since it requires multiple additional parameters and if statements to chose which action to perform. Also, each action in the function can require a different set of parameters.

Example#

In Core.sol there is changeOwnership function used to change either curator address, dao address or canvas admin address. It is used both to propose a new owner and to accept ownership. This makes the function complex and requires additional parameters. Also in case the canvas admin is being changed, canvasId is needed, but if curator is being changed the canvasId parameter is not used. This makes gas consumption for each of these actions higher.

Recommendation#

I would recommend splitting the changeOwnership function, and making a different function for each action. You could make changeCurator, acceptCurator, changeDao, acceptDao, changeCanvasAdmin, acceptCanvasAdmin. This requires 5 more functions than before to be made, but the code is more readable and consumes less gas when changing ownership. Of course this requires changeOwnership function in Canvas.sol to be split into 6 as well.

Other examples#

Create.sol

  • setCreatorsOrRefs can be split into setCreators and setRefs
  • createOrUpdateCanvas can be split into createCanvas, updateCanvas, updateTraits and updateMintPass

Core.sol

  • changeProtocolParameter
  • approveCanvas can be split into curateCanvas and approveCanvas
  • setVrfResult can be split into setVrfResult, setVrfPending and resetVrf

Customize.sol

  • customizeToken can be split into updateTitle, updateSubtitle and updateRefs
  • updateIdentifiers can be split into updateChipIds and updatePurchaseIds

Funds.sol

  • payoutAuctionOrDutchRefund can be split into claimExpiredRefund, payoutAuction and claimDutchRefund