alaydin / DAO

A web site for NFT DAO's to create proposals and vote them
dao-alaydin.vercel.app
0 stars 0 forks source link

i have a question #1

Open RedWilly opened 1 year ago

RedWilly commented 1 year ago

what is the dao = iDao(0xf4910C763eD4e47A585E2D34baA9A4b611aE448C) for becuase i check the address and it the OpenSea Collections (OPENSTORE)

Can you include a little bit of docs

The iDao contract is expected to have the function balanceOf function balanceOf(address, uint256) external view returns(uint256);

In the Dao contract, the balanceOf function from the iDao contract is likely being used to check the balance of specific tokens held by an address. It is used within the isValidOwner modifier to determine if the calling address has a sufficient balance of valid tokens to perform certain actions.

but when i person check the holder and the id i got zero, even though it shows the address is a holder.

could you explain to me how a little bit of details how this works. and a bit of docs might help, trying to build a similar project

alaydin commented 1 year ago

Hello, First of all I should say that I haven't been engaged in NFTs for a long while, and I created this project while I was learning Solidity and React. I don't remember the contract details.

Looks like I considered the case of token being in ERC-1155 standart, that's why there is an array of valid tokens. iDao(0xf4910C763eD4e47A585E2D34baA9A4b611aE448C) in the constructor gets the Opensea shared contract address for creating NFTs. I can't remember how I utilized it, but here is how you should do it imo: Initiate dao variable in constructor with your NFT contract address. If it is ERC-721, you can use balanceOf(owner (address)) - pretty straightforward. If it is ERC-1155, balanceOf() gets two parameters which are owner (address), id (uint256). That's where you loop through valid tokens.

I also noticed that I created modifier isValidOwner(address _addr) but never used in the contract. That should replace checkVoteEligibility function, and we should use the modifier on voteOnProposal function. However, there are a few things you might want to consider. checkVoteEligibility uses newProposal.canVote variable which takes the list of eligible addresses at the moment of proposal creation (kinda snapshot). It checks whether the potential voter's address is in the list or not. The reason why I did that was to prevent manipulations on voting in cases of NFT owners sending their NFTs to different wallets to vote again. I think it would be better to remove this problem by making Proposals[_id].alreadyVoted to be based on NFT ID, rather than holder's address. That would also help to allow NFT holders to vote for the number of NFTs they hold.

I think that contract has a lot to improve, so I would just prefer creating a new project instead of writing docs for it. I believe you can find more useful sources on the internet for this kind of project. Regardless of that, if you give more details on what you want to achieve, I can help you with it.

Have a great day

RedWilly commented 1 year ago

Thanks for the reply. I'm interested in creating a DAO where NFT holders can propose and vote, much like your current project. Your work is already a great foundation, but I plan to use ERC721 tokens instead of ERC1155. For the voting mechanism, I'd like to calculate the total NFTs held by each user and use that as a basis for voting, essentially creating a weighted system.

I would appreciate it if you could help enhance the project. Am still learning solidity because the last contract i tried to write after testing it didnt go so well. But i can work and improve the frontend UI.

Also your suggestion to use Proposals[_id].alreadyVoted to store on NFT ID rather than holder's address can be a good has it will fix Address Manipulation problem but another issue i think might occur later on will be storage costs especially if the nft collection is large, also this will also increase the gas fee dramatically, since each proposal will store ID to check track. Also if a wayout is found i think this can add complexity to the contract it self and the voting process, as you'll need to track and verify NFT IDs for each voter.

alternately instead of storing NFT IDs for each vote, you can consider adding mapping that associates NFT IDs with addresses but doesn't store every NFT ID within the contract. "mapping(address => uint256[]) public nftOwnership;" - hopefully am not wrong

so in the mapping you associate each address with an array of NFT IDs that they own. When a user votes or propose, you can check if they own the NFT by looking up their address in nftOwnership and then check if the NFT ID is among the ones they own. This might reduce the gas cost compared to storing all NFT IDs in the contract, but maintain some security.

Anyway thanks for the quick reply

RedWilly commented 1 year ago

Also i with the suggestion i made above not sure that will work because nft ID, can chnage overtime. so another suggestion or solution will be to sue a seperate contract let say NFTRegistry. So when user buy or transfers an NFT, they can interact with this contract to update their NFT holdings. in the registry contract. example if they buy, mint, sell or transfer a new nft the NFT Registry contract should updat to reflect their current NFT holdings. not sure if this can be achieved by emitting events when NFTs are minted or transferred or by having a corresponding function in the NFT Registry to update the NFT holdings.

And with the dao contract, you can check a user's NFT holdings by referencing the NFT Registry. When a user votes, the contract can query the NFTRegistry Contract to determine the NFT IDs associated with their address at that moment.

mycontract.sol>

// SPDX-License-Identifier: GPL-3.0
// Contract Auto send the mint to the DEV wallet 
//Whitelist wallet wont pay for mint
pragma solidity ^0.8.14;

