Function execution right based on external contract state, ballot and proposal example

Question

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++;
            }

        }
}

Answer 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.