Increase line coverage

bosonprotocol · Nov 28, 2023 · a221393 · a221393
1 parent df1e1d7
commit a221393
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 29 deletions.
diff --git a/contracts/protocol/bases/BeaconClientBase.sol b/contracts/protocol/bases/BeaconClientBase.sol
@@ -105,19 +105,6 @@ abstract contract BeaconClientBase is BosonTypes, BosonErrors {
         return IBosonExchangeHandler(protocolDiamond).onPremintedVoucherTransferred(_tokenId, _to, _from, _rangeOwner);
     }
 
-    /**
-     * @notice Gets the info about the seller associated with the sellerId.
-     *
-     * @param _sellerId - the id of the seller
-     * @return exists - the seller was found
-     * @return seller - the seller associated with the _sellerId
-     */
-    function getBosonSeller(uint256 _sellerId) internal view returns (bool exists, Seller memory seller) {
-        address protocolDiamond = IClientExternalAddresses(BeaconClientLib._beacon()).getProtocolAddress();
-
-        (exists, seller, ) = IBosonAccountHandler(protocolDiamond).getSeller(_sellerId);
-    }
-
     /**
      * @notice Gets the info about the seller associated with the address.
      *

diff --git a/contracts/protocol/facets/SellerHandlerFacet.sol b/contracts/protocol/facets/SellerHandlerFacet.sol
@@ -155,33 +155,27 @@ contract SellerHandlerFacet is SellerBase {
                 if (_seller.treasury == address(0)) revert InvalidAddress();
 
                 // Check if new treasury is already a royalty recipient
-                uint256 royaltyRecipientId = lookups.royaltyRecipientIndexBySellerAndRecipient[_seller.id][
-                    _seller.treasury
-                ];
+                mapping(address => uint256) storage royaltyRecipientIndexBySellerAndRecipient = lookups
+                    .royaltyRecipientIndexBySellerAndRecipient[_seller.id];
+                uint256 royaltyRecipientId = royaltyRecipientIndexBySellerAndRecipient[_seller.treasury];
 
                 RoyaltyRecipient[] storage royaltyRecipients = lookups.royaltyRecipientsBySeller[_seller.id];
                 if (royaltyRecipientId != 0) {
                     // If the new treasury is already a royalty recipient, remove it
-                    // TODO: check if can be refactored to use with removeRoyaltyRecipient
+                    royaltyRecipientId--; // royaltyRecipientId is 1-based, so we need to decrement it to get the index
                     uint256 lastRoyaltyRecipientsId = royaltyRecipients.length - 1;
                     if (royaltyRecipientId != lastRoyaltyRecipientsId) {
                         royaltyRecipients[royaltyRecipientId] = royaltyRecipients[lastRoyaltyRecipientsId];
-                        lookups.royaltyRecipientIndexBySellerAndRecipient[_seller.id][
-                            royaltyRecipients[royaltyRecipientId].wallet
-                        ] = royaltyRecipientId;
+                        royaltyRecipientIndexBySellerAndRecipient[royaltyRecipients[royaltyRecipientId].wallet] =
+                            royaltyRecipientId +
+                            1;
                     }
-                    delete royaltyRecipients[lastRoyaltyRecipientsId];
+                    royaltyRecipients.pop();
                 }
 
                 // Update treasury
                 seller.treasury = _seller.treasury;
 
-                // Update default royalty recipient
-                lookups.royaltyRecipientsBySeller[_seller.id][0].wallet = _seller.treasury;
-
-                // Update treasury
-                seller.treasury = _seller.treasury;
-
                 updateApplied = true;
 
                 emit RoyaltyRecipientsChanged(_seller.id, royaltyRecipients, msgSender());

diff --git a/test/protocol/SellerHandlerTest.js b/test/protocol/SellerHandlerTest.js
@@ -716,7 +716,7 @@ describe("SellerHandler", function () {
 
           // Attempt to create a seller expecting revert
           await expect(
-            accountHandler.connect(rando).createSeller(seller, emptyAuthToken, voucherInitValues)
+            accountHandler.connect(assistant).createSeller(seller, emptyAuthToken, voucherInitValues)
           ).to.revertedWithCustomError(bosonErrors, RevertReasons.REGION_PAUSED);
         });
 
@@ -725,7 +725,7 @@ describe("SellerHandler", function () {
 
           // Attempt to Create a seller, expecting revert
           await expect(
-            accountHandler.connect(rando).createSeller(seller, emptyAuthToken, voucherInitValues)
+            accountHandler.connect(assistant).createSeller(seller, emptyAuthToken, voucherInitValues)
           ).to.be.revertedWithCustomError(bosonErrors, RevertReasons.MUST_BE_ACTIVE);
         });
 
@@ -986,6 +986,15 @@ describe("SellerHandler", function () {
             accountHandler.connect(admin).createSeller(seller, emptyAuthToken, voucherInitValues)
           ).to.revertedWithCustomError(bosonErrors, RevertReasons.CLONE_CREATION_FAILED);
         });
+
+        it("Royalty percentage is above the limit", async function () {
+          voucherInitValues.royaltyPercentage = "50000"; //50%
+
+          // Attempt to Create a seller, expecting revert
+          await expect(
+            accountHandler.connect(assistant).createSeller(seller, emptyAuthToken, voucherInitValues)
+          ).to.be.revertedWithCustomError(bosonErrors, RevertReasons.INVALID_ROYALTY_PERCENTAGE);
+        });
       });
     });
 
@@ -2223,6 +2232,42 @@ describe("SellerHandler", function () {
           .withArgs(seller.id, pendingSellerUpdateStruct, emptyAuthTokenStruct, admin.address);
       });
 
+      it("update treasury, when it's already one of the royalty recipients", async function () {
+        // add some royalty recipients
+        const newRoyaltyRecipients = new RoyaltyRecipientList([
+          new RoyaltyRecipient(other1.address, "100", "other1"),
+          new RoyaltyRecipient(other2.address, "200", "other2"),
+          new RoyaltyRecipient(other3.address, "300", "other3"),
+        ]);
+        await accountHandler.connect(admin).addRoyaltyRecipients(seller.id, newRoyaltyRecipients.toStruct());
+
+        // Default royalty recipient is set
+        let expectedRoyaltyRecipientList = new RoyaltyRecipientList([
+          new RoyaltyRecipient(ZeroAddress, voucherInitValues.royaltyPercentage, DEFAULT_ROYALTY_RECIPIENT),
+          ...newRoyaltyRecipients.royaltyRecipients,
+        ]);
+
+        let royaltyRecipientList = RoyaltyRecipientList.fromStruct(
+          await accountHandler.connect(rando).getRoyaltyRecipients(seller.id)
+        );
+        expect(royaltyRecipientList).to.deep.equal(expectedRoyaltyRecipientList, "Royalty recipient list mismatch");
+
+        // Update seller's treasury
+        seller.treasury = other1.address;
+        await accountHandler.connect(assistant).updateSeller(seller, emptyAuthToken);
+
+        // Default royalty recipient is set
+        expectedRoyaltyRecipientList = new RoyaltyRecipientList([
+          new RoyaltyRecipient(ZeroAddress, voucherInitValues.royaltyPercentage, DEFAULT_ROYALTY_RECIPIENT),
+          new RoyaltyRecipient(other3.address, "300", "other3"),
+          new RoyaltyRecipient(other2.address, "200", "other2"),
+        ]);
+        royaltyRecipientList = RoyaltyRecipientList.fromStruct(
+          await accountHandler.connect(rando).getRoyaltyRecipients(seller.id)
+        );
+        expect(royaltyRecipientList).to.deep.equal(expectedRoyaltyRecipientList, "Royalty recipient list mismatch");
+      });
+
       context("💔 Revert Reasons", async function () {
         it("The sellers region of protocol is paused", async function () {
           // Pause the sellers region of the protocol