• Reviewed commit hash: 6e0821c0ee0c659b8c44ed9750d927e3e6c31f86

GitHub - public-awesome/names at 6e0821c0ee0c659b8c44ed9750d927e3e6c31f86

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

</aside>

Incorrect renewal fee refunded when transferring NFTs

<aside> 💬 Feature is not live yet but we acknowledge that this is a bug. This is not enabled anywhere from the platform, and can only be done by invoking the contract directly.

</aside>

Intended functionality

Users can deposit funds to their NFT using the FundRenewal message to automatically renew their name service during renewal time. The deposited funds should be refunded to the original user when the NFT is transferred or sent to another address.

Issue

In contracts/marketplace/src/execute.rs:200, the execute_update_ask function updates the seller to the recipient before sending the renewal fund. Consequently, this causes the funds to be refunded to the new seller instead of the old seller.

    // refund any renewal funds and update the seller
    let mut ask = asks().load(deps.storage, ask_key(token_id))?;
    ask.seller = seller.clone();
    if !ask.renewal_fund.is_zero() {
        let msg = BankMsg::Send {
            to_address: ask.seller.to_string(),
            amount: coins(ask.renewal_fund.u128(), NATIVE_DENOM),
        };
        res = res.add_message(msg);
        ask.renewal_fund = Uint128::zero();
    }
    asks().save(deps.storage, ask_key(token_id), &ask)?;

Proof of concept

The test case is written inside the name-minter contract because all required contracts are set up already.

#[test]
    fn test_incorrect_renewal_fee_refund() {
        // reproduced in contracts/name-minter/src/integration_tests.rs
        
        let mut app = instantiate_contracts(None, None, None);

        mint_and_list(&mut app, NAME, USER, None).unwrap();

        // mint 1000 funds to user
        let renewal_fee = coins(1000_u128, NATIVE_DENOM);

        app.sudo(CwSudoMsg::Bank({
            BankSudo::Mint {
                to_address: USER.to_string(), 
                amount: renewal_fee.clone(),
            }
        })).unwrap();

        // user renew domain name
        let msg = MarketplaceExecuteMsg::FundRenewal {
             token_id: NAME.to_string(),
        };

        app.execute_contract(
            Addr::unchecked(USER), 
            Addr::unchecked(MKT), 
            &msg, 
            &renewal_fee,
        ).unwrap();

        // verify user have no money 
        let res = app
            .wrap()
            .query_balance(USER.to_string(), NATIVE_DENOM)
            .unwrap();
        assert_eq!(res.amount, Uint128::new(0));

        // user sends the nft to bob 
        let bob : &str = &"bob";

        let msg = Sg721NameExecuteMsg::TransferNft {
            recipient: bob.to_string(),
            token_id: NAME.to_string(),
        };
        app.execute_contract(
            Addr::unchecked(USER),
            Addr::unchecked(COLLECTION),
            &msg,
            &[],
        ).unwrap();

        // the renewal fee should refunded back to user, however it incorrectly sent to bob
        let user_balance = app
            .wrap()
            .query_balance(USER.to_string(), NATIVE_DENOM)
            .unwrap().amount;
        assert_eq!(user_balance, renewal_fee[0].amount);

        let bob_balance = app
            .wrap()
            .query_balance(bob.clone(), NATIVE_DENOM)
            .unwrap().amount;
        assert_eq!(bob_balance, Uint128::zero());

        /*

        Suggestion: modify contracts/marketplace/src/execute.rs:215-226 into the following:

        // refund any renewal funds and update the seller
        let mut ask = asks().load(deps.storage, ask_key(token_id))?;
        // ask.seller = seller.clone();
        if !ask.renewal_fund.is_zero() {
            let msg = BankMsg::Send {
                to_address: ask.seller.to_string(), 
                amount: coins(ask.renewal_fund.u128(), NATIVE_DENOM),
            };
            res = res.add_message(msg);
            ask.renewal_fund = Uint128::zero();
        } ask.seller = seller.clone();
        asks().save(deps.storage, ask_key(token_id), &ask)?;

        */
    }

Suggestion

Update the seller to the new owner after refunding the renewal fund to the old owner.

Patch

https://github.com/public-awesome/names/commit/0ed8d33ce5e82e57cd41ecfb8eb84d409857742f

Users can renew names without paying the required fees

<aside> 💬 Feature is not live yet and we are currently in process of completing the feature set which will include free renewals if there is no demand for it.

</aside>