<aside> 💬 The text inside callouts represents the Stargaze team’s response.

</aside>

Decreasing the royalty fee to zero causes collection to be untradeable in marketplace

<aside> 💬 Acknowledged as low severity.

</aside>

Intended functionality

• The marketplace contract allows owners to sell their NFTs.
• The NFT collection creator can execute the UpdateCollectionInfo message to decrease the royalty share to zero.

Issue

The payout function in the marketplace contract sends the royalty without verifying the amount to send is higher than zero. If the NFT collection creator decides to decrease the royalty share to zero, the NFT collection will not be tradable in the marketplace.

This issue will also occur if the creator specifies the royalty share as zero (instead of None) during the sg721 contract instantiation phase.

match collection_info.royalty_info {
        // If token supports royalties, payout shares to royalty recipient
        Some(royalty) => {
            let amount = coin((payment * royalty.share).u128(), NATIVE_DENOM);
            if payment < (network_fee + Uint128::from(finders_fee) + amount.amount) {
                return Err(StdError::generic_err("Fees exceed payment"));
            }
            res.messages.push(SubMsg::new(BankMsg::Send {
                to_address: royalty.payment_address.to_string(),
                amount: vec![amount.clone()],
            }));

Suggestion

Ensure the royalty amount is higher than zero.

if amount.amount > Uint128::zero() {
                res.messages.push(SubMsg::new(BankMsg::Send {
                    to_address: royalty.payment_address.to_string(),
                    amount: vec![amount.clone()],
                }));
            }

Patch

Merge pull request #507 from public-awesome/yubrew/royalties-add-tests · public-awesome/launchpad@c61254f

Proof of concept

#[test]
fn test_zero_royalty_fee_fail() {
    // reproduced in contracts/marketplace/src/testing/tests/bids.rs

    use sg721::{ExecuteMsg as Sg721ExecuteMsg, UpdateCollectionInfoMsg, RoyaltyInfoResponse};
    use cosmwasm_std::{Timestamp, Empty, Decimal};
    use sg721_base::msg::CollectionInfoResponse;

    let vt = standard_minter_template(1);
    let (mut router, owner, bidder, creator) =
        (vt.router, vt.accts.owner, vt.accts.bidder, vt.accts.creator);
    let marketplace = setup_marketplace(&mut router, owner).unwrap();
    let minter_addr = vt.collection_response_vec[0].minter.clone().unwrap();
    let collection = vt.collection_response_vec[0].collection.clone().unwrap();
    let start_time = Timestamp::from_nanos(GENESIS_MINT_START_TIME);
    setup_block_time(&mut router, GENESIS_MINT_START_TIME, None);
    let token_id = 1;

    // mint nft
    mint(&mut router, &creator, &minter_addr);

    // authorize nft
    approve(&mut router, &creator, &collection, &marketplace, token_id);

    // Creator lowers royalty to zero
    let royalty_info: Option<RoyaltyInfoResponse> = Some(RoyaltyInfoResponse {
        payment_address: creator.to_string(),
        share: Decimal::percent(0),
    });

    let update_collection_msg: Sg721ExecuteMsg<CollectionInfoResponse, Empty> = Sg721ExecuteMsg::UpdateCollectionInfo { 
        collection_info: UpdateCollectionInfoMsg {
            description: None,
            image: None,
            external_link: None,
            explicit_content: None,
            royalty_info: Some(royalty_info),
        }
    };

    router.execute_contract(creator.clone(), collection.clone(), &update_collection_msg, &[]).unwrap();

    // creator list nft in marketplace
    let set_ask = ExecuteMsg::SetAsk {
        sale_type: SaleType::FixedPrice,
        collection: collection.to_string(),
        token_id,
        price: coin(100, NATIVE_DENOM),
        funds_recipient: None,
        reserve_for: None,
        expires: start_time.plus_seconds(MIN_EXPIRY + 1),
        finders_fee_bps: Some(0),
    };

    router.execute_contract(
        creator.clone(),
        marketplace.clone(),
        &set_ask,
        &listing_funds(LISTING_FEE).unwrap(),
    ).unwrap();

    // bidder bids
    let set_bid_msg = ExecuteMsg::SetBid {
        sale_type: SaleType::FixedPrice,
        collection: collection.to_string(),
        token_id,
        finders_fee_bps: None,
        expires: start_time.plus_seconds(MIN_EXPIRY + 1),
        finder: None,
    };

    // error here
    router
        .execute_contract(
            bidder.clone(),
            marketplace,
            &set_bid_msg,
            &coins(100, NATIVE_DENOM),
        )
    .unwrap();
}

Creators can increase royalty share by initializing royalty info as None

<aside> 💬 Acknowledged as medium severity.

</aside>