[SCH] Smart Contract Hacking 5편 - NFT2

Task1

---

NFT 마켓플레이스 컨트랙트를 만드는 과제다. 빈 공간을 채워보자!

// SPDX-License-Identifier: GPL-3.0-or-later 
pragma solidity ^0.8.13;

import "@openzeppelin/contracts/token/ERC721/IERC721.sol";

/**
 * @title OpenOcean
 * @author JohnnyTime (https://smartcontractshacking.com)
 */
contract OpenOcean {

    // TODO: Complete this contract functionality

    // TODO: Constants

    // TODO: Item Struct

    // TODO: State Variables and Mappings
    
    constructor() {}

    // TODO: List item function
    // 1. Make sure params are correct
    // 2. Increment itemsCounter
    // 3. Transfer token from sender to the contract
    // 4. Add item to listedItems mapping
    function listItem(address _collection, uint256 _tokenId, uint256 _price) external {

    }

    // TODO: Purchase item function 
    // 1. Check that item exists and not sold
    // 2. Check that enough ETH was paid
    // 3. Change item status to "sold"
    // 4. Transfer NFT to buyer
    // 5. Transfer ETH to seller
    function purchase(uint _itemId) external payable {
        
    }
    
}

몇 번 삽질 끝에 아래와 같이 컨트랙트를 만들었다.

// SPDX-License-Identifier: GPL-3.0-or-later 
pragma solidity ^0.8.13;

import "@openzeppelin/contracts/token/ERC721/IERC721.sol";

/**
 * @title OpenOcean
 * @author JohnnyTime (https://smartcontractshacking.com)
 */
contract OpenOcean {

    // TODO: Complete this contract functionality

    // TODO: Constants
    uint256 public constant maxPrice = 100 ether;

    // TODO: Item Struct
    struct Item {
        uint256 itemId;
        address collectionContract;
        uint256 tokenId;
        uint256 price;
        address payable seller;
        bool isSold;
    }

    // TODO: State Variables and Mappings
    uint256 public itemCounter;
    Item[] public item;
    mapping(uint256 itemId => Item item) public listedsItems;


    constructor() {}

    // TODO: List item function
    function listItem(address _collection, uint256 _tokenId, uint256 _price) external {
        require(_collection != address(0), "Collection doesn't exist");
        require(_price <= maxPrice, "Can't list more than max price");
        itemCounter++;
        IERC721(_collection).transferFrom(msg.sender, address(this), _tokenId);
        item.push(Item(
            itemCounter,
            _collection,
            _tokenId,
            _price,
            payable(msg.sender),
            false
        ));
    }

    // TODO: Purchase item function 
    function purchase(uint _itemId) external payable {
        Item storage itemToBuy = item[_itemId];
        require(itemToBuy.isSold == false, "Already sold");
        require(msg.value >= itemToBuy.price, "Not enough fund");
        itemToBuy.isSold = true;
        address collection = itemToBuy.collectionContract;
        IERC721(collection).transferFrom(address(this), msg.sender, itemToBuy.tokenId);
        itemToBuy.seller.transfer(msg.value);   
    }
    
}

여기까지 피드백

---

1. 당연한 얘기지만 struct를 만들고 빈 struct를 새로 만들어줘야 사용가능하다.

2. 다시 보니까 listedsItems을 전혀 활용하지 않았음. 모범답안대로 하는 것이 훨씬 깔끔한 것 같다. 첫 번째는 listItem()에서 수정할 내용. 두 번째는 purchase()에서 수정할 내용.

//내가 쓴 코드
        item.push(Item(
            itemCounter,
            _collection,
            _tokenId,
            _price,
            payable(msg.sender),
            false
        ));

//모범 답안
        listedsItems[itemCounter] = Item(
            itemCounter,
            _collection,
            _tokenId,
            _price,
            payable(msg.sender),
            false
        ));

