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

Use short reason strings can save gas #317

Open
code423n4 opened this issue Dec 1, 2021 · 1 comment
Open

Use short reason strings can save gas #317

code423n4 opened this issue Dec 1, 2021 · 1 comment
Labels
bug Something isn't working G (Gas Optimization) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

WatchPug

Vulnerability details

Every reason string takes at least 32 bytes.

Use short reason strings that fits in 32 bytes or it will become more expensive.

Instances include:

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Permissions.sol#L135-L141

  function _notSameBlock() internal {
    require(
      block.number > lastBlock[_msgSender()],
      "Can't carry out actions in the same block"
    );
    lastBlock[_msgSender()] = block.number;
  }

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L223-L223

require(!auction.active, "Cannot claim tokens on an active auction");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L230-L230

require(redemption <= remaining.add(1), "Cannot claim more tokens than available");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L659-L659

require(auction.startingTime > 0, "No auction available for the given id");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/AuctionEscapeHatch.sol#L177-L177

require(!active, "Cannot exit early on an active auction");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/AuctionParticipant.sol#L136-L136

require(_index > replenishingIndex, "Cannot replenishingIndex to old value");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/DAO.sol#L56-L56

require(block.timestamp >= getEpochStartTime(epoch + 1), "Cannot advance epoch until start of new epoch");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/ERC20Permit.sol#L81-L81

require(balance >= value, "ERC20Permit: transfer amount exceeds balance");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/ERC20Permit.sol#L122-L122

require(balance >= value, "ERC20Permit: transfer amount exceeds balance");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Malt.sol#L65-L65

require(_service != address(0), "Cannot use address 0 as transfer service");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/MovingAverage.sol#L412-L412

require(_sampleLength > 0, "Cannot have 0 second sample length");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/RewardSystem/RewardDistributor.sol#L154-L154

require(forfeited <= _globals.declaredBalance, "Cannot forfeit more than declared");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/RewardSystem/RewardDistributor.sol#L275-L275

require(amount <= _globals.declaredBalance, "Can't decrement more than total reward balance");

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Timelock.sol#L118-L121

    require(
      eta >= block.timestamp.add(delay),
      "Timelock::queueTransaction: Estimated execution block must satisfy delay."
    );

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Timelock.sol#L165-L176

    require(
      queuedTransactions[txHash],
      "Timelock::executeTransaction: Transaction hasn't been queued."
    );
    require(
      block.timestamp >= eta,
      "Timelock::executeTransaction: Transaction hasn't surpassed time lock."
    );
    require(
      block.timestamp <= eta.add(gracePeriod),
      "Timelock::executeTransaction: Transaction is stale."
    );

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Timelock.sol#L195-L198

    require(
      success,
      "Timelock::executeTransaction: Transaction execution reverted."
    );
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Dec 1, 2021
code423n4 added a commit that referenced this issue Dec 1, 2021
@0xScotch 0xScotch added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Dec 9, 2021
@GalloDaSballo
Copy link
Collaborator

Finding is Valid, most projects will setup a table of codes to be able to explain their errors, alternatively try to keep the strings short

Notice that the gas savings happen only on deployment as you're paying for cost of storing the string in the smart contracts bytecode

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) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants