SIP-44 and SIP-46 audit remediations (Synthetixio#476)

Author: justin j. moses

contracts/FeePool.sol

@@ -17,6 +17,7 @@ import "./FeePoolState.sol";
 import "./FeePoolEternalStorage.sol";
 import "./DelegateApprovals.sol";
 
+
 // https://docs.synthetix.io/contracts/FeePool
 contract FeePool is Proxyable, SelfDestructible, LimitedSetup, MixinResolver {
     using SafeMath for uint;
@@ -79,6 +80,7 @@ contract FeePool is Proxyable, SelfDestructible, LimitedSetup, MixinResolver {
     bytes32 private constant CONTRACT_SYNTHETIXSTATE = "SynthetixState";
     bytes32 private constant CONTRACT_REWARDESCROW = "RewardEscrow";
     bytes32 private constant CONTRACT_DELEGATEAPPROVALS = "DelegateApprovals";
+    bytes32 private constant CONTRACT_REWARDSDISTRIBUTION = "RewardsDistribution";
 
     bytes32[24] private addressesToCache = [
         CONTRACT_SYSTEMSTATUS,
@@ -90,7 +92,8 @@ contract FeePool is Proxyable, SelfDestructible, LimitedSetup, MixinResolver {
         CONTRACT_ISSUER,
         CONTRACT_SYNTHETIXSTATE,
         CONTRACT_REWARDESCROW,
-        CONTRACT_DELEGATEAPPROVALS
+        CONTRACT_DELEGATEAPPROVALS,
+        CONTRACT_REWARDSDISTRIBUTION
     ];
 
     /* ========== ETERNAL STORAGE CONSTANTS ========== */
@@ -155,6 +158,11 @@ contract FeePool is Proxyable, SelfDestructible, LimitedSetup, MixinResolver {
         return DelegateApprovals(requireAndGetAddress(CONTRACT_DELEGATEAPPROVALS, "Missing DelegateApprovals address"));
     }
 
+    function rewardsDistribution() internal view returns (IRewardsDistribution) {
+        return
+            IRewardsDistribution(requireAndGetAddress(CONTRACT_REWARDSDISTRIBUTION, "Missing RewardsDistribution address"));
+    }
+
     function recentFeePeriods(uint index)
         external
         view
@@ -245,7 +253,7 @@ contract FeePool is Proxyable, SelfDestructible, LimitedSetup, MixinResolver {
      * @notice The RewardsDistribution contract informs us how many SNX rewards are sent to RewardEscrow to be claimed.
      */
     function setRewardsToDistribute(uint amount) external {
-        address rewardsAuthority = resolver.getAddress("RewardsDistribution");
+        address rewardsAuthority = rewardsDistribution();
         require(messageSender == rewardsAuthority || msg.sender == rewardsAuthority, "Caller is not rewardsAuthority");
         // Add the amount of SNX rewards to distribute on top of any rolling unclaimed amount
         _recentFeePeriodsStorage(0).rewardsToDistribute = _recentFeePeriodsStorage(0).rewardsToDistribute.add(amount);
@@ -539,7 +547,6 @@ contract FeePool is Proxyable, SelfDestructible, LimitedSetup, MixinResolver {
         //          return _value;
         //      }
         //      return fee;
-
     }
 
     /**

contracts/MixinResolver.sol

@@ -14,10 +14,17 @@ contract MixinResolver is Owned {
 
     uint public constant MAX_ADDRESSES_FROM_RESOLVER = 24;
 
-    constructor(address _owner, address _resolver, bytes32[24] _addressesToCache) public Owned(_owner) {
+    constructor(address _owner, address _resolver, bytes32[MAX_ADDRESSES_FROM_RESOLVER] _addressesToCache)
+        public
+        Owned(_owner)
+    {
         for (uint i = 0; i < _addressesToCache.length; i++) {
             if (_addressesToCache[i] != bytes32(0)) {
                 resolverAddressesRequired.push(_addressesToCache[i]);
+            } else {
+                // End early once an empty item is found - assumes there are no empty slots in
+                // _addressesToCache
+                break;
             }
         }
         resolver = AddressResolver(_resolver);
@@ -70,7 +77,7 @@ contract MixinResolver is Owned {
     }
 
     /* ========== INTERNAL FUNCTIONS ========== */
-    function updateAddressCache(bytes32 name) internal {
+    function appendToAddressCache(bytes32 name) internal {
         resolverAddressesRequired.push(name);
         require(resolverAddressesRequired.length < MAX_ADDRESSES_FROM_RESOLVER, "Max resolver cache size met");
         // Because this is designed to be called internally in constructors, we don't

contracts/MultiCollateralSynth.sol

@@ -22,7 +22,7 @@ contract MultiCollateralSynth is Synth {
     ) public Synth(_proxy, _tokenState, _tokenName, _tokenSymbol, _owner, _currencyKey, _totalSupply, _resolver) {
         multiCollateralKey = _multiCollateralKey;
 
-        updateAddressCache(multiCollateralKey);
+        appendToAddressCache(multiCollateralKey);
     }
 
     /* ========== VIEWS ======================= */

contracts/PurgeableSynth.sol

@@ -27,7 +27,7 @@ contract PurgeableSynth is Synth {
         uint _totalSupply,
         address _resolver
     ) public Synth(_proxy, _tokenState, _tokenName, _tokenSymbol, _owner, _currencyKey, _totalSupply, _resolver) {
-        updateAddressCache(CONTRACT_EXRATES);
+        appendToAddressCache(CONTRACT_EXRATES);
     }
 
     /* ========== VIEWS ========== */

contracts/Synth.sol

@@ -8,6 +8,7 @@ import "./interfaces/IExchanger.sol";
 import "./interfaces/IIssuer.sol";
 import "./MixinResolver.sol";
 
+
 // https://docs.synthetix.io/contracts/Synth
 contract Synth is ExternStateToken, MixinResolver {
     /* ========== STATE VARIABLES ========== */

contracts/SystemStatus.sol

@@ -2,6 +2,7 @@ pragma solidity 0.4.25;
 
 import "./Owned.sol";
 
+
 // https://docs.synthetix.io/contracts/SystemStatus
 contract SystemStatus is Owned {
     struct Status {
@@ -151,7 +152,7 @@ contract SystemStatus is Owned {
         delete synthSuspension[currencyKey];
     }
 
-    /* ========== INTERNL FUNCTIONS ========== */
+    /* ========== INTERNAL FUNCTIONS ========== */
 
     function _requireAccessToSuspend(bytes32 section) internal view {
         require(accessControl[section][msg.sender].canSuspend, "Restricted to access control list");