Navigation Menu

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

Open
code423n4 opened this issue Mar 18, 2022 · 1 comment
Open

Gas Optimizations #18

code423n4 opened this issue Mar 18, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

unchecked direction can be used at mintLongShortTokens function in PrePOMarket.sol

Target codebase

mintLongShortTokens function in PrePOMarket.sol can use unchecked directory.

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/PrePOMarket.sol#L122

require(_amount > _fee, "Minting amount too small");
_collateral.transferFrom(msg.sender, _treasury, _fee);
_amount -= _fee;

It is obvious that _amount - _fee will not be underflown since it has require(_amount > _fee, ...). Hence, can use unchecked directory,

Proposed change

require(_amount > _fee, "Minting amount too small");
_collateral.transferFrom(msg.sender, _treasury, _fee);
unchecked {
    _amount -= _fee;
}

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
PrePOMarket mintLongShortTokens 198115 198055 -60

Deployments - average gas change

Contract Before After Change
PrePOMarketFactory 4182011 4180271 -1740

unchecked direction can be used at redeem function in PrePOMarket.sol

Target codebase

redeem function in PrePOMarket.sol can use unchecked directory.

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/PrePOMarket.sol#L169

require(_collateralOwed > _fee, "Redemption amount too small");
_collateral.transfer(_treasury, _fee);
_collateralOwed -= _fee;

It is obvious that _collateralOwed - _fee will not be underflown since it has require(_collateralOwed > _fee, ...). Hence, can use unchecked directory,

Proposed change

require(_collateralOwed > _fee, "Redemption amount too small");
_collateral.transfer(_treasury, _fee);
unchecked {
    _collateralOwed -= _fee;
}

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
PrePOMarket mintLongShortTokens 100708 100653 -55

Deployments - average gas change

Contract Before After Change
PrePOMarketFactory 4182011 4180271 -1740

Target codebase

Proposed change

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
PrePOMarket extractRewards 135652 131459 -4193

Deployments - average gas change

Contract Before After Change
ExecutorManager 548814 536945 -11869

Unchecked can be used at deposit function in Collateral.sol

Target codebase

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/Collateral.sol#L73

require(_amountToDeposit > _fee, "Deposit amount too small");
_baseToken.safeTransfer(_treasury, _fee);
_amountToDeposit -= _fee;

_amountToDeposit - _fee will not be underflown since require(_amountToDeposit > _fee, ...).

Proposed change

require(_amountToDeposit > _fee, "Deposit amount too small");
_baseToken.safeTransfer(_treasury, _fee);
unchecked {
    _amountToDeposit -= _fee;
}

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
PrePOMarket extractRewards 135652 131459 -4193

Deployments - average gas change

Contract Before After Change
ExecutorManager 548814 536945 -11869

uint256 _shares does not need to be initialized with 0 at deposit function in Collateral.sol

Target codebase

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/Collateral.sol#L81

uint256 _shares = 0;

Proposed change

uint256 _shares;

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
Collateral deposit 273176 273117 -59

Deployments - average gas change

Contract Before After Change
TestCollateral 3383940 3382344 -1596

unchecked can be used at withdraw function at Collateral.sol

Target codebase

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/Collateral.sol#L171

require(_amountWithdrawn > _fee, "Withdrawal amount too small");
_baseToken.safeTransfer(_treasury, _fee);
_amountWithdrawn -= _fee;

It is obvious that _amountWithdrawn - _fee will not be underflown since it has require(_amountWithdrawn > _fee, ...).

Proposed change

require(_amountWithdrawn > _fee, "Withdrawal amount too small");
_baseToken.safeTransfer(_treasury, _fee);
unchecked {
    _amountWithdrawn -= _fee;
}

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
Collateral withdraw 162848 162788 -60

Deployments - average gas change

Contract Before After Change
TestCollateral 3383940 3382200 -1740

Unchecked can be used at recordDeposit function in CollateralDepositRecord.sol

Target codebase

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/CollateralDepositRecord.sol#L37-L38

require(
    _amount + _globalDepositAmount <= _globalDepositCap,
    "Global deposit cap exceeded"
);
require(
    _amount + _accountToNetDeposit[_sender] <= _accountDepositCap,
    "Account deposit cap exceeded"
);
_globalDepositAmount += _amount;
_accountToNetDeposit[_sender] += _amount;

Both _globalDepositAmount + _amount and _accountToNetDeposit[_sender] + _amount will not be overflown because it checks _amount + _globalDepositAmount <= _globalDepositCap and _amount + _accountToNetDeposit[_sender] <= _accountDepositCap before.

Proposed change

require(
    _amount + _globalDepositAmount <= _globalDepositCap,
    "Global deposit cap exceeded"
);
require(
    _amount + _accountToNetDeposit[_sender] <= _accountDepositCap,
    "Account deposit cap exceeded"
);
unchecked {
    _globalDepositAmount += _amount;
    _accountToNetDeposit[_sender] += _amount;
}

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
CollateralDepositRecord recordDeposit 66022 65298 -724

Deployments - average gas change

Contract Before After Change
CollateralDepositRecord 750519 744047 -6472

Unchecked can be used at recordWithdrawal function in CollateralDepositRecord.sol

Target codebase

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/CollateralDepositRecord.sol#L47
https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/CollateralDepositRecord.sol#L52

if (_globalDepositAmount > _amount) {
    _globalDepositAmount -= _amount;
} else {
    _globalDepositAmount = 0;
}
if (_accountToNetDeposit[_sender] > _amount) {
    _accountToNetDeposit[_sender] -= _amount;
} else {
    _accountToNetDeposit[_sender] = 0;
}

_globalDepositAmount - _amount and _accountToNetDeposit[_sender] - _amount will not be underflown since they are wrapped by the if statement _globalDepositAmount > _amount or _accountToNetDeposit[_sender] > _amount.

Proposed change

if (_globalDepositAmount > _amount) {
    unchecked {
        _globalDepositAmount -= _amount;
    }
} else {
    _globalDepositAmount = 0;
}
if (_accountToNetDeposit[_sender] > _amount) {
    unchecked {
        _accountToNetDeposit[_sender] -= _amount;
    }
} else {
    _accountToNetDeposit[_sender] = 0;
}

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
CollateralDepositRecord recordWithdrawal 33023 32895 -128

Deployments - average gas change

Contract Before After Change
CollateralDepositRecord 750519 736492 -14027

Unchecked can be used at allowAccounts function in AccountAccessController.sol

Target codebase

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/AccountAccessController.sol#L44

for (uint256 _i = 0; _i < _accounts.length; _i++) {
    _allowedAccounts[_allowedAccountsIndex][_accounts[_i]] = true;
    emit AccountAllowed(_accounts[_i]);
}

Proposed change

for (uint256 _i = 0; _i < _accounts.length; ) {
    _allowedAccounts[_allowedAccountsIndex][_accounts[_i]] = true;
    emit AccountAllowed(_accounts[_i]);
    unchecked {
        _i++;
    }
}

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
MockAccountAccessController allowAccounts 115094 114786 -308

Deployments - average gas change

Contract Before After Change
MockAccountAccessController 948869 946721 -2148

Unchecked can be used at blockAccounts function in AccountAccessController.sol

Target codebase

https://github.com/code-423n4/2022-03-prepo/blob/main/contracts/core/AccountAccessController.sol#L55-L58

for (uint256 _i = 0; _i < _accounts.length; _i++) {
    _blockedAccounts[_blockedAccountsIndex][_accounts[_i]] = true;
    emit AccountBlocked(_accounts[_i]);
}

Proposed change

for (uint256 _i = 0; _i < _accounts.length; ) {
    _blockedAccounts[_blockedAccountsIndex][_accounts[_i]] = true;
    emit AccountBlocked(_accounts[_i]);
    unchecked {
        _i++;
    }
}

Gas changes with the proposed changes

Methods - average gas change

Contract Methods Before After Change
MockAccountAccessController blockAccounts 115120 114812 -308

Deployments - average gas change

Contract Before After Change
MockAccountAccessController 948869 946721 -2148
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Mar 18, 2022
code423n4 added a commit that referenced this issue Mar 18, 2022
@ramenforbreakfast
Copy link
Collaborator

Excellent submission, well documented and clearly outlines gas savings.
This report and #5 will be referenced as the source submissions for common optimization duplicates.

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)
Projects
None yet
Development

No branches or pull requests

2 participants