import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.5/contracts/token/ERC721/extensions/ERC721Enumerable.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.5/contracts/access/Ownable.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.5/contracts/utils/Counters.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.5/contracts/token/ERC20/IERC20.sol";

contract NFTDAO is ERC721Enumerable, Ownable {
    using Strings for uint256;

    using Counters for Counters.Counter;

    Counters.Counter private _tokenIds;

    string public baseURI;
    string public baseExtension = ".json";
    string public notRevealedURI;

    uint256 public cost = 0.03 ether;
    uint256 public maxSupply = 3000;
    uint256 public maxMintAmount = 100;

    bool public revealed = false;
    bool public paused = false;
    mapping(address => bool) public whitelisted;

    constructor(
        string memory _name,
        string memory _symbol,
        string memory _initBaseURI,
        string memory _notRevealedURI
    ) ERC721(_name, _symbol) {
        setBaseURI(_initBaseURI);
        setNotRevealedURI(_notRevealedURI);
        _safeMint(msg.sender, 1);
    }

    // internal
    function _baseURI() internal view virtual override returns (string memory) {
        return baseURI;
    }

    // public
    function mint(uint256 _mintAmount) public payable {
        require(!revealed);
        require(!paused);
        require(_mintAmount > 0);
        require(balanceOf(msg.sender) + _mintAmount <= maxMintAmount);
        uint256 supply = totalSupply();
        require(supply + _mintAmount <= maxSupply);

        if (msg.sender != owner()) {
            if (whitelisted[msg.sender] != true) {
                require(msg.value >= cost * _mintAmount);
            }
        }

        for (uint256 i = 1; i <= _mintAmount; i++) {
            _safeMint(msg.sender, supply + i);
            _tokenIds.increment();
        }

        require(payable(owner()).send(address(this).balance));
    }

    function count() public view returns (uint256) {
        return _tokenIds.current();
    }

    function walletOfOwner(address _owner)
        public
        view
        returns (uint256[] memory)
    {
        uint256 ownerTokenCount = balanceOf(_owner);
        uint256[] memory tokenIds = new uint256[](ownerTokenCount);
        for (uint256 i; i < ownerTokenCount; i++) {
            tokenIds[i] = tokenOfOwnerByIndex(_owner, i);
        }
        return tokenIds;
    }

    function tokenURI(uint256 tokenId)
        public
        view
        virtual
        override
        returns (string memory)
    {
        require(
            _exists(tokenId),
            "ERC721Metadata: URI query for nonexistent token"
        );

        string memory currentBaseURI = _baseURI();

        if (revealed) {
            return
                bytes(currentBaseURI).length > 0
                    ? string(
                        abi.encodePacked(
                            currentBaseURI,
                            tokenId.toString(),
                            baseExtension
                        )
                    )
                    : "";
        } else {
            return notRevealedURI;
        }
    }

    //only owner
    function setCost(uint256 _newCost) public onlyOwner {
        cost = _newCost;
    }

    function setmaxMintAmount(uint256 _newmaxMintAmount) public onlyOwner {
        maxMintAmount = _newmaxMintAmount;
    }

    function setNotRevealedURI(string memory _newNotRevealedURI)
        public
        onlyOwner
    {
        notRevealedURI = _newNotRevealedURI;
    }

    function setBaseURI(string memory _newBaseURI) public onlyOwner {
        baseURI = _newBaseURI;
    }

    function setBaseExtension(string memory _newBaseExtension)
        public
        onlyOwner
    {
        baseExtension = _newBaseExtension;
    }

    function setReveal() public onlyOwner {
        revealed = !revealed;
    }

    function setPause() public onlyOwner {
        paused = !paused;
    }

    function addWhitelistUser(address _user) public onlyOwner {
        whitelisted[_user] = true;
    }

    function removeWhitelistUser(address _user) public onlyOwner {
        whitelisted[_user] = false;
    }

    function withdraw() public payable onlyOwner {
        require(payable(msg.sender).send(address(this).balance));
    }

     receive() external payable {}
}
alaydin commented 1 year ago

I haven't checked your contract in detail but you should note that owners can directly send NFTs or use other contracts to sell. So you have to keep NFTRegistry contract updated all the time which requires an additional script to trigger contract functions regularly.

I want to mention something else. There are many web3 APIs those could help us get all the NFTs a wallet holding. A change in the contract could turn it into this: example -> a wallet has 7 NFTs, we will get that from the front-end, voting will have a parameter named "amount" referring to amount of nfts he is holding, front-end will send that to contract function. Holder will vote once, but his vote will have an effect of 7. Continuing that, now we also have NFT IDs, we just need to figure out how to use them in contract.

I can't go in detail with coding as I'm a bit occupied these days, but you may want to check Alchemy and Moralis APIs before constructing your contract if you haven't done already. Those may give you different perspectives.