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

1 stars 1 forks source link

Gas Optimizations #264

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Contract Storage

Don't save (e.g. merkle tree) where logs or calldata do the trick.

Affected code

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleDropFactory.sol#L22

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleIdentity.sol#L23

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleResistor.sol#L39

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleVesting.sol#L32

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/PermissionlessBasicPoolFactory.sol#L44

Variable packing

We can save storage slots by safely reducing the size of some variables and rearraging them.

Affected code

MerkleIdentity.sol

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleIdentity.sol#L21

Save 2 storage slots per Merkle tree by changing to:

struct MerkleTree {
    bytes32 metadataMerkleRoot;  // root of merkle tree whose leaves are uri strings to be assigned to minted NFTs
    bytes32 ipfsHash; // ipfs hash of complete uri dataset, as redundancy so that merkle proof remain computable
    address nftAddress; // address of NFT contract to be minted
    address priceGateAddress;  // address price gate contract
    address eligibilityAddress;  // address of eligibility gate contract
    uint32 eligibilityIndex; // enables re-use of eligibility contracts
    uint32 priceIndex; // enables re-use of price gate contracts
}

Eligibility address, index and price index get packed into one slot.

MerkleResistor.sol

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleResistor.sol#L27

Save 2 storage slots per Tranche by changing the struct to:

struct Tranche {
    uint totalCoins; // total coins released after vesting complete
    uint currentCoins; // unclaimed coins remaining in the contract, waiting to be vested
    uint64 startTime; // start time of the vesting schedule
    uint64 endTime;   // end time of the vesting schedule
    uint128 lastWithdrawalTime; // keep track of last time user claimed coins to compute coins owed for this withdrawal
    uint coinsPerSecond;  // how many coins are emitted per second, this value is cached to avoid recomputing it    
}

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleResistor.sol#L37

Save 2 storage slots per MerkleTree by changing to:

struct MerkleTree {
    bytes32 merkleRoot; // merkle root of tree whose leaves are ranges of vesting schedules for each recipient
    bytes32 ipfsHash; // ipfs hash of the entire data set represented by the merkle root, in case our servers go down
    uint tokenBalance; // amount of tokens allocated to this tree (this prevents trees from sharing tokens)
    uint128 minEndTime; // minimum length (offset, not absolute) of vesting schedule in seconds
    uint128 maxEndTime; // maximum length (offset, not absolute) of vesting schedule in seconds
    uint64 pctUpFront; // percent of vested coins that will be available and withdrawn upon initialization
    address tokenAddress; // address of token to be distributed
}

Times get packed into one slot and address + pctUpFront into another.

MerkleVesting.sol

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleVesting.sol#L19

As with the others, save 3 storage slots per Tranche by changing the struct to:

struct Tranche {
    uint totalCoins;  // total number of coins released to an address after vesting is completed
    uint currentCoins; // how many coins are left unclaimed by this address, vested or unvested
    uint64 startTime; // when the vesting schedule is set to start, possibly in the past
    uint64 endTime;  // when the vesting schedule will have released all coins
    uint64 lastWithdrawalTime; // the last time a withdrawal occurred, used to compute unvested coins
    uint64 lockPeriodEndTime; // the first time at which coins may be withdrawn
    uint coinsPerSecond; // an intermediate value cached to reduce gas costs, how many coins released every second
}

PermissionlessBasicPoolFactory.sol

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/PermissionlessBasicPoolFactory.sol#L15

Save 2 storage slots per Receipt by packing the times and the id. Change struct to:

struct Receipt {
    uint128 id;   // primary key
    uint64 timeDeposited;  // the time the deposit was made
    uint64 timeWithdrawn;  // the time the deposit was withdrawn, or 0 if not withdrawn yet
    uint amountDepositedWei;  // amount of tokens originally deposited
    address owner;  // the owner of the deposit
}

Save 4 storage slots per Pool by packing times, id, taxPerCapita and numReceipts:

struct Pool {
    uint32 id; // primary key
    uint32 numReceipts;  // number of receipts issued
    uint64 startTime;  // the time that the pool begins
    uint64 endTime;    // time that the pool ends
    uint16 taxPerCapita;  // portion of rewards that go to the contract creator
    uint[] rewardsWeiPerSecondPerToken; // array of reward rates, this number gets multiplied by time and tokens (not wei) to determine rewards
    uint[] rewardsWeiClaimed;  // bookkeeping of how many rewards have been paid out for each token
    uint[] rewardFunding;  // bookkeeping of how many rewards have been supplied for each token
    uint maximumDepositWei;  // the size of the pool, maximum sum of all deposits
    uint totalDepositsWei;  // current sum of all deposits    
    address depositToken;  // token that user deposits (stakes)
    address excessBeneficiary;  // address that is able to reclaim unused rewards
    address[] rewardTokens;  // array of token contract addresses that stakers will receive as rewards
    mapping (uint => Receipt) receipts;  // mapping of receipt ids to receipt structs
}

Unused Parameter

The initialize function asks for the destination address, but requires it to be the msg.sender.

Recommendation: Remove the parameter and use msg.sender directly.

Affected Code

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleResistor.sol#L134

illuzen commented 2 years ago

Valid, but debatable. Putting it as an event means it has to be accessed from archive node, whereas if it's in state, it can be queried from light node.

gititGoro commented 2 years ago

While the event log vs storage choice is debatable, it's nonetheless a good gas optimization suggestion by the warden.