code-423n4 / 2022-05-factorydao-findings

1 stars 1 forks source link

Gas Optimizations #222

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Gas Report

Table of Contents

Caching storage variables in memory to save gas

IMPACT

Anytime you are reading from storage more than once, it is cheaper in gas cost to cache the variable in memory: a SLOAD cost 100gas, while MLOAD and MSTORE cost 3 gas.

PROOF OF CONCEPT

Instances include:

MerkleDropFactory.sol

scope: addMerkleTree()

MerkleEligibility.sol

scope: addGate()

MerkleResistor.sol

scope: addMerkleTree()

MerkleVesting.sol

scope: addMerkleRoot()

TOOLS USED

Manual Analysis

MITIGATION

cache these storage variables in memory

Calldata instead of memory for RO function parameters

PROBLEM

If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.

Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.

PROOF OF CONCEPT

Instances include:

MerkleDropFactory.sol

scope: withdraw()

MerkleDropFactory.sol:88: bytes32[] memory proof

MerkleEligibility.sol

scope: isEligible()

MerkleEligibility.sol:70: bytes32[] memory proof

scope: passThruGate()

MerkleEligibility.sol:85: bytes32[] memory proof

MerkleIdentity.sol

scope: withdraw()

MerkleIdentity.sol:116: string memory uri, bytes32[] memory addressProof, bytes32[] memory metadataProof

scope: isEligible()

MerkleIdentity.sol:152: bytes32[] memory proof

scope: verifyMetadata()

MerkleIdentity.sol:163: string memory uri, bytes32[] memory proof

MerkleLib.sol

scope: verifyProof()

MerkleLib.sol:17: bytes32[] memory proof

MerkleResistor.sol

scope: initialize()

MerkleResistor.sol:134: bytes32[] memory proof

MerkleVesting.sol

scope: initialize()

MerkleVesting.sol:104: bytes32[] memory proof

PermissionlessBasicPoolFactory.sol

scope: addPool()

PermissionlessBasicPoolFactory.sol:95: uint[] memory rewardsWeiPerSecondPerToken
PermissionlessBasicPoolFactory.sol:99: address[] memory rewardTokenAddresses

VoterID.sol

scope: createIdentityFor()

VoterID.sol:122: string memory uri

scope: setTokenURI()

VoterID.sol:162: string memory uri

scope: safeTransferFrom()

VoterID.sol:215: bytes memory data

TOOLS USED

Manual Analysis

MITIGATION

Replace memory with calldata

Comparison Operators

IMPACT

In the EVM, there is no opcode for >= or <=. When using greater than or equal, two operations are performed: > and =.

Using strict comparison operators hence saves gas

PROOF OF CONCEPT

Instances include:

FixedPricePassThruGate.sol

FixedPricePassThruGate.sol:48

MerkleDropFactory.sol

MerkleDropFactory.sol:90

MerkleIdentity.sol

MerkleIdentity.sol:124

MerkleResistor.sol

MerkleResistor.sol:179

MerkleVesting.sol

MerkleVesting.sol:153

SpeedBumpPriceGate.sol

SpeedBumpPriceGate.sol:67

TOOLS USED

Manual Analysis

MITIGATION

Replace <= with <, and >= with >. Do not forget to increment/decrement the compared variable

However, if 1 is negligible compared to the value of the variable, we can omit the increment.

Constructor parameters should be avoided when possible

IMPACT

Constructor parameters are expensive. The contract deployment will be cheaper in gas if they are hard coded instead of using constructor parameters.

PROOF OF CONCEPT

Instances include:

MerkleEligibility.sol

MerkleEligibility.sol:35 address _gateMaster

MerkleIdentity.sol

MerkleIdentity.sol:52 address _mgmt

PermissionlessBasicPoolFactory.sol

PermissionlessBasicPoolFactory.sol:75 address _globalBeneficiary, uint _globalTaxPerCapita

VoterID.sol

VoterID.sol:108 address ooner, address minter, string memory nomen, string memory symbowl

TOOLS USED

Manual Analysis

MITIGATION

Hardcode the state variable with its initial value instead of writing it during contract deployment with constructor parameters.

Custom Errors

IMPACT

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here

Custom errors are defined using the error statement

PROOF OF CONCEPT

Instances include:

FixedPricePassThruGate.sol

FixedPricePassThruGate.sol:48
FixedPricePassThruGate.sol:54

MerkleDropFactory.sol

MerkleDropFactory.sol:77
MerkleDropFactory.sol:90
MerkleDropFactory.sol:92
MerkleDropFactory.sol:98
MerkleDropFactory.sol:107

MerkleEligibility.sol

MerkleEligibility.sol:86
MerkleEligibility.sol:89

MerkleIdentity.sol

MerkleIdentity.sol:97
MerkleIdentity.sol:124
MerkleIdentity.sol:127

MerkleResistor.sol

MerkleResistor.sol:82
MerkleResistor.sol:83
MerkleResistor.sol:121
MerkleResistor.sol:136
MerkleResistor.sol:138
MerkleResistor.sol:144
MerkleResistor.sol:149
MerkleResistor.sol:171
MerkleResistor.sol:175
MerkleResistor.sol:204

MerkleVesting.sol

MerkleVesting.sol:89
MerkleVesting.sol:106
MerkleVesting.sol:113
MerkleVesting.sol:141
MerkleVesting.sol:145
MerkleVesting.sol:147

