search

exo-digital-labs / ERC721R

https://erc721r.super.site/
MIT License
241 stars 61 forks source link

Fix: Refund issue #8

Closed solomeowl closed 2 years ago

solomeowl commented 2 years ago

RefundAddress can transfer nfts to other address and get refund. It means team still can rug. So I changed transferFrom(msg.sender, refundAddress, tokenId); to transferFrom(msg.sender, address(0), tokenId);

royosherove commented 2 years ago

essentially burning it?

royosherove commented 2 years ago

This can enable a malicious actor to buy and refund all tokens and burn the entire collection. no?

lawweiliang commented 2 years ago

Why transfer to zero? It essentially burn it. To put it in english, if the project have 10,000 nft, 3 people refund, the project will left out 9993. It does not make sense.

Send the refund nft token back to the contract owner address is totally fine in my opinion because that address usually is multisign address own by the team.

If really want to place safe, I would rather set a modifier that did not allow the smart contract owner call the refund function. Problem solved.

Somehow, if the team really want to rug pull, they can withdraw money using the withdraw function and leave the project and we can't do anything.

elie222 commented 2 years ago

Yes, I agree with this approach. We do need to close off this loophole of the team refunding and emptying all funds from the contract before the refund period is over.

I agree that burning NFTs isn't helpful and will lead to nasty scenarios very quickly.