Project - Multisig Wallet

[thecil]

Hey @theurer, hope you are great.

I’ve tested your contract, it does looks quite amazing to be honest, you have made a very nice job man! congrats

• After deployment, setOwners to set the 2 extra owners.
• Deposit 3 eth and check balance on getContractInfo and getUserBalance, all good.
• requestTransfer with 1st owner to transfer 3 eth to an external address (non-owner), check with getPendingTransfer
• approveTransfer with 2nd and 3rd owner, check getPendingTransfer again, it have 3 approvalCounter
• Now for completeTransfer i tried it with the external address that should get the funds, so it triggers properly and then withdraw with the same account, I got my 3 eth perfectly in its wallet .

Now here are some improvements (cuz there is always room for improvements )

Instead of returning an string message when a transaction is completed properly, you could use an Event, that way you could achieve the same goal but scalabe for further develops (events are the way that you track your conctract activity for the dapp, if you want to show a success message in your dapp after the transaction is completed, you have to track the complete event instead the returned value of the function).

    function completeTransfer() public returns(string memory){
        require(approvalCounter == 2 || approvalCounter == 3);
        for(uint i = 0; i <= 2; i++){
            ownersList[i].hasApprovedTransfer = false;
        }
        
        uint previousSenderBalance = balance[transferRequester];
        
        _transfer(transferRequester, transctionLog[0].recipient, transctionLog[0].amount);
        
        pendingTransfer = false;
        approvalCounter = 0;
        
        assert(balance[transferRequester] == previousSenderBalance - transctionLog[0].amount);
        
        return ("Transfer was successful!");
    }

This 2 functions shares almost the same require conditions, might be better to create a modifier for them.

    function requestTransfer(uint _amount, address _recipient)public {
        require(balance[msg.sender] >= _amount, "Balance not sufficient.");
        require(msg.sender == ownersList[0].owner || msg.sender == ownersList[1].owner || msg.sender == ownersList[2].owner, "You don't have access to this contract!");
        transctionLog.push(Transactions(_recipient, _amount));
        pendingTransfer = true;
        transferRequester = msg.sender;
        approvalCounter = 1;
    }
    
    function approveTransfer() public {
        require((msg.sender == ownersList[0].owner || msg.sender == ownersList[1].owner || msg.sender == ownersList[2].owner) && msg.sender != transferRequester, "You are not allowed to approve this transfer.");
        for(uint i = 0; i <= 2; i++){
            if(ownersList[i].owner == msg.sender && ownersList[i].hasApprovedTransfer == false){
                approvalCounter++;
                ownersList[i].hasApprovedTransfer = true;
            }
        }
    }

Should be something like this

    modifier onlySetOwners(){
        require(msg.sender == ownersList[0].owner || msg.sender == ownersList[1].owner || msg.sender == ownersList[2].owner, "You don't have access to this contract!");
        _;
    }

Then your functions will looks like this:

    function requestTransfer(uint _amount, address _recipient)public onlySetOwners{
        require(balance[msg.sender] >= _amount, "Balance not sufficient.");
        
        transctionLog.push(Transactions(_recipient, _amount));
        pendingTransfer = true;
        transferRequester = msg.sender;
        approvalCounter = 1;
    }
    
    function approveTransfer() public onlySetOwners{
        require(msg.sender != transferRequester, "You are not allowed to approve this transfer.");
        
        for(uint i = 0; i <= 2; i++){
            if(ownersList[i].owner == msg.sender && ownersList[i].hasApprovedTransfer == false){
                approvalCounter++;
                ownersList[i].hasApprovedTransfer = true;
            }
        }
    }

Overall you have made a great work if you did not peak into the solutions
Amazing work man, congrats!

Carlos Z

[Maia]

Blockchain is a very new subject to me and I have no choice other than research on it and try to understand how to proceed with the project. I’m having a hard time to find material on the subject. However, I came across a material from 2017, which I could understand more clearly. But still I have many doubts. And even with the material, could not deploy the contract without error. Anyways, here is what I have by parts and will also post the entire coding. I went already on mapping explanation video as well:

Anyone should be able to deposit ether into the smart contract

- The contract creator should be able to input (1): the addresses of the owners and (2): the numbers of approvals required for a transfer, in the constructor. For example, input 3 addresses and set the approval limit to 2.

