fix: correct manager fee and ubi settings upgrade

packages/contracts/contracts/DirectPayments/DirectPaymentsFactory.sol

@@ -98,31 +98,37 @@ contract DirectPaymentsFactory is AccessControlUpgradeable, UUPSUpgradeable {
         string memory _projectId,
         string memory _ipfs,
         DirectPaymentsPool.PoolSettings memory _settings,
-        DirectPaymentsPool.SafetyLimits memory _limits
+        DirectPaymentsPool.SafetyLimits memory _limits,
+        uint32 _managerFeeBps
     ) external onlyProjectOwnerOrNon(_projectId) returns (DirectPaymentsPool pool) {
-        return _createPool(_projectId, _ipfs, _settings, _limits, true);
+        return _createPool(_projectId, _ipfs, _settings, _limits, _managerFeeBps, true);
     }
 
     function createPool(
         string memory _projectId,
         string memory _ipfs,
         DirectPaymentsPool.PoolSettings memory _settings,
-        DirectPaymentsPool.SafetyLimits memory _limits
+        DirectPaymentsPool.SafetyLimits memory _limits,
+        uint32 _managerFeeBps
     ) external onlyProjectOwnerOrNon(_projectId) returns (DirectPaymentsPool pool) {
-        return _createPool(_projectId, _ipfs, _settings, _limits, false);
+        return _createPool(_projectId, _ipfs, _settings, _limits, _managerFeeBps, false);
     }
 
     function _createPool(
         string memory _projectId,
         string memory _ipfs,
         DirectPaymentsPool.PoolSettings memory _settings,
         DirectPaymentsPool.SafetyLimits memory _limits,
+        uint32 _managerFeeBps,
         bool useBeacon
     ) internal returns (DirectPaymentsPool pool) {
         //TODO: add check if msg.sender is whitelisted
 
         _settings.nftType = nextNftType;
-        bytes memory initCall = abi.encodeCall(DirectPaymentsPool.initialize, (nft, _settings, _limits, this));
+        bytes memory initCall = abi.encodeCall(
+            DirectPaymentsPool.initialize,
+            (nft, _settings, _limits, _managerFeeBps, this)
+        );
 
         if (useBeacon) {
             pool = DirectPaymentsPool(address(new BeaconProxy(address(impl), initCall)));

packages/contracts/contracts/DirectPayments/DirectPaymentsPool.sol

@@ -86,7 +86,6 @@ contract DirectPaymentsPool is
         IIdentityV2 uniquenessValidator;
         IERC20Upgradeable rewardToken;
         bool allowRewardOverride;
-        uint32 managerFeeBps;
     }
 
     struct SafetyLimits {
@@ -113,6 +112,8 @@ contract DirectPaymentsPool is
     LimitsData public globalLimits;
     DirectPaymentsFactory public registry;
 
+    uint32 public managerFeeBps;
+
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor(ISuperfluid _host, IV3SwapRouter _swapRouter) GoodCollectiveSuperApp(_host, _swapRouter) {}
 
@@ -127,7 +128,7 @@ contract DirectPaymentsPool is
     }
 
     function getManagerFee() public view override returns (address feeRecipient, uint32 feeBps) {
-        return (settings.manager, settings.managerFeeBps);
+        return (settings.manager, managerFeeBps);
     }
 
     /**
@@ -140,12 +141,14 @@ contract DirectPaymentsPool is
         ProvableNFT _nft,
         PoolSettings memory _settings,
         SafetyLimits memory _limits,
+        uint32 _managerFeeBps,
         DirectPaymentsFactory _registry
     ) external initializer {
         registry = _registry;
         settings = _settings;
         limits = _limits;
         nft = _nft;
+        managerFeeBps = _managerFeeBps;
         _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); // when using factory this gives factory role which then set role to the real msg.sender
         _setupRole(MANAGER_ROLE, _settings.manager);
         _setupRole(MINTER_ROLE, _settings.manager);
@@ -380,7 +383,8 @@ contract DirectPaymentsPool is
      * @dev Sets the settings for the pool.
      * @param _settings The new pool settings.
      */
-    function setPoolSettings(PoolSettings memory _settings) public onlyRole(MANAGER_ROLE) {
+    function setPoolSettings(PoolSettings memory _settings, uint32 _managerFeeBps) public onlyRole(MANAGER_ROLE) {
+        managerFeeBps = _managerFeeBps;
         if (_settings.nftType != settings.nftType) revert NFTTYPE_CHANGED();
         if (_settings.manager == address(0)) revert EMPTY_MANAGER();
 

packages/contracts/contracts/GoodCollective/GoodCollectiveSuperApp.sol

@@ -68,7 +68,7 @@ abstract contract GoodCollectiveSuperApp is SuperAppBaseFlow {
 
     IGoodCollectiveSuperApp.Stats public stats;
 
-    uint256[48] private _reserved;
+    uint256[45] private _reserved;
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor(ISuperfluid _host, IV3SwapRouter _swapRouter) SuperAppBaseFlow(_host) {

packages/contracts/contracts/GoodCollective/IGoodCollectiveSuperApp.sol

@@ -17,6 +17,7 @@ interface IGoodCollectiveSuperApp {
         address lastManagerFeeRecipient;
         uint256 protocolFees;
         uint256 managerFees;
+        // adding fields MUST update GoodCollectiveSuperApp storage layout
     }
 
     function getAdminFee() external view returns (address admin, uint32 feeBps);

packages/contracts/contracts/Interfaces.sol

@@ -22,5 +22,4 @@ struct PoolSettings {
     IMembersValidator membersValidator;
     IIdentityV2 uniquenessValidator;
     IERC20Upgradeable rewardToken;
-    uint32 managerFeeBps;
 }

packages/contracts/contracts/UBI/UBIPool.sol

@@ -57,10 +57,6 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
         // max number of members in a pool
         uint32 maxMembers;
         bool onlyMembers;
-        // max number of members that can claim in a day maxPeriodClaimers <= maxMembers
-        uint32 maxPeriodClaimers;
-        // min daily claim amount, daily amount will be 0 if <minClaimAmount
-        uint minClaimAmount;
     }
 
     struct PoolStatus {
@@ -81,11 +77,21 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
         uint32 membersCount;
     }
 
+    struct ExtendedSettings {
+        // max number of members that can claim in a day maxPeriodClaimers <= maxMembers
+        uint32 maxPeriodClaimers;
+        // min daily claim amount, daily amount will be 0 if <minClaimAmount
+        uint minClaimAmount;
+        // fees taken from income to the pool manager
+        uint32 managerFeeBps;
+    }
+
     PoolSettings public settings;
     UBISettings public ubiSettings;
     PoolStatus public status;
 
     UBIPoolFactory public registry;
+    ExtendedSettings public extendedSettings;
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor(ISuperfluid _host, IV3SwapRouter _swapRouter) GoodCollectiveSuperApp(_host, _swapRouter) {}
@@ -101,7 +107,7 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
     }
 
     function getManagerFee() public view override returns (address feeRecipient, uint32 feeBps) {
-        return (settings.manager, settings.managerFeeBps);
+        return (settings.manager, extendedSettings.managerFeeBps);
     }
 
     /**
@@ -112,11 +118,13 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
     function initialize(
         PoolSettings memory _settings,
         UBISettings memory _ubiSettings,
+        ExtendedSettings memory _extendedSettings,
         UBIPoolFactory _registry
     ) external initializer {
         registry = _registry;
         settings = _settings;
         ubiSettings = _ubiSettings;
+        extendedSettings = _extendedSettings;
         _verifyPoolSettings(_settings);
         _verifyUBISettings(_ubiSettings);
         _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); // when using factory this gives factory role which then set role to the real msg.sender
@@ -140,7 +148,7 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
      * the daily balance is determined by dividing current pool by the cycle length
      * @return The amount of GoodDollar the user can claim
      */
-    function distributionFormula() internal returns (uint256) {
+    function distributionFormula() public returns (uint256) {
         // once every claim cycle
         uint256 currentDay = getCurrentDay();
         if (currentDay >= status.currentDay + ubiSettings.claimPeriodDays) {
@@ -155,7 +163,7 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
 
             uint256 prevPeriodClaimers = status.periodClaimers;
             status.dailyUbi = nextDailyUbi;
-            if (status.dailyUbi <= ubiSettings.minClaimAmount) status.dailyUbi = 0;
+            if (status.dailyUbi <= extendedSettings.minClaimAmount) status.dailyUbi = 0;
 
             emit UBICalculated(
                 currentDay,
@@ -194,7 +202,7 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
             nextPeriodPool / max((status.periodClaimers * 10500) / 10000, ubiSettings.minActiveUsers)
         );
 
-        if (nextDailyUbi < ubiSettings.minClaimAmount) nextDailyUbi = 0;
+        if (nextDailyUbi < extendedSettings.minClaimAmount) nextDailyUbi = 0;
     }
 
     /**
@@ -241,7 +249,7 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
         if ((ubiSettings.maxMembers > 0 || ubiSettings.onlyMembers) && hasRole(MEMBER_ROLE, claimer) == false)
             revert NOT_MEMBER(claimer);
 
-        if (ubiSettings.maxPeriodClaimers > 0 && status.periodClaimers >= ubiSettings.maxPeriodClaimers)
+        if (extendedSettings.maxPeriodClaimers > 0 && status.periodClaimers >= extendedSettings.maxPeriodClaimers)
             revert MAX_PERIOD_CLAIMERS_REACHED(status.periodClaimers);
 
         // calculats the formula up today ie on day 0 there are no active users, on day 1 any user
@@ -314,9 +322,13 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
      * @dev Sets the safety limits for the pool.
      * @param _ubiSettings The new safety limits.
      */
-    function setUBISettings(UBISettings memory _ubiSettings) public onlyRole(MANAGER_ROLE) {
+    function setUBISettings(
+        UBISettings memory _ubiSettings,
+        ExtendedSettings memory _extendedSettings
+    ) public onlyRole(MANAGER_ROLE) {
         _verifyUBISettings(_ubiSettings);
         ubiSettings = _ubiSettings;
+        extendedSettings = _extendedSettings;
         emit UBISettingsChanged(_ubiSettings);
     }
 
@@ -371,7 +383,8 @@ contract UBIPool is AccessControlUpgradeable, GoodCollectiveSuperApp, UUPSUpgrad
         // current day has already been updated which means
         // that the dailyUbi has been updated
         if (status.currentDay == getCurrentDay() && status.dailyUbi > 0) {
-            if (ubiSettings.maxPeriodClaimers > 0 && status.periodClaimers >= ubiSettings.maxPeriodClaimers) return 0;
+            if (extendedSettings.maxPeriodClaimers > 0 && status.periodClaimers >= extendedSettings.maxPeriodClaimers)
+                return 0;
             return hasClaimed(_member) ? 0 : status.dailyUbi;
         }
         return estimateNextDailyUBI();

packages/contracts/contracts/UBI/UBIPoolFactory.sol

@@ -87,30 +87,33 @@ contract UBIPoolFactory is AccessControlUpgradeable, UUPSUpgradeable {
         string memory _projectId,
         string memory _ipfs,
         PoolSettings memory _settings,
-        UBIPool.UBISettings memory _limits
+        UBIPool.UBISettings memory _limits,
+        UBIPool.ExtendedSettings memory _extendedSettings
     ) external onlyProjectOwnerOrNon(_projectId) returns (UBIPool pool) {
-        return _createPool(_projectId, _ipfs, _settings, _limits, true);
+        return _createPool(_projectId, _ipfs, _settings, _limits, _extendedSettings, true);
     }
 
     function createPool(
         string memory _projectId,
         string memory _ipfs,
         PoolSettings memory _settings,
-        UBIPool.UBISettings memory _limits
+        UBIPool.UBISettings memory _limits,
+        UBIPool.ExtendedSettings memory _extendedSettings
     ) external onlyProjectOwnerOrNon(_projectId) returns (UBIPool pool) {
-        return _createPool(_projectId, _ipfs, _settings, _limits, false);
+        return _createPool(_projectId, _ipfs, _settings, _limits, _extendedSettings, false);
     }
 
     function _createPool(
         string memory _projectId,
         string memory _ipfs,
         PoolSettings memory _settings,
         UBIPool.UBISettings memory _limits,
+        UBIPool.ExtendedSettings memory _extendedSettings,
         bool useBeacon
     ) internal returns (UBIPool pool) {
         //TODO: add check if msg.sender is whitelisted
 
-        bytes memory initCall = abi.encodeCall(UBIPool.initialize, (_settings, _limits, this));
+        bytes memory initCall = abi.encodeCall(UBIPool.initialize, (_settings, _limits, _extendedSettings, this));
 
         if (useBeacon) {
             pool = UBIPool(address(new BeaconProxy(address(impl), initCall)));