Project - Multisig Wallet

[thecil]

Great Job, I tested your contract, although it works ok, it could be a little bit confusing.

Steps I made:

• depositMoney() (from 1st owner) 3 ethers.
• createTransfer() 1 ether (from 2nd owner) to 2nd owner.
• viewTrasnferStatus() to check transfer data.
• approveTransfer() (from 3rd owner) , transaction approve and send.
• check balance of 2nd owner.

What I could suggest you:
Try to add a message for each require, that can made way easier to debug why a transaction is failing, when creating the transfer with 2nd owner, i tried to also approve it with the same owner, so your require(_approver != record[_index].initiator); jumps in correctly, but i dint know why it was failing until i read the function logic.

Overall a very good job, congratulations

If you have any more questions, please let us know so we can help you!

Carlos Z.

[bbss]

Hi thecil,

I’m still having some confusions regarding the payable keyword. If I write a smart contract that receive funds, should the smart contract’s address itself receive funds? Or should the smart contract have a state variable, of type address, that will receive the funds? Meaning, how should a smart contract receive and hold funds? Variant a or b below?

a. The smart contract’s address itself receives and holds funds; the variable ‘balance’ just keeps track of the smart contract’s balance.

contract ContractA {
  uint256 balance;

  function addFunds() public payable {
    balance += msg.value;
  }
}

b. The smart contract has, as a state variable, an address ‘fundsAddress’ that is used to keep funds; the variable ‘balance’ keeps track of the contract’s balance.

contract ContractB {
  address fundsAddress;
  uint256 balance;

  constructor(address _fundsAddress) {
    fundsAddress = _fundsAddress;
  }

  function addFunds() public payable {
    fundsAddress.transfer(msg.value);
    balance += msg.value;
  }
}

Regarding the reason why the method payTransferRequest is called inside createTransferRequest(), I wanted to cover the case when indeed it’s just one approval limit in the constructor. I could get rid of this case, if it’s unjustified.

[bbss]

Hi thecil,

After playing in remix with variants A and B, I realized that B is impossible to implement. I mean, if you keep funds in an external address, you cannot retrieve value from that external address.

For this reason, all funds are kept in the smart contract’s address itself. Every payable function adds funds to this smart contract address. address(this) is the current smart contract’s address, msg.value represents how much value was sent, msg.sender represents the sender address.

I’ve programed, in Remix, a basic ContractA implementation to solidify the knowledge.

pragma solidity 0.7.5;


contract ContractA {
    uint256 balance;
    
    function addFunds() public payable {
        balance += msg.value;
    }
    
    function getFunds() public view returns(uint256) {
        return balance;
    }
    
    function getContractFunds() public view returns(uint256) {
        return address(this).balance;
    }
    
    function withdrawFunds() public {
        msg.sender.transfer(balance);
        balance = 0;
    }
    
}

Basically, getFunds and getContractFunds retrieve the same result, so you don’t need the balance variable.

With these observations in mind, I’ve come to the following version:

pragma solidity 0.7.5;

contract MultiSignWallet {
    uint transferId = 0;
    
    address[] ownwers;
    uint nrApprovals;
    
    struct TransferRequest {
        uint ammount;
        address payable sendAddress;
        bool paid;
    }
    
    mapping(uint => TransferRequest) transferRequests;
    mapping(uint => address[]) signaturesPerTransferRequest;
    
    constructor(address[] memory _owners, uint _nrApprovals) {
        require(_owners.length > 0, "Please add at least one owner");
        require(_nrApprovals > 0, "There should be at least one approval.");
        require(_nrApprovals <= _owners.length, "The number of approvals should be at most the number of owners");
        ownwers = _owners;
        nrApprovals = _nrApprovals;
    }
    
    // returns transfer id
    function createTransferRequest(uint _ammount, address payable _toAddress) public returns (uint) {
        require(addressIsOwner(msg.sender), "A transfer request can only be performed by an owner");
        
        TransferRequest memory transfer = TransferRequest(_ammount, _toAddress, false);
        
        uint currentTransferId = transferId;
        transferId++;
        
        transferRequests[currentTransferId] = transfer;
        signaturesPerTransferRequest[currentTransferId].push(msg.sender);
        if (signaturesPerTransferRequest[currentTransferId].length >= nrApprovals) {
            payTransferRequest(currentTransferId);
        }
        
        return currentTransferId;
    }
    
    function retrieveTransferRequest(uint _id) public view returns (uint, address, bool) {
        
        return (transferRequests[_id].ammount, transferRequests[_id].sendAddress, transferRequests[_id].paid);
    }
    
    // returns true if the necessy number of sgnatures has been reached
    function signTransferRequest(uint _transferId) public returns(bool) {
        require(transferRequests[_transferId].ammount > 0, "Cound not find a transfer with this id.");
        require(!transferRequests[_transferId].paid, "This transfer request is already paid.");
        require(addressIsOwner(msg.sender), "Only an owner can sign the transfer request");
        
        bool callerAlreadySigned = false;
        for(uint i = 0; i < signaturesPerTransferRequest[_transferId].length; i++) {
            if (msg.sender == signaturesPerTransferRequest[_transferId][i]) {
                callerAlreadySigned = true;
                break;
            }
        }
        
        require(!callerAlreadySigned, "A owner cannot sign a transfer request multiple times.");
        
        signaturesPerTransferRequest[_transferId].push(msg.sender);
        if (signaturesPerTransferRequest[_transferId].length >= nrApprovals) {
            payTransferRequest(_transferId);
            return true;
        }
        
        return false;
    }
    
    function addFunds() public payable {
        require(addressIsOwner(msg.sender), "Only an owner can add funds.");
    }
    
    function getAvailableFunds() public view returns (uint256) {
        return address(this).balance;
    }
    
    function addressIsOwner(address _caller) private view returns(bool) {
        for(uint i = 0; i < ownwers.length; i++) {
            if (_caller == ownwers[i]) {
                return true;
            }
        }
        
        return false;
    }
    
    function payTransferRequest(uint _id) private {
        require(address(this).balance >= transferRequests[_id].ammount, "The contract does not as enough funds to complete the transfer.");
        transferRequests[_id].sendAddress.transfer(transferRequests[_id].ammount);
        transferRequests[_id].paid = true;
    }
}

Thank you again for your patience in examining my code! I could not do the ‘FlipCoin’ homework (or probably any other app) without figuring out this basic idea.

[MentalChai]

Hello,
I met the requirements of the the assignment.
I feel like there is a lot of room for improvement if this was for production.

pragma solidity 0.7.5;


contract MultiSigner {
    
    struct TransferRequest {
        uint amount;
        address Sender;
        address Recipient;
        uint approvals;
    }

    event transferEvent(uint amount, address indexed depositedFrom, address indexed depositedTo);

    mapping(address => bool) owners;
    mapping(address => uint) balance;
    mapping(uint => TransferRequest) transfer_requests;
    TransferRequest request;
    
    uint transferKey = 0;
    uint approvalLimit = 0;
    constructor() {
        owners[msg.sender] = true;
    }

    // 1. Anyone can deposit
    function deposit() public payable returns (uint){
        //require(balance[msg.sender] >= msg.value, "Sender has insufficient funds");
        balance[msg.sender] += msg.value;
        return balance[msg.sender];
    }
    
    function getBalance(address person) public view returns (uint){
        return balance[person];
    }
    
    // 2A. Creator should be able to update owners
    function addOwner(address newOwner) public returns (bool){
        require(isOwner(msg.sender), "Only the Owner can add owners");
        owners[newOwner] = true;
        return true;
    }
    function removeOwner(address exOwner) public returns (bool) {
        require(isOwner(msg.sender), "Only the Owner can remove owners");
        owners[exOwner] = false;
        return false;
    }
    function isOwner(address owner) internal view returns (bool) {
        return owners[owner];
    }
    
    
    // 2B. Creator should be able to update number of approvals for a transfer?
    function updateApprovalLimit(uint limit) public {
        require(isOwner(msg.sender), "Only the Owner can update the limits");
        approvalLimit = limit;
    }
    
        
    function getApprovalLimit() public view returns (uint){
        return approvalLimit;
    }
    
    // 3A. Any owner can create a transfer request (Amount, Recipient)
    function createTransferRequest(uint amount, address recipient) public returns (uint){
        require(isOwner(msg.sender), "Only the Owner can create a transfer request");
        
        // Create Request
        request = TransferRequest(amount, msg.sender, recipient, 0);

        //// Add to Dictionary
        transferKey += 1;
        //transfer_requests[11] = request;
        transfer_requests[transferKey] = request;
        
        // Assert everything okay
        //assert(transfer_requests.length == request_count + 1);
        return transferKey;
        
    }
    
    function viewTransferRequest(uint transferId) public view returns (uint, address, address, uint){
        return (transfer_requests[transferId].amount, transfer_requests[transferId].Sender, transfer_requests[transferId].Recipient, transfer_requests[transferId].approvals);
    }
    
    // 4. Owners can approve transfer_requests
    function approveTransfer(uint transferId) public returns (uint){
        require(isOwner(msg.sender), "Only the Owner can approve transfer requests");
        transfer_requests[transferId].approvals += 1;
        
        
        if (transfer_requests[transferId].approvals >= approvalLimit){
            // make the transfer
            balance[transfer_requests[transferId].Sender] -= transfer_requests[transferId].amount;
            balance[transfer_requests[transferId].Recipient] += transfer_requests[transferId].amount;
            emit transferEvent(transfer_requests[transferId].amount, transfer_requests[transferId].Sender, transfer_requests[transferId].Recipient);
            return 1;
        }
        else {
            return 0;
        }
        
        // assert everything is okay
        
    }

}

[makinitnow]

Hi,

I have a first question as I am startng to write the Multisig wallet. I do not understand if the contract creator is intended to be one of the owners or a separate entity? So what to set in the constructor would be the contractCreator = msg.sender. I do not understand this very well. So the msg.sender is the address that adds the address of the owners, pushes them in the array of addresses.
So everyone can deposit, but withdrawal is restricted to the owners and this are separate addresses defined in the array and not the msg.sender addresses?
Sorry, hope it’s clear what I am asking.

[Capplequoppe]

In my solution I had the contract creator saved separate from the “co owners”. I defined the list of owners as an array to support a custom number of owners. You could always check so that the sender is in the list of owners and if not you can add him/her automatically. As has been mentioned, there are not a single answer to the assignment.

[Capplequoppe]

I like the idea of having everything dynamic, however you don’t want the scenario where the creator can change the limit back to 1 whenever he wants and by doing so disable the purpose of having multiple approvals for security. But as you said, It’s all about meeting the requirements for the assignment so you can move to the next assignment.

[Nile]

Here’s my no peeking solution. Took me a while, but learned alot! Looking forward to seeing what Filip did. Any suggestions are very much appreciated!

Filename: AnonymousFund.sol

pragma solidity 0.7.5;
pragma abicoder v2;

import "./AnonymousBase.sol";

contract AnonymousFund is AnonymousBase {
    
    //constructor takes number of approvals required for transfer, and founder addresses
    constructor(uint _requiredApprovals, address[] memory _founders) {
        //add required approvals
        requiredApprovals = _requiredApprovals;
        
        //add owner to founder addresses
        addFounder(msg.sender);
        
        //add other founder addresses
        for (uint i = 0; i < _founders.length; i++){
            addFounder(_founders[i]);
        }
    }
    
    //deposit function
    function deposit() public payable {
        //log
        emit deposited(msg.value, msg.sender);
    }
    
    //transfer request function (only for founders)
    function transferRequest(uint _transferAmount, address payable _transferAddress) onlyFounders public{
        transferAmount = _transferAmount;
        transferAddress = _transferAddress;
    }
    
    //approve function (only for founders)
    function approve() public onlyFounders{
        //require founders to approve only once
        require(approved[msg.sender] == false, "No voting twice, you sneaky.");
        
        //add 1 to approvals counter
        approvals += 1;
        
        //switch approved mapping to already approved
        approved[msg.sender] = true;
    }
    
    //transfer function (only for founders)
    function transfer() public onlyFounders {
        //require approvals
        require(approvals >= requiredApprovals, "Need more approvals, you hasty.");
        
        //require withdrawal amount to be less than contract balance
        require (address(this).balance >= transferAmount, "Insufficient funds, you greedy.");
        
        //transfer
        transferAddress.transfer(transferAmount);
        
        //log
        emit transferred(transferAmount, transferAddress);
        
        //reset
        reset();
    }
    
    //get contract balance function
    function FundBalance() public view returns(uint){
        return address(this).balance;
    }
    
    //get approvals function
    function Approvals() public view returns(uint){
        return approvals;
    }
    
}

Filename: AnonymousBase.sol

pragma solidity 0.7.5;

contract AnonymousBase {
    
    //array of founder addresses
    address[] founders;
    
    //number of founder approvals required for transfer
    uint requiredApprovals;
    
    //number of founder approvals
    uint approvals = 0;
    
    //mapping of founders already approved
    mapping (address => bool) approved;
    
    //amount to transfer
    uint transferAmount;
    
    //address to transfer
    address payable transferAddress;
    
    //logs
    event deposited(uint amount, address indexed depositedFrom);
    event transferred(uint amount, address indexed transferredTo);
    
    //require msg.sender to be a founder address
    modifier onlyFounders{
        address founder;
        uint i = 0;
        while (i < founders.length){
            if(msg.sender == founders[i]){
                founder = founders[i];
                i = founders.length;
            }
        i++;
        }
        require(msg.sender == founder, "I don't know you like that, you nobody.");
        _;
    }
    
    //add founder function
    function addFounder(address _address) internal{
        //add to founders array
        founders.push(_address);
        
        //add to approved mapping
        approved[msg.sender] = false;
    }
    
    //reset function
    function reset() internal{
        //reset transfer request
        delete transferAmount;
        delete transferAddress;
        
        //reset approvals
        approvals = 0;
        for (uint i = 0; i < founders.length; i++){
            approved[founders[i]] = false;
        }
    }
    
}

[LukeAppleby]

Like some others I see here, gave this a go without watching past the first video.

https://github.com/LukeAppleB/escp101/tree/main/Wallet%20Project

Please take a look and let me know what you guys think.

Awesome course, didn’t realize how much I picked up until this project.

Thanks Filip - see you in 201!

[mars1]

It’s a mess, but does the job. I included also a function to add new owners.
Definitely beginner code. I used arrays where mappings would have been better, but at some point I needed to move on. A very nice starting project and introduction to solidity.

pragma solidity 0.7.5;
pragma abicoder v2;
//
contract Divide {

  function percent(uint numerator, uint denominator, uint precision) internal returns(uint quotient) {

         // caution, check safe-to-multiply here
        uint _numerator  = numerator * 10 ** (precision+1);
        // with rounding of last digit
        uint _quotient =  ((_numerator / denominator) + 5) / 10;
        return ( _quotient);
  }

}

contract Bank is Divide {
    
    mapping(address => uint) balance;
    mapping(address => address[]) accountOwners;
    mapping(address => bool[]) txApproval;
    
    struct tx_struct {
        uint txId;
        uint txType; //request 0 for newOwner, 1 for transfer
        address recipient;
        uint amount;
        bool fulfilled;
    }
    
    tx_struct[] txArray;
    
    address creator;
    
    constructor(address _owner){
        accountOwners[msg.sender].push(msg.sender);
        accountOwners[msg.sender].push(_owner);
        creator = msg.sender;
    }
    
    
    event transferRequest(uint amount, address sender, address recipient);
    
    
    // you can only add owners
    function addOwner(address newOwner) private {
        accountOwners[creator].push(newOwner);
    }
    
    function getNumberOwners() public view returns(uint) {
        return accountOwners[creator].length;
    }
    
    // function for deposit; anyone can deposit any amount
    function deposit() public payable returns(uint){
        balance[creator] += msg.value;
        return balance[creator];
    }

    
    // set the ratio of approvals required in 3 digits
    function getApproval(uint _txId) private returns(bool) {
        uint approved = countApproval(_txId);
        if(percent(approved, accountOwners[creator].length, 3) >= 666){
            return true;
        }
        else{
            return false;
        }
    }
    
    
    // anyone can create a transfer request: public transfer function
    function transfer(address recipient, uint amount) public payable {
        require(balance[creator] >= amount, "Balance not sufficient");
        require(creator != recipient, "Don't transfer money to yourself");
        // an event for the transfer should be emitted
        balance[creator] -= amount;
        balance[recipient] += amount;
    }
    
    
    // create request
    function createRequest(uint _txType, address _newOwner, uint _amount) public 
    returns(uint) {
        bool _isOwner = false;
        for(uint i = 0; i < accountOwners[creator].length; i++){
            if (accountOwners[creator][i] == msg.sender){
                _isOwner = true;
            }
        }
        if(_isOwner){
            uint length = txArray.length;
            txArray.push(tx_struct(length, _txType, _newOwner, _amount, false));
            newSignature();
            sign(length);
            return length;
        }
        else{
            require(false, "You need to be a owner in order to create a request");
            _;
        }
    }
    
    function getNumberRequest() public view returns(uint){
        return txArray.length;
    }

    // get signature
    function sign(uint _txId) public {
        require(txApproval[msg.sender][_txId] != true);
        txApproval[msg.sender][_txId] = true;
        
        if(txArray[_txId].txType == 0){
            if(getApproval(_txId)){
                addOwner(txArray[_txId].recipient);
                txArray[_txId].fulfilled = true;
            }
        }
        else {
            if(getApproval(_txId)){
                transfer(txArray[_txId].recipient, txArray[_txId].amount);
                txArray[_txId].fulfilled = true;
            }
        }
        
        
    }
    
    
    // get current balance of the wallet;
    function getBalance() public view returns(uint){
        bool inMultiSig = false;
        for(uint i = 0; i < accountOwners[creator].length; i++){
            if (accountOwners[creator][i] == msg.sender){
                inMultiSig = true;
            }
        }
        if(inMultiSig){
            return balance[creator];
        }
        else{
            return balance[msg.sender];
        }
        
    }
    
    
    // count approval
    function countApproval(uint _txId) public returns(uint){
        uint _approved = 0;
        for(uint i = 0; i < accountOwners[creator].length; i++){
            if(txApproval[accountOwners[creator][i]][_txId] == true){
                _approved += 1;
            }
        }
        return _approved;
    }
    // reset approval
    function newSignature() private{
        for(uint i = 0; i < accountOwners[creator].length; i++){
            txApproval[accountOwners[creator][i]].push(false);
        }
    }
}

[thecil]

Great Job, I tested your contract, although it works ok, it could be a little bit confusing.

Steps I made:

• addFunds() 3 ethers from each owner (9 total).
• getAvailableFunds() to check contract balance
• createTransferRequest() 8 ether (from 1st owner) to 3rd owner.
• retrieveTransferRequest() to check transfer data.
• signTransferRequest() (from 1st owner) , require jumps in, 1st owner has already signer (because he create the transfer request).
• signTransferRequest() (from 2nd owner), transaction approve and send.
• check balance of 3rd owner.

Overall a very good job, congratulations

If you have any more questions, please let us know so we can help you!

Carlos Z.

[thecil]

Hey @LukeAppleby, hope you are great.

I’m trying your contract and i found something that i do not understand quite well.

    // constructor with 3 owners
    constructor(uint reqApprovals, address owner2, address owner3) {
        reqApprovalsForTransfer = reqApprovals;
        owners[owner2] = 1;
        owners[owner3] = 1;
    }

owners array is not defined anywhere in the contract.

    function getListIdWithTransferId (uint id) private view returns (uint) {
        for ( uint i = 0; i < transferQueue.length; i++ ) {
            if (transferQueue[i].id == id) return i;
        }
    }

This function should return a uint variable at the end of its logic.

Also the funds can’t be withdraw from the contract, when you makeTransferRequest it must be approveRequest by the owners, when it reachs the approve limit, it will be sent automatically to the address specifed, but is not a transfer to the address itself, is an internal transfer and then that address just have more internal funds to use inside the contract, but there is no way to withdraw the funds from the contract.

Overall a very good job, the getTransferQueueLength is a cleaver way to track how amny request you have in place, also the getLiquidity to differentiate whenever is an msg.sender balance or the contract balance in general.

congratulations

If you have any more questions, please let us know so we can help you!

Carlos Z.

[thecil]

Hey @makinitnow, hope you are well.

@makinitnow

I do not understand if the contract creator is intended to be one of the owners or a separate entity?

Is up to you, for simplicity it can be both (contract owner and also multisig participant owner), but if you want to specify it on a different way, is completely valid

@makinitnow

So what to set in the constructor would be the contractCreator = msg.sender. I do not understand this very well. So the msg.sender is the address that adds the address of the owners, pushes them in the array of addresses.

msg.sender is the keyword referred to the account/address that execute the function, so in the constructor, the address that deploy the contract will be msg.sender by that moment.

@makinitnow

So everyone can deposit, but withdrawal is restricted to the owners and this are separate addresses defined in the array and not the msg.sender addresses?

withdrawals are limited to owners (participants of the multisig), also you can limit deposits to owners by using a modifier.

If you have any more questions, please let us know so we can help you!

Carlos Z.

[ivanovp]

Hello, here i have my solution. I added possibility to create multiple wallets with single contract.

pragma solidity 0.7.5;

contract MultisigWalletSystem {
    struct TransferRequest {
        uint id;
        address payable to;
        uint walletId;
        uint amount;
        address[] approvals;
        uint8 status;
    }
    
    uint8 constant TransferRequestStatusPending = 0;
    uint8 constant TransferRequestStatusDone = 1;
    uint8 constant TransferRequestStatusFailed = 2;
    
    struct Wallet {
        uint id;
        address[] owners;
        uint balance;
        uint approvalTreshold;
    }
    
    uint transferRequestIdSequence;
    mapping(uint => TransferRequest) transferRequests;
    uint walletIdSequence;
    mapping(uint => Wallet) wallets;
    
    function createWallet(uint treshold, address[] memory owners) external returns(uint){
        walletIdSequence++;
        Wallet storage wallet = wallets[walletIdSequence];
        wallet.id = walletIdSequence;
        wallet.owners.push(msg.sender);
        for (uint i = 0; i < owners.length; i++) {
            wallet.owners.push(owners[i]);
        }
        wallet.approvalTreshold = treshold;
        return walletIdSequence;
    }
    
    // returns owners list, balance, approvalTreshold
    function getWallet(uint walletId) external view returns(address[] memory, uint, uint) {
        Wallet memory wallet = wallets[walletId];
        return(wallet.owners, wallet.balance, wallet.approvalTreshold);
    }
    
    // returns balance after deposit
    function deposit(uint walletId) external payable returns (uint) {
        require(_walletHasAddress(walletId, msg.sender), "Permission denied. You are not wallet owner");
        wallets[walletId].balance += msg.value;
        return wallets[walletId].balance;
    }
    
    function createTransfer(uint walletId, uint amount, address payable to) external returns (uint) {
        require(_walletHasAddress(walletId, msg.sender), "Permission denied. You are not wallet owner");
        require(wallets[walletId].balance >= amount, "Insufficient wallet balance");
        transferRequestIdSequence++;
        TransferRequest storage request = transferRequests[transferRequestIdSequence];
        request.to = to;
        request.walletId = walletId;
        request.amount = amount;
        request.approvals.push(msg.sender);
        request.status = TransferRequestStatusPending;
        return transferRequestIdSequence;
    }
    
    function _walletHasAddress(uint _walletId, address _address) internal view returns (bool) {
        address[] memory owners = wallets[_walletId].owners;
        return _containsAddress(owners, _address);
    }
    
    function _containsAddress(address[] memory haystack, address _needle) internal pure returns(bool) {
         for (uint i = 0; i < haystack.length; i++) {
            if (haystack[i] == _needle) {
                return true;
            }
        }
        
        return false;
    }
    
    // returns transferDirection, walletId, amount, approvals list, status
    function getTransferRequest(uint transferRequestId) external view returns (address, uint, uint, address[] memory, uint8){
        TransferRequest memory request = transferRequests[transferRequestId];
        return(request.to, request.walletId, request.amount, request.approvals, request.status);
    }
    
    function approveTransfer(uint transferRequestId) external {
        TransferRequest storage request = transferRequests[transferRequestId];
        require(_walletHasAddress(request.walletId, msg.sender), "Permission denied. You are not wallet particapant");
        require(!_containsAddress(request.approvals, msg.sender), "You have your approval already sent");
        request.approvals.push(msg.sender);
        if (request.approvals.length == wallets[request.walletId].approvalTreshold) {
            _fireTransfer(transferRequestId);
        }
    }
    
    
    function _fireTransfer(uint transferRequestId) internal {
        TransferRequest storage request = transferRequests[transferRequestId];
        Wallet storage wallet = wallets[request.walletId];
        if (wallet.balance < request.amount) {
            request.status = TransferRequestStatusFailed;
        } else {
            request.to.transfer(request.amount);
            wallet.balance -= request.amount;
            request.status = TransferRequestStatusDone;
        }
    }
}

