Swivel v3 contest - __141345__'s results

Typo

"can can" should be "can".

Tokens/ZcToken.sol Creator/ZcToken.sol 127:

        // some 5095 tokens may have custody of underlying and can can just burn PTs and transfer underlying out, while others rely on external custody

Unused return values can be removed

Creator/VaultTracker.sol VaultTracker/VaultTracker.sol

143-146:
  function matureVault(uint256 c) external authorized(admin) returns (bool) {
    maturityRate = c;
    return true;
  }

But the return value of matureVault() is not used.

Marketplace/MarketPlace.sol

101-102:
    // NOTE we don't check the return of this simple operation
    IVaultTracker(market.vaultTracker).matureVault(exchangeRate);

Suggestion: Remove the return line from the function.

ZcToken mint() unneccessary check

When mintZcTokenAddingNotional() is called, the return value of mint() in ZcToken is checked.

Marketplace\MarketPlace.sol

118:    if (!IZcToken(market.zcToken).mint(t, a)) { revert Exception(28, 0, 0, address(0), address(0)); }
249:    if (!IZcToken(market.zcToken).mint(z, a)) { revert Exception(28, 0, 0, address(0), address(0)); }
287:    if (!IZcToken(market.zcToken).mint(t, a)) { revert Exception(28, 0, 0, address(0), address(0)); }

However, in ZcToken.sol: Tokens\ZcToken.sol

    function mint(address t, uint256 a) external onlyAdmin(address(redeemer)) returns (bool) {
        _mint(t, a);
        return true;
    }

Which calls the following: Creator\Erc20.sol

    function _mint(address to, uint256 amount) internal virtual {
        totalSupply += amount;

        // Cannot overflow because the sum of all user
        // balances can't exceed the max uint256 value.
        unchecked {
            balanceOf[to] += amount;
        }

        emit Transfer(address(0), to, amount);
    }

The only exception is overflow: totalSupply += amount.

The check for return value of mint is unneccessary.

Suggestion: Just call mint()directly:

    IZcToken(market.zcToken).mint(t, a);

++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:

• Changing > 0 with != 0.
• Enable the Optimizer.