//내가 쓴 코드
    function purchase(uint _itemId) external payable {
        Item storage itemToBuy = item[_itemId];
        require(itemToBuy.isSold == false, "Already sold");
        require(msg.value >= itemToBuy.price, "Not enough fund");
        itemToBuy.isSold = true;
        address collection = itemToBuy.collectionContract;
        IERC721(collection).transferFrom(address(this), msg.sender, itemToBuy.tokenId);
        itemToBuy.seller.transfer(msg.value);   
    }

//모범답안
    function purchase(uint _itemId) external payable {
        Item storage item = listedsItems[_itemId]; //listedsItems 활용
        require(!item.isSold, "Already sold"); // "!" 활용
        require(msg.value >= item.price, "Not enough fund");
        item.isSold = true;
        //address collection = itemToBuy.collectionContract; 삭제
        IERC721(item.collectionContract).transferFrom(address(this), msg.sender, item.tokenId);
        //item.seller.transfer(msg.value);  
        (bool success, ) = listedItems[_itemId].seller.call{value: msg.value}("");
        require(success, "Transfer of ETH Failed!!!");
    }

셀러한테 이더를 보낼 때 transfer 대신 call{value: msg.value}를 사용했다. 전송 결과를 확실하게 확인하기 위함이다. solidity by example을 참고하면 transfer와 send는 이더를 보낼 때 추천하지 않고, call{value: msg.value}("")를 추천한다고 한다. 전송 결과를 확인할 수 있음과 동시에 가스를 조절할 수 있기 때문이다.

3. mapping을 만들 때는 타입만 작성하면 된다!

//내가 적은 코드
mapping(uint256 itemId => Item item) public listedsItems;

//모범답안
mapping(uint256 => Item) public listedsItems;

Task2

---

빈 공간을 채워보자.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import "../../src/erc721-2/OpenOcean.sol";
import "../../src/utils/DummyERC721.sol";

/**
@dev run "forge test --match-contract ERC7212" 

*/
contract TestERC7212 is Test {
    uint128 public constant CUTE_NFT_PRICE = 5 ether;
    uint128 public constant BOOBLES_NFT_PRICE = 7 ether;

    OpenOcean public marketPlace;
    DummyERC721 public cuteNFT;
    DummyERC721 public booblesNFT;

    address deployer;
    address user1;
    address user2;
    address user3;

    uint256 init_user1_bal;
    uint256 init_user2_bal;
    uint256 init_user3_bal;

    function setUp() public {
        deployer = address(1);
        user1 = address(2);
        user2 = address(3);
        user3 = address(4);

        vm.deal(deployer, 100 ether);
        vm.deal(user1, 100 ether);
        vm.deal(user2, 100 ether);
        vm.deal(user3, 100 ether);

        init_user1_bal = user1.balance;
        init_user2_bal = user2.balance;
        init_user3_bal = user3.balance;

        vm.prank(deployer);
        marketPlace = new OpenOcean();

        vm.startPrank(user1);
        cuteNFT = new DummyERC721("Crypto Cuties", "CUTE", 1000);
        cuteNFT.mintBulk(30);
        vm.stopPrank();

        vm.startPrank(user3);
        booblesNFT = new DummyERC721("Rare Boobles", "BOO", 10000);
        booblesNFT.mintBulk(120);
        vm.stopPrank();
    }

    function test_NFT() public {
        console.log("Running Listing Tests...");

        // TODO: User1 lists Cute NFT tokens 1-10 for 5 ETH each

        // TODO: Check that Marketplace owns 10 Cute NFTs

        // TODO: Checks that the marketplace mapping is correct (All data is correct), check the 10th item.


        // TODO: User3 lists Boobles NFT tokens 1-5 for 7 ETH each

        // TODO: Check that Marketplace owns 5 Booble NFTs

        // TODO: Checks that the marketplace mapping is correct (All data is correct), check the 15th item.


        // All Purchases From User2 //

        // TODO: Try to purchase itemId 100, should revert

        // TODO: Try to purchase itemId 3, without ETH, should revert

        // TODO: Try to purchase itemId 3, with ETH, should work
        // TODO: Can't purchase sold item

        // TODO: User2 owns itemId 3 -> Cuties tokenId 3

        // TODO: User1 got the right amount of ETH for the sale

        // TODO: Purchase itemId 11

        // TODO: User2 owns itemId 11 -> Boobles tokenId 1
        
        // TODO: User3 got the right amount of ETH for the sale

    }


}

