From a2213933f689bc57230ee06125ed9274ce0f6349 Mon Sep 17 00:00:00 2001 From: zajck Date: Tue, 28 Nov 2023 11:08:37 +0100 Subject: [PATCH] Increase line coverage --- contracts/protocol/bases/BeaconClientBase.sol | 13 ----- .../protocol/facets/SellerHandlerFacet.sol | 22 +++------ test/protocol/SellerHandlerTest.js | 49 ++++++++++++++++++- 3 files changed, 55 insertions(+), 29 deletions(-) diff --git a/contracts/protocol/bases/BeaconClientBase.sol b/contracts/protocol/bases/BeaconClientBase.sol index 4b40d216e..1cb19cdea 100644 --- 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 index f68dc50ba..90030d374 100644 --- 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 index e137f2fef..73053dae1 100644 --- 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