refactor: update membership pricing to have fee added on top of price

packages/contracts/src/spaces/facets/membership/IMembership.sol

@@ -63,7 +63,6 @@ interface IMembershipBase {
     error Membership__InvalidPricingModule();
     error Membership__AlreadyMember();
     error Membership__InsufficientPayment();
-    error Membership__PriceTooLow();
     error Membership__MaxSupplyReached();
     error Membership__InvalidTokenId();
     error Membership__NotExpired();

packages/contracts/src/spaces/facets/membership/MembershipBase.sol

@@ -20,28 +20,27 @@ abstract contract MembershipBase is IMembershipBase {
     using SafeTransferLib for address;
 
     function __MembershipBase_init(Membership memory info, address spaceFactory) internal {
-        MembershipStorage.Layout storage ds = MembershipStorage.layout();
+        MembershipStorage.Layout storage $ = MembershipStorage.layout();
 
-        ds.spaceFactory = spaceFactory;
-        ds.pricingModule = info.pricingModule;
-        ds.membershipCurrency = CurrencyTransfer.NATIVE_TOKEN;
-        ds.membershipMaxSupply = info.maxSupply;
+        $.spaceFactory = spaceFactory;
+        $.pricingModule = info.pricingModule;
+        $.membershipCurrency = CurrencyTransfer.NATIVE_TOKEN;
+        $.membershipMaxSupply = info.maxSupply;
 
         if (info.freeAllocation > 0) {
             _verifyFreeAllocation(info.freeAllocation);
-            ds.freeAllocation = info.freeAllocation;
+            $.freeAllocation = info.freeAllocation;
         }
 
-        ds.freeAllocationEnabled = true;
+        $.freeAllocationEnabled = true;
 
         if (info.price > 0) {
-            _verifyPrice(info.price);
             IMembershipPricing(info.pricingModule).setPrice(info.price);
         }
 
         if (info.duration > 0) {
             _verifyDuration(info.duration);
-            ds.membershipDuration = info.duration;
+            $.membershipDuration = info.duration;
         }
     }
 
@@ -64,24 +63,39 @@ abstract contract MembershipBase is IMembershipBase {
         );
     }
 
+    function _getMembershipPrice(
+        uint256 totalSupply
+    ) internal view virtual returns (uint256 membershipPrice) {
+        address pricingModule = _getPricingModule();
+        if (pricingModule == address(0)) return 0;
+        uint256 freeAllocation = _getMembershipFreeAllocation();
+        membershipPrice = IMembershipPricing(pricingModule).getPrice(freeAllocation, totalSupply);
+    }
+
     function _getProtocolFee(uint256 membershipPrice) internal view returns (uint256) {
         IPlatformRequirements platform = _getPlatformRequirements();
+        uint256 baseFee = platform.getMembershipFee();
+        if (membershipPrice == 0) return baseFee;
+        uint256 bpsFee = BasisPoints.calculate(membershipPrice, platform.getMembershipBps());
+        return FixedPointMathLib.max(bpsFee, baseFee);
+    }
 
-        uint256 minPrice = platform.getMembershipMinPrice();
-
-        if (membershipPrice < minPrice) return platform.getMembershipFee();
-
-        return BasisPoints.calculate(membershipPrice, platform.getMembershipBps());
+    function _getTotalMembershipPayment(
+        uint256 membershipPrice
+    ) internal view returns (uint256 totalRequired, uint256 protocolFee) {
+        protocolFee = _getProtocolFee(membershipPrice);
+        if (membershipPrice == 0) return (protocolFee, protocolFee);
+        return (membershipPrice + protocolFee, protocolFee);
     }
 
     function _transferIn(address from, uint256 amount) internal returns (uint256) {
-        MembershipStorage.Layout storage ds = MembershipStorage.layout();
+        MembershipStorage.Layout storage $ = MembershipStorage.layout();
 
         // get the currency being used for membership
         address currency = _getMembershipCurrency();
 
         if (currency == CurrencyTransfer.NATIVE_TOKEN) {
-            ds.tokenBalance += amount;
+            $.tokenBalance += amount;
             return amount;
         }
 
@@ -94,7 +108,7 @@ abstract contract MembershipBase is IMembershipBase {
         uint256 finalAmount = balanceAfter - balanceBefore;
         if (finalAmount != amount) Membership__InsufficientPayment.selector.revertWith();
 
-        ds.tokenBalance += finalAmount;
+        $.tokenBalance += finalAmount;
         return finalAmount;
     }
 
@@ -150,31 +164,9 @@ abstract contract MembershipBase is IMembershipBase {
     }
 
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
-    /*                           PRICING                          */
+    /*                           RENEWAL                          */
     /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
 
-    function _verifyPrice(uint256 newPrice) internal view {
-        uint256 minFee = _getPlatformRequirements().getMembershipFee();
-        if (newPrice < minFee) Membership__PriceTooLow.selector.revertWith();
-    }
-
-    /// @dev Makes it virtual to allow other pricing strategies
-    function _getMembershipPrice(
-        uint256 totalSupply
-    ) internal view virtual returns (uint256 membershipPrice) {
-        // get free allocation
-        uint256 freeAllocation = _getMembershipFreeAllocation();
-        address pricingModule = _getPricingModule();
-
-        IPlatformRequirements platform = _getPlatformRequirements();
-
-        if (pricingModule == address(0)) return platform.getMembershipFee();
-
-        membershipPrice = IMembershipPricing(pricingModule).getPrice(freeAllocation, totalSupply);
-        uint256 minPrice = platform.getMembershipMinPrice();
-        if (membershipPrice < minPrice) return platform.getMembershipFee();
-    }
-
     function _setMembershipRenewalPrice(uint256 tokenId, uint256 pricePaid) internal {
         MembershipStorage.layout().renewalPriceByTokenId[tokenId] = pricePaid;
     }
@@ -183,16 +175,10 @@ abstract contract MembershipBase is IMembershipBase {
         uint256 tokenId,
         uint256 totalSupply
     ) internal view returns (uint256) {
-        MembershipStorage.Layout storage ds = MembershipStorage.layout();
-        IPlatformRequirements platform = _getPlatformRequirements();
-
-        uint256 minFee = platform.getMembershipFee();
-        uint256 renewalPrice = ds.renewalPriceByTokenId[tokenId];
-
-        if (renewalPrice != 0) return FixedPointMathLib.max(renewalPrice, minFee);
-
-        uint256 price = _getMembershipPrice(totalSupply);
-        return FixedPointMathLib.max(price, minFee);
+        MembershipStorage.Layout storage $ = MembershipStorage.layout();
+        uint256 renewalPrice = $.renewalPriceByTokenId[tokenId];
+        if (renewalPrice != 0) return renewalPrice;
+        return _getMembershipPrice(totalSupply);
     }
 
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
@@ -207,16 +193,15 @@ abstract contract MembershipBase is IMembershipBase {
     }
 
     function _setMembershipFreeAllocation(uint256 newAllocation) internal {
-        MembershipStorage.Layout storage ds = MembershipStorage.layout();
-        ds.freeAllocation = newAllocation;
-        ds.freeAllocationEnabled = true;
+        MembershipStorage.Layout storage $ = MembershipStorage.layout();
+        ($.freeAllocation, $.freeAllocationEnabled) = (newAllocation, true);
         emit MembershipFreeAllocationUpdated(newAllocation);
     }
 
     function _getMembershipFreeAllocation() internal view returns (uint256) {
-        MembershipStorage.Layout storage ds = MembershipStorage.layout();
+        MembershipStorage.Layout storage $ = MembershipStorage.layout();
 
-        if (ds.freeAllocationEnabled) return ds.freeAllocation;
+        if ($.freeAllocationEnabled) return $.freeAllocation;
 
         return _getPlatformRequirements().getMembershipMintLimit();
     }

packages/contracts/src/spaces/facets/membership/MembershipFacet.sol

@@ -98,23 +98,26 @@ contract MembershipFacet is IMembership, MembershipJoin, ReentrancyGuard, Facet
 
     /// @inheritdoc IMembership
     function setMembershipPrice(uint256 newPrice) external onlyOwner {
-        _verifyPrice(newPrice);
         IMembershipPricing(_getPricingModule()).setPrice(newPrice);
     }
 
     /// @inheritdoc IMembership
-    function getMembershipPrice() external view returns (uint256) {
-        return _getMembershipPrice(_totalSupply());
+    function getMembershipPrice() external view returns (uint256 totalRequired) {
+        (totalRequired, ) = _getTotalMembershipPayment(_getMembershipPrice(_totalSupply()));
     }
 
     /// @inheritdoc IMembership
-    function getMembershipRenewalPrice(uint256 tokenId) external view returns (uint256) {
-        return _getMembershipRenewalPrice(tokenId, _totalSupply());
+    function getMembershipRenewalPrice(
+        uint256 tokenId
+    ) external view returns (uint256 totalRequired) {
+        (totalRequired, ) = _getTotalMembershipPayment(
+            _getMembershipRenewalPrice(tokenId, _totalSupply())
+        );
     }
 
     /// @inheritdoc IMembership
-    function getProtocolFee() external view returns (uint256) {
-        return _getProtocolFee(_getMembershipPrice(_totalSupply()));
+    function getProtocolFee() external view returns (uint256 protocolFee) {
+        (, protocolFee) = _getTotalMembershipPayment(_getMembershipPrice(_totalSupply()));
     }
 
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
@@ -131,7 +134,6 @@ contract MembershipFacet is IMembership, MembershipJoin, ReentrancyGuard, Facet
             Membership__InvalidFreeAllocation.selector.revertWith();
         }
 
-        // verify newLimit is not more than the allowed platform limit
         _verifyFreeAllocation(newAllocation);
         _setMembershipFreeAllocation(newAllocation);
     }

packages/contracts/src/spaces/facets/membership/MembershipStorage.sol

@@ -31,10 +31,9 @@ library MembershipStorage {
         bool freeAllocationEnabled;
     }
 
-    function layout() internal pure returns (Layout storage l) {
-        bytes32 slot = STORAGE_SLOT;
+    function layout() internal pure returns (Layout storage $) {
         assembly {
-            l.slot := slot
+            $.slot := STORAGE_SLOT
         }
     }
 }

packages/contracts/src/spaces/facets/membership/join/MembershipJoin.sol

@@ -74,22 +74,21 @@ abstract contract MembershipJoin is
         uint256 totalSupply = _totalSupply();
         uint256 membershipPrice = _getMembershipPrice(totalSupply);
         uint256 freeAllocation = _getMembershipFreeAllocation();
-        uint256 prepaidSupply = _getPrepaidSupply();
 
         joinDetails.basePrice = membershipPrice;
         if (freeAllocation > totalSupply) {
             return joinDetails;
         }
 
         // Check if this is a free join due to prepaid supply
+        uint256 prepaidSupply = _getPrepaidSupply();
         if (prepaidSupply > 0) {
             joinDetails.isPrepaid = true;
             return joinDetails;
         }
 
-        // Regular paid join
-        joinDetails.amountDue = membershipPrice;
-        joinDetails.shouldCharge = true;
+        (uint256 totalRequired, ) = _getTotalMembershipPayment(membershipPrice);
+        (joinDetails.amountDue, joinDetails.shouldCharge) = (totalRequired, true);
     }
 
     /// @notice Handles the process of joining a space
@@ -144,9 +143,7 @@ abstract contract MembershipJoin is
         }
 
         // Consume prepaid membership if applicable
-        if (joinDetails.isPrepaid) {
-            _reducePrepay(1);
-        }
+        if (joinDetails.isPrepaid) _reducePrepay(1);
 
         _validateUserReferral(receiver, referral);
 
@@ -186,10 +183,8 @@ abstract contract MembershipJoin is
     }
 
     function _validateUserReferral(address receiver, ReferralTypes memory referral) internal view {
-        if (referral.userReferral != address(0)) {
-            if (referral.userReferral == receiver || referral.userReferral == msg.sender) {
-                Membership__InvalidAddress.selector.revertWith();
-            }
+        if (referral.userReferral == receiver || referral.userReferral == msg.sender) {
+            Membership__InvalidAddress.selector.revertWith();
         }
     }
 
@@ -320,8 +315,7 @@ abstract contract MembershipJoin is
             sender,
             receiver,
             joinDetails.amountDue,
-            ownerProceeds,
-            joinDetails.basePrice
+            ownerProceeds
         );
     }
 