PermissionlessBasicPoolFactory.sol

PermissionlessBasicPoolFactory.sol:112
PermissionlessBasicPoolFactory.sol:148
PermissionlessBasicPoolFactory.sol:159
PermissionlessBasicPoolFactory.sol:160
PermissionlessBasicPoolFactory.sol:182
PermissionlessBasicPoolFactory.sol:183
PermissionlessBasicPoolFactory.sol:184
PermissionlessBasicPoolFactory.sol:185
PermissionlessBasicPoolFactory.sol:199
PermissionlessBasicPoolFactory.sol:211
PermissionlessBasicPoolFactory.sol:213
PermissionlessBasicPoolFactory.sol:214
PermissionlessBasicPoolFactory.sol:215
PermissionlessBasicPoolFactory.sol:234
PermissionlessBasicPoolFactory.sol:244
PermissionlessBasicPoolFactory.sol:245
PermissionlessBasicPoolFactory.sol:246
PermissionlessBasicPoolFactory.sol:254
PermissionlessBasicPoolFactory.sol:263
PermissionlessBasicPoolFactory.sol:271
PermissionlessBasicPoolFactory.sol:315
PermissionlessBasicPoolFactory.sol:316

SpeedBumpPriceGate.sol

SpeedBumpPriceGate.sol:67
SpeedBumpPriceGate.sol:80

VoterID.sol

VoterID.sol:123
VoterID.sol:124
VoterID.sol:184
VoterID.sol:199
VoterID.sol:217
VoterID.sol:239
VoterID.sol:240
VoterID.sol:262
VoterID.sol:305
VoterID.sol:306
VoterID.sol:437
VoterID.sol:449
VoterID.sol:450

TOOLS USED

Manual Analysis

MITIGATION

Replace require and revert statements with custom errors.

For instance, in MerkleEligibility.sol:

Replace

require(msg.sender == gateMaster, "Only gatemaster may call this.");

with

if (msg.sender != gateMaster) {
        revert IsNotGateMaster(msg.sender);
}

and define the custom error in the contract

error IsNotGateMaster(address _address);

Default value initialization

IMPACT

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

PROOF OF CONCEPT

Instances include:

MerkleDropFactory.sol

MerkleDropFactory.sol:17: uint public numTrees = 0;

MerkleEligibility.sol

MerkleEligibility.sol:31: uint public numGates = 0;

MerkleLib.sol

MerkleLib.sol:22: uint i = 0;

MerkleResistor.sol

MerkleResistor.sol:24: uint public numTrees = 0;
MerkleResistor.sol:176: uint currentWithdrawal = 0;

MerkleVesting.sol

MerkleVesting.sol:16: uint public numTrees = 0;
MerkleVesting.sol:150: uint currentWithdrawal = 0;

PermissionlessBasicPoolFactory.sol

PermissionlessBasicPoolFactory.sol:115: uint i = 0;
PermissionlessBasicPoolFactory.sol:141: uint i = 0;
PermissionlessBasicPoolFactory.sol:168: uint i = 0;
PermissionlessBasicPoolFactory.sol:224: uint i = 0;
PermissionlessBasicPoolFactory.sol:249: uint i = 0;
PermissionlessBasicPoolFactory.sol:266: uint i = 0;

VoterID.sol

VoterID.sol:69: uint public numIdentities = 0;

TOOLS USED

Manual Analysis

MITIGATION

Remove explicit initialization for default values.

Event emitting of local variable

PROBLEM

When emitting an event, using a local variable instead of a storage variable saves gas.

PROOF OF CONCEPT

Instances include:

MerkleDropFactory.sol

MerkleDropFactory.sol:61

MerkleResistor.sol

MerkleResistor.sol:122

MerkleVesting.sol

MerkleVesting.sol:72

TOOLS USED

Manual Analysis

MITIGATION

The storage variable is read multiple times in the function and it is recommended to cache it into memory, then passing the cached variable in the emit statement, as explained in the cache paragraph

Prefix increments

IMPACT

Prefix increments are cheaper than postfix increments.

PROOF OF CONCEPT

Instances include:

MerkleLib.sol

MerkleLib.sol:22 i += 1

PermissionlessBasicPoolFactory.sol

PermissionlessBasicPoolFactory.sol:115 i++
PermissionlessBasicPoolFactory.sol:141 i++
PermissionlessBasicPoolFactory.sol:168 i++
PermissionlessBasicPoolFactory.sol:224 i++
PermissionlessBasicPoolFactory.sol:249 i++
PermissionlessBasicPoolFactory.sol:266 i++

TOOLS USED

Manual Analysis

MITIGATION

change variable++ to ++variable, and variable += 1 to ++variable.

Unnecessary computation

IMPACT

When emitting an event that includes a new and an old value, it is cheaper in gas to avoid caching the old value in memory. Instead, emit the event, then save the new value in storage.

PROOF OF CONCEPT

Instances include:

VoterID.sol

VoterID.sol:154

TOOLS USED

Manual Analysis

MITIGATION

Replace

address oldOwner = _owner_;
_owner_ = newOwner;
emit OwnerUpdated(oldOwner, newOwner);

with

emit OwnerUpdated(_owner_, newOwner);
_owner_ = newOwner;
illuzen commented 2 years ago

all duplicates