Project - Multisig Wallet

jonp · February 11, 2021, 3:25pm

Thanks for the reply. Here’s my solution. For simplicity I’ve included all contracts in the same file. I’m interested why embedding a mapping inside a Struct is seen as bad practice as it would be seen as good practice in OOP. The approvers are a member property of a particular transaction so seemed natural to include that in the Struct. I also attempted the exercise without watching the double mapping video and therefore didn’t really think a double mapping was necessary.

Please refer to this stack overflow comment regarding embedding mappings:

https://stackoverflow.com/questions/66072737/use-mappings-inside-structs-in-solidity

pragma solidity 0.8.0;
pragma abicoder v2;

contract Ownership {
    
    //mapping that is checked when an address attempts to approve a transaction.
    mapping(address => bool) public owners; 
    
    modifier anyOwner() {
        require(owners[msg.sender] == true, "you are not an owner of this wallet!");
        _;
    }
}

contract Wallet {
    
    uint balance;
    Tx[] public txs;
    
    struct Tx {
        address initiator;
        address payable recipient;
        uint amount;
        mapping(address => bool) approvers;
        uint approvals;
    }
    
    modifier checkRecipient(address payable recipient) {
        require(msg.sender != recipient, "sender and recipient of this transfer are the same!");
        _;
    }
    
    modifier checkFunds(uint amount) {
        require(balance >= amount, "You do not have enough funds to perform this transfer");
        _;
    }
    
    modifier checkTransaction(uint transactionId) {
        require(transactionId < txs.length, "transaction with supplied id does not exist!");
        _;
    }
    
    event deposited(address indexed sender, uint amount);
    event transferred(address payable indexed recipient, uint amount);
    
    function deposit() public payable {
        
        balance += msg.value;
        emit deposited(msg.sender, msg.value);
    }
    
    function _transfer(Tx storage transaction) internal checkFunds(transaction.amount) returns (uint) {
        transaction.recipient.transfer(transaction.amount);
        balance -= transaction.amount;
        emit transferred(transaction.recipient, transaction.amount);
        return balance;
    }
}

contract MultiSigWallet is Ownership, Wallet {
    
    uint approvalLimit;
    
    event deployed(address indexed owner);
    event transferRequest(address indexed initiator, address payable indexed recipient, uint amount);
    event transactionApproved(address indexed approver, uint transaction);

    constructor(uint _approvalLimit, address[] memory _owners) {
        require(_approvalLimit > 0, "at least one approver is required for multisig");
        require(_owners.length >= _approvalLimit, "not enough owners to satisfy approval limit!");
        approvalLimit = _approvalLimit;
        owners[msg.sender] = true;
        emit deployed(msg.sender);
        
        for (uint8 i = 0; i<_owners.length; i++) {
            owners[_owners[i]] = true;
            emit deployed(_owners[i]);
        }
    }
    
    function requestTransfer(address payable recipient, uint amount) public anyOwner checkFunds(amount) checkRecipient(recipient) returns (uint) {
        Tx storage transaction = txs.push();
        transaction.initiator = msg.sender;
        transaction.recipient = recipient;
        transaction.amount = amount;
        transaction.approvals = 0;
        emit transferRequest(msg.sender, recipient, amount);
        return txs.length;
    }
    
    function approveTransfer(uint transactionId) public anyOwner checkTransaction(transactionId)  {
        require(txs[transactionId].approvals < approvalLimit, "transaction has already been approved and transferred!");
        require(txs[transactionId].approvers[msg.sender] == false, "you have already approved this transaction!");
        require(txs[transactionId].initiator != msg.sender, "you are the initiator so cannot approve!");
        txs[transactionId].approvals += 1;
        
        if (txs[transactionId].approvals == approvalLimit) {
            _transfer(txs[transactionId]);
        } 
    }
}

thecil · February 11, 2021, 5:29pm

Mappings can only have a data location of storage and thus are allowed for state variables, as storage reference types in functions, or as parameters for library functions. They cannot be used as parameters or return parameters of contract functions that are publicly visible. These restrictions are also true for arrays and structs that contain mappings.

https://docs.soliditylang.org/en/v0.7.0/types.html#mapping-types

For this exercise, use a mapping inside an struct could lead into complex understanding, since this course teach you the basics, in your contract for example your mapping approvers is not doing nothing, where is being use?

only here (approveTransfer function):
require(txs[transactionId].approvers[msg.sender] == false, "you have already approved this transaction!");

But it should be: when anyOwner want to approveTransfer, the mapping approvers should change the value of the boolean to true, because one of the owner is signing the tx.

Try this to check why your require is not failing:

pragma solidity 0.7.5;

contract helloworld {
    
    mapping(address=>bool) approvals;
    
    function test() public view returns(bool){
        return approvals[msg.sender];
    }
}

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

Carlos Z.

jonp · February 12, 2021, 3:35pm

Hi @thecil

Thanks for the explanation. I can see the limitation now as the mapping inside the struct isn’t visible when I interrogate the txs property.

Good spot regarding the approvers! My bad, not enough manual test cases done by me

Ignacio · February 12, 2021, 10:21pm

Hi, I have been stuck in the validation of a transaction step. Is it possible to have an array within a struct within an array?? For instance in my code I have been wanting to append addresses that approve a transaction (address[] approved_by;). This list of addresses are recorded within a struct (struct TransfersHist{ ... }) that contains information about the transaction, and each transaction is an element of the transactions array (TransfersHist[] transfers_hist;). Is this a viable method or should I forcefully use double mappings. I appreciate any help.

I tried to include many comments on my code so is more readable:

pragma solidity 0.7.5;
pragma abicoder v2;

contract MULTISIG{
    
    // n_approvals
    uint Napproval = 2;
    address[] Validators;  // all validator (aprovers) addresses are appended here
    
    // store Client in a mapping, where every element of the map is a struct
    mapping (address => Cli) Client;
    struct Cli{
        bool isOwner;
        uint funds;
        address adrs;                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                               
    }
    
    // transfers history struct
    struct TransfersHist{
        uint id;
        address payer;
        address recipient;
        uint amount;
        uint n_approvals;       // number of approvals out of Napproval=2 (defined at the top)
        bool already_approved;  // if the transaction already went through this will be true
        address[] approved_by;  // validators that approved a transaction will be appended here
    }
    
    TransfersHist[] transfers_hist;  // transfers_hist is a vector which elements are structs
    
    
    // contract creator
    address contract_creator;
    
    modifier only_contract_creator{
        require(msg.sender == contract_creator);
        _;
    }
    // n_approvals needed
    constructor() {
        contract_creator = msg.sender;
        Client[contract_creator] = Cli(true, Client[contract_creator].funds, contract_creator);  // contract_creator is by default a validator
        Validators.push(contract_creator);                                                       // contract creator is by default validator
    }
    
    
    // create logs
    event add_funds_log(address deposit_address, uint amount_added, uint current_balance);
    event client_added_log(address client_address, bool is_client_validator);
    event succesfull_transfer_log(uint transaction_id, address sender_address, address recipient_address, uint amount_transfered);
    
    
    // add funds function, anyone can add funds
    function addFunds() public payable returns(uint){
        Client[msg.sender].funds += msg.value;
        emit add_funds_log(msg.sender, msg.value, Client[msg.sender].funds);
        return Client[msg.sender].funds;
    }
    
    // Add clients and their validator status
    function addClient(bool _isValidator, address add_address) public {
        Cli memory new_client = Cli(_isValidator, Client[add_address].funds, add_address);
        Client[add_address] = new_client;
        emit client_added_log(add_address, _isValidator);
        
        if (Client[add_address].isOwner == true){  // if client is set as validartor, they are appended to the validators list
            Validators.push(add_address);
        }
    }
    
    
    // get client 
    function getClient() public view returns(Cli memory){
        return Client[msg.sender];
    }
    
    // get Validators
    function getValidators() public view returns(address[] memory){
        return (Validators);
    }
    
    
    // transact function moves money from one account to another
    function transact_proceed(address _from, address _to, uint _amount) private {
        Client[_from].funds -= _amount;
        Client[_to].funds += _amount;
    }
    
    // Aprove transaction function
    function Aprove(uint _transaction_ID) public {
        require(Client[msg.sender].isOwner == true);
        TransfersHist memory current_transaction = transfers_hist[_transaction_ID]; // refer to the element of the array, each element is a struct
        require(current_transaction.already_approved == false, "The transaction already went through");
        
        // check that addresses have not approved a transacition
        for (uint i=0; i<current_transaction.approved_by.length; i++){
            require(msg.sender != current_transaction.approved_by[i], "This address has already approved the transaction");
        }
        current_transaction.n_approvals += 1;                  // add +1 to the count of number of approvals
        //address[] memory new_validator = [msg.sender];             
        current_transaction.approved_by.push(msg.sender);   // add approved address to the "aproved_by" array
        
        // check if the number of approvals meet Napp=2, if so proceed with the transaction
        if (current_transaction.n_approvals >= Napproval){
            transact(current_transaction.payer, current_transaction.recipient, current_transaction.amount);  // call fucntion transact
            current_transaction.already_approved = true;                                                     // change transaction status
        }
    }
    
    // post transaction into transfers_hist vector, this then is proceeded once the number of approvals is >= 2
    function transact(address _from, address _to, uint _amount) public {
       require(Client[_from].funds >= _amount, "Not enough balance in account");
       require(_from != _to, "Can't auto-deposit");
       
       uint n0 = 0;                           // initial number of approvals
       address[] memory Validators_init;      // empty list, initial addresses that approved the transaction
       transfers_hist.push(TransfersHist(transfers_hist.length, _from, _to, _amount, n0, false, Validators_init));  // add an element to transaction history array
    }
    
    
       
    function getTranscactionHist() public view returns(TransfersHist[] memory){
        return transfers_hist;
    }
}

ajcapo90 · February 13, 2021, 10:28am

Hi, I’m very exciting about my progress during the execution of this exercise. I haven’t had a dev background, so I’ve needed to check some tips, but in the end I’ve already built all the functions from scratch by myself!

Thanks a lot @filip for this amazing course, I’m looking forward to diving into the new one ETH programming 201!!

Here my solution:

pragma solidity 0.7.5;
pragma abicoder v2;

contract Wallet{
    
    //Define an array of owners
    address[] public owners;
    
    //state var that define the minimum nr. of approvals, initialized into constructor
    uint limit;
    
    constructor(address _owners1, address _owners2, address _owners3) {
        owners.push(msg.sender);
        owners.push(_owners1);
        owners.push(_owners2);
        owners.push(_owners3);
        limit = 2;
    }
    
    modifier onlyOwners(){
        require(msg.sender == owners[0] || msg.sender == owners[1] || msg.sender == owners[2] || msg.sender == owners[3]);
        _;
    }
    
    struct Transfer{
        uint id;
        uint approvals;
        address from;
        address payable to;
        uint amount;
    }
    
    //Array of Transfer Request Pending
    Transfer[] transferRequest;
    
    mapping(address => mapping(uint => bool)) approvals;
    mapping(address => uint) balance;
    
    event balanceAdded(uint amount, address indexed depositedTo);
    event transferRequestSubmitted(uint id, address indexed from, address indexed to, uint amount);
    event transferSended(uint id, uint approvals, address indexed to, uint amount);
    
    //Anyone can add funds into the Wallet
    function deposit() public payable returns(uint){
        balance[msg.sender] += msg.value;
        emit balanceAdded(msg.value, msg.sender);
        return balance[msg.sender];
    }
    
    //The transfer will push into the array, no other actions right now..
    function transfer(address payable _to, uint _amount) public onlyOwners returns(Transfer[] memory, uint){
        Transfer memory newTransfer = Transfer(transferRequest.length, 0, msg.sender, _to, _amount);
        transferRequest.push(newTransfer);
        emit transferRequestSubmitted(newTransfer.id, newTransfer.from, newTransfer.to, newTransfer.amount);
        return (transferRequest, address(this).balance);
    }
    
    //take back an ID transaction, control that the approver is different and add +1 in approvals
    function approve(uint _transferId) public onlyOwners returns(uint, uint){
        Transfer storage transferToApprove = transferRequest[_transferId];
        require(transferToApprove.from != msg.sender && approvals[msg.sender][_transferId] == false);
        transferToApprove.approvals += 1;
        approvals[msg.sender][_transferId] = true;
        //when approvals = 2 --> if there are enough funds transfer Wallet's balance to "address to"
        if(transferToApprove.approvals == limit){
            require(transferToApprove.amount <= address(this).balance);
            uint oldBalance = balance[transferToApprove.to];
            transferToApprove.to.transfer(transferToApprove.amount);
            balance[transferToApprove.to] += transferToApprove.amount;
            assert(balance[transferToApprove.to] == oldBalance + transferToApprove.amount);
            emit transferSended(transferToApprove.id, transferToApprove.approvals, transferToApprove.to, transferToApprove.amount);
        }
        return(address(this).balance, balance[transferToApprove.to]);
    }
    
    //check if an approver has approved a specific transfer request or not.
    function checkApprover(address _approver, uint _transferId) public view returns(bool){
        return approvals[_approver][_transferId];
    }
}

ajcapo90 · February 13, 2021, 10:48am

@thecil If I had tried to slip the Wallet Owners definitions to another contract, it would have given back to me this error:

Here is the code:

pragma solidity 0.7.5;
pragma abicoder v2;

contract WalletOwners{
    
    address[] public owners;
    
    constructor(address _owners1, address _owners2, address _owners3){
        owners.push(msg.sender);
        owners.push(_owners1);
        owners.push(_owners2);
        owners.push(_owners3);
    }
    
    modifier onlyOwners(){
        require(msg.sender == owners[0] || msg.sender == owners[1] || msg.sender == owners[2] || msg.sender == owners[3]);
        _;
    }
}

contract Wallet is WalletOwners{

    uint limit;
    
    constructor() {
        limit = 2;
    }

If I had put the word “abstract” before “contract Wallet”, that error would have been dissolved; but when I deployed I could see another one:

Can you help me on this?
Thanks!

thecil · February 13, 2021, 10:27pm

Hey @ajcapo90, hope you are great.

Abstracts contracts are contracts that have partial function definitions. You cannot create an instance of an abstract contract. An abstract contract must be inherited by a child contract for utilizing its functions. Abstract contracts help in defining the structure of a contract and any class inheriting from it must ensure to provide an implementation for them. If the child contract does not provide the implementation for incomplete functions, even its instance cannot be created. The function signatures terminate using the semicolon, ;, character.

So what is important to understand is that abstracts contracts should not have a constructor, because their like a uninitialized library (like a tool box), constructor should only be used on the parent contract (main), because is the method to set some parameters at the first initialization of the contract (like setting the owners before the contract can be deployed).

So, basically just move the constructor of WalletOwners to the Wallet contract, add the limit =2; to the new constructor and try it again.

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

Carlos Z.

thecil · February 13, 2021, 10:39pm

Hey @Ignacio, hope you are great.

To be honest, it is way easy to just use double mapping, because it solve that problem you are showing.

mapping(address => mapping(uint => bool)) approvals;

      //An owner should not be able to vote twice.
      require(approvals[msg.sender][_id] == false, "Transfer already signed by this account");

In this example, require just ask "is this msg.sender has signed this transaction _id ?

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

Carlos Z.

ajcapo90 · February 14, 2021, 8:03am

Thanks a lot Carlos @thecil, I’ve transferred the whole constructor into Wallet and now it works correctly! Have a nice Sunday!

Ignacio · February 15, 2021, 11:41pm

Thanks Carlos! Here is the fixed code, everything working now

pragma solidity 0.7.5;
pragma abicoder v2;

contract MULTISIG{
    
    // n_approvals
    uint Napproval = 2;
    address[] Validators;  // all validator (aprovers) addresses are appended here
    
    // store Client in a mapping, where every element of the map is a struct
    mapping (address => Cli) Client;
    struct Cli{
        bool isOwner;
        uint funds;
        address adrs;                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                               
    }
    
    // transfers history struct
    struct TransfersHist{
        uint id;
        address payer;
        address recipient;
        uint amount;
        uint n_approvals;       // number of approvals out of Napproval=2 (defined at the top)
        bool already_approved;  // if the transaction already went through this will be true
    }
    
    TransfersHist[] transfers_hist;  // transfers_hist is a vector which elements are structs
    
    // approvals mapping, approvals[id]["address"] = 0|1
    mapping (address => mapping (uint => uint)) approvals;
    
    // contract creator
    address contract_creator;
    
    modifier only_contract_creator{
        require(msg.sender == contract_creator);
        _;
    }
    // n_approvals needed
    constructor() {
        contract_creator = msg.sender;
        Client[contract_creator] = Cli(true, Client[contract_creator].funds, contract_creator);  // contract_creator is by default a validator
        Validators.push(contract_creator);                                                       // contract creator is by default validator
    }
    
    
    // create logs
    event add_funds_log(address deposit_address, uint amount_added, uint current_balance);
    event client_added_log(address client_address, bool is_client_validator);
    event succesfull_transfer_log(uint transaction_id, address sender_address, address recipient_address, uint amount_transfered);
    
    
    // add funds function, anyone can add funds
    function addFunds() public payable returns(uint){
        Client[msg.sender].funds += msg.value;
        emit add_funds_log(msg.sender, msg.value, Client[msg.sender].funds);
        return Client[msg.sender].funds;
    }
    
    // Add clients and their validator status
    function addClient(bool _isValidator, address add_address) public {
        Cli memory new_client = Cli(_isValidator, Client[add_address].funds, add_address);
        Client[add_address] = new_client;
        emit client_added_log(add_address, _isValidator);
        
        if (Client[add_address].isOwner == true){  // if client is set as validartor, they are appended to the validators list
            Validators.push(add_address);
        }
    }
    
    
    // get client 
    function getClient() public view returns(Cli memory){
        return Client[msg.sender];
    }
    
    // get Validators
    function getValidators() public view returns(address[] memory){
        return (Validators);
    }
    
    
    // transact function moves money from one account to another
    function transact_proceed(address _from, address _to, uint _amount) private {
        Client[_from].funds -= _amount;
        Client[_to].funds += _amount;
    }
    
    // Aprove transaction function
    function Aprove(uint _transaction_ID) public {
        require(Client[msg.sender].isOwner == true);
        TransfersHist storage current_transaction = transfers_hist[_transaction_ID]; // refer to the element of the array, each element is a struct
        require(current_transaction.already_approved == false, "The transaction already went through");
        
        // check that addresses have not approved a transacition
        require(approvals[msg.sender][_transaction_ID] == 0, "This address has already approved the transaction");
        
        current_transaction.n_approvals += 1;         // add +1 to the count of number of approvals
        approvals[msg.sender][_transaction_ID] = 1;   // change the state of the approvals mapping
        
        // check if the number of approvals meet Napp=2, if so proceed with the transaction
        if (current_transaction.n_approvals >= Napproval){
            transact_proceed(current_transaction.payer, current_transaction.recipient, current_transaction.amount);  // call fucntion transact
            current_transaction.already_approved = true;                                                             // change transaction status
            emit succesfull_transfer_log(_transaction_ID, current_transaction.payer, current_transaction.recipient, current_transaction.amount);
        }
    }
    
    // post transaction into transfers_hist vector, this then is proceeded once the number of approvals is >= 2
    function transact(address _from, address _to, uint _amount) public {
       require(Client[_from].funds >= _amount, "Not enough balance in account");
       require(_from != _to, "Can't auto-deposit");
       
       uint n0 = 0;
       transfers_hist.push(TransfersHist(transfers_hist.length, _from, _to, _amount, n0, false));  // add an element to transaction history array
       
    }
    
    
       
    function getTranscactionHist() public view returns(TransfersHist[] memory){
        return transfers_hist;
    }
}

voljumet · February 16, 2021, 10:03am

this is my way to sove it:
Ownable.sol :

pragma solidity 0.7.5;

contract Ownable {
    
    address[] public owners;
    uint numberOfSignatures;
    bool owner = false;

    modifier onlyOwner { 
        for (uint8 i = 0; i < owners.length; i++){
                if (owners[i] == msg.sender){
                    owner = true;
                }
            }
        require(owner, "You are not allowed to interact with the contract!"); 
        owner = false;
        _; // run the function
    }
}

Wallet.sol :

pragma solidity 0.7.5;
pragma abicoder v2;

import "./Ownable.sol";

contract Wallet is Ownable {
    
    bool checker = false;
    struct Transfer {
        address ownerAddress;
        uint value;
    }
    
    struct Approval {
        address signing;
        address needsSignature;
    }
    
    Transfer[] transferRequests;
    Approval[] approval;
    
    constructor(address addr1, address addr2, uint _numberOfSignatures){
        adder(msg.sender);
        adder(addr1);
        adder(addr2);
        numberOfSignatures = _numberOfSignatures;
    }
    
    //support function to constructor
    function adder(address addrIn) internal{
        owners.push(addrIn);
    }
    
    function deposit() public payable returns (uint){
        return address(this).balance;
    }
    
    function getBalance() public view returns (uint){
        return address(this).balance;
    }
    
    function requestTransfer(uint requestSum) public onlyOwner returns (string memory){
        if(address(this).balance >= requestSum){
            transferRequests.push(Transfer(msg.sender,requestSum));
            return "Transfer request sent!";
        } else {
            return "Not enough tokens!";
        }
    }
    
    function getResquests() public view returns(Transfer[] memory){
        return transferRequests;
    }
    
    function approve(address toAddr) public onlyOwner {
        approval.push(Approval(msg.sender, toAddr));
    }
    
    function checkApprovals(address toAddr) public returns(address[] memory){
        address[] memory temp = new address[](approval.length);
        for(uint i = 0;i < approval.length ;i++){
            if(toAddr == approval[i].needsSignature){
                temp[i] = approval[i].signing;
                if(checker){
                    delete approval;
                }
            }
        }
        checker = false;
        return temp;
    }
    
    function withdraw(uint amount) public onlyOwner returns (string memory, uint) {
        uint sum = address(this).balance;
        require(sum >= amount, "Insufficient funds");
        string memory message;
        if(checkApprovals(msg.sender).length >= numberOfSignatures){
            checker = true;
            sum -= amount;
            msg.sender.transfer(amount);
            checkApprovals(msg.sender);
            message = "Transfer complete";
        } else {
            message = "Transfer not complete";
        }
        return (message,address(this).balance);
    }
}

Tomasz · February 18, 2021, 11:23am

pragma solidity 0.7.5;

contract MultisigWallet {
    mapping( address => mapping (address => uint) ) confirmation;
    mapping ( address => bool ) owners;
    address[] aowners;
    uint min_approvals;
    
    constructor(address[] memory _owners, uint _min_approvals){
        require(_min_approvals <= _owners.length, "Not enougth owners");
        min_approvals = _min_approvals;
        aowners =_owners;
        for(uint i=0; i < _owners.length; i++){
            owners[_owners[i]] = true;
        }
    }

    event balanceAdded(uint amount, address depositedFrom);
    
    function deposit() public payable returns (uint) {
        emit balanceAdded(msg.value, msg.sender);
        return address(this).balance;
    }
    
    function transfer(uint _amount, address payable _to) public  returns(uint){
        require(owners[msg.sender], "Not an owner");
        require(address(this).balance >= _amount,"Insufficient funds");
        
        confirmation[msg.sender][_to] = _amount;
        
        uint count = 0;
        for(uint i = 0; i < aowners.length; i++){
            if(confirmation[ aowners[i] ][_to] == _amount)
                count++;
        }
        
        if(count >= min_approvals){
            _to.transfer(_amount);
            for(uint i = 0; i < aowners.length; i++){
                if(confirmation[ aowners[i] ][_to] == _amount)
                    delete confirmation[ aowners[i] ][_to];
            }
        }
        return _amount;
    }
    
    function getBalance() public view returns (uint){
        return address(this).balance;
    }
}

seblife · February 17, 2021, 11:17pm

This is my minimalistic solution prior to watching any of the helper videos. Now that I have watched the videos, I would have probably changed what I did, but I’ll just leave it as it is.

pragma solidity 0.8.1;

/* 
    For a transfer to go through 2 owners out of 3 must make subsequents  
    calls to the transfer() function with the same address and amount and 
    the transfer will be completed.
*/

contract MultisigWallet {
    
    address[3] owners;
    
    /* 
        This variable equals -1, if no one as requested a transfer
        otherwise it stores the index of the one who requeted the transfer 
        as well as the toAddress and the amount waiting for a second owner 
        to request the same thing.
    */
    int prior_requester_index = -1;
    uint prior_amount;
    address payable prior_toAddress;
    
    constructor(address _owner1, address _owner2, address _owner3) {
        owners[0] = _owner1;
        owners[1] = _owner2;
        owners[2] = _owner3;
    }
    
    modifier WalletOwner {
        require(msg.sender == owners[0] || msg.sender == owners[1] || msg.sender == owners[2], 
        "Not an owner of this wallet.");
        _;
    }
    
    function transfer(address payable _toAddress, uint _amount) external WalletOwner {
        
        int sender_index = getSenderIndex();
        
        // sender is the 1st one to request the transfer
        if(prior_requester_index == -1) {
            require(_amount <= address(this).balance, "Insufficient funds.");
            prior_requester_index = getSenderIndex();
            prior_amount = _amount;
            prior_toAddress = _toAddress;
        }
        // An other owner has already requested a the transfer.
        // 2nd owner must send exact same amount to the same address to proceed
        else {
            require(prior_requester_index != sender_index, "2 owners must be different for transfer to occur.");
            require(prior_toAddress == _toAddress && prior_amount == _amount, "Address/Amount differ from prior owner's request.");
            
            // _amount >= balance already validated
            _toAddress.transfer(_amount);
            clearState();
        }
    }
    
    function getSenderIndex() private WalletOwner view returns (int) {
        if(msg.sender == owners[0]) {
            return 0;
        }
        else if(msg.sender == owners[1]) {
            return 1;
        }
        else
            return 2;
    }
    
    // Anyone can deposit
    function deposit() external payable { }
    
    function getBalance() external view WalletOwner returns (uint) {
        return address(this).balance;
    }
    
    // Cancel prior request made by other owner
    function clearState() public WalletOwner {
        prior_requester_index = -1;
        prior_amount = 0;
        prior_toAddress = payable(address(0));
    }
}

thecil · February 18, 2021, 5:16pm

Hey @seblife, hope you are great.

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

Steps I made:

deposit() (from 3rd owner) 11 ethers, getBalance.
transfer() 10 ethers (from 2nd owner) to 1st owner.
click same trasnfer() but with 3rd owner, transaction approve and send.
check balance of 1st owner.

It is confusing because I have to click 2 times the same transfer() function with 2 different accoutns, which in the eyes of the user can be seen has sending 2 transactions, not signing the same.

Still is a good solution, congratulations!

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

Carlos Z.

Bhujanga · February 18, 2021, 6:31pm

Wow, what a journey! In the end, I needed Filip’s help to process everything in the right way and get the syntax right, but I am happy that I have been on the right track with many things and the basic understanding of what kind of information is needed to make a MultiSigWallet possible.

I still have a long way to go though when it comes to mastering Solidity and the way to think here. Thank you @Filip for the awesome course! Can’t wait to revisit everything and continue learning.

Bhujanga · February 18, 2021, 10:21pm

Hello fellows,

I am trying to improve the MultiSigWallet and want to implement a check against duplicates in the owner address array. Somehow I am kinda stuck. What is wrong with this piece of code?

 constructor(address[] memory _owners, uint _limit){
      bool isDuplicate = false;
       
        owners = _owners;
        limit = _limit;
       for(uint i = 0;i<_owners.length;i++){
           for(uint j = i+1;j<=_owners.length;j++){
                if(_owners[i] == _owners[j])
            {
                isDuplicate = true;
            }
            else{
                isDuplicate = false;
            }
           }
            
        }
        assert(isDuplicate == false);
        
    }

thecil · February 19, 2021, 2:36am

Hey @Bhujanga, hope you are great.

The problem from my perspective in your code is that you are running a complex (in terms of computing resoucers) a double loop (used commonly to iterate double arrays), also the constructor should be used to initialize functions, not to have a complete logic on it, you can try to resume that code into a function that your constructor can call later.

Maybe a require could do the trick…

Carlos Z

bbss · February 20, 2021, 1:51pm

I’m attaching my code:

pragma solidity 0.7.5;

contract MultiSignWallet {
    uint transferId = 0;
    
    address[] ownwers;
    uint nrApprovals;
    address payable walletFunds;
    uint fundsAvailable;
    
    struct TransferRequest {
        uint ammount;
        address payable sendAddress;
        bool paid;
    }
    
    mapping(uint => TransferRequest) transferRequests;
    mapping(uint => address[] signatures) signaturesPerTransferRequest;
    
    constructor(address[] _owners, uint _nrApprovals, address payable _walletFunds) {
        ownwers = _owners;
        nrApprovals = _nrApprovals;
        walletFunds = _walletFunds;
    }
    
    // returns transfer id
    function createTransferRequest(uint _ammount, address payable _toAddress) public payable returns (uint) {
        require(addressIsOwner(msg.sendAddress), "A transfer request can only be performed by an owner");
        
        TransferRequest transfer = TransferRequest(_ammount, _toAddress, false);
        
        address[] signatures;
        signatures.push(msg.sendAddress);
        
        uint currentTransferId = transferId;
        transferRequests[currentTransferId] = transfer;
        signaturesPerTransferRequest[currentTransferId] = signatures;
        if (signaturesPerTransferRequest[currentTransferId].length >= nrApprovals) {
            payTransferRequest(currentTransferId);
        }
        
        transferId++;
        
        return currentTransferId;
    }
    
    // returns true if the necessy number of sgnatures has been reached
    function signTransferRequest(uint _transferId) public payable returns(bool) {
        require(transferRequests[_transferId], "Cound not find a transfer with this id.");
        require(!transferRequests[_transferId].paid, "This transfer equest is already paid.");
        require(addressIsOwner(msg.sendAddress), "Only an owner can sign the transfer request");
        
        bool callerAlreadySigned = false;
        for(uint i = 0; i < signaturesPerTransferRequest[_transferId]; i++) {
            if (msg.sendAddress == signaturesPerTransferRequest[_transferId][i]) {
                callerAlreadySigned = true;
                break;
            }
        }
        
        require(!callerAlreadySigned, "A owner cannot sign a transfer request multiple times.");
        
        signaturesPerTransferRequest[_transferId].push(msg.sendAddress);
        if (signaturesPerTransferRequest[_transferId] >= nrApprovals) {
            payTransferRequest(_transferId);
            return true;
        }
        
        return false;
    }
    
    function addFunds(uint _amount) public payable {
        require(addressIsOwner(msg.sendAddress), "Only an owner can add funds.");
        
        fundsAvailable += _amount;
        walletFunds.transfer(_amount);
    }
    
    function addressIsOwner(address _caller) private returns(bool) {
        for(uint i = 0; i < ownwers.length; i++) {
            if (msg.sendAddress == ownwers[i]) {
                return true;
            }
        }
        
        return false;
    }
    
    function payTransferRequest(uint _id) private payable {
        transferRequests[_id].sendAddress.transfer(transferRequests[_id].ammount);
        walletFunds -= transferRequests[_id].ammount;
        transferRequests[_id].paid = true;
    }
}

dan.mac · February 20, 2021, 7:27pm

ownable

pragma solidity 0.7.5;
pragma abicoder v2;
contract Ownable {
    address internal owner;
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _; //run the function
    }
    constructor(){
        owner = msg.sender;
    }    
}

Multisig

pragma solidity 0.7.5;
pragma abicoder v2;
import "./Ownable.sol";



contract Multisig is Ownable {

    address[] owners;
    uint limit;

    mapping(address => uint) users_balance;
    
    struct Transfers{
        uint id;
        address sender;
        address receiver;
        uint amount;
    }
    
    Transfers[] transfer_book;
    
    mapping (address => mapping(uint => bool)) Requests;


    function ChangeLimit(uint _limit) public onlyOwner{   //Change the number of signatures needed
      
        
        limit = _limit;
    }
    
    function ChangeOwners(address[] memory _owners) public onlyOwner{  //Change the owners' addresses
  
        address[] storage o = owners;
        uint l = _owners.length - 1 ;
        
        for (uint i= 0 ; i<=l ; i++){
            
            o.push(_owners[i]);
        }
    }
     
    function CheckOwners() public view returns(address[] memory) { //Check Owners 
        
        return owners;
    }
     
    function deposit() public payable returns (uint)  {
        users_balance[msg.sender] += msg.value;
    
        return users_balance[msg.sender];
    }
    
    function CheckBalance() public view returns (uint) {
        
        return users_balance[msg.sender];
    }
     
    function CreateTransferRequest(address _receiver, uint _amount) public {
        require(_amount <= users_balance[msg.sender], "Your account doesn't have the funds needed for this transfer.");
        transfer_book.push(Transfers(transfer_book.length,msg.sender, _receiver, _amount));
        users_balance[msg.sender] -= _amount;
    }
    
    function approveTransfer(uint _transferId) public returns (string memory){
        
        uint size = owners.length-1;
        bool _isOwner;
        //address[] memory o = owners; 
        for (uint i = 0; i <= size; i++){
            if (owners[i] == msg.sender) {
                _isOwner = true;
                //break;
            }
        }
        if (_isOwner){
            Requests[msg.sender][_transferId] = true;
        
            if (checkTransferState(_transferId)){
                Sendfunds(_transferId);
                
                return "The transfer request was approved by you and the transfer was performed";
            }
            else{
                return "You have approved this transfer request";
            }
        }
        else {
            return "You must be an owner to perform this action";
        }
    }
    
    function Sendfunds(uint _transferId) internal {
        address payable _receiver = payable(transfer_book[_transferId].receiver);
        _receiver.transfer(transfer_book[_transferId].amount);
    }
    
    function checkTransferState(uint _transferId) view public returns (bool){
        
        
       
        uint approvals = 0;
        
        for (uint i = 0 ; i <= owners.length -1 ; i++) {
            
            if (Requests[owners[i]][_transferId]){
                
                approvals += 1;
            }
        }
        
        if (approvals >= limit){
            
            return true;
        }
        
        else{
            
            return false;
        }
        
    }
    
    function checkTransferInfo (uint t_id) public view returns (Transfers memory){
        
        return transfer_book[t_id];
    }
    
    function checkOwnAcceptedTransf(uint _transferId) public view returns(bool){
        return Requests[msg.sender][_transferId]; 
    }
    
    
    function CheckIfIamOwner() public view returns(bool){
        uint size = owners.length-1;
        bool _isOwner;
        for (uint i = 0; i <= size; i++){
            if (owners[i] == msg.sender) {
                _isOwner = true;
                
            }
        }
        return _isOwner;
        
    }
}

Jason_Mack · February 21, 2021, 7:46am

pragma solidity 0.7.5;
pragma abicoder v2;

contract Wallet {
address[] public owners;
uint limit;

struct Transfer{
    uint amount;
    address payable receiver;
    uint approvals;
    bool hasBeenSent;
    uint id;
}

event TransferRequestCreated(uint _id, uint _amount, address _initiator, address _receiver);
event ApprovalReceived(uint _id, uint _approvals, address _approver);
event TransferApproved(uint _id);

Transfer[] transferRequests;

mapping(address => mapping(uint => bool)) approvals;

//Should only allow people in the owners list to continue the execution.
modifier onlyOwners(){
    bool owner = false;
    for(uint i=0; i<owners.length;i++){
        if(owners[i] == msg.sender){
            owner = true;
        }
    }
    require(owner == true);
    _;
}
//Should initialize the owners list and the limit 
constructor(address[] memory _owners, uint _limit) {
    owners = _owners;
    limit = _limit;
}

//Empty function
function deposit() public payable {}

//Create an instance of the Transfer struct and add it to the transferRequests array
function createTransfer(uint _amount, address payable _receiver) public onlyOwners {
    emit TransferRequestCreated(transferRequests.length, _amount, msg.sender, _receiver);
    transferRequests.push(
        Transfer(_amount, _receiver, 0, false, transferRequests.length)
    );

}

//Set your approval for one of the transfer requests.
//Need to update the Transfer object.
//Need to update the mapping to record the approval for the msg.sender.
//When the amount of approvals for a transfer has reached the limit, this function should send the transfer to the recipient.
//An owner should not be able to vote twice.
//An owner should not be able to vote on a tranfer request that has already been sent.
function approve(uint _id) public onlyOwners {
    require(approvals[msg.sender][_id] == false);
    require(transferRequests[_id].hasBeenSent == false);

    approvals[msg.sender][_id] = true;
    transferRequests[_id].approvals++;

    emit ApprovalReceived(_id, transferRequests[_id].approvals, msg.sender);

    if(transferRequests[_id].approvals >= limit){
        transferRequests[_id].hasBeenSent = true;
        transferRequests[_id].receiver.transfer(transferRequests[_id].amount);
        emit TransferApproved(_id);
    }
}

//Should return all transfer requests
function getTransferRequests() public view returns (Transfer[] memory){
    return transferRequests;
}

}