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 #195

Open
code423n4 opened this issue Jul 15, 2022 · 3 comments
Open

Gas Optimizations #195

code423n4 opened this issue Jul 15, 2022 · 3 comments
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix)

Comments

@code423n4
Copy link
Contributor

Table of Contents

  • Use Calldata instead of Memory for Read Only Function Parameters
  • Using Elements Smaller than 32 bytes (256 bits) Might Use More Gas

Use Calldata instead of Memory for Read Only Function Parameters

Issue

It is cheaper gas to use calldata than memory if the function parameter is read only.
Calldata is a non-modifiable, non-persistent area where function arguments are stored,
and behaves mostly like memory. More details on following link.
link: https://docs.soliditylang.org/en/v0.8.15/types.html#data-location

PoC

./Creator/ZcToken.sol:31:    constructor(uint8 _protocol, address _underlying, uint256 _maturity, address _cToken, address _redeemer, string memory _name, string memory _symbol, uint8 _decimals) 
./Creator/Creator.sol:36:    string memory n,
./Creator/Creator.sol:37:    string memory s,
./Swivel/Swivel.sol:495:  function setFee(uint16[] memory i, uint16[] memory d) external authorized(admin) returns (bool) {
./Marketplace/MarketPlace.sol:68:    string memory n,
./Marketplace/MarketPlace.sol:69:    string memory s

Mitigation

Change memory to calldata

Using Elements Smaller than 32 bytes (256 bits) Might Use More Gas

Issue

Since EVM operates on 32 bytes at a time, it acctually cost more gas to use elements smaller than 32 bytes.
Reference: https://docs.soliditylang.org/en/v0.8.15/internals/layout_in_storage.html

PoC

./Creator/ZcToken.sol:17:    uint8 public immutable protocol;
./Creator/VaultTracker.sol:26:  uint8 public immutable protocol;
./Creator/Creator.sol:31:    uint8 p,
./Creator/Creator.sol:38:    uint8 d
./Swivel/Swivel.sol:35:  uint16 constant public MIN_FEENOMINATOR = 33;
./Swivel/Swivel.sol:37:  uint16[4] public feenominators;
./Swivel/Swivel.sol:495:  function setFee(uint16[] memory i, uint16[] memory d) external authorized(admin) returns (bool) {
./Marketplace/MarketPlace.sol:22:  mapping (uint8 => bool) public paused;
./Marketplace/MarketPlace.sol:65:    uint8 p,
./Marketplace/MarketPlace.sol:336:  function pause(uint8 p, bool b) external authorized(admin) returns (bool) {
./Marketplace/MarketPlace.sol:346:  modifier unpaused(uint8 p) {
./Marketplace/Compounding.sol:50:  function underlying(uint8 p, address c) internal view returns (address) {
./VaultTracker/VaultTracker.sol:26:  uint8 public immutable protocol;

Mitigation

I suggest using uint256 instead of anything smaller.

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

robrobbins commented Aug 22, 2022

constructor args cannot be calldata, thus that suggestion is incorrect

@robrobbins
Copy link
Collaborator

fixes for the arrays in swivel.setFee (now changeFee) were addressed in a different chore.

calldata has been used where possible otherwise

@robrobbins
Copy link
Collaborator

Swivel-Finance/gost#428

@robrobbins robrobbins added resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) wontfix out of scope, a non-issue, or something already addressed and removed wontfix out of scope, a non-issue, or something already addressed labels Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix)
Projects
None yet
Development

No branches or pull requests

2 participants