Reserve contest - Breeje's results

QA Report

[L-1] USE OF FLOATING PRAGMA

Impact: swcregistry

File: libraries/Fixed.sol

3:    pragma solidity ^0.8.9;

Link to Code

File: plugins/aave/ERC20.sol

3:    pragma solidity ^0.6.0;

Link to Code

File: plugins/aave/ReentrancyGuard.sol

3:    pragma solidity >=0.6.0 <0.8.0;

Link to Code

[L-2] setMinTradeVolume METHOD USES INCORRECT REQUIRE CHECK

As MIN_TRADE_VOLUME is already set by default, the minTradeVolume assigned by the governance should be greater than or equal to MIN_TRADE_VOLUME and not less than or equal to.

File: p1/mixins/Trading.sol

135:       function setMinTradeVolume(uint192 val) public governance {
136:            require(val <= MIN_TRADE_VOLUME, "invalid minTradeVolume");
137:            emit MinTradeVolumeSet(minTradeVolume, val);
138:            minTradeVolume = val;
139:        }

Link to Code

[L-3] WRONG OPERATOR USED

In discharge method of RedemptionBatteryLib, the function should return if anyone of battery.redemptionRateFloor or battery.scalingRedemptionRate equals zero as suggested in Natspec.

So && operator should be replaced with || operator.

File: p1/mixins/RedemptionBattery.sol

25:       // for either: set to 0 to disable

38:       if (battery.redemptionRateFloor == 0 && battery.scalingRedemptionRate == 0) return;

Link to Code

[NC-1] USE REQUIRE INSTEAD OF ASSERT

Assert should not be used except for tests, require should be used.

Intances (20):

• BackingManager.sol Instance 1
• BasketHandler.sol Instance 1
• StRSR.sol Instance 1
• RecollateralizationLib.sol Instance 1
• RewardableLib.sol Instance 1
• RewardableLib.sol Instance 2
• TradeLib.sol Instance 1
• TradeLib.sol Instance 2
• TradeLib.sol Instance 3
• TradeLib.sol Instance 4
• Trading.sol Instance 1
• StaticATokenLM.sol Instance 1
• Asset.sol Instance 1
• Asset.sol Instance 2
• Asset.sol Instance 3
• FiatCollateral.sol Instance 1
• RTokenAsset.sol Instance 1
• GnosisTrade.sol Instance 1
• GnosisTrade.sol Instance 2
• GnosisTrade.sol Instance 3

[NC-2] EVENT IS MISSING INDEXED FIELDS

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

Instances (1):

• IDistributor.sol Instance 1

[NC-3] SPELLING MISTAKE IN NATSPEC

Spelling of Auction should be corrected. Spelling of Maintain should be corrected.

File: contracts/plugins/trading/GnosisTrade.sol

47:      // from this trade's acution will all eventually go to origin.

• Link to Code

File: p1/BackingManager.sol

89:      /// Mointain the overall backing policy; handout assets otherwise

• Link to Code

Gas Optimizations

[GAS-1] ++I/I++ SHOULD BE UNCHECKED{++I}/UNCHECKED{I++} WHEN IT IS NOT POSSIBLE FOR THEM TO OVERFLOW

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop.

Instances (60):

• Array.sol Instance 1
• Array.sol Instance 2
• Array.sol Instance 3
• Fixed.sol Instance 1
• Fixed.sol Instance 2
• String.sol Instance 1
• AssetRegistry.sol Instance 1
• AssetRegistry.sol Instance 2
• AssetRegistry.sol Instance 3
• AssetRegistry.sol Instance 4
• BackingManager.sol Instance 1
• BackingManager.sol Instance 2
• BasketHandler.sol Instance 1
• BasketHandler.sol Instance 2
• BasketHandler.sol Instance 3
• BasketHandler.sol Instance 4
• BasketHandler.sol Instance 5
• BasketHandler.sol Instance 6
• BasketHandler.sol Instance 7
• BasketHandler.sol Instance 8
• BasketHandler.sol Instance 9
• BasketHandler.sol Instance 10
• BasketHandler.sol Instance 11
• BasketHandler.sol Instance 12
• BasketHandler.sol Instance 13
• BasketHandler.sol Instance 14
• BasketHandler.sol Instance 15
• BasketHandler.sol Instance 16
• BasketHandler.sol Instance 17
• BasketHandler.sol Instance 18
• BasketHandler.sol Instance 19
• BasketHandler.sol Instance 20
• BasketHandler.sol Instance 21
• BasketHandler.sol Instance 22
• Distributor.sol Instance 1
• Distributor.sol Instance 2
• Distributor.sol Instance 3
• Distributor.sol Instance 4
• RToken.sol Instance 1
• RToken.sol Instance 2
• RToken.sol Instance 3
• RToken.sol Instance 4
• RToken.sol Instance 5
• RToken.sol Instance 6
• RToken.sol Instance 7
• RToken.sol Instance 8
• RToken.sol Instance 9
• RToken.sol Instance 10
• RToken.sol Instance 11
• RToken.sol Instance 12
• RToken.sol Instance 13
• StRSR.sol Instance 1
• StRSR.sol Instance 2
• RecollateralizationLib.sol Instance 1
• RecollateralizationLib.sol Instance 2
• RecollateralizationLib.sol Instance 3
• RewardableLib.sol Instance 1
• RewardableLib.sol Instance 2
• RewardableLib.sol Instance 3
• Trading.sol Instance 1

[GAS-2] X += Y COSTS MORE GAS THAN X = X + Y FOR STATE VARIABLES

Instances (36):

• Fixed.sol Instance 1
• Fixed.sol Instance 2
• Fixed.sol Instance 3
• Fixed.sol Instance 4
• Fixed.sol Instance 5
• Fixed.sol Instance 6
• Fixed.sol Instance 7
• Fixed.sol Instance 8
• Auth.sol Instance 1
• BasketHandler.sol Instance 1
• BasketHandler.sol Instance 2
• BasketHandler.sol Instance 3
• Distributor.sol Instance 1
• Distributor.sol Instance 2
• RToken.sol Instance 1
• RToken.sol Instance 2
• RToken.sol Instance 3
• RToken.sol Instance 4
• RToken.sol Instance 5
• StRSR.sol Instance 1
• StRSR.sol Instance 2
• StRSR.sol Instance 3
• StRSR.sol Instance 4
• StRSR.sol Instance 5
• StRSR.sol Instance 6
• StRSR.sol Instance 7
• StRSR.sol Instance 8
• StRSR.sol Instance 9
• StRSR.sol Instance 10
• StRSR.sol Instance 11
• StRSR.sol Instance 12
• StRSR.sol Instance 13
• RecollateralizationLib.sol Instance 1
• RecollateralizationLib.sol Instance 2

[GAS-3] SPLITTING REQUIRE() STATEMENTS THAT USE && SAVES GAS

Saves 3 gas per instance.

Instances (32):

• Auth.sol Instance 1
• Auth.sol Instance 2
• BackingManager.sol Instance 1
• BasketHandler.sol Instance 1
• Broker.sol Instance 1
• Deployer.sol Instance 1-14
• Deployer.sol Instance 15
• Furnace.sol Instance 1
• RevenueTrader.sol Instance 1
• RToken.sol Instance 1
• RToken.sol Instance 2
• RToken.sol Instance 3
• RToken.sol Instance 4
• RToken.sol Instance 5
• StRSR.sol Instance 1
• StRSR.sol Instance 2
• TradeLib.sol Instance 1
• TradeLib.sol Instance 2
• Asset.sol Instance 1

[GAS-4] NO NEED TO EXPLICITLY INITIALIZE VARIABLES WITH DEFAULT VALUES

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address…). Explicitly initializing it with its default value wastes gas.