내가 적은 테스트 코드는 다음과 같다.

   function test_NFT() public {
        console.log("Running Listing Tests...");

        // TODO: User1 lists Cute NFT tokens 1-10 for 5 ETH each
        for(uint256 i=1; i < 11; i++) {
            vm.prank(user1);
            marketPlace.listItem(cuteNFT, i, CUTE_NFT_PRICE);
        }

        // TODO: Check that Marketplace owns 10 Cute NFTs
        assertEq(cuteNFT.balanceOf(marketPlace), 10);

        // TODO: Checks that the marketplace mapping is correct (All data is correct), check the 10th item.
        assertEq(marketPlace.listedsItems(10).itemId, 10);
        assertEq(marketPlace.listedsItems(10).collectionContract, cuteNFT);
        assertEq(marketPlace.listedsItems(10).tokenId, 10);
        assertEq(marketPlace.listedsItems(10).price, CUTE_NFT_PRICE);
        assertEq(marketPlace.listedsItems(10).seller, user1);
        assertEq(marketPlace.listedsItems(10).isSold, false);

        // TODO: User3 lists Boobles NFT tokens 1-5 for 7 ETH each
        for(uint256 i=1; i < 6; i++) {
            vm.prank(user3);
            marketPlace.listItem(booblesNFT, i, BOOBLES_NFT_PRICE);
        }    

        // TODO: Check that Marketplace owns 5 Booble NFTs
        assertEq(booblesNFT.balanceOf(marketPlace), 5);

        // TODO: Checks that the marketplace mapping is correct (All data is correct), check the 15th item.
        assertEq(marketPlace.listedsItems(15).itemId, 15);
        assertEq(marketPlace.listedsItems(15).collectionContract, booblesNFT);
        assertEq(marketPlace.listedsItems(15).tokenId, 15);
        assertEq(marketPlace.listedsItems(15).price, BOOBLES_NFT_PRICE);
        assertEq(marketPlace.listedsItems(15).seller, user3);
        assertEq(marketPlace.listedsItems(15).isSold, false);

        // All Purchases From User2 //

        // TODO: Try to purchase itemId 100, should revert
        vm.expectRevert("itemId dosen't exist");
        vm.prank(user2);
        marketPlace.purchase{value: BOOBLES_NFT_PRICE}(100);

        // TODO: Try to purchase itemId 3, without ETH, should revert
        vm.expectRevert("Can't purchase without fund");
        vm.prank(user2);
        marketPlace.purchase(3);

        // TODO: Try to purchase itemId 3, with ETH, should work
        vm.prank(user2);
        marketPlace.purchase{value: CUTE_NFT_PRICE}(3);

        // TODO: Can't purchase sold item
        vm.expectRevert("Can't purchase sold item");
        vm.prank(user2);
        marketPlace.purchase{value: CUTE_NFT_PRICE}(3);

        // TODO: User2 owns itemId 3 -> Cuties tokenId 3
        assertEq(cuteNFT.balanceOf(user2), 3);

        // TODO: User1 got the right amount of ETH for the sale
        assertEq(vm.balance(user1), 100 - (CUTE_NFT_PRICE)*10 + (CUTE_NFT_PRICE)*1);

        // TODO: Purchase itemId 11
        vm.prank(user2);
        marketPlace.purchase{value: BOOBLES_NFT_PRICE}(11);

        // TODO: User2 owns itemId 11 -> Boobles tokenId 1
        assertEq(booblesNFT.ownerOf(1), user2);
        
        // TODO: User3 got the right amount of ETH for the sale
        assertEq(vm.balance(user1), 100 - (BOOBLES_NFT_PRICE)*5 + (BOOBLES_NFT_PRICE)*1);

    }

