code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

Gas Optimizations #122

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago
++I COSTS LESS GAS COMPARED TO I++ OR I += 1

The demo of the gas compare can be seen here.

Swivel/Swivel.sol

108:      unchecked {i++;}
269:      unchecked {i++;}
417-419:      
      unchecked {
        i++;
      }
510-512:
      unchecked {
        x++;
      }
503-565:
      unchecked {
        i++;
      }

Suggestion: Using ++i and ++x instead of i++ and x++ to increment the value of an uint variable.

X = X + Y IS CHEAPER THAN X += Y

The demo of the gas comparsion can be seen here.

In Swivel.sol, there are multiple. Swivel/Swivel.sol 121, 158, 193, 222, 287, 318, 348, 383:

    filled[hash] += a;

Creator/VaultTracker.sol VaultTracker/VaultTracker.sol These 2 files are the same

67:     vlt.redeemable += interest;
68:     vlt.notional += a;
102:    vlt.redeemable += interest;
103:    vlt.notional -= a;
174:    from.redeemable += interest;
175:    from.notional -= a;
193:    to.redeemable += newVaultInterest;
194:    to.notional += a;
213:    oVault.notional -= a;
230:    sVault.redeemable += interest;
234:    sVault.notional += a;

Creator/ZcToken.sol Tokens/ZcToken.sol These 2 files are the same

115:    allowance[holder][msg.sender] -= previewAmount;
134:    allowance[holder][msg.sender] -= principalAmount;  

Suggestion: Consider use X = X + Y to save gas

USE CALLDATA INSTEAD OF MEMORY

Swivel/Swivel.sol

495:  function setFee(uint16[] memory i, uint16[] memory d) external authorized(admin) returns (bool) {}

When arguments are read-only on external functions, the data location can be calldata.

Swivel/Swivel.sol

495:  function setFee(uint16[] memory i, uint16[] memory d) external authorized(admin) returns (bool) {}

Guaranteed non overflow/underflow can be unchecked

Swivel/Swivel.sol

195-199:
    uint256 premiumFilled = (a * o.premium) / o.principal;

    IErc20 uToken = IErc20(o.underlying);
    // transfer underlying tokens, then take fee
    Safe.transferFrom(uToken, msg.sender, o.maker, a - premiumFilled);

  /// @param a Amount of volume (principal) being filled by the taker's initiate 

premiumFilled must be smaller than a.

291-293:
    uint256 principalFilled = (a * o.principal) / o.premium;
    // transfer underlying from initiating party to exiting party, minus the price the exit party pays for the exit (premium), and the fee.
    Safe.transferFrom(uToken, o.maker, msg.sender, principalFilled - a);

  /// @param a Amount of volume (interest) being filled by the taker's exit

principalFilled must be bigger than a.

358-363:
    uint256 premiumFilled = (a * o.premium) / o.principal;
    Safe.transfer(uToken, o.maker, a - premiumFilled);

    // transfer premium-fee to floating exit party
    uint256 fee = premiumFilled / feenominators[3];
    Safe.transfer(uToken, msg.sender, premiumFilled - fee);

premiumFilled must be smaller than a, and bigger than fee.

388-395:
    uint256 principalFilled = (a * o.principal) / o.premium;

    // ...

    IErc20 uToken = IErc20(o.underlying);
    // transfer principal-premium-fee back to fixed exit party now that the interest coupon and zcb have been redeemed
    uint256 fee = principalFilled / feenominators[1];
    Safe.transfer(uToken, msg.sender, principalFilled - a - fee);

principalFilled must be bigger than sum of a and fee.

Suggestion: Those guaranteed non-underflow can be unchecked to save some gas.

FUNCTIONS GUARANTEED TO REVERT WHEN CALLED BY NORMAL USERS CAN BE MARKED PAYABLE

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Creator/Creator.sol

30-39:
      function create () external authorized(marketPlace) returns (address, address) {}
