refactor: avoid duplicated currencies

contracts/RightsManager.sol

@@ -105,6 +105,38 @@ contract RightsManager is
         address newImplementation
     ) internal override onlyAdmin {}
 
+    /// @notice Checks if the given distributor is active and not blocked.
+    /// @param distributor The address of the distributor to check.
+    /// @return True if the distributor is active, false otherwise.
+    function _checkActiveDistributor(
+        address distributor
+    ) internal returns (bool) {
+        IRegistrableVerifiable _v = IRegistrableVerifiable(syndication);
+        return _v.isActive(distributor); // is active status in syndication
+    }
+
+    /// @notice Checks if the given content is active and not blocked.
+    /// @param contentId The ID of the content to check.
+    /// @return True if the content is active, false otherwise.
+    function _checkActiveContent(
+        uint256 contentId
+    ) internal view returns (bool) {
+        IReferendumVerifiable _v = IReferendumVerifiable(referendum);
+        return _v.isActive(contentId); // is active in referendum
+    }
+
+    /// @notice Checks if the given content is approved for distribution.
+    /// @param to The address attempting to distribute the content.
+    /// @param contentId The ID of the content to be distributed.
+    /// @return True if the content is approved, false otherwise.
+    function _checkApprovedContent(
+        address to,
+        uint256 contentId
+    ) internal view returns (bool) {
+        IReferendumVerifiable _v = IReferendumVerifiable(referendum);
+        return _v.isApproved(to, contentId); // is approved by referendu,
+    }
+
     /// @notice Modifier to restrict access to the holder only or their delegate.
     /// @param contentId The content hash to give distribution rights.
     /// @dev Only the holder of the content and the delegated holder can pass this validation.
@@ -146,35 +178,6 @@ contract RightsManager is
         _;
     }
 
-    /// @notice Checks if the given distributor is active and not blocked.
-    /// @param distributor The address of the distributor to check.
-    /// @return True if the distributor is active, false otherwise.
-    function _checkActiveDistributor(
-        address distributor
-    ) internal returns (bool) {
-        return IRegistrableVerifiable(syndication).isActive(distributor);
-    }
-
-    /// @notice Checks if the given content is active and not blocked.
-    /// @param contentId The ID of the content to check.
-    /// @return True if the content is active, false otherwise.
-    function _checkActiveContent(
-        uint256 contentId
-    ) internal view returns (bool) {
-        return IReferendumVerifiable(referendum).isActive(contentId);
-    }
-
-    /// @notice Checks if the given content is approved for distribution.
-    /// @param to The address attempting to distribute the content.
-    /// @param contentId The ID of the content to be distributed.
-    /// @return True if the content is approved, false otherwise.
-    function _checkApprovedContent(
-        address to,
-        uint256 contentId
-    ) internal view returns (bool) {
-        return IReferendumVerifiable(referendum).isApproved(to, contentId);
-    }
-
     /// @inheritdoc ITreasury
     /// @notice Sets a new treasury fee for a specific token.
     /// @param newTreasuryFee The new fee amount to be set.

contracts/base/upgradeable/CurrencyManagerUpgradeable.sol

@@ -15,8 +15,8 @@ abstract contract CurrencyManagerUpgradeable is
     /// @custom:storage-location erc7201:currencymanagarupgradeable
     struct CurrencyManagerStorage {
         // Maps currency addresses to their index in the supported tokens array
-        mapping(address => uint256) _supportedCurrencyMap; 
-         // Array of supported currency addresses
+        mapping(address => uint256) _supportedCurrencyMap;
+        // Array of supported currency addresses
         address[] _supportedCurrencies;
     }
 
@@ -45,16 +45,15 @@ abstract contract CurrencyManagerUpgradeable is
 
     /// @notice Initializes the currency manager. To be called during contract initialization.
     function __CurrencyManager_init() internal onlyInitializing {}
+
     function __CurrencyManager_init_unchained() internal onlyInitializing {}
 
     /// @notice Adds a currency to the list of supported currencies.
     /// @param currency The address of the currency to add.
     function _addCurrency(address currency) internal {
         CurrencyManagerStorage storage $ = _getCurrencyManagerStorage();
-        // TODO avoid duplicate currencies...
-        // if ($._supportedCurrencyMap[currency] != 0)
-
-
+        // avoid duplicate currencies...
+        if (isCurrencySupported(currency)) return;
         $._supportedCurrencies.push(currency);
         // Add the last index for the current stored currency as value for mapping
         $._supportedCurrencyMap[currency] = $._supportedCurrencies.length;

contracts/modules/RentModule.sol

@@ -205,7 +205,6 @@ contract RentModule is
 
         // Add access to content for N days to account..
         IRightsManager(drmAddress).grantAccess(rentalWatcher, contentId, cond);
-        // return the resulting data, the deadline..
         return abi.encode(rentRegistry[contentId][rentalWatcher], currency);
     }
 

test/Syndication.ts

@@ -38,6 +38,23 @@ async function getSyndicatableFakeGovernor() {
 }
 
 
