0

i'm building a sample application which require voting. I think it needs to be split into 2 contracts where the owner of Ballot will create Proposal contracts. Splitting simply because the logic is got too complex to be clearly understandable in a single contract.

Question: what is the proper way to let only the contributors from Ballot contract vote for proposals.

My implementation - simply a call to Bellot to check if the address is in contributors. I've read that there are a lot of security flaws in interacting between 2 contracts(since i'm new to solidity might not know all details), so i want to verify if this is correct solution.

Ballot

contract Ballot {
    address public manager;
    mapping (address => bool) public contributors;

    Proposal[] proposals;
    mapping(address => Proposal[]) proposalsByAddress;

    constructor(address _creator) public {
        manager = _creator;
    }

    function createProposal() public {
        Proposal memory newProposal = new Proposal();
        proposalsByAddress[msg.sender].push(newProposal);

        //to be able to returl all porposals later
        proposals.push(newProposal);
    }

    function checkIfContributor(uint address _address) public view returns (bool) {
        require(proposalsByAddress[msg.sender]);

        if (contributors[_address]) {
            return true;
        } else {
            return false;
        }
    }
}

Proposal:

contract Proposal {
        Ballot ballot;
        bool complete;
        uint approveCount;
        uint rejectCount;
        mapping (address => bool) voters;

        constructor() {
            ballot = new Ballot(msg.sender);
        }

        function vote(bool _vote) {
            require(ballot.checkIfContributor(msg.sender));
            require(!voters[msg.sender]);

            voters[msg.sender] = true;
            if (_vote) {
                approveCount++;  
            } else {
                rejectCount++;
            }

        }
}
1

You have some of it but I see circular logic.

You have a Ballot contract that creates new Proposals and when a new Proposal instantiates, it creates a new Ballot contract. So, what's going on?

As I understand it, there should be one Ballot contract and many Proposal contracts. The proposal contracts need an interface to the Ballot contract that deployed them.

Change,

ballot = new Ballot(msg.sender);

to

ballot = Ballot(msg.sender);

That will assign ballet to the Ballot contract at the address of msg.sender, which should be a Ballot.

As a matter of style, I would change this line:

if (contributors[_address]) {
    return true;
} else {
    return false;
}

to

return contributors[_address];

Since this is a read-only function I'd even consider passing in address proposal instead of relying on msg.sender so anyone can check anything.

Consider:

function isVoter(address proposal, address voter) public view returns(bool canIndeed) {
  require(proposalsByAddress[proposal]); // not a proposal (not ours), invalid request
  return contributors[voter]; // is a contributor overall
}

Hope it helps.

  • Thanks! yeah it all sounds right. just forget that new will actually make a contract instead of initializing an interface. However, generally - is this the way this kind of relationship checks are done? – user1935987 Dec 25 '18 at 22:14
  • 1
    There is more than one way to approach it. Your hub and spoke approach is reasonable and you’re catching the important checks. – Rob Hitchens - B9lab Dec 26 '18 at 3:42

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.