atilatech / arthouse Goto Github PK

William Entriken and is one of the original authors of the ERC721 NFT standard.

He hosts weekly web3 call to answer questions every Tuesday at 6PM EST. THis is a great opportunity to ask and learn more about web3, crypto and NFTs. Here are some questions and topics to discuss with William.

Update: The answers to these questions can be found at Episode 26—Vj Deliria @ 22:36 (notes). Thanks so much @fulldecent and @dtedesco1 for organizing this and answering my questions!

Questions

• Thoughts on using pull instead of push? What are some better alterantives?

  • https://fravoll.github.io/solidity-patterns/pull_over_push.html

  • arthouse/contracts/Market.sol

    Lines 150 to 156 in 6bee14f

    	_allowForPull(seller, sellerPayment);
    	idToMarketItem[itemId].owner = payable(msg.sender);
    	idToMarketItem[itemId].sold = true;
    	_itemsSold.increment();
    	_allowForPull(payable(owner), marketPayment);
    	IERC721(nftContract).transferFrom(address(this), msg.sender, tokenId);

• Thoughts on EIPS-2981 token standard (royalties)

• If a smart contract is storing address, won't it become arbitrarly large? Who is paying for these ongoing storage costs?

• What does this comment mean?
  https://gist.github.com/dabit3/eb8866adc22bd86cabf5e6604b408e4a?permalink_comment_id=4091958#gistcomment-4091958

• How inefficient is the return statement and the Emit in minting an NFT token

• When people said bored ape was bad and inefficient, what did they mean?

• Do Comments add gas fees

• Do we need to use pull payment if we already use nnReentrant modifier?

• Am I following check-effects-interaction properly? Is it necessary if we have reentrancy guard?

• Should _allowForPull() be it's own function?

• Is the following line a security risk, what if the smart contract is not ERC721 compliant

   IERC721(nftContract).transferFrom(msg.sender, address(this), tokenId);
       return itemId;
  

• why do we need setApproval for if the sender is the one transferring the NFT to the smart contract?

• Why are some strings lowercase while others are uppercase

  • a49bd90

Reading the itemId from the transaction result is causing potential race conditions where tests pass in isolation but fail when run together. To solve this, this helper function was created to get the item ID from the smart contract when it's needed.

It should be able to make updates to the smart contracts without changing the address which users interact with and losing storage space. See Proxy Delegate and Eternal Storage patterns.

Our Market.Sol smart Contracts calls the transferFrom() function on any provided nftContract address. These may not always be valid IERC721 compliant smart contract addresses. Is this a security vulnerability? Confirm that this is either a safe assumption to make or what can be done to improve such calls.

Example:

This tweet suggests using safeTransferFrom. See: https://gist.github.com/ecmendenhall/9408082d8f3cfe50076642d8745fc6d3