Fixes to cover SigmaPrime audit

.solhint.json

@@ -2,7 +2,7 @@
   "extends": "solhint:recommended",
   "plugins": ["prettier"],
   "rules": {
-    "code-complexity": ["error", 12],
+    "code-complexity": ["error", 13],
     "compiler-version": ["error", ">=0.8.4"],
     "func-visibility": ["error", { "ignoreConstructors": true }],
     "max-line-length": ["error", 120],

contracts/NetworkRegistryShaman.sol

@@ -2,11 +2,14 @@
 pragma solidity ^0.8.13;
 
 import { IBaal } from "@daohaus/baal-contracts/contracts/interfaces/IBaal.sol";
+import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 
 import { NetworkRegistry } from "./NetworkRegistry.sol";
 
 // import "hardhat/console.sol";
 
+error NetworkRegistryShaman_NotManagerShaman();
+
 /**
  * @title A cross-chain network registry and Baal shaman module to distribute funds escrowed in 0xSplit based
  * on member activity.
@@ -30,24 +33,27 @@ contract NetworkRegistryShaman is NetworkRegistry {
     IBaal public baal;
     /// @notice The amount of shares to mint to new members
     uint256 public sharesToMint;
-    /// @notice Wether or not to burn shares if a memeber activityMultiplier is set to zero
+    /// @notice Wether or not to burn shares if a member activityMultiplier is set to zero
     bool public burnShares;
 
     /**
      * @notice A modifier to check if the registry has been setup as a manager shaman module
      */
     modifier isManagerShaman() {
-        if (isMainRegistry()) {
-            require(
-                address(baal) != address(0) && baal.isManager(address(this)),
-                "NetworkRegistryShaman: !init || ! manager"
-            );
-        }
+        if (!isMainRegistry() || address(baal) == address(0) || !baal.isManager(address(this)))
+            revert NetworkRegistryShaman_NotManagerShaman();
         _;
     }
 
+    // /**
+    //  * @notice emitted when the shaman config is updated
+    //  * @param _sharesToMint new amount of shares to mint to registered members
+    //  * @param _burnShares wether or not to burn shares to inactive members
+    //  */
+    // event ShamanConfigUpdated(uint256 _sharesToMint, bool _burnShares);
+
     /**
-     * @notice Initializs the registry shaman contract
+     * @notice Initializes the registry shaman contract
      * @dev Initialization parameters are abi-encoded through the NetworkRegistrySummoner contract
      * @param _initializationParams abi-encoded parameters
      */
@@ -79,11 +85,14 @@ contract NetworkRegistryShaman is NetworkRegistry {
      * @notice Updates shaman config parameters
      * @dev Must only be called by owner or updater (latter should never apply)
      * @param _sharesToMint The amount of shares to mint to new members
-     * @param _burnShares Wether or not to burn shares if a memeber activityMultiplier is set to zero
+     * @param _burnShares Wether or not to burn shares if a member activityMultiplier is set to zero
      */
     function setShamanConfig(uint256 _sharesToMint, bool _burnShares) external onlyOwnerOrUpdater {
         burnShares = _burnShares;
         sharesToMint = _sharesToMint;
+        // TODO: temporarily disabled to avoid reaching maximum contract size.
+        // This should be enabled in the next iteration
+        // emit ShamanConfigUpdated(sharesToMint, burnShares);
     }
 
     /**
@@ -99,13 +108,11 @@ contract NetworkRegistryShaman is NetworkRegistry {
         uint32 _startDate
     ) internal override isManagerShaman {
         super._setNewMember(_member, _activityMultiplier, _startDate);
-        if (isMainRegistry()) {
-            address[] memory _receivers = new address[](1);
-            _receivers[0] = _member;
-            uint256[] memory _amounts = new uint256[](1);
-            _amounts[0] = sharesToMint;
-            baal.mintShares(_receivers, _amounts);
-        }
+        address[] memory _receivers = new address[](1);
+        _receivers[0] = _member;
+        uint256[] memory _amounts = new uint256[](1);
+        _amounts[0] = sharesToMint;
+        baal.mintShares(_receivers, _amounts);
     }
 
     /**
@@ -116,12 +123,14 @@ contract NetworkRegistryShaman is NetworkRegistry {
      */
     function _updateMember(address _member, uint32 _activityMultiplier) internal override isManagerShaman {
         super._updateMember(_member, _activityMultiplier);
-        if (_activityMultiplier == 0 && isMainRegistry() && burnShares) {
-            address[] memory _from = new address[](1);
-            _from[0] = _member;
-            uint256[] memory _amounts = new uint256[](1);
-            _amounts[0] = sharesToMint;
-            baal.burnShares(_from, _amounts);
+        address[] memory _to = new address[](1);
+        _to[0] = _member;
+        uint256[] memory _amounts = new uint256[](1);
+        _amounts[0] = sharesToMint;
+        if (_activityMultiplier > 0 && IERC20(baal.sharesToken()).balanceOf(_member) == 0) {
+            baal.mintShares(_to, _amounts);
+        } else if (_activityMultiplier == 0 && burnShares) {
+            baal.burnShares(_to, _amounts);
         }
     }
 }

contracts/NetworkRegistrySummoner.sol

@@ -41,14 +41,14 @@ contract NetworkRegistrySummoner {
         NetworkRegistry registry = NetworkRegistry(registryAddress);
         registry.initialize(_initializationParams);
         emit NetworkRegistrySummoned(registryAddress, _details, _initializationParams);
-        return address(registryAddress);
+        return registryAddress;
     }
 
     /**
      * @notice Summons a new NetworkRegistry deterministically using the create2 opcode
      * @dev Singleton contract must inherit NetworkRegistry
      * Using the same {_singleton} {salt} multiple time will revert
-     * @param _singleton NetoworkRegistry singleton contract address
+     * @param _singleton NetworkRegistry singleton contract address
      * @param _details registry name/details as string
      * @param _initializationParams abi-encoded parameters used to setup the registry
      * @param _saltNonce unique salt nonce for the contract deployment

contracts/fixtures/SplitMain.sol

@@ -72,7 +72,7 @@ contract SplitMain is ISplitMain {
      * STORAGE - CONSTANTS & IMMUTABLES
      */
 
-    /// @notice constant to scale uints into percentages (1e6 == 100%)
+    /// @notice constant to scale UINT values into percentages (1e6 == 100%)
     uint256 public constant PERCENTAGE_SCALE = 1e6;
     /// @notice maximum distributor fee; 1e5 = 10% * PERCENTAGE_SCALE
     uint256 internal constant MAX_DISTRIBUTOR_FEE = 1e5;

contracts/interfaces/IMemberRegistry.sol

@@ -57,6 +57,22 @@ interface IMemberRegistry {
 
     function batchUpdateMember(address[] memory _members, uint32[] memory _activityMultipliers) external;
 
+    /**
+     * @notice Adds and/or updates a set of members on the registry
+     * @dev Make sure array parameters are of the same length
+     * Activity multiplier could be set within 0-100 (%) range (i.e. 50 -> part-time 100 -> full-time)
+     * but it's up to the implementer to establish the multiplier boundaries
+     * @param _members A list of member addresses to be added to the registry
+     * @param _activityMultipliers Activity multipliers for each new member
+     * @param _startDates A list of dates when each member got active
+     */
+
+    function addOrUpdateMembersBatch(
+        address[] memory _members,
+        uint32[] memory _activityMultipliers,
+        uint32[] memory _startDates
+    ) external;
+
     /**
      * @notice Updates seconds active for each member in the registry since the last update epoch
      * @dev manages a lastActivityUpdate state variable to update activity based on last update epoch.

contracts/interfaces/INetworkMemberRegistry.sol

@@ -24,9 +24,9 @@ interface INetworkMemberRegistry is IMemberRegistry, ISplitManager {
     }
 
     /**
-     * @notice Initializs the registry contract
+     * @notice Initializes the registry contract
      * @dev Initialization parameters are abi-encoded through the NetworkRegistrySummoner contract.
-     * It should also call initialzer methods from parent contracts
+     * It should also call initializer methods from parent contracts
      * @param _initializationParams abi-encoded parameters
      */
     function initialize(bytes memory _initializationParams) external;
@@ -91,7 +91,8 @@ interface INetworkMemberRegistry is IMemberRegistry, ISplitManager {
 
     /**
      * @notice Adds a new set of members to the registry and sync with replicas
-     * @dev It should forward messages to sync all registered replicas
+     * @dev Must be used only if registries are in sync. It can only be called by the main registry owner
+     * {validNetworkParams} verifies for matching network param sizes & {msg.value}
      * {msg.value} must match the total fees required to pay the Connext relayer to execute messages on the destination
      * @param _members A list of member addresses to be added to the registry
      * @param _activityMultipliers Activity multipliers for each new member
@@ -118,7 +119,8 @@ interface INetworkMemberRegistry is IMemberRegistry, ISplitManager {
 
     /**
      * @notice Updates the activity multiplier for a set of existing members and sync with replicas
-     * @dev It should forward messages to sync all registered replicas
+     * @dev Must be used only if registries are in sync. It can only be called by the main registry owner
+     * {validNetworkParams} verifies for matching network param sizes & {msg.value}
      * {msg.value} must match the total fees required to pay the Connext relayer to execute messages on the destination
      * @param _members A list of existing members
      * @param _activityMultipliers New activity multipliers for each member
@@ -142,7 +144,7 @@ interface INetworkMemberRegistry is IMemberRegistry, ISplitManager {
     function syncUpdateSecondsActive(uint32[] calldata _chainIds, uint256[] calldata _relayerFees) external payable;
 
     /**
-     * @notice Updates the 0xsplit distribution on all networks based on member activity during the last epoch.
+     * @notice Updates the 0xSplit distribution on all networks based on member activity during the last epoch.
      * @dev It should forward messages to sync all registered replicas
      * - The registry must hold the controller role of the 0xSplit contract
      * - Addresses in _sortedList must be in the member registry

contracts/interfaces/ISplitManager.sol

@@ -19,7 +19,7 @@ interface ISplitManager {
      *  - Total allocations from all members must be equal to 0xSplit PERCENTAGE_SCALE
      * @param _sortedList sorted list (ascending order) of members to be considered in the 0xSplit distribution
      * @return _receivers list of eligible recipients (non-zero allocation) for the next split distribution
-     * @return _percentAllocations list of split allocations for each eligible recipeint
+     * @return _percentAllocations list of split allocations for each eligible recipient
      */
     function calculate(address[] memory _sortedList) external view returns (address[] memory, uint32[] memory);
 
@@ -39,7 +39,7 @@ interface ISplitManager {
     function calculateTotalContributions() external view returns (uint256 total);
 
     /**
-     * @notice Updates the 0xsplit distribution based on member activity during the last epoch.
+     * @notice Updates the 0xSplit distribution based on member activity during the last epoch.
      * @dev permissionless action, however the registry must hold the controller role of the 0xSplit contract
      * Verify if the address list is sorted, has no duplicates and is valid.
      * Addresses in _sortedList must be in the member registry
@@ -77,13 +77,13 @@ interface ISplitManager {
 
     /**
      * @notice Accepts control of the current 0xSplit contract
-     * @dev should accept control of the current 0xsplit in the state
+     * @dev should accept control of the current 0xSplit in the state
      */
     function acceptSplitControl() external;
 
     /**
      * @notice Cancel controller transfer of the current 0xSplit contract
-     * @dev should cancel a previous request to update the controller of the current 0xsplit contract
+     * @dev should cancel a previous request to update the controller of the current 0xSplit contract
      */
     function cancelSplitControlTransfer() external;
 }

contracts/mocks/ConnextMock.sol

@@ -50,7 +50,7 @@ contract ConnextMock {
 
     /**
      * @notice Emitted when `xcall` is called on the origin domain of a transfer.
-     * @param transferId - The unique identifier of the crosschain transfer.
+     * @param transferId - The unique identifier of the cross-chain transfer.
      * @param nonce - The bridge nonce of the transfer on the origin domain.
      * @param messageHash - The hash of the message bytes (containing all transfer info) that were bridged.
      * @param params - The `TransferInfo` provided to the function.
@@ -125,7 +125,7 @@ contract ConnextMock {
      * to this chain), we will custody the tokens here.
      *
      * @param _params - The TransferInfo arguments.
-     * @return bytes32 - The transfer ID of the newly created crosschain transfer.
+     * @return bytes32 - The transfer ID of the newly created cross-chain transfer.
      */
     function _xcall(
         TransferInfo memory _params,
@@ -256,7 +256,7 @@ contract ConnextMock {
             }
         }
 
-        // Send the crosschain message.
+        // Send the cross-chain message.
         _sendMessageAndEmit(
             transferId,
             _params,

contracts/registry/MemberRegistry.sol

@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.7;
 
-import "@prb/math/src/UD60x18.sol";
-
 // import "hardhat/console.sol";
 
 /**
@@ -38,7 +36,7 @@ abstract contract MemberRegistry {
     struct Member {
         /// @notice member address
         address account;
-        /// @notice active time in seconds
+        /// @notice total active time in seconds
         uint32 secondsActive;
         /// @notice timestamp where member started activities
         /// @dev timestamp format in seconds
@@ -78,8 +76,9 @@ abstract contract MemberRegistry {
      * @notice emitted after the an existing member is updated
      * @param _member member address
      * @param _activityMultiplier new member activity multiplier
+     * @param _startDate timestamp the member started activities in seconds
      */
-    event UpdateMember(address indexed _member, uint32 _activityMultiplier);
+    event UpdateMember(address indexed _member, uint32 _activityMultiplier, uint32 _startDate);
     /**
      * @notice emitted after each time a member registry activity is updated
      * @param _member member address
@@ -132,35 +131,35 @@ abstract contract MemberRegistry {
         Member storage member = members[memberIdxs[_member] - 1];
         member.activityMultiplier = _activityMultiplier;
 
-        emit UpdateMember(_member, _activityMultiplier);
+        emit UpdateMember(_member, _activityMultiplier, member.startDate);
     }
 
     /**
      * @notice Updates seconds active for each member in the registry since the last update epoch
      * @dev manages a lastActivityUpdate state variable to update activity based on last update epoch.
      * However for new members it should update seconds based each member startDate.
-     * Notice function is set as virtual so base functionality can be overriden by the implementer
+     * Notice function is set as virtual so base functionality can be overridden by the implementer
      */
     function _updateSecondsActive() internal virtual {
         uint32 currentDate = uint32(block.timestamp);
+        uint256 membersLength = totalMembers();
         // update struct with total seconds active and seconds in last claim
-        uint256 i;
-        for (i = 0; i < members.length; ) {
+        for (uint256 i = 0; i < membersLength; ) {
             Member storage _member = members[i];
             uint32 newSecondsActive = 0;
             if (_member.activityMultiplier > 0) {
                 uint32 initDate = _member.secondsActive > 0 ? lastActivityUpdate : _member.startDate;
-                uint256 activeSeconds = currentDate - initDate;
+                uint256 totalSeconds = currentDate - initDate;
                 // multiply by modifier and divide by 100 to get modifier % of seconds
-                newSecondsActive = uint32((activeSeconds * _member.activityMultiplier) / 100);
+                newSecondsActive = uint32((totalSeconds * _member.activityMultiplier) / 100);
             }
             _member.secondsActive += newSecondsActive;
             emit UpdateMemberSeconds(_member.account, newSecondsActive);
             unchecked {
-                i++; // gas optimization: very unlikely to overflow
+                ++i; // gas optimization: very unlikely to overflow
             }
         }
-        emit RegistryActivityUpdate(currentDate, i);
+        emit RegistryActivityUpdate(currentDate, membersLength);
         lastActivityUpdate = currentDate;
     }
 
@@ -201,12 +200,13 @@ abstract contract MemberRegistry {
         address[] memory _members = new address[](members.length);
         uint32[] memory _activityMultipliers = new uint32[](members.length);
         uint32[] memory _startDates = new uint32[](members.length);
-        for (uint256 i = 0; i < members.length; ) {
+        uint256 membersLength = totalMembers();
+        for (uint256 i = 0; i < membersLength; ) {
             _members[i] = members[i].account;
             _activityMultipliers[i] = members[i].activityMultiplier;
             _startDates[i] = members[i].startDate;
             unchecked {
-                i++;
+                ++i;
             }
         }
         return (_members, _activityMultipliers, _startDates);

hardhat.config.ts

@@ -240,7 +240,7 @@ const config: HardhatUserConfig = {
           // https://hardhat.org/hardhat-network/#solidity-optimizer-support
           optimizer: {
             enabled: true,
-            runs: 200, // 800
+            runs: 10, // Trade-off to avoid getting Contract code size warnings
           },
         },
       },

test/networkRegistry/NetworkRegistry.behavior.ts

@@ -238,9 +238,7 @@ describe("NetworkRegistry E2E tests", function () {
 
       // Validate member's percent allocation
       const calcContributions = await Promise.all(
-        l1Splits._receivers.map(
-          async (member: string) => await l1NetworkRegistry["calculateContributionOf(address)"](member),
-        ),
+        l1Splits._receivers.map(async (member: string) => await l1NetworkRegistry["calculateContributionOf"](member)),
       );
       const totalContributions = await l1NetworkRegistry.calculateTotalContributions();
 
@@ -376,9 +374,7 @@ describe("NetworkRegistry E2E tests", function () {
 
       // Validate member's percent allocation
       const calcContributions = await Promise.all(
-        l1Splits._receivers.map(
-          async (member: string) => await l1NetworkRegistry["calculateContributionOf(address)"](member),
-        ),
+        l1Splits._receivers.map(async (member: string) => await l1NetworkRegistry["calculateContributionOf"](member)),
       );
       const totalContributions = await l1NetworkRegistry.calculateTotalContributions();