These will both fit in a uint128 allowing them to fit in the same slot.
feeRate and protocolUnclaimedFees may be packed in storage
As we know that feeRate is bounded above by 1e18, it will fit in a uint64, This would leave 192bits for protocolUnclaimedFees - enough for 6.2 * 10^39 Ether - we can then safely pack these values together in the same slot.
Unnecessary bounds checking of provided enum values
On this line we check that the provided enum value is any of the valid enum values for TokenType:
This is unnecessary as if an invalid value for the enum were passed, this would be automatically picked up by solidity and fail before we even reach this line of code.
We often check if a tokenId is odd or even to see if a user is interacting with an option or a vault. As we only need to check the final bit to determine this we don't need all the overhead of the modulo operator.
We can instead just use a bitwise and with 1 (x & 1) to perform the same check for cheaper.
outdoteth
commented
2 years ago
List of contents:
feeRateandprotocolUnclaimedFeesmay be packed in storageVault struct may be packed further
The
currentStrikeanddutchAuctionReserveStrikefields of theVaultstruct current use a full slot eachhttps://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L83-L84
However we know that the greatest value which these can take is
6765e18:https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L92
These will both fit in a
uint128allowing them to fit in the same slot.feeRateandprotocolUnclaimedFeesmay be packed in storageAs we know that
feeRateis bounded above by1e18, it will fit in auint64, This would leave 192bits forprotocolUnclaimedFees- enough for 6.2 * 10^39 Ether - we can then safely pack these values together in the same slot.Unnecessary bounds checking of provided enum values
On this line we check that the provided enum value is any of the valid enum values for
TokenType:https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L171
This is unnecessary as if an invalid value for the enum were passed, this would be automatically picked up by solidity and fail before we even reach this line of code.
See this finding: https://github.com/code-423n4/2021-06-realitycards-findings/issues/9
Use bitwise and instead of modulo 2
We often check if a
tokenIdis odd or even to see if a user is interacting with an option or a vault. As we only need to check the final bit to determine this we don't need all the overhead of the modulo operator.We can instead just use a bitwise and with 1 (
x & 1) to perform the same check for cheaper.See: https://twitter.com/clemlak/status/1521973218864771073?cxt=HHwWgsC9wYWlkZ8qAAAA
Some arithmetic can be made unchecked
In a number of places we perform checked arithmetic where we know the result cannot over/underflow.
vault.durationDays <= 256so this won't overflow until about 80 years time. https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L238No vault with
tokenId == 0exists: https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L265 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L459Will not overflow unless protocol fee is above 100% (which should be prevented): https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L289
The multiplications here will fit nicely in a
uint256for even the largest strike price so checking for overflows is unnecessary: https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L417-L419