47:   function setAdmin(address a) external authorized(admin) returns (bool) {
54:   function setMarketPlace(address m) external authorized(admin) returns (bool) {

Creator/VaultTracker.sol VaultTracker/VaultTracker.sol

49:   function addNotional(address o, uint256 a) external authorized(admin) returns (bool) {
82:   function removeNotional(address o, uint256 a) external authorized(admin) returns (bool) {
113:  function redeemInterest(address o) external authorized(admin) returns (uint256) {
143:  function matureVault(uint256 c) external authorized(admin) returns (bool) {
152:  function transferNotionalFrom(address f, address t, uint256 a) external authorized(admin) returns (bool) {
208:  function transferNotionalFee(address f, uint256 a) external authorized(admin) returns(bool) {

Creator/ZcToken.sol Tokens/ZcToken.sol

140:    function burn(address f, uint256 a) external onlyAdmin(address(redeemer)) returns (bool) {
147:    function mint(address t, uint256 a) external onlyAdmin(address(redeemer)) returns (bool) {

Marketplace/MarketPlace.sol

45:   function setSwivel(address s) external authorized(admin) returns (bool) {
53:   function setAdmin(address a) external authorized(admin) returns (bool) {
64-70:
      function createMarket() external authorized(admin) unpaused(p) returns (bool) {
115:  function mintZcTokenAddingNotional(uint8 p, address u, uint256 m, address t, uint256 a) external authorized(swivel) unpaused(p) returns (bool) {
148:  function authRedeem(uint8 p, address u, uint256 m, address f, address t, uint256 a) public authorized(markets[p][u][m].zcToken) returns (uint256 underlyingAmount) {
176:  function redeemZcToken(uint8 p, address u, uint256 m, address t, uint256 a) external authorized(swivel) unpaused(p) returns (uint256) {
201:  function redeemVaultInterest(uint8 p, address u, uint256 m, address t) external authorized(swivel) unpaused(p) returns (uint256) {
247:  function custodialInitiate(uint8 p, address u, uint256 m, address z, address n, uint256 a) external authorized(swivel) unpaused(p) returns (bool) {
265:  function custodialExit(uint8 p, address u, uint256 m, address z, address n, uint256 a) external authorized(swivel) unpaused(p) returns (bool) {
283:  function p2pZcTokenExchange(uint8 p, address u, uint256 m, address f, address t, uint256 a) external authorized(swivel) unpaused(p) returns (bool) {
301:  function p2pVaultExchange(uint8 p, address u, uint256 m, address f, address t, uint256 a) external authorized(swivel) unpaused(p) returns (bool) {
328:  function transferVaultNotionalFee(uint8 p, address u, uint256 m, address f, uint256 a) external authorized(swivel) returns (bool) {
336:  function pause(uint8 p, bool b) external authorized(admin) returns (bool) {

Swivel/Swivel.sol

428:  function setAdmin(address a) external authorized(admin) returns (bool) {
436:  function scheduleWithdrawal(address e) external authorized(admin) returns (bool) {
447:  function blockWithdrawal(address e) external authorized(admin) returns (bool) {
457:  function withdraw(address e) external authorized(admin) returns (bool) {
473:  function scheduleFeeChange() external authorized(admin) returns (bool) {
483:  function blockFeeChange() external authorized(admin) returns (bool) {
495:  function setFee(uint16[] memory i, uint16[] memory d) external authorized(admin) returns (bool) {
522:  function scheduleApproval(address e) external authorized(admin) returns (bool) {
533:  function blockApproval(address e) external authorized(admin) returns (bool) {
544:  function approveUnderlying(address[] calldata u, address[] calldata c) external authorized(admin) returns (bool) {
620:  function authRedeemZcToken(uint8 p, address u, address c, address t, uint256 a) external authorized(marketPlace) returns(bool) {

> 0 can be replaced with != 0 when dealing with uint.

!= 0 costs less gas compared to > 0 for unsigned integers with the optimizer enabled (6 gas). Here is the code as a demo. Opcode discussion.

Creator/VaultTracker.sol VaultTracker/VaultTracker.sol

54:     if (vlt.notional > 0) {
59:     if (maturityRate > 0) { 
93:     if (maturityRate > 0) { 
123:    if (maturityRate > 0) { 
165:    if (maturityRate > 0) { 
181:    if (to.notional > 0) {
184:    if (maturityRate > 0) {
222:    if (maturityRate > 0) { 

Suggestion:

robrobbins commented 2 years ago

el dupos