피드백

---

1. 첫 부분 approve 빠짐. 항상 NFT나 erc20 토큰을 이동시키기 위해선 approve 먼저 해줘야 한다는 것을 잊지말자.

        for (uint i = 1; i <= 10; i++) {
            vm.startPrank(user1);
            cuteNFT.approve(address(marketPlace), i);
            marketPlace.listItem(address(cuteNFT), i, CUTE_NFT_PRICE);
            vm.stopPrank();
        }

2. 인스턴스와 address를 구분해서 써야 한다. DummyERC721 타입의 변수를 만들고, 그 변수에 인스턴스를 넣어준 것 뿐이지 그것 자체로 컨트랙트의 주소가 될 수 없다. 따라서 앞에 address를 붙여서 사용해야 한다.

    DummyERC721 public cuteNFT;

    cuteNFT = new DummyERC721("Crypto Cuties", "CUTE", 1000);

	//잘못된 예
    marketPlace.listItem(cuteNFT, i, CUTE_NFT_PRICE);

	//올바른 예
    marketPlace.listItem(address(cuteNFT), i, CUTE_NFT_PRICE);

3. 테스트 코드 후반부에 vm.prank(user2);를 남발했는데, 자주 사용할 것 같으면 vm.startPrank()와 vm.stopPrank()를 애용하도록 하자. 중간에 vm.expertRevert() 사용해도 상관 없음!

4. struct 내부를 확인할 때, 각 요소에 할당해준 다음 각자 assertEq()로 확인해주는 것이 맞다.

		//잘못된 예
        assertEq(marketPlace.listedsItems(15).itemId, 15);
        assertEq(marketPlace.listedsItems(15).collectionContract, booblesNFT);
        assertEq(marketPlace.listedsItems(15).tokenId, 15);
        assertEq(marketPlace.listedsItems(15).price, BOOBLES_NFT_PRICE);
        assertEq(marketPlace.listedsItems(15).seller, user3);
        assertEq(marketPlace.listedsItems(15).isSold, false);


		//올바른 예
        (
            uint256 itemId,
            address collectionContract,
            uint256 tokenId,
            uint256 price,
            address payable seller,
            bool isSold
        ) = marketPlace.listedItems(10);

        assertEq(itemId, 10);
        assertEq(collectionContract, address(cuteNFT));
        assertEq(tokenId, 10);
        assertEq(price, CUTE_NFT_PRICE);
        assertEq(seller, user1);
        assertEq(isSold, false);

5. balance를 체크하고 싶을 때는 vm.balance를 사용하면 된다.

		//잘못된 예
        assertEq(vm.balance(user1), 100 - (CUTE_NFT_PRICE)*10 + (CUTE_NFT_PRICE)*1);
		
		//올바른 예
	    assertEq(user1.balance > (init_user1_bal + CUTE_NFT_PRICE - 0.2 ether), true);

6. vm.expectRevert()를 쓸 때 중간에 글자를 넣어주면 에러가 난다.

		//잘못된 예
        vm.expectRevert("Can't purchase");
        
        //올바른 예
        vm.expectRevert();

나중에 알고보니 중간에 글자를 넣어주면 에러가 나는 것이 아니라, 호출하려는 함수의 require()문 속에 있는 에러 메세지와 일치하지 않으면 에러가 나는 것이다.

require(itemToBuy.isSold == false, "Already sold");

위 코드는 purchase() 함수의 첫 번째 require() 문이다. 이를 테스트 할 때는 vm.expectRevert("Already sold"); 라고 써줘야 하고, 이와 다른 문구를 사용하면 에러가 난다!

다 쓰고 나니까 내가 작성한 테스트 코드는 완전 엉망진창이네.. 그래도 이렇게 적나라하게 피드백을 하지 않으면 실력이 늘지 않으니까 긍정적으로 생각하자! 의식적으로 계속 피드백 루프를 돌리다보면 어느 순간 실력은 늘어나 있을거라 생각한다.