Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #261

Open
code423n4 opened this issue May 14, 2022 · 1 comment
Open

Gas Optimizations #261

code423n4 opened this issue May 14, 2022 · 1 comment

Comments

@code423n4
Copy link
Contributor

List of contents:

  • Vault struct may be packed further
  • feeRate and protocolUnclaimedFees may be packed in storage
  • Unnecessary bounds checking of provided enum values
  • Use bitwise and instead of modulo 2
  • Some arithmetic can be made unchecked

Vault struct may be packed further

The currentStrike and dutchAuctionReserveStrike fields of the Vault struct current use a full slot each

https://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 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:

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: code-423n4/2021-06-realitycards-findings#9

Use bitwise and instead of modulo 2

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.

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 <= 256 so this won't overflow until about 80 years time.
https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L238

No vault with tokenId == 0 exists:
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#L459

Will 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 uint256 for 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

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels May 14, 2022
code423n4 added a commit that referenced this issue May 14, 2022
@outdoteth
Copy link
Collaborator

high quality report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants