0

I implemented multisig 2 of 3 contract. Please give me feedback on codestyle, security and usability. On codereview there is only two solidity questions, so I posted here.

see on github

pragma solidity >=0.4.22 <0.6.0;
contract multisig2of3 {
    address signer1;
    address signer2;
    address signer3;
    address payable receiver1;
    address payable receiver2;
    address payable receiver3;

    constructor(address owner1, address owner2, address owner3) public {
        assert(owner1 != owner2);
        assert(owner2 != owner3);
        assert(owner3 != owner1);
        signer1 = owner1;
        signer2 = owner2;
        signer3 = owner3;
    }

    function deposit() public payable {
        if (msg.value > 1 ether || address(this).balance > 1 ether) revert();
    }

    function setReceiver(address payable newReceiver) public {
        if (msg.sender == signer1) receiver1 = newReceiver;
        if (msg.sender == signer2) receiver2 = newReceiver;
        if (msg.sender == signer3) receiver3 = newReceiver;
    }

    function withdraw() public {
        address payable approvedReceiver;
        if (receiver1 == receiver2) approvedReceiver = receiver1;
        if (receiver2 == receiver3) approvedReceiver = receiver2;
        if (receiver3 == receiver1) approvedReceiver = receiver3;
        require(approvedReceiver != address(0));
        receiver1 = address(0);
        receiver2 = address(0);
        receiver3 = address(0);
        approvedReceiver.transfer(address(this).balance);
    }
}

closed as too broad by Ismael, shane, goodvibration, tayvano, flygoing Apr 18 at 14:37

Please edit the question to limit it to a specific problem with enough detail to identify an adequate answer. Avoid asking multiple distinct questions at once. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

2

You asked for style tips. These are matters of style.

For production, I dislike the ambiguous pragma. There are good reasons to avoid it.

In the deposit function, I would use 3 require() with error messages instead of the complex express with OR ||, for readabilty.

Both are of minor importance.

It is doubtful anyone here will testify "it is secure" given the risk/reward ratio. I might suggest a bug bounty. For example https://solidified.io, to get more eyes on it, with an incentive.

Hope it helps.

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