Instances (2):

• Fixed.sol Instance 1
• Fixed.sol Instance 2

[GAS-5] USE CALLDATA INSTEAD OF MEMORY FOR FUNCTION ARGUMENTS THAT DO NOT GET MUTATED

Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage.

Instances (7):

• Array.sol Instance 1
• Array.sol Instance 2
• Deployer.sol Instance 1
• Deployer.sol Instance 2
• Deployer.sol Instance 3
• Distributor.sol Instance 1
• Distributor.sol Instance 2

[GAS-6] USE ASSEMBLY TO CHECK FOR ADDRESS(0)

Saves 6 gas per instance

Instances (91):

• Auth.sol Instance 1
• ComponentRegistry.sol Instance 1
• ComponentRegistry.sol Instance 2
• ComponentRegistry.sol Instance 3
• ComponentRegistry.sol Instance 4
• ComponentRegistry.sol Instance 5
• ComponentRegistry.sol Instance 6
• ComponentRegistry.sol Instance 7
• ComponentRegistry.sol Instance 8
• ComponentRegistry.sol Instance 9
• ComponentRegistry.sol Instance 10
• AssetRegistry.sol Instance 1
• BasketHandler.sol Instance 1
• BasketHandler.sol Instance 2
• Broker.sol Instance 1
• Broker.sol Instance 2
• Deployer.sol Instance 1-14
• Deployer.sol Instance 15
• Distributor.sol Instance 1
• Main.sol Instance 1
• RevenueTrader.sol Instance 1
• RevenueTrader.sol Instance 2
• StRSR.sol Instance 1-2
• StRSR.sol Instance 3
• StRSR.sol Instance 3
• StRSR.sol Instance 4-5
• StRSR.sol Instance 4-5
• StRSR.sol Instance 6
• StRSR.sol Instance 7-8
• StRSR.sol Instance 9-10
• StRSRVotes.sol Instance 1
• StRSRVotes.sol Instance 2
• Component.sol Instance 1
• RecollateralizationLib.sol Instance 1-2
• RecollateralizationLib.sol Instance 3-4
• Trading.sol Instance 1
• Trading.sol Instance 2
• ERC20.sol Instance 1-2
• ERC20.sol Instance 3-4
• ERC20.sol Instance 5
• ERC20.sol Instance 6-7
• ERC20.sol Instance 8
• ERC20.sol Instance 9-10
• StaticATokenLM.sol Instance 1
• StaticATokenLM.sol Instance 2
• StaticATokenLM.sol Instance 3
• StaticATokenLM.sol Instance 4
• StaticATokenLM.sol Instance 5
• StaticATokenLM.sol Instance 6
• StaticATokenLM.sol Instance 7
• StaticATokenLM.sol Instance 8
• StaticATokenLM.sol Instance 9
• StaticATokenLM.sol Instance 10
• StaticATokenLM.sol Instance 11
• StaticATokenLM.sol Instance 12
• StaticATokenLM.sol Instance 13
• StaticATokenLM.sol Instance 14
• StaticATokenLM.sol Instance 15
• StaticATokenLM.sol Instance 16
• Asset.sol Instance 1
• Asset.sol Instance 2
• CTokenFiatCollateral.sol Instance 1
• CTokenNonFiatCollateral.sol Instance 1
• CTokenSelfReferentialCollateral.sol Instance 1
• EURFiatCollateral.sol Instance 1
• NonFiatCollateral.sol Instance 1
• RTokenAsset.sol Instance 1
• GnosisTrade.sol Instance 1
• GnosisTrade.sol Instance 2

[GAS-7] NOT USING THE NAMED RETURN VARIABLES WHEN A FUNCTION RETURNS, WASTES DEPLOYMENT GAS

Instances (8):

