Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SHF-02C] Non-Uniform Royalty Recipient ID Definition #873

Closed
zajck opened this issue Jan 8, 2024 · 0 comments · Fixed by #887
Closed

[SHF-02C] Non-Uniform Royalty Recipient ID Definition #873

zajck opened this issue Jan 8, 2024 · 0 comments · Fixed by #887
Assignees
Labels
documentation Improvements or additions to documentation v2.4.0

Comments

@zajck
Copy link
Member

zajck commented Jan 8, 2024

SHF-02C: Non-Uniform Royalty Recipient ID Definition

Type Severity Location
Standard Conformity SellerHandlerFacet.sol:L555

Description:

The royalty recipient IDs are 1-based in the Boson Protocol system, however, they are expected to be passed in as 0-based in the SellerHandlerFacet::updateRoyaltyRecipients function.

Example:

function updateRoyaltyRecipients(
    uint256 _sellerId,
    uint256[] calldata _royaltyRecipientIds,
    RoyaltyRecipient[] calldata _royaltyRecipients
) external sellersNotPaused nonReentrant {
    // Cache protocol lookups and sender for reference
    ProtocolLib.ProtocolLookups storage lookups = protocolLookups();

    // Make sure admin is the caller and get the seller
    address treasury;
    {
        (Seller storage seller, ) = validateAdminStatus(lookups, _sellerId);
        treasury = seller.treasury;
    }

    if (_royaltyRecipientIds.length != _royaltyRecipients.length) revert ArrayLengthMismatch();

    RoyaltyRecipient[] storage royaltyRecipients = lookups.royaltyRecipientsBySeller[_sellerId];
    // uint256 royaltyRecipientIdsLength = _royaltyRecipientIds.length; // TODO can be optimized?
    uint256 royaltyRecipientsLength = royaltyRecipients.length;
    for (uint256 i = 0; i < _royaltyRecipientIds.length; ) {
        uint256 royaltyRecipientId = _royaltyRecipientIds[i];

        if (royaltyRecipientId >= royaltyRecipientsLength) revert InvalidRoyaltyRecipientId();

Recommendation:

Given that the current implementation is efficient, we advise proper documentation to be introduced to the SellerHandlerFacet::updateRoyaltyRecipients function indicating that the IDs are meant to be passed in as 0-based.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation v2.4.0
Projects
None yet
1 participant