@@ -368,8 +362,7 @@ abstract contract MembershipJoin is
             sender,
             receiver,
             joinDetails.amountDue,
-            ownerProceeds,
-            joinDetails.basePrice
+            ownerProceeds
         );
     }
 
@@ -378,16 +371,15 @@ abstract contract MembershipJoin is
         address payer,
         address receiver,
         uint256 paymentRequired,
-        uint256 ownerProceeds,
-        uint256 membershipPrice
+        uint256 ownerProceeds
     ) internal {
         // account for owner's proceeds
         if (ownerProceeds != 0) _transferIn(payer, ownerProceeds);
 
         _releaseCapturedValue(transactionId, paymentRequired);
         _deleteCapturedData(transactionId);
 
-        _mintMembershipPoints(receiver, membershipPrice);
+        _mintMembershipPoints(receiver, paymentRequired);
     }
 
     /// @notice Issues a membership token to the receiver
@@ -501,22 +493,25 @@ abstract contract MembershipJoin is
 
     function _renewMembership(address payer, uint256 tokenId) internal {
         address receiver = _ownerOf(tokenId);
-
         if (receiver == address(0)) Membership__InvalidAddress.selector.revertWith();
 
         uint256 duration = _getMembershipDuration();
-        uint256 membershipPrice = _getMembershipRenewalPrice(tokenId, _totalSupply());
+        uint256 basePrice = _getMembershipRenewalPrice(tokenId, _totalSupply());
+        (uint256 totalRequired, ) = _getTotalMembershipPayment(basePrice);
 
-        if (membershipPrice > msg.value) Membership__InvalidPayment.selector.revertWith();
+        if (totalRequired > msg.value) Membership__InvalidPayment.selector.revertWith();
 
-        _mintMembershipPoints(receiver, membershipPrice);
+        // Collect protocol fee (transfers from payer)
+        uint256 protocolFee = _collectProtocolFee(payer, basePrice);
 
-        uint256 protocolFee = _collectProtocolFee(payer, membershipPrice);
+        // Calculate owner proceeds (what goes to space owner)
+        uint256 ownerProceeds = totalRequired - protocolFee;
 
-        uint256 remainingDue = membershipPrice - protocolFee;
-        if (remainingDue > 0) _transferIn(payer, remainingDue);
+        // Transfer owner proceeds to contract
+        if (ownerProceeds > 0) _transferIn(payer, ownerProceeds);
 
-        uint256 excess = msg.value - membershipPrice;
+        // Handle excess payment
+        uint256 excess = msg.value - totalRequired;
         if (excess > 0) {
             CurrencyTransfer.transferCurrency(
                 _getMembershipCurrency(),
@@ -526,6 +521,7 @@ abstract contract MembershipJoin is
             );
         }
 
+        _mintMembershipPoints(receiver, totalRequired);
         _renewSubscription(tokenId, uint64(duration));
     }