+// helper function to approve a distributor by "governance"
+async function syndicationWithGovernance() {
+  const [owner] = await getAccounts()
+  const fees = hre.ethers.parseUnits('0.3', 'ether')
+  const [distributorAddress, distributorContract] = await switcher(deployDistributor)
+  const [syndication] = await switcher(deployInitializedSyndication)
+
+  // !IMPORTANT the approval is only allowed for governance
+  // A VALID GOVERNOR IS SET IN A REAL USE CASE..
+  // eg: https://docs.openzeppelin.com/contracts/4.x/api/governance#GovernorTimelockControl
+  // set the owner address as governor for test purposes..
+  await (await syndication.setGovernance(owner.address)).wait()
+  // register account and approve by "governance" 
+  await (await syndication.register(distributorAddress, { value: fees })).wait()
+  return [syndication, distributorContract, distributorAddress];
+}
+
 describe('Syndication', function () {
   describe("Initialization", async () => {
     it('Should have been initialized with the right penalty rate.', async function () {
@@ -200,6 +217,19 @@ describe('Syndication', function () {
       expect(afterQuitBalance).to.be.equal(penal)
     })
 
+    it('Should fail with InvalidDistributorContract.', async function () {
+      const fees = hre.ethers.parseUnits('0.3', 'ether') // expected fees paid in contract..
+      const invalidDistributor = hre.ethers.ZeroAddress // invalid contract
+      const [distributorAddress] = await switcher(deployDistributor)
+      const [syndication] = await switcher(deployInitializedSyndication)
+
+      // register a valid distributor then attemp quit with an invalid one.
+      await (await syndication.register(distributorAddress, { value: fees })).wait()
+      await expect(syndication.quit(invalidDistributor)).to.be.revertedWithCustomError(
+        syndication, 'InvalidDistributorContract'
+      )
+    })
+
     it('Should revert the correct amount to manager after quit.', async function () {
       const [owner] = await getAccounts()
       const fees = hre.ethers.parseUnits('0.3', 'ether') // expected fees paid in contract..
@@ -239,23 +269,6 @@ describe('Syndication', function () {
 
   describe('Approve', function () {
 
-    // helper function to approve a distributor by "governance"
-    async function syndicationWithGovernance() {
-      const [owner] = await getAccounts()
-      const fees = hre.ethers.parseUnits('0.3', 'ether')
-      const [distributorAddress, distributorContract] = await switcher(deployDistributor)
-      const [syndication] = await switcher(deployInitializedSyndication)
-
-      // !IMPORTANT the approval is only allowed for governance
-      // A VALID GOVERNOR IS SET IN A REAL USE CASE..
-      // eg: https://docs.openzeppelin.com/contracts/4.x/api/governance#GovernorTimelockControl
-      // set the owner address as governor for test purposes..
-      await (await syndication.setGovernance(owner.address)).wait()
-      // register account and approve by "governance" 
-      await (await syndication.register(distributorAddress, { value: fees })).wait()
-      return [syndication, distributorContract, distributorAddress];
-    }
-
     it('Should emit Approved event after approve successfully.', async function () {
       const [syndication, , distributorAddress] = await syndicationWithGovernance()
       expect(await syndication.approve(distributorAddress)).to.emit(syndication, 'Approved')
@@ -284,32 +297,15 @@ describe('Syndication', function () {
   })
 
   describe('Revoke', function () {
-
-    // helper function to approve a distributor by "governance"
-    async function syndicationWithGovernance() {
-      const [owner] = await getAccounts()
-      const fees = hre.ethers.parseUnits('0.3', 'ether')
-      const [distributorAddress, distributorContract] = await switcher(deployDistributor)
-      const [syndication] = await switcher(deployInitializedSyndication)
-
-      // !IMPORTANT the approval is only allowed for governance
-      // A VALID GOVERNOR IS SET IN A REAL USE CASE..
-      // eg: https://docs.openzeppelin.com/contracts/4.x/api/governance#GovernorTimelockControl
-      // set the owner address as governor for test purposes..
-      await (await syndication.setGovernance(owner.address)).wait()
-      // register account and approve by "governance" 
-      await (await syndication.register(distributorAddress, { value: fees })).wait()
-      return [syndication, distributorContract, distributorAddress];
-    }
-
     it('Should emit Revoked event after revoked successfully.', async function () {
       const [syndication, , distributorAddress] = await syndicationWithGovernance()
-      // after being active can be blocked again..
+      await (await syndication.approve(distributorAddress)).wait()
       expect(await syndication.revoke(distributorAddress)).to.emit(syndication, 'Revoked')
     })
 
     it('Should set valid revoked state after approval.', async function () {
       const [syndication, , distributorAddress] = await syndicationWithGovernance()
+      await (await syndication.approve(distributorAddress)).wait()
       await (await syndication.revoke(distributorAddress)).wait()
       expect(await syndication.isBlocked(distributorAddress)).to.be.true
     })