[kirtan-amin]

Hi,
I was wondering if this contract needs to allow multiple wallets or is this a one wallet per deployed contract ?

[thecil]

Hey @ivanovp

I tested your contract, although it works ok, it could be a little bit confusing.

Steps I made:

• createWallet() threshold = 2, array of 3 wallets [“wal1”,“wal2”,"wal3].
• getWallet() id = 1, get data correctly.
• deposit() 5 ether (from 1st owner) to wallet id 1.
• createTransfer() 4 ethers, from 1st owner to 2nd owner (wallet id1)
• getTransferRequest() id = 1, get data correctly.
• approveTransfer() with the 3 owners, but the transaction never got sent to the address requested in the transfer.
  Maybe i’m doing something wrong, the threshold maybe?

Your contract looks very solid, i like the idea that can be multiple owners that can manage their own funds, like multiple instances for the same method (multisig). Nice idea!

Carlos Z

[vineet]

pragma solidity 0.7.5;
pragma abicoder v2;

import "./Ownable.sol";

contract Wallet is Ownable {
    
    address[] public owners;
    uint limit;
    
    struct Transfer {
        uint id;
        uint noOfApprovals;
        address from;
        address payable to;
        uint amount;
        bool approved;
    }
    
    Transfer[] transferRequests;
    
    mapping(address => mapping(uint => bool)) approvals;
    mapping(address => uint) balance;
    
    modifier onlySignatory() { 
        bool isSignatory;
        for(uint i=0; i<limit; i++){
            if(msg.sender == owners[i]) {
                isSignatory = true;
                break;
            }
        }
        require(isSignatory, "You are not a signatory");
        _;
    }
    
    constructor (uint _limit, address[] memory _owners) {
        require(_limit >= 2, "More than 2 signatures required");
        require(_owners.length >= 2, "More than 2 signatories required");
        require(_owners.length >= _limit, "Limit exceeds number of signatories");
        bool _duplicate;
        for(uint i=0; i<_owners.length; i++) {
            for(uint j= i+1; j< _owners.length; j++) {
                if(_owners[i] == _owners[j]) {
                    _duplicate = true;
                    break;
                }
            }
            if(_duplicate == false) {
                owners.push(_owners[i]);
            }
        }
        require(_duplicate == false, "Duplicate Signatory");
        limit = _limit;
    }
    
    function deposit() public payable returns (uint) {
        balance[msg.sender] += msg.value;
        return balance[msg.sender];
    }
    
    function getBalance() public view returns (uint) {
        return balance[msg.sender];
    }
    
    function createTransferRequest(address payable _to, uint _amount)  public {
        require(_amount > 0 && balance[msg.sender] >= _amount && _to != msg.sender, "error here");
        transferRequests.push(Transfer(transferRequests.length, 0, msg.sender, _to, _amount, false));
    }
    
    function approve(uint _id) public onlySignatory {
        require(approvals[msg.sender][_id] == false, "You have already signed on this transaction");
        approvals[msg.sender][_id] = true;
        transferRequests[_id].noOfApprovals++;
        checkApprovals(_id);
    }
    
    
    function checkApprovals(uint _id) public {
          require(transferRequests[_id].approved == false, "Transaction already approved");
        if(transferRequests[_id].noOfApprovals == limit) {
          transferRequests[_id].to.transfer(transferRequests[_id].amount);
          transferRequests[_id].approved = true;
          balance[transferRequests[_id].to] += transferRequests[_id].amount;
          balance[transferRequests[_id].from] -= transferRequests[_id].amount;
        }
          
    }
    
    function transactionStatus(uint _id) public view returns(bool){
        return transferRequests[_id].approved;
    }
}

[Imhotep]

Hello thecil you have’nt looked at my other code yet that I did after this one. Can you take a look at how I did it without the double mapping. I still plan on using the double mapping just to see how it works but I think I got it done without it before I have looked at any videos. It is under Imhotep and I did it about a week ago.

[LuizCyriloA]

pragma solidity 0.7.5;
pragma abicoder v2;

contract Multisig {

address owner;
mapping(address=>uint) private depositReceived; //just to know who made deposits
mapping(address=>bool) private owners; //if true, means we already added the owner
uint public approvalsRequired; //# of approvals required in order to confirm the transaction request
uint public contractBalance; //amount received by the contract. We can only transfer what we have.
uint public totalRequests; //total # of requests made.

event transactionSent(address _to, uint _amount);
event transactionRequested(address _to, uint _amount, address _owner);


struct TransferRequests {
    address payable to;
    uint amount;
    bool sent; //transaction already performed/sent..
    uint totalSigned; //nro of owners that already signed!
    mapping(address=>bool) signatures; //who signed.. I want to control so no onwer can approve twice.
}

mapping(uint=>TransferRequests) requests;

constructor(uint _approvals) {
    require(_approvals > 0, "Approval limite must be greater than 0.");
    
    approvalsRequired = _approvals;
    
    owner = msg.sender;
}

modifier OnlyOwner() {
    require(msg.sender == owner, "Only the owner can add more owners.");
    _;
}

function deposit() public payable returns (uint){
    require(msg.value > 0, "Deposit value must be greater than 0.");
    
    uint previousAmount = depositReceived[msg.sender];
    
    depositReceived[msg.sender] += msg.value;
    contractBalance += msg.value; 
    
    assert(depositReceived[msg.sender] == previousAmount + msg.value);
    
    return msg.value;
    
}

//add the owners allowed to approve a transaction
function addOwners(address _ownerSig) public OnlyOwner() {
    require(owners[_ownerSig] == false, "Owner already added.");
    
    owners[_ownerSig] = true;
}


//creating the request.
function createRequest(address payable _to, uint _amount) public{
    require(_amount > 0, "Amount must be greater than 0.");
    require(msg.sender != _to,"Not allowed to send ether to yourself.");
    require(owners[msg.sender] == true, "Only the owners can create transfers.");
    require(contractBalance >= _amount, "Not enough funds to transfer.");
   
    //create the details of the request 
    TransferRequests storage newRequest = requests[totalRequests++];
    newRequest.to = _to;
    newRequest.amount = _amount;
    newRequest.sent = false;
    newRequest.totalSigned = 0;
    
    depositReceived[_to] += _amount;
    contractBalance -= _amount; //even that not approved yet, must deduct from the total available.
    
    emit transactionRequested(_to, _amount, msg.sender);
    
}

//return details of the request so you owner can check before approving.
function getRequest(uint _txid) public view returns (address,uint, bool, uint) {
    return (requests[_txid].to,requests[_txid].amount,requests[_txid].sent,requests[_txid].totalSigned);
}

function approveRequest(uint _txid) public {
    require(owners[msg.sender] == true, "Only Owners can approve transaction.");
    require(requests[_txid].sent == false, "Transaction already transfered.");
    require(requests[_txid].signatures[msg.sender] == false, "Owner cannot approve twice same transaction.");
    
    TransferRequests storage request = requests[_txid];
    request.totalSigned++;
    request.signatures[msg.sender] = true; //saving the owners that already signed.
    
    if (request.totalSigned >= approvalsRequired) { //ok, the needed number of signatures was reached. Perform the transfer.
        transfer(request.to, request.amount);
        request.sent = true;
    }
}

function transfer(address payable _to, uint _amount) private {

    _to.transfer(_amount);
    
    emit transactionSent(_to, _amount);
}

}Preformatted text

[thecil]

Hey @Imhotep

I forgot about your code, could you please share it again?

Carlos Z