0

I have recently started working on solidity. I want to create a smart contract where users can mint and own the LAND (NFT) by clicking Buy Now button on UI and send LAND Coordinates(x and y) to the contract. I have created a separate smart contract for that. Now I want to revert transaction if some user tries to buy/mint an NFT that is already minted/owned by some other address using same coordinates, so what is best way to do this? Please check buyLand() function or go through my code and suggest me better practice to do so. Thank You!

// SPDX-License-Identifier: MIT
pragma solidity >=0.4.22 <0.9.0;

interface ILandContract {
    function mint(address to, string memory tokenURI) external;
    function getTokenCount() external view returns(uint256);
}

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/access/Ownable.sol";

contract MintingContract is ReentrancyGuard, Ownable {

    IERC20 private Token;
    address private _landContractAddress;
    uint256 private _landPrice;
    uint256 public numberOfSold;

    struct ParcelsSold{
        uint x;
        uint y;
    }

    mapping (uint256 => ParcelsSold) coords;
    string[] private _landCoordinates;

    event LandSold(
        uint256 timestamp,
        address buyer,
        uint256 amount
    );

    constructor(address landContractAddress, IERC20 tokenAddress, uint256 initialLandPrice){
        _landContractAddress = landContractAddress;
        Token = tokenAddress;
        _landPrice = initialLandPrice;
        numberOfSold = 0;
    }

    function buyLand(string memory tokenURI, uint x, uint y) public nonReentrant {

        for(uint i=0; i< numberOfSold; i++){
            require(!(coords[i].x == x && coords[i].y == y), "Minting : Parcel Sold Already");
        }
        Token.transferFrom(msg.sender, owner(), _landPrice);
        ILandContract land = ILandContract(_landContractAddress);
        land.mint(msg.sender, tokenURI);
        numberOfSold++;

        uint256 tokenId = land.getTokenCount();

        ParcelsSold memory sold = ParcelsSold(x, y);
        coords[tokenId] = sold;

        emit LandSold(block.timestamp, msg.sender, _landPrice);
    }

}

2 Answers 2

1

This for(uint i=0; i< numberOfSold is an unbounded loop and is a big no-no. Eventually, the cost of 1 transaction will be over the block gas limit, and it will revert. At this point buyLand would be bricked.

Try:

//       x,                  y
mapping (uint256 => mapping (uint256 => ParcelInfo)) parcels;

You cannot enumerate through a mapping. And you don't want to, again, because that would be an unbounded loop. However, you may want to keep track of the plots sold, for instance with a:

// Token ids perhaps?
uint256[] allSold;

But never have your Solidity code do a for on it. This increases the complexity of your contract. So think hard about what is needed in your contract, and what can be outsourced to Web2.0 servers.


To improve: you do land.mint and then tokenId = land.getTokenCount(). That does not sound right. Right now you are saved by the fact that Ethereum is "single-thread". Instead, the mint should return the new tokenId, something like:

uint256 tokenId = land.mint(msg.sender, tokenURI);
1
  • Thank you so much, i got the point and implementing it Feb 16 at 13:16
0

this uses an iterable mapping pattern

pragma solidity ^0.8.14;

import "hardhat/console.sol";

contract Family {

    mapping (address => string) public familyMembers;   // store data
    mapping (address => bool) public isInMap;   // need this as we can't distinguish a zero cause not in the map from is in map but is zero
    address[] public keys;  // something we can enumerate and use each element as the key for the familyMembers map

    constructor () {
        addFamilyMember(address(0x1), "alice");
        addFamilyMember(address(0x2), "bob");
        addFamilyMember(address(0x3), "cam");
        addFamilyMember(address(0x4), "doris");
        addFamilyMember(address(0x5), "");        //  <---
        addFamilyMember(address(0x5), "eve");     //  <---
        addFamilyMember(address(0x6), "fud");     //  <---
        addFamilyMember(address(0x6), "fred");    //  <---

        iterateFamilyMembers();
    }

    function addFamilyMember(address _addr, string memory _name) public {
        
        familyMembers[_addr] = _name;
        if(!isInMap[_addr]) {
            isInMap[_addr] = true;
            keys.push(_addr);
        }
    }

    function iterateFamilyMembers() public view {
        for(uint i; i < keys.length; ++i){
            console.log("address: %s, name: %s", keys[i], familyMembers[keys[i]]);
        }
    }
}

output:

console.log:
 address: 0x0000000000000000000000000000000000000001, name: alice
 address: 0x0000000000000000000000000000000000000002, name: bob
 address: 0x0000000000000000000000000000000000000003, name: cam
 address: 0x0000000000000000000000000000000000000004, name: doris
 address: 0x0000000000000000000000000000000000000005, name: eve
 address: 0x0000000000000000000000000000000000000006, name: fred

this is a pattern for an iterable mapping. we need an array to complement the mapping, as it's iterable. we also put the address of each map element in the array so we can use that as the key to pull the values from the map as we iterate the array.

we also need another map to tell us whether an element has been added already or not. we need that as there is no null in solidity so in some cases there is no way for us to be certain whether a zero or empty string returned from a mapping is because it wasn't found, or it was found and the zero or empty string is a valid value.

In your familyMember / name example, perhaps this wouldn't be an issue, but in other cases it would.

An example of where it would be a problem is a mapping of address and account balance. A balance of zero is valid and common. When we look up the address in the mapping and get a zero, was the address in there with a zero balance or just not there? We need to know that to know whether to add another element to the array or not.

If we didn't want to make the mapping iterable, it probably wouldn't matter why we got a zero or an empty string.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.