• AssetRegistry.sol Instance 1
• RToken.sol Instance 1
• RecollateralizationLib.sol Instance 1
• TradeLib.sol Instance 1
• TradeLib.sol Instance 2
• Asset.sol Instance 1
• EURFiatCollateral.sol Instance 1
• Governance.sol Instance 1

[GAS-8] USAGE OF UINTS/INTS SMALLER THAN 32 BYTES (256 BITS) INCURS OVERHEAD

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

Checkout this Solidity Doc

Instances (21):

File: IDistributor.sol

7:     struct RevenueShare {
8:          uint16 rTokenDist; // {revShare} A value between [0, 10,000]
9:          uint16 rsrDist; // {revShare} A value between [0, 10,000]
10:     }

12:     /// Assumes no more than 1024 independent distributions.
13:     struct RevenueTotals {
14:         uint24 rTokenTotal; // {revShare}
15:         uint24 rsrTotal; // {revShare}
16:     }

File: StRSRP1Votes.sol

22:     struct Checkpoint {
23:         uint48 fromBlock;
24:         uint224 val;
25:     }

File: p1/mixins/RecollateralizationLib.sol

24:      struct TradingRules {
25:         uint192 minTradeVolume; // {UoA}
26:         uint192 maxTradeSlippage; // {1}
27:     }

29:     struct TradeInfo {
30:         IAsset sell;
31:         IAsset buy;
32:         uint192 sellAmount; // {sellTok}
33:         uint192 buyAmount; // {buyTok}
34:         uint192 sellPrice; // {UoA/sellTok} can be 0
35:         uint192 buyPrice; // {UoA/buyTok}
36:     }

116:    struct BasketRange {
117:        uint192 top; // {BU}
118:        uint192 bottom; // {BU}
119:    }

284:    struct MaxSurplusDeficit {
285:        CollateralStatus surplusStatus; // starts SOUND
286:        uint192 surplus; // {UoA}
287:        uint192 deficit; // {UoA}
288:    }

Link to Code

File: RToken.sol

67:      uint192 public basketsNeeded; // D18{BU}

72:      uint192 private allVestAt; // D18{fractional block number}

74:      uint192 private lastIssRate;

81:      uint192 when;

83:      uint192 amtBaskets;

Link to Code

[GAS-9] STATE VARIABLES CAN BE PACKED INTO FEWER STORAGE SLOTS

Instances (1):

File: interfaces/IDeployer.sol L19:L48

    struct DeploymentParams {
        // === Revenue sharing ===
        RevenueShare dist; // revenue sharing splits between RToken and RSR
        //
        // === Trade sizing ===
        uint192 minTradeVolume; // {UoA}
        uint192 rTokenMaxTradeVolume; // {UoA}
        //
        // === Freezing ===
        uint48 shortFreeze; // {s} how long an initial freeze lasts
        uint48 longFreeze; // {s} how long each freeze extension lasts
        //
        // === Rewards (Furnace + StRSR) ===
        uint192 rewardRatio; // the fraction of available revenues that stRSR holders get each PayPeriod
        uint48 rewardPeriod; // {s} the atomic unit of rewards, determines # of exponential rounds
        //
        // === StRSR ===
        uint48 unstakingDelay; // {s} the "thawing time" of staked RSR before withdrawal
        //
        // === BackingManager ===
        uint48 tradingDelay; // {s} how long to wait until starting auctions after switching basket
        uint48 auctionLength; // {s} the length of an auction
        uint192 backingBuffer; // {1} how much extra backing collateral to keep
        uint192 maxTradeSlippage; // {1} max slippage acceptable in a trade
        //
        // === RToken ===
        uint192 issuanceRate; // {1/block} number of RToken to issue per block / (RToken value)
        uint192 scalingRedemptionRate; // {1/hour} max fraction of supply that can be redeemed hourly
        uint256 redemptionRateFloor; // {qRTok/hour} the lowest possible hourly redemption limit
    }

Link to Code