pragma solidity 0.7.5;

contract multiSigWallet {
    mapping (address => uint8) private _owners;
    
    uint constant MIN_SIGNATURES = 2;
    uint private _TransactionsIdx;
    
    struct transaction {
    address from;
    address to;
    uint amount;
    uint8 signatureCount;
    //bellow it guarantees there are more owners addresses than required signatures
    mapping (address => uint8) signatures;

• Anyone of the owners should be able to create a transfer request. The creator of the transfer request will specify what amount and to what address the transfer will be made.

 function transferTo(address to, uint amount)
    validOwner
    public {
        require (address(this).balance >= amount);
        uint transactionId = _transactionIdx++;
        
        transaction memory transaction;
        transaction.from = msg.sender;
        transaction.to = to;
        transaction.amount = amount;
        transaction.signatureCount = 0;
        
        _transaction[transactionID] = transaction;
        _pendingTransactions.push(transactionId);
        
        transactionCreated(msg.sender, to, amount, transactionId);

• Owners should be able to approve transfer requests.

- When a transfer request has the required approvals, the transfer should be sent.

 function signTransaction (uint transactionId);
        validOwner;
        public {
            transaction storage transaction = _transactions[transactionId];
            
            //transaction must exist
           require(0x0 != transaction.from);
           //cannot sign a transaction more than once 
           require(transaction.signatures[msg.sender]) = 1;
           transaction.signatureCount++;
           
           transactionsSigned(msg.sender, transactionId);
           if (transaction.signatureCount >= MIN _SIGNATURES) {
               require (address(this).balance >= amount);
               transaction.to.transfer(amount);
               transactionCompleted(transaction.from, transaction.to, transaction.amount, transactionId);
               deleteTransaction(transactionId);           }
            }
       }

[Maia]

Now here is the entire contract which still cannot be deployed, there is an error. Anyways, I need to have access to more material on this subject and go back and forth continuously until I get it.

pragma solidity 0.7.5;

contract multiSigWallet {
    mapping (address => uint8) private _owners;
    
    uint constant MIN_SIGNATURES = 2;
    uint private _TransactionsIdx;
    
    struct transaction {
    address from;
    address to;
    uint amount;
    uint8 signatureCount;
    //bellow it guarantees there are more owners addresses than required signatures
    mapping (address => uint8) signatures;
        
    }
    
    mapping (uint => Transaction) private _transactions;
    uint [] private _pendingTransactions;
    
    
    
    modifier isOwner() {
        require (msg.sender == _owner);
        _;
        
    }
    
    modifier validOwner() {
        require(msg.sender == _owner || _owners[msg.sender] ==1);
        _;
    }
    
    event DepositFunds(address from, uint amout);
    event TransactionCreated (address from, address to, uint amount, uint transactionId);
    event TransactionCompleted(address from, address to, uint amount, uint transactionId);
    event TransactionSigned(address by, uint transactionId);
    
    function multiSigWallet( )
    public {
        _owner = msg.sender;
    }
    
    function addOwner (address newOwner)
    isOwner
    public {
        _owners [newOwner] = 1;
        
    }
    
    function removeOwner(address existingOwner)
    isOwner
    public {
        _owners[existingOwner] = 0;
    }
    
    function deposit()
    validOwner
        public
        payable {
        DepositFunds(msg.sender, msg.value);
        }
    
    function withdraw (uint amount);
        validOwner
        public {
        require(address(this). >= amount);
        msg.sender.transfer(amount);
        withdrawFunds(msg.sender, amount);
        
        }
        
    function transferTo(address to, uint amount)
    validOwner
    public {
        require (address(this).balance >= amount);
        uint transactionId = _transactionIdx++;
        
        transaction memory transaction;
        transaction.from = msg.sender;
        transaction.to = to;
        transaction.amount = amount;
        transaction.signatureCount = 0;
        
        _transaction[transactionID] = transaction;
        _pendingTransactions.push(transactionId);
        
        transactionCreated(msg.sender, to, amount, transactionId);
        
        function getPendingTransactions()
        validOwner;
        public
        returns(uint[]) {
            return _pendingTransactions;
            
        }
        
        function signTransaction (uint transactionId);
        validOwner;
        public {
            transaction storage transaction = _transactions[transactionId];
            
           require(0x0 != transaction.from);
           require(transaction.signatures[msg.sender]) = 1;
           transaction.signatureCount++;
           
           transactionsSigned(msg.sender, transactionId);
           if (transaction.signatureCount >= MIN _SIGNATURES) {
               require (address(this).balance >= amount);
               transaction.to.transfer(amount);
               transactionCompleted(transaction.from, transaction.to, transaction.amount, transactionId);
               deleteTransaction(transactionId);           }
            }
       }
           
           
           functiom deleteTransaction(uint, transactionId);
           validOwner
           public {
           uint8 replace = 0;
           for(uint i = 0; i <_pendingtransactions.lenght; i++) {
               if(1 == replace) {
                   _pendingtransactions[i-1] = _pendingTransactions[i];
                 } else if (transactionId) ==  _pendingTransactions[i] {
                     replace - 1;
                     
                 }
           }
           delete _pendingtransactions[_pendingtransactions.lenght - 1];
           _pendingtransactions.lenght -- ;
           delete _transaction[transactionId];
          }
               
          function walletBalance()
          constant
          public
          returns(uint) {
              return
          }
               
            
            
        }

[theurer]

Wow, thank you so much @thecil for taking your time and giving me such a great feedback ^^ like I mentioned before I didn’t want to look at the hints or solution at all because I think it’s the best way to learn it like that because you are required to solve problems by yourself and not depend on the help of others that much. I’m actually quite surprised my code worked out that well!

Now to the part where I can improve.

• I will definitely use events from now on, they make much more sense than just returning basic string.

• I will also try to avoid repeating code like the required statements and put them in a modifier

Again thank you for the feedback, helps me a lot ^^

Till

[Jazmin]

Hi,

Here is my code:

//“SPDX-License-Identifier: UNLICENSED”
pragma solidity 0.7.5;
pragma abicoder v2;

//import "./MSOwnable.sol";

contract multiSig{
    
    address[] owners; //array of owners addresses
    uint requiredSigs; //number of signatures required
    bool isOwner; //boolean to determine if the sender is an owner

    modifier onlyOwners{
     // uint i=0; //set counter to 0
      //uint j=owners.length; //number of owners
      isOwner = false; //assume sender is not the owner

      // loop through the owners, if it matches the sender then set isOwner to true
      for(uint i=0; i < owners.length; i++){
           if (owners[i] == msg.sender){
              isOwner = true;
           }
      }
      require(isOwner == true, "You are not an owner");
      _;  // proceed
    }
    
    
    constructor (address[] memory _owners, uint _requiredSigs) {
        require(_requiredSigs > 0, "Required signatures must be greater than 0");
        require(_owners.length >= _requiredSigs, "There must be more owners than required signatures");
        owners = _owners; // the address that deploys the contract i.e. the first sender
        requiredSigs = _requiredSigs; //set the number of signatures required to send a tx
    }
        
    struct Transfer {
        uint id; //id of the transfer
        address payable recipient; //address to send to
        uint amount; //amount
        uint signatures; //number of signatures already signed
        bool sent; //track whether the tx has been sent
     }

     Transfer[] transfers;  //array of transfers

    //double mapping Address => (transferID:true/false) keep track of which owners have signed
    mapping (address => mapping(uint => bool)) signatures;

    event depositDone(uint amount, address indexed depositedFrom); //deposit event
    event signed(uint transferID, address signer);  //signed event
    event transferred(uint transferID, address signer, uint amount);  //sent event

    //deposits into SC
    function deposit() public payable returns (uint){
        emit depositDone(msg.value, msg.sender);      //log event - who sent and how much
        return address(this).balance;
    }

    function getBalance() public view returns(uint){
        return address(this).balance;         // return the balance of this address
    }

    //create a transaction
    function createTransfer(uint _amount,address payable _recipient) public onlyOwners{
        require(address(this)!= _recipient, "Cannot send to yourself");

        uint TxID = transfers.length; //set transactionID to start at 0 (transfers.length will be one more than the current number of txs)

        transfers.push(
            Transfer(TxID,_recipient,_amount,0,false) // add transfer to the array, set signatures to 0, set sent status to false
            );
    }

    //sign a tx
    function sign(uint _TransferID) public onlyOwners returns(uint){

        require(signatures[msg.sender][_TransferID] == false, "You have already approved");
        require(transfers[_TransferID].sent == false, "This transfer has already been sent!");

        signatures[msg.sender][_TransferID] = true; //update signatures to track that this owner has signed

        transfers[_TransferID].signatures += 1; //update transfer to update the number of signatures
        emit signed(_TransferID,msg.sender);

        //if signatures >= limit, send the transfer
        //}
        //}
        return transfers[_TransferID].signatures; //return number of signatures
    }

    //send a tx
    function send(uint _TransferID) public onlyOwners returns (uint){
        require(address(this).balance >= transfers[_TransferID].amount, "Balance not sufficient");
        require(transfers[_TransferID].signatures >= requiredSigs, "Not enough signatures");
        require(transfers[_TransferID].sent == false, "This transaction has already been sent");

        transfers[_TransferID].sent = true; //set status as sent
        transfers[_TransferID].recipient.transfer(transfers[_TransferID].amount); //transfer function - send funds

        emit transferred(_TransferID,transfers[_TransferID].recipient,transfers[_TransferID].amount);

        return address(this).balance;
    }
}

I tried to use a separate contract for the Ownable section but I was getting this error:

not sure what I’m doing wrong there, when I put everything in the same contract it works…

Feedback is welcome!

Thanks,
Jazmin

[juanmcba]

pragma solidity 0.7.5;
pragma abicoder v2;

contract MultiSig {
    
    address[] owners;
    uint limit;
    
    struct Transfer{
        uint amount;
        address payable recipient;
        uint approvalNumber;
        bool hasBeenSent;
        uint id;
    }
    
    event transferRequestCreated(uint _id, uint _amount, address _initiator, address _receiver);
    event approvalReceived(uint _id, uint _approvalNumber, address _approver);
    event transferApproved(uint _id);
    
    Transfer[] transferRequests;
    
    mapping (address => mapping(uint => bool)) approvals;
    
    modifier onlyOwners(){
        bool owner = false;
        for(uint i=0; i<owners.length; i++){
            if(owners[i] == msg.sender){
                owner = true;
            }
        }
        require(owner == true);
        _;
    }
    
    constructor(address[] memory _owners, uint _limit){
        owners = _owners;
        limit = _limit;
    }

    
    function deposit() public payable {
    }
    
 
    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));
    }
    
    function approve(uint _id) public onlyOwners{
        
        require(approvals[msg.sender][_id] == false);
        require(transferRequests[_id].hasBeenSent == false);
        
        approvals[msg.sender][_id] = true;
        transferRequests[_id].approvalNumber++;
        
        emit approvalReceived(_id, transferRequests[_id].approvalNumber, msg.sender);
        
        if(transferRequests[_id].approvalNumber >= limit){
            transferRequests[_id].hasBeenSent = true;
            transferRequests[_id].recipient.transfer(transferRequests[_id].amount);
            emit transferApproved(_id);
        }
    }
    
    function getTransferRequests() public view returns(Transfer[] memory){
        return transferRequests;
    }
}

[vili]

Hello, please see below my multisig Wallet. Every time you deploy the contract you have have to enter the owner’s wallet. Please let me know whether the whole idea of the wallet sound good.

pragma solidity 0.7.5;
pragma abicoder v2;

contract MultisigWallet {
    
    
  mapping(address => uint) balance;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    struct Owner {
        address from;
        bool signed;
    }
    
   Owner[] public owners;

    function deposit () public payable returns (uint) {
        balance[msg.sender] += msg.value;
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }
    
    function addOwner(address from) public {
        owners.push(Owner(msg.sender, false));
    }
    
    function signedCompleted (uint _index) public {
        Owner storage owner = owners[_index];
        owner.signed = !owner.signed;
    }
    
     function getOwner (uint _index) public returns (address, bool){
        return (owners[_index].from, owners[_index].signed);
    }
    
     function transfer(address recipient, uint amount) public {
         uint ownersApproved = 0;
        for (uint i=0; i<owners.length; i++){
            if (owners[i].signed == true){
                ownersApproved += ownersApproved;
            }
        }
        require(ownersApproved >= 2, "Not all the owners approved the trnasaction"); 
       require(balance[msg.sender] >= amount, "Balance not sufficient");
        require(msg.sender != recipient, "Don't transfer money to yourself");
        
        uint previousSenderBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == previousSenderBalance - amount);
    }
    
    function _transfer(address from, address to, uint amount) private {
        balance[from] -= amount;
        balance[to] += amount;
    }
}

[thecil]

That is just a warning message that is shown on contracts that does not have a constructor, you can avoid it since its only a suggestion. Apparently the constructor is in your MSOwnable contract, and not in the multiSig, thats why it suggest to mark it as abstract.

Carlos Z

[yestome]

Below is my code for the multi-sig wallet. I haven’t looked at the solution yet as I prefer to keep trying but need some hints for execution of the transfer once signatory-limit is reached (remix flags an error I didn’t know how to resolve; my code is based on this source: https://solidity-by-example.org/sending-ether/).
I was also curious to get your comments on other errors / imperfections my code might have and more specifically an answer to the doubt (once more) regarding execution of the transfer, namely:

• why the idea of executing the transfer from within the approve function instead of moving the transfer execution into a separate function / is there any risk involved in tying these 2 functionalities into a single function? I understand that by keeping them together the transaction automatically executes once limit is reached (instead of requiring an authorised user to manually trigger the transfer), so that might be an argument in favour but still have doubts as to the safety of this bundling strategy.
  1 more question: doesn the deposit function need a function body or is the header sufficient for accepting incoming transfers?
  1 more comment: I just realised that my code is pending to add EVENTS which I’ll do once I understand how executing the transfer works.

pragma solidity 0.8.4;
pragma abicoder v2;

contract MultisigWallet {

address[] public owners; // authorised owners
uint limit; // min number of signatories

struct Transfer {
    address payable to;
    uint amount;
    uint totalSignatures; // changed name to avoid confusion as "approvals" is also used for a mapping
    bool transferComplete; // "hasBeenSent" is somewhat unclear as it could relate to a trx-request or a transfer
    uint id;
}

Transfer[] public transferRequests;

// mapping [address][transferID] => true/false
// example: mapping[msg.sender][5] = true
mapping(address => mapping(uint => bool)) approvals;
// mapping for checking multiwallet owners
mapping(address => bool) isOwner; 

//Should only allow people in the owners list to continue the execution.
modifier onlyOwners(){
    require(isOwner[msg.sender], "not an owner");
    _;
}

//Should initialize the owners list and the limit 
constructor(address[] memory _owners, uint _limit) {
    require(_owners.length > 0, "no owners registered");
    require(_limit > 0 && _limit <= _owners.length);

    for (uint i = 0; i <= _owners.length; i++){
        address owner = _owners[i];

        require(owner != address(0), "invalid owner");
        require(!isOwner[owner], "owner not unique");

        isOwner[owner] = true;
        owners.push(owner);
    }
    limit = _limit;
}

//Create an instance of the Transfer struct and add it to the transferRequests array
// assure owner is part of signatories -> taken care by modifier onlyOwners
function createTransfer(address payable _to, uint _amount) public onlyOwners {
    
    transferRequests.push(Transfer({
        to: _to,
        amount: _amount,
        totalSignatures: 0,
        transferComplete: false,
        id: 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 transfer request that has already been sent.
function approve(uint _id) public onlyOwners {
    require(_id > transferRequests.length, "no Transfers exists to date"); // a transfer needs to exist 
    require(approvals[msg.sender][_id] != true, "can't approve same transfer twice"); // an single owner can not approve the same transfer twice
    require(!transferRequests[_id].transferComplete, "transfer already executed"); // an owner should not be able to vote on a transfer request that has already been sent.
    
    
    Transfer storage transferRequest = transferRequests[_id]; // load transfer with id-number 
    
    approvals[msg.sender][_id] = true; // update the mapping by recording the owner's approval for this txID
    
    transferRequest.totalSignatures += 1; // update amount of signatures of this trfRequest

    require(transferRequest.totalSignatures >= limit, "insufficient signatures"); // check for sufficient signatures before executing transfer

    transferRequest.transferComplete = true; // first set transferComplete status to true
    
    (bool sent) = transferRequest.to.call.amount(transferRequest.amount); // transfer amount (remix compiler error, don't know how to resolve)
    require(sent, "transfer failed");
     
}

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

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

}

Thanks a lot for your help with this.

[thecil]

Hey @yestome, hope you are ok.

Some few words about your contract

@yestome

(bool sent) = transferRequest.to.call.amount(transferRequest.amount); // transfer amount (remix compiler error, don't know how to resolve)

I dont know exactly what you are trying to achieve here, by using call is to an external function, also you are not referring properly to any element in the transferRequest array.
Example should be something like transferRequest[id].amount

@yestome

why the idea of executing the transfer from within the approve function instead of moving the transfer execution into a separate function / is there any risk involved in tying these 2 functionalities into a single function?

Both approaches are valid, that just depends on what you want to achieve, execute the transaction manually or automatically after reaching the approval limit, if you are wondering about the most security measures I advice you to take the Smart Contract Security course that we have in the academy.
https://academy.ivanontech.com/products/ethereum-smart-contract-security

@yestome

1 more question: doesn the deposit function need a function body or is the header sufficient for accepting incoming transfers?

Also depends on the use case, it can have a body logic if its need it, if not, the header info is enough to receive incoming deposits.

Carlos Z

[Yeleva]

Hi @thecil. I got my contract working yesterday and am just tidying it up along with writing up some design and development notes prior to posting here.

Before doing so, I wanted to quickly ask you about coding standards. Are there any generally adopted style guides that devs typically follow for Solidity? It’d be nice to be compliant with the general consensus here.

A brief duck-duck-go gave me this https://github.com/ethereum/solidity/blob/v0.8.0/docs/style-guide.rst for example.

Many thanks.

[yestome]

Hello Carlos,

Thank you for your comments and tips!

Regarding the transfer: As I was unsure how to actually action the transfer I searched for a method for doing this (link I posted) and was aware this was likely to be incorrect. You mention the “id” should be added which obviously makes sense but then the approve-function doesn’t request this parameter, so the question remains how to get the “id” information into the approve function? Would appreciate receiving further indications on this - thank you.
Regarding security: yes, that course is next on my list - thanks for the hint all the same
Deposit function: ok - understood -

[yestome]

please ignore the question regarding the “id” parameter as I’ve noted it is requested in the approve function - still struggling with the correct syntax for the transfer. - thank you

[yestome]

Hello thecil,

Following your instructions in the above post I have removed the “payable” keyword in the function header of the createTransfer function but without the keyword the contracts doesn’t compile:
“creation of MultisigWallet errored: VM error: invalid opcode. invalid opcode The execution might have thrown. Debug the transaction to get more information.”
If I add the keyword back, the contract compiles ok but the contract doesn’t deploy after adding 3 addresses and the limit parameter.
I’ve assured the 3 addresses are placed within quotes and braced in square brackets, but the error persists.
I would be grateful for your indications to resolve the error - thank you in advance. Code below:

pragma solidity 0.7.5;
pragma abicoder v2;

contract MultisigWallet {

address[] public owners; // authorised owners
uint limit; // min number of signatories

struct Transfer {
    address payable to;
    uint amount;
    uint totalSignatures; // changed name to avoid confusion as "approvals" is also used for a mapping
    bool transferComplete; // "hasBeenSent" is somewhat unclear as it could relate to a trx-request or a transfer
    uint id;
}

Transfer[] transferRequests;

event transferCreated(address _to, uint _amount, uint _id);
event transferApproved(uint _id);
//event transferComplete();

// mapping [address][transferID] => true/false
// example: mapping[msg.sender][5] = true
mapping(address => mapping(uint => bool)) approvals;
// mapping for checking multiwallet owners
mapping(address => bool) isOwner; 

//Should only allow people in the owners list to continue the execution.
modifier onlyOwners(){
    require(isOwner[msg.sender], "not an owner");
    _;
}

//Should initialize the owners list and the limit 
constructor(address[] memory _owners, uint _limit) {
    require(_owners.length > 0, "no owners registered");
    require(_limit > 0 && _limit <= _owners.length);

    for (uint i = 0; i <= _owners.length; i++){
        address owner = _owners[i];

        require(owner != address(0), "invalid owner");
        require(!isOwner[owner], "owner not unique");

        isOwner[owner] = true;
        owners.push(owner);
    }
    limit = _limit;
}

//Create an instance of the Transfer struct and add it to the transferRequests array
// assure owner is part of signatories -> taken care by modifier onlyOwners
function createTransfer(address payable _to, uint _amount) public onlyOwners { 
    
    transferRequests.push(
        Transfer(_to, _amount, 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 transfer request that has already been sent.
function approve(uint _id) public onlyOwners {
    require(_id > transferRequests.length, "no Transfers exists to date"); // a transfer needs to exist 
    require(approvals[msg.sender][_id] != true, "can't approve same transfer twice"); // an single owner can not approve the same transfer twice
    require(!transferRequests[_id].transferComplete, "transfer already executed"); // an owner should not be able to vote on a transfer request that has already been sent.
    
    
    Transfer storage transferRequest = transferRequests[_id]; // load transfer with id-number 
    
    approvals[msg.sender][_id] = true; // update the mapping by recording the owner's approval for this txID
    
    transferRequest.totalSignatures += 1; // update amount of signatures of this trfRequest

    require(transferRequest.totalSignatures >= limit, "insufficient signatures"); // check for sufficient signatures before executing transfer

    transferRequest.transferComplete = true; // first set transferComplete status to true
    
    transferRequests[_id].to.transfer(transferRequests[_id].amount); // transfer amount to recipient
     
}

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

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

}

[Deini-Atakan]

pragma solidity 0.7.5;

contract Wallet{
    
    address[] owners;
    uint amountNeededForApproval;
    
    uint public completedRequestCount;
    uint public totalRequestCount;
    
    constructor(address _owner1, address _owner2, uint _amountForApproval){
        transferRequests.push();
        owners.push(_owner1);
        owners.push(_owner2);
        owners.push(msg.sender);
        amountNeededForApproval = _amountForApproval;
    }
    
    struct transferRequest{//THIS IS TO CREATE REQUESTS
        address payable to;
        uint amount;
        uint approved;
        uint requestId;
        bool completed;
    }
    
    transferRequest[] transferRequests;
    
    mapping (address=>mapping(uint => bool)) approvedBy;
    
    //EVENTS    
    
    
    event transferRequestCreated(address indexed requestCreator, address indexed to, uint amount);
    event transferCompleted(address indexed to, uint amount);
    
    
    //ONLY OWNER CHECK FOR NOT LETTING OTHER PEOPLE SIGN FOR APPROVAL
    modifier ownersOnly{
        require(msg.sender == owners[0] || msg.sender == owners[1] || msg.sender == owners[2],"You aren't one of the owners!");
        _;
    }
    
    //TO SEE WHO ARE THE OWNERS
    function displayOwners() public view returns(address,address,address){
        return(owners[0],owners[1],owners[2]);
    }
    
    //ANYONE CAN DEPOSIT AND SEE THE BALANCE OF THE CONTRACT/WALLET
    function deposit() public payable returns(uint walletBalance){
        return address(this).balance;
    }
    function getBalance() public view returns(uint EtherBalance){
        return address(this).balance/ 10**18;
    }
    
    //TRANSFER REQUEST AND SIGNING PROCESS
    function createTransferRequest(address payable _to, uint _amount)public ownersOnly{
        transferRequests.push(transferRequest(_to,_amount,0,transferRequests.length,false));
        totalRequestCount++;
        emit transferRequestCreated(msg.sender,_to,_amount);
    }
    function signTransferRequest(uint _index)public ownersOnly returns(uint totalApprovals){
        require(approvedBy[msg.sender][_index] == false,"You have already signed this transaction.");//checking if this owner already approved
        transferRequests[_index].approved ++;
        approvedBy[msg.sender][_index] = true;
        return (transferRequests[_index].approved);
    }
    function tryTransfer(uint _index)public ownersOnly returns(bool success){
        return _transfer(_index);//Checking if transfer is possible then doing it if possible
    }
    function showApproved(uint _index)public view returns(uint){
        return transferRequests[_index].approved;
    }
    
    //ACTUAL TRANSFER HAPPENING
    function _transfer(uint _index)private ownersOnly returns(bool){
        require(transferRequests[_index].approved >= amountNeededForApproval, "Not enough owners approved this transaction yet");
        require(address(this).balance >= transferRequests[_index].amount, "Not enough funds in the wallet");
        require(transferRequests[_index].completed == false,"Transfer already done before");
        completedRequestCount++;
        transferRequests[_index].completed = true;
        transferRequests[_index].to.transfer(transferRequests[_index].amount*10**18);
        emit transferCompleted(transferRequests[_index].to,transferRequests[_index].amount);
        return true;
    }
}

It works fine right now and I think I nailed it but I would be happy if you guys check it out and point out my mistakes if there are or any improvements are appreciated.

@yestome

/ mapping [address][transferID] => true/false
// example: mapping[msg.sender][5] = true
mapping(address => mapping(uint => bool)) approvals;
// mapping for checking multiwallet owners
mapping(address => bool) isOwner; 

By the way, before I checked here I couldn’t figure out how to check if an owner signed the transaction already but thanks to this code now I know what to do. Thanks @yestome

[yestome]

my pleasure ! (even though I’m still quite far from being knowledgeable in this space

[thecil]

@yestome

//Should initialize the owners list and the limit 
constructor(address[] memory _owners, uint _limit) {
    require(_owners.length > 0, "no owners registered");
    require(_limit > 0 && _limit <= _owners.length);

    for (uint i = 0; i <= _owners.length; i++){
        address owner = _owners[i];

        require(owner != address(0), "invalid owner");
        require(!isOwner[owner], "owner not unique");

        isOwner[owner] = true;
        owners.push(owner);
    }
    limit = _limit;
}

The problem in on the constructor, what are you trying to achieve with the loop iterator?

Carlos Z

[jahh]

Wow! im gonna have to do this course a couple of times haha…
I had an error that said " Parser Error: expected indentifer but got ‘(’ receive() payable external { " it’s at the end of this code, i can’t figure out what it would like me to change hopefully someone can shed some light on the situation?
thank you

pragma solidity 0.5.7;
contract wallets{
event Deposit(address indexed sender, uint amount, uint balance);
event submitTransaction(
address indexed owner,
uint indexed txIndexed,
aaddress indexed to,
uint vale,
bytes data
);
event confirmTransaction(address indexed owner, uint indexed txIndexed);
event revokeConfirmation(address indexed owner, uint indexed txIndexed);
event excuteTransaction(address indexed owner, uint indexed txIndexed);

    function submitTransaction (){}
    function confirmTransaction (){}
    function excuteTransaction (){}
    function revokeConfirmation (){}
    
     address[] public owners;
mapping(address => bool) public isOwner;
uint public numConfirmationsRequired;

struct Transaction {
    address to;
    uint value;
    bytes data;
    bool executed;
    uint numConfirmations;

}
mapping(uint => mapping(address => bool)) public isConfirmed;

Transaction[] public transactions;

modifier onlyOwner() {
    require(isOwner[msg.sender], "not owner");
    _;
}


modifier txExists(uint _txIndex) {
    require(_txIndex < transactions.length, "tx does not exist");
    _;
}

modifier notExecuted(uint _txIndex) {
    require(!transactions[_txIndex].executed, "tx already executed");
    _;
}

modifier notConfirmed(uint _txIndex) {
    require(!isConfirmed[_txIndex][msg.sender], "tx already confirmed");
    _;
}

constructor(address[] memory _owners, uint _numConfirmationsRequired) {
    require(_owners.length > 0, "owners required");
    require(
        _numConfirmationsRequired > 0 && _numConfirmationsRequired <= _owners.length,
        "invalid number of required confirmations"
    );

    for (uint i = 0; i < _owners.length; i++) {
        address owner = _owners[i];

        require(owner != address(0), "invalid owner");
        require(!isOwner[owner], "owner not unique");

        isOwner[owner] = true;
        owners.push(owner);
    }

    numConfirmationsRequired = _numConfirmationsRequired;
}

receive() payable external {
    emit Deposit(msg.sender, msg.value, address(this).balance);
}

[yestome]

adding the owners addresses to the owners array -

[thecil]

@jahh

receive() payable external {
    emit Deposit(msg.sender, msg.value, address(this).balance);
}

You have missed the keyword function at the start of that one

Carlos Z