Project - Multisig Wallet

Ethereum Smart Contract Programming 101
#826

Thank you @mcgrane5, I really appreciate you taking time to review and leave feedback! I like this double-mapping approach, it’s much simpler than what I ended up doing. :smile_cat:

Question with that approach, how is gas or speed/efficiency impacted by needed to store/update 2 separate pieces of info for each signature (once in the approvals double-mapping, and then once more in the Request.numOfApprovals), rather than the single location (via the current Request.Signatures approach)?

I just started the Ethereum Smart Contract Programming 201 course and I’m just starting to get some insight into what effects gas, but I think the best method of finding this out will likely be through experience (my own and from others). :smiley_cat:

#828

mmm i thought i did it right but i didn’t . maybe i misunderstood the assignment. i thought it was meant that if person A deposited 100wei, person b could make a transfer of eg 50 wei because it was like a shared wallet. in my contract i have a balance mapping and I added a TXfrom to the transaction struct and fill it with the msgsender of the caller of the addtransaction and later have the balance reduced from that account. but in the addtransaction i have a require that the msg.sender his/her balance >= TXamount. So I am not sure if i did it right or not. my wallet is basically that i and three others are owners but you can only make transactions for the amount that you deposited in it yourself. was that how it was supposed to be?

1 Like
#829

Hi everybody, that is my attempt after watching the project introduction video.

pragma solidity ^0.8.7;
pragma abicoder v2;
import "./Ownable.sol";
import "./Destroyable.sol";

contract MultisigWallet is Ownable,Destroyable{

    //number of approvals required to validate a transfer
    uint requiredApprovals;
    //Transfer number used as key for transfer mapping storage
    uint numRequest;
    //  mapping to store owners  
    mapping(address => Owner) owners;
    //mapping storage for transfer requests
    mapping(uint => Transfer) requestedtransfers;
    //emit an event on new deposit
    event DepositDone(uint _amount,address _account);
    //emit an event on transfer creation
    event TransferRequest(uint _id,address recipient,uint _amount);
   //emit an event when ether is sent to an address
   event TransferCompleted(uint _id,address recipient,address _owner,uint _amount);
   // emit an event when a transfer approval is reached
   event TransferApproved(uint _tranferId);
    struct Transfer{
        uint Id;
        uint amount;
        address recipient;
        uint approvalCount;
        address requestCreator;
        address[] signatories;
    }
    struct Owner{
        uint id;
        uint account;
    }

    modifier  ownlyOwners{
       require(owners[msg.sender].id != 0,"You must be a Owner before create transfert");
        _;
    }

    constructor(uint _requiredApprovals,address[] memory _owners){
        requiredApprovals = _requiredApprovals;
        numRequest = 0;
        for(uint i = 0;i < _owners.length; i++){
            owners[_owners[i]] = _newOwner(i+1);
        }
    }


    function _newOwner(uint _ownerID) private pure returns(Owner memory){
        return Owner(_ownerID,0);
    }


    function deposit() public payable ownlyOwners{
        owners[msg.sender].account += msg.value;
    }

    function createTransfer(uint _amount,address _recipient) public  ownlyOwners{
            // check for the owners amount
            require(owners[msg.sender].account >= _amount,"No enough funds");
            // increment the request number and use it as request id and key in the mapping
            numRequest++;
            Transfer storage newtransfer = requestedtransfers[numRequest];
            // set the transfer request parameters
            newtransfer.Id = numRequest;
            newtransfer.amount = _amount;
            newtransfer.recipient = _recipient;
            newtransfer.approvalCount = 0;
            newtransfer.requestCreator = msg.sender;
            // emit transfer request creation event
            emit TransferRequest(newtransfer.Id,_recipient,_amount);
    }

    function approveTransfer(uint _transferID) public ownlyOwners{
        // store the transfer to local storage
        Transfer storage transferToApprove = requestedtransfers[_transferID];
        require(transferToApprove.approvalCount < requiredApprovals,"Transfer already approved");
        // prevent the creator of a transfer to approve it
        require(msg.sender != transferToApprove.requestCreator,"you cannot approve your own transfer request");
        // prevent an owner to double approve transfer
        for(uint i = 0;i < transferToApprove.signatories.length;i++){
            require(transferToApprove.signatories[i] != msg.sender,"Cannot approve transfer twice");
        }
        // record owners that had approved transfer addresses
        transferToApprove.signatories.push(msg.sender);
        // increment approval count and chech if the required is meet 
        transferToApprove.approvalCount++;
        if (transferToApprove.approvalCount == requiredApprovals){
            emit TransferApproved(transferToApprove.Id);
        }
    }
    
    
    function sendTransfer(uint _transferID) public ownlyOwners {
         // store the transfer to local storage
        Transfer storage transferToSend = requestedtransfers[_transferID];
        //only the creator of a contrac can send it
        require(msg.sender == transferToSend.requestCreator,"you can only send your own created transfers");
        //check for approvals
        require(transferToSend.approvalCount == requiredApprovals,"Transfer must be approved by all approvals");
        // check the amount of owners that request transfer
        require(owners[transferToSend.requestCreator].account >= transferToSend.amount,"No enough funds");
        // reduce user account buy the amount transfer
        owners[transferToSend.requestCreator].account -= transferToSend.amount;
        payable(transferToSend.recipient).transfer(transferToSend.amount);
        emit TransferCompleted(transferToSend.Id,transferToSend.recipient,transferToSend.requestCreator,transferToSend.amount);
    }


    function getOwner(address _ownerAddr) public view returns(Owner memory){
        return owners[_ownerAddr];
    }


    function getTransfer(uint _index) public view returns(Transfer memory){
        return requestedtransfers[_index];
    }
    
    function getContractBalance() public view onlyCreator returns(uint) {
        return address(this).balance;
    }
}

pragma solidity ^0.8.7;

contract Ownable{
  
  //the creator of the contract
  address creator;

  modifier onlyCreator(){
    require(msg.sender == creator);
    _;
  }
  constructor() {
    creator = msg.sender;
  }


}

pragma solidity ^0.8.7;

contract Ownable{
  
  //the creator of the contract
  address creator;

  modifier onlyCreator(){
    require(msg.sender == creator);
    _;
  }
  constructor() {
    creator = msg.sender;
  }
}
#830

i have this, but am confused about where the money is :slight_smile: what do you do with transferring it from where? and if you want to check if there is enough balance to make the transaction:

pragma solidity 0.7.5;
pragma abicoder v2;

import "./isowner.sol";

contract Bank is isowner {
    
    uint amountsign;
    mapping (address => uint) Balance;

    //address[] owners etc in isowner;
    uint txid = 0;

    constructor (address _owner2, address _owner3, address _owner4, uint _amountsign)  {
    owners.push(msg.sender);
    owners.push(_owner2);
    owners.push(_owner3);
    owners.push(_owner4);
    amountsign = _amountsign;
    }

    mapping (address => mapping(uint => bool)) approvals;
        
    struct TX {
        uint txid;
        address txfrom;
        address txto;
        uint txamount;
        uint approvalsreceived;
        bool txsent;
    }
    
    TX[] transaction;

    function deposit () payable public  {
        Balance[msg.sender] += msg.value;
      }
    
    //show who are the owners - so i can check
    function getOwners () public view returns (address [] memory) {
        return owners;
    }

    //show what is  the balance of the person who is interacting with the contract- though not sure who gets the balance of
    //deposited and from what will it be sent? how do you check if there is enough balance? does an owner can only sent 
    //as much i he/she has deposited?? i don't underand

    function getBalance () public view returns (uint){
        return Balance[msg.sender];
        }
    
    // if person A deposits 1 eth, and person b makes a transfer from what wallet does it go? i now have msg.sender
    //but i think that is incorrect
    function addTransaction (address payable _to, uint _amount) public isOwner  {
        require (Balance[msg.sender] >= _amount, "you cannot transfer that much");
        TX memory newTX = TX(txid, msg.sender, _to, _amount, 0, false);
        transaction.push(newTX);
        txid += 1;
    }
    
    //view transaction of a certain id
    function viewtransaction (uint _txid)  public view returns (TX memory){
        return transaction[_txid];
        }
    
    //approve txs
    function approveTX (uint _txid)  payable public isOwner {
        require (msg.sender != transaction[_txid].txfrom, "you are the transactionmaker, you cannot approve this transaction");
        require (isCoOwner == true, "you are not a co-owner, you cannot approve this transaction");
        require (approvals[msg.sender][_txid] == false, "you have already approved, you cannot approve twice");

        transaction[_txid].approvalsreceived += 1;
        approvals[msg.sender][_txid] = true;

        address payable TransferTo = payable (transaction[_txid].txto);
        
        if (transaction[_txid].approvalsreceived >= amountsign) {
   
        //here... from what do you substract the amount that is sent?
        Balance[transaction[_txid].txfrom] -= transaction[_txid].txamount;
        TransferTo.transfer(transaction[_txid].txamount);
        transaction[_txid].txsent = true;
        }
    }
    
    //check how many apporvals there are
    function getapprovalamount (uint _txid) public view returns (uint) {
        return transaction[_txid].approvalsreceived ;
    }

    //to check if a certain person has true or false at a certain tarnsaction
    function getapprovaltxperson (uint _txid, address _address) public view returns (bool) {
        return approvals[_address][_txid];
    }
}

and this is the onlyowner (bit double, i used one just to check something:

pragma solidity 0.7.5;

contract isowner {
    
bool isCoOwner;
    
address[] public owners;
    
modifier isOwner {

isCoOwner = false; 
for (uint i=0; i < owners.length; i++) {
    if (msg.sender == owners[i]) {
        isCoOwner = true;
         }
    }
 require(isCoOwner == true);
        _;
    }   

//to check if you are false or true
function isOwner2 () public returns (bool) {   
isCoOwner = false; 
for (uint i=0; i < owners.length; i++) {
    if (msg.sender == owners[i]) {
        isCoOwner = true;
         }
         
    }
    return isCoOwner;
}

//modifier onlyOwner {
//        require(isCoOwner == true);
 //       _;
 //   }

}
1 Like
#831

hey minke your code now works i have fixed it below. Note that i only fixed it in the most bare minimum way to make it work you should explore further and try to make this better.

**So why wouldnt it work?
firstly was the constructor you cannot use the format your employing to “push multiple owners” into the woeners array one after the other. your better off passing an array of owners into the constructor. secondly look at your approve transfr functuon. this you ned to get rid of

address payable TransferTo = payable (transaction[_txid].txto);

you may be able to keep it but its unesssecary. other than this i dont think i changed anything. consider the code below

pragma solidity 0.7.5;
pragma abicoder v2;



contract Bank {
    
    uint amountsign;
    mapping (address => uint) Balance;

    bool isCoOwner;
    
    address[] public owners;
        
    modifier isOwner {

    isCoOwner = false; 
    for (uint i=0; i < owners.length; i++) {
        if (msg.sender == owners[i]) {
            isCoOwner = true;
            }
        }
    require(isCoOwner == true);
            _;
    }   

    //address[] owners etc in isowner;
    uint txid = 0;

    constructor (address[] memory _owners, uint _amountsign)  {
        owners = _owners;
        amountsign = _amountsign;
    }

    mapping (address => mapping(uint => bool)) approvals;
        
    struct TX {
        uint txid;
        address txfrom;
        address payable txto;
        uint txamount;
        uint approvalsreceived;
        bool txsent;
    }
    
    TX[] transaction;

    function deposit () payable public  {
        Balance[msg.sender] += msg.value;
      }
    
    //show who are the owners - so i can check
    function getOwners () public view returns (address [] memory) {
        return owners;
    }

    //show what is  the balance of the person who is interacting with the contract- though not sure who gets the balance of
    //deposited and from what will it be sent? how do you check if there is enough balance? does an owner can only sent 
    //as much i he/she has deposited?? i don't underand

    function getBalance () public view returns (uint){
        return Balance[msg.sender];
        }
    
    // if person A deposits 1 eth, and person b makes a transfer from what wallet does it go? i now have msg.sender
    //but i think that is incorrect
    function addTransaction (address payable _to, uint _amount) public isOwner  {
        require (Balance[msg.sender] >= _amount, "you cannot transfer that much");
        TX memory newTX = TX(txid, msg.sender, _to, _amount, 0, false);
        transaction.push(newTX);
        txid += 1;
    }
    
    //view transaction of a certain id
    function viewtransaction (uint _txid)  public view returns (TX memory){
        return transaction[_txid];
        }
    
    //approve txs
    function approveTX (uint _txid)  payable public isOwner {
        require (msg.sender != transaction[_txid].txfrom, "you are the transactionmaker, you cannot approve this transaction");
        require (isCoOwner == true, "you are not a co-owner, you cannot approve this transaction");
        require (approvals[msg.sender][_txid] == false, "you have already approved, you cannot approve twice");

        transaction[_txid].approvalsreceived += 1;
        approvals[msg.sender][_txid] = true;

        //address payable TransferTo = payable (transaction[_txid].txto);
        
        if (transaction[_txid].approvalsreceived >= amountsign) {
   
        //here... from what do you substract the amount that is sent?
            Balance[transaction[_txid].txfrom] -= transaction[_txid].txamount;
            transaction[_txid].txto.transfer(transaction[_txid].txamount);
            transaction[_txid].txsent = true;
        }
    }
    
    //check how many apporvals there are
    function getapprovalamount (uint _txid) public view returns (uint) {
        return transaction[_txid].approvalsreceived ;
    }

    //to check if a certain person has true or false at a certain tarnsaction
    function getapprovaltxperson (uint _txid, address _address) public view returns (bool) {
        return approvals[_address][_txid];
    }
}
1 Like
#832

Here is my code fort the multisigwallet.

pragma solidity 0.8.10;
contract MultiSigWallet {

    event Deposit(address indexed sender, uint amount, uint balance);

    event SubmitTransaction(

        address indexed owner,

        uint indexed txIndex,

        address indexed to,

        uint value,

        bytes data

    );

    //["0x5B38Da6a701c568545dCfcB03FcB875f56beddC4", "0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2", "0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db"]

    event ConfirmTransaction(address indexed owner, uint indexed txIndex);

    event RevokeConfirmation(address indexed owner, uint indexed txIndex);

    event ExecuteTransaction(address indexed owner, uint indexed txIndex);

    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;

    //mapping[address][transferid] => true/false

    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() external payable {

        emit Deposit(msg.sender, msg.value, address(this).balance);

    }

    function submitTransaction( address _to, uint _value, bytes memory _data ) public onlyOwner {

        uint txIndex = transactions.length;

        transactions.push(

            Transaction({

                to: _to,

                value: _value,

                data: _data,

                executed: false,

                numConfirmations: 0

            })

        );

        emit SubmitTransaction(msg.sender, txIndex, _to, _value, _data);

    }

    function confirmTransaction(uint _txIndex) public onlyOwner txExists(_txIndex) notExecuted(_txIndex) notConfirmed(_txIndex) {

        Transaction storage transaction = transactions[_txIndex];

        transaction.numConfirmations += 1;

        isConfirmed[_txIndex][msg.sender] = true;

        emit ConfirmTransaction(msg.sender, _txIndex);

    }

    function executeTransaction(uint _txIndex)public onlyOwner txExists(_txIndex) notExecuted(_txIndex) {

        Transaction storage transaction = transactions[_txIndex];

        require(

            transaction.numConfirmations >= numConfirmationsRequired,

            "cannot execute tx"

        );

        transaction.executed = true;

        (bool success, ) = transaction.to.call{value: transaction.value}(

            transaction.data

        );

        require(success, "tx failed");

        emit ExecuteTransaction(msg.sender, _txIndex);

    }

    function revokeConfirmation(uint _txIndex) public onlyOwner txExists(_txIndex) notExecuted(_txIndex) {

        Transaction storage transaction = transactions[_txIndex];

        require(isConfirmed[_txIndex][msg.sender], "tx not confirmed");

        transaction.numConfirmations -= 1;

        isConfirmed[_txIndex][msg.sender] = false;

        emit RevokeConfirmation(msg.sender, _txIndex);

    }

    function getOwners() public view returns (address[] memory) {

        return owners;

    }

    function getTransactionCount() public view returns (uint) {

        return transactions.length;

    }

    function getTransaction(uint _txIndex) public view returns (

            address to,

            uint value,

            bytes memory data,

            bool executed,

            uint numConfirmations

        )

    {

        Transaction storage transaction = transactions[_txIndex]; return (

            transaction.to,

            transaction.value,

            transaction.data,

            transaction.executed,

            transaction.numConfirmations

        );

    }

}
#833

Okay, I tried to do it on my own, but without succes. Everytime I want to add a new transaction, it keeps reverting. I tried looking on the web for the answer (and even looked a little bit in the studygroup), but I still don’t know what I did wrong. I deposited money (works as it should), then I create a transfer, but I don’t know why the transaction keeps reverting. Can anyone see what I did wrong?

pragma solidity 0.7.5;
pragma abicoder v2;

contract Wallet {

    //EVENTS
    event transferRequestAdded(uint _id, address RequestedBy, address recipient, uint amount);
    event ApprovalReceived(uint _id, uint _approvals, address _approver);
    event TransferSent (uint _id);
    event depositDone(uint _amount, address indexed DepositedFrom);    

    //VARIABLES
    address[] public owners;
    uint public limit;
    uint private AddressBalance;

    struct Transfer{
        uint Amount;
        address payable Recipient;
        uint Approvals;
        bool Confirmed;
    }
    Transfer[] transferRequests;
    
    mapping(address => mapping(uint => bool)) approvals;    
    mapping(address => bool) public IsOwner;

    //MODIFIERS
    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) {

        for(uint i=0; i <owners.length; i++){
            IsOwner[_owners[i]] = true;
        }
        owners = _owners;
        limit = _limit;
        
    }

    //FUNCTIONS
    
    function deposit() public payable returns(uint){
        require(msg.value >= 0);
        AddressBalance += msg.value;
        return(AddressBalance);
    }

    function getBalance() public view returns(uint){
        return AddressBalance;
    }    
    
 
    
    function createTransfer(address payable receiver, uint amount) public onlyOwners{
        require(AddressBalance >= amount, "You can't send more than the Balance!");

        Transfer memory newTransfer = Transfer(amount, receiver, 0, false);
        transferRequests.push(newTransfer);
        emit transferRequestAdded(transferRequests.length, msg.sender, receiver, amount);
    }

    function getTransferRequests(uint _id) public view returns (Transfer[] memory){
        _id = transferRequests.length;
        return(transferRequests);
    }    

    function ApproveTransaction (uint _id) public onlyOwners {
    // 

        require(approvals[msg.sender][_id] == false);
        require(transferRequests[_id].Confirmed == false);

        approvals[msg.sender] [_id] = true;
        transferRequests[_id].Approvals++;
        emit ApprovalReceived(_id, transferRequests[_id].Approvals, msg.sender);
        if(transferRequests[_id].Approvals >= limit){
            transferRequests[_id].Confirmed = true;
            transferRequests[_id].Recipient.transfer(transferRequests[_id].Amount);

        }
    }
}
1 Like
#834

hey @Sil. the problem is in your only owners modifer. Consider the below

 modifier onlyOwners(){
        bool Owner = false;
        for(uint i=0; i < owners.length; i++){
            if(owners[i] == msg.sender){
                Owner == true;
            }
            require(Owner == true);
        }
        _;
    }

here you are checking Owner == true; npt doing an assignemnt. change that line to Owner = true; you could aslo make this modifier more effient by breaking out of the loop once the owner has been found to save computations. so ideally your modifier should be

modifier onlyOwners(){
        bool Owner = false;
        for(uint i=0; i < owners.length; i++){
            if(owners[i] == msg.sender){
                Owner ==true;
                break;
            }
          
        }
         require(Owner == true);
        _;
    }

also one other thing is you shoud replace your addressBalance variable with a mapping. Your current stup will only allow you to store the balance for one person. you need a mapping to be able to store the balance of individual users. so replace addressBalance with mapping(address => uint) balance. Then in your deposit function and getBalance function replace addressBalance with balance[msg.sender]

1 Like
#835

Okay, I’ve started from scratch again, and now there aren’t any failures in my smart contract.
I feel like it can always be better, so tips are more than welcome. Everything works as it should. I’ve also added some extra things to expand the functionality to the MultiSig contract.
You can find the contract below.

pragma solidity 0.7.5;
pragma abicoder v2;

contract MultiSigWallet{

    //EVENTS
    event Deposited(address from, uint amount);
    event RequestAdded(address to, uint amount, address from);
    event RequestConfirmed(address from);
    event TransactionSent(address to, uint amount);


    //VARIABLES

    address[] Owners;
    uint MaxApprovals;

    struct Transfer{
        uint amount;
        address payable receiver;
        uint approvals;
        bool confirmed;
        address initiater;
    }
    Transfer[] TransferRequests;

    mapping(address => mapping(uint => bool)) confirmations;
    mapping(address => bool) IsOwner;
    mapping(address => uint) Balances;

    modifier onlyOwners(){
        bool owner = false;
        for(uint i=0; i < Owners.length; i++){
            if(Owners[i] == msg.sender){
                owner = true;
                break;
            }
        }
        require(owner == true);        
        _;
    }

    //CONSTRUCTOR

    constructor(address[] memory _owners, uint _limit) {
        for(uint i=0; i < Owners.length; i++){
            IsOwner[_owners[i]] = true;

        }

        Owners = _owners;
        MaxApprovals = _limit;
    }

    //FUNCTIONS

    function deposit() public payable returns(uint){
        require(Balances[msg.sender] >= 0);
        Balances[msg.sender] += msg.value;
        emit Deposited(msg.sender, msg.value);
        return(Balances[msg.sender]);
    }

    function getBalance() public view returns(uint){
        return(Balances[msg.sender]);
    }

    function createTransfer(address payable receiver, uint amount) public onlyOwners{
        require(msg.sender != receiver);
        require(Balances[msg.sender] >= amount);

        Transfer memory newTransfer = Transfer(amount, receiver, 0, false, msg.sender);
        emit RequestAdded(receiver, amount, msg.sender);
        TransferRequests.push(newTransfer);
    }

    function getTransferRequests(uint _id) public view returns (Transfer[] memory){
        _id = TransferRequests.length;
        return(TransferRequests);
    }

    function ApproveTransaction (uint _id) public onlyOwners {

        require(TransferRequests[_id].confirmed == false);
        require(TransferRequests[_id].initiater != msg.sender);
        require(confirmations[msg.sender][_id] == false);       
        TransferRequests[_id].approvals ++;
        emit RequestConfirmed(msg.sender);

        if(TransferRequests[_id].approvals >= MaxApprovals){
            TransferRequests[_id].confirmed = true;
            Balances[msg.sender] -= TransferRequests[_id].amount;
            emit TransactionSent(TransferRequests[_id].receiver, TransferRequests[_id].amount);
            TransferRequests[_id].receiver.transfer(TransferRequests[_id].amount);
        }
    }

    function getApprovals(uint _id) public view returns(uint){
        return(TransferRequests[_id].approvals);
        
    }

    function AddOwner(address newOwner) public onlyOwners {
        IsOwner[newOwner] = true;
        Owners.push(newOwner);
    }

    function getOwners() public view returns(address[] memory){
        return(Owners);
    }                        

}

Again, If someone sees something that could be better, be my guest and say it.

#836

Thanks for the tips. I kinda chose for one simple AddressBalance, As I thought it would be easier if it were just a MultiSigWallet. I did added it to my updated MultiSig Smart contract. Thanks for the tips. Really appreciate it!!

1 Like
#837 
pragma solidity 0.7.5;
pragma abicoder v2;

contract MultiSigWallet{

    //EVENTS
    event Deposited(address from, uint amount);
    event RequestAdded(address to, uint amount, address from);
    event RequestConfirmed(address from);
    event TransactionSent(address to, uint amount);


    //VARIABLES

    address[] Owners;
    uint MaxApprovals;

    struct Transfer{
        uint amount;
        address payable receiver;
        uint approvals;
        bool confirmed;
        address initiater;
    }
    Transfer[] TransferRequests;

    mapping(address => mapping(uint => bool)) confirmations;
    mapping(address => bool) IsOwner;
    mapping(address => uint) Balances;

    modifier onlyOwners(){
        bool owner = false;
        for(uint i=0; i < Owners.length; i++){
            if(Owners[i] == msg.sender){
                owner = true;
                break;
            }
        }
        require(owner == true);        
        _;
    }

    //CONSTRUCTOR

    constructor(address[] memory _owners, uint _limit) {
        for(uint i=0; i < Owners.length; i++){
            IsOwner[_owners[i]] = true;

        }

        Owners = _owners;
        MaxApprovals = _limit;
    }

    //FUNCTIONS

    function deposit() public payable returns(uint){
        require(Balances[msg.sender] >= 0);
        Balances[msg.sender] += msg.value;
        emit Deposited(msg.sender, msg.value);
        return(Balances[msg.sender]);
    }

    function getBalance() public view returns(uint){
        return(Balances[msg.sender]);
    }

    function createTransfer(address payable receiver, uint amount) public onlyOwners{
        require(msg.sender != receiver);
        require(Balances[msg.sender] >= amount);

        Transfer memory newTransfer = Transfer(amount, receiver, 0, false, msg.sender);
        emit RequestAdded(receiver, amount, msg.sender);
        TransferRequests.push(newTransfer);
    }

    function getTransferRequests(uint _id) public view returns (Transfer[] memory){
        _id = TransferRequests.length;
        return(TransferRequests);
    }

    function ApproveTransaction (uint _id) public onlyOwners {

        require(TransferRequests[_id].confirmed == false);
        require(TransferRequests[_id].initiater != msg.sender);
        require(confirmations[msg.sender][_id] == false);       
        TransferRequests[_id].approvals ++;
        emit RequestConfirmed(msg.sender);

        if(TransferRequests[_id].approvals >= MaxApprovals){
            TransferRequests[_id].confirmed = true;
            Balances[msg.sender] -= TransferRequests[_id].amount;
            emit TransactionSent(TransferRequests[_id].receiver, TransferRequests[_id].amount);
            TransferRequests[_id].receiver.transfer(TransferRequests[_id].amount);
        }
    }

    function getApprovals(uint _id) public view returns(uint){
        return(TransferRequests[_id].approvals);
        
    }

    function AddOwner(address newOwner) public onlyOwners {
        IsOwner[newOwner] = true;
        Owners.push(newOwner);
    }

    function getOwners() public view returns(address[] memory){
        return(Owners);
    }                        

}

Here is my updated code. If someone finds something that could be better, be my guest and tell it to me.

1 Like
#838

Felt like I was following this course pretty well until we got to this assignment… Definitely was not able to complete this assignment independently. I understand other people’s code but I know I couldn’t come up with it myself. I am completely new to coding and have only completed the JS course as background, but I’d love to know if anyone else found themselves in a similar situation or if I’m just in over my head lol.

1 Like
#839

@Matias_Haller yes do not worry you are not alone. stick at it everyday. and keep doing the courses ask questions where needed and you will be astonished at your progress in 1 month. its key to practice and progress everyday. remember we are always here to help

#840

My man, you are not alone on this, i also felt the same way, but I promised to keep searching and practicing, I also realised that the more I practice the better my understanding.

1 Like
#841

Hi everyone. Here’s my multisig wallet. I watched the first minute or two of the “Project Assistance” video but I wanted to have a go at doing it myself with as little help as possible. I’ll watch all the help videos now and probably find simple explanations of stuff it took me hours to figure out by myself but oh well!

It’s pretty janky in a lot of ways and there are some things I couldn’t figure out so I just worked around them however I could.

Things I’d like to do better and get help with:

  1. I couldn’t figure out how to do a dynamically sized array with a constructor (for setting multiple owners at the point of deployment). So the only thing I have in my constructor is a “Number of Approvers” argument, and I made an “addOwner” function to be used to add more owners after the contract is deployed. I think the main drawback of this is that it allows the contract to exist in a state where the number of required approvers is greater than the number of owners that actually exist.

  2. I gave up on having a “transactionId” for each transaction and just ended up using the index of the array for reference. I couldn’t work out how to “get” a Transaction (struct) from within an array, using one of it’s own parameters (txId). So I ended up just using it’s index place to “get” it and then things get complicated once you start removing transactions from the array so “txId” and the transaction’s index number don’t match at all. So I just got rid of txId, it just felt a bit redundant and confusing.

  3. My method of removing transactions from the array involves a for loop that cycles through each transaction in the array and copies / shifts them to maintain order. This would be quite gas-intensive and I don’t think keeping the transactions in order is all that important so I might look into cheaper ways of removing items from an array.

  4. Order of functions. This didn’t seem to make a difference but through general chopping and changing the functions have ended up in no sort of sensical order. Let me know if this is a big deal :sweat_smile:

Anyway, there’s probably loads of other sub-optimal stuff but here it is. I’ve tried to comment the code as best I can…

pragma solidity 0.7.5;

contract Ownable {

    address[] public owners; //stores addresses of the owners in an array

    uint noOfApprovers; //number of owners required to approve a transaction

    bool[] alreadyApproved; //copied to each Transaction object to track which owners have already approved which transactions

    mapping (address => bool) isOwner;

    mapping (address => uint) approvalTracking;


    //restricts usage of a function to only addresses marked as "true" in the "isOwner" mapping

    modifier onlyOwner{

        require (isOwner[msg.sender] == true, "Only an owner of this wallet is permitted to take this action");

        _;

    }


    //adds a new owner to the wallet with all the privileges of the original contract deployer

    function addOwner (address _owner) public onlyOwner {

        alreadyApproved.push(false);

        approvalTracking[_owner] = owners.length;        

        owners.push(_owner);

        isOwner[_owner] = true;

    }

}

separate file

pragma solidity 0.7.5;

pragma abicoder v2;

import "./OwnableWal.sol";

contract multisigWallet is Ownable{


    //sets number of required approvers and establishes the deployer of the contract as an "owner"

    constructor(uint _noOfApprovers) {

        isOwner[msg.sender] = true;

        addOwner(msg.sender);

        noOfApprovers = _noOfApprovers;

    }

    struct Transaction {

        uint amount;

        address payable recipient;

        address sender;

        uint approvals; //number of approvals this transaction has

        bool[] _alreadyApproved; //an array that corresponds with the "owners" array for preventing duplicate approvals

    }

    Transaction[] pendingTransactions; //where the queue of pending transactions will be stored

    Transaction currentTx; //a variable for building a Transaction to be added to the array

    function deposit () public payable {

    }


    //returns the total balance of the contract

    function getWalletBalance() public view onlyOwner returns (uint){

        return address(this).balance;

    }


    //checks balance then starts a transfer / withdrawal and queues it for approval in the "pendingTransactions" array

    function requestSendFunds(uint _amount, address payable _recipient) public onlyOwner {

        require(address(this).balance >= _amount, "Insufficient funds to initiate this transaction");

        currentTx = Transaction(_amount, _recipient, address(this), 1, alreadyApproved);

        //allows transaction to execute immediately if noOfApprovers was set to 1

        if(currentTx.approvals >= noOfApprovers){

            _recipient.transfer(_amount);

        }


        //if noOfApprovers set higher than 1, record 1 approval from msg.sender and add transaction to array for more approvals

        else{

            currentTx._alreadyApproved[approvalTracking[msg.sender]] = true;

            pendingTransactions.push(currentTx);

        }

    }


    //triggered by "approveTx" only if the required number of approvers has been met

    //checks balance then completes the transaction

    function doSendFunds (uint _amount, address payable _recipient, uint _index) private {

        require(address(this).balance >= _amount, "Insufficient funds to complete this transaction");

        _recipient.transfer(_amount);

        removeTx(_index);

    }


    //triggered by "doSendFunds". Removes the completed transaction from the array

    function removeTx (uint _index) private {

        for (uint i = _index; i < pendingTransactions.length - 1; i++) {

            pendingTransactions[i] = pendingTransactions[i+1];

        }

        pendingTransactions.pop();

    }


    //returns how many transactions are awaiting approval

    function noOfPendingTxs() public view onlyOwner returns(uint){

        return (pendingTransactions.length);

    }


    //returns recipient and amount of a transaction. Queried using index number

    function getTxInfo(uint _index) public view onlyOwner returns(uint, address){

        Transaction memory txToReturn = pendingTransactions[_index];

        return(txToReturn.amount, txToReturn.recipient);

    }


    //checks that msg.sender hasn't already approved the transaction they are trying to approve then flags the transaction as approved by msg.sender

    //increments the "approvals" counter in the Transaction struct

    //checks if the required number of approvals has been met, if so triggers "doSendFunds"

    function approveTx (uint index) public onlyOwner {

        require(pendingTransactions[index]._alreadyApproved[approvalTracking[msg.sender]] == false, "You have already approved this transaction");

        pendingTransactions[index]._alreadyApproved[approvalTracking[msg.sender]] = true;

        pendingTransactions[index].approvals ++;

        if (pendingTransactions[index].approvals >= noOfApprovers){

                doSendFunds(pendingTransactions[index].amount, pendingTransactions[index].recipient, index);          

        }

    }

}
1 Like
#842

Using Phillips final code, I notice there is a “owner” blue function button at the bottom of the deployed section of solidity. Why is that there? I thought that would only show there if there was a function named “owner”? What routine does it access/send an address to? Am I missing something? Thanks

#843

Hi @DanDD for all public state variables (variables that aren’t declared inside a function) automatic getters are generated it means if you program a contract like this:

pragma solidity 0.7.5;

contract PublicGetter {
    uint public num;
}

You automatically get a function num which will give the state variable value. You can read a bit more in solidity documentation in Visibility and Getters Contracts Documentation

#844

Hi @tom88norman well done! You figured out how to finish the project so I think that’s a great step! I try to follow Mark Zuckerberg quote Done is better than perfect so about your points I think:

  1. When I did the project I had the same problem because I was using a base class how you did, and the base class had the owners arrays so I had to google what are the ways to pass parameters to constructors and I found this article that help me a lot Solidity Inheritance turns out that there are two ways to pass parameters to constructor and which works for me was this way:
    constructor(address[] memory _owners, uint _limitApprovals) Ownables(_owners){
        limitApprovals = _limitApprovals;
    }

If you can see after the function parameters I called the parent class and pass the owners array that I received from the constructor.

  1. I saw your code and I think it’s a good approach as long as you don’t need a transaction log maybe you could emit events for every transaction step how Filip did in his code.

  2. I have a similar doubt about the better way to avoid gas increasing, my approach was setting a state for every transaction without removing it but I think in long term is expensive because I don’t get rid of transactions so as long as more transactions we have more space we need to use it means more costs. Maybe someone can help us with this doubt.

  3. I think we have to read solidity style guide to be aware if we’re following the conventions.

#845

Hi, first of all, this was an interesting project, we have to use what we learned to solve it! so thank Filip for this project.

Steps

I used 2 files, 1 for managing all owners permissions and the another for wallet logic

ownables.sol

pragma solidity 0.7.5;

contract Ownables{

    address[] owners;

    constructor(address[] memory _owners){
        owners  = _owners;
    }

    modifier isOwner () {
        bool owner = false;
        uint index = 0;

        while( index < owners.length && owner == false) {
            owner = owners[index] == msg.sender;
            index++;
        }
        require(owner, "Just owner can use this method");
        _;
    }

    function ownerDifferentThanRequestor(address _requestorAddress) internal {
    
        if (_requestorAddress == msg.sender) {
            revert("approvers have to be different than requestor");
        }
     
    }
}

wallet.sol

pragma solidity 0.7.5;

import "ownables.sol";

contract Wallet is Ownables{

    //minimum approvals to do the transaction.
    uint limitApprovals;
    
    struct PendingTransaction{
        address requestor;
        address to;
        uint amount;
        uint id;
        address[] approvers;
        bool completed;
    }


    PendingTransaction[] pendingTransactions;

    constructor(address[] memory _owners, uint _limitApprovals) Ownables(_owners){
        limitApprovals = _limitApprovals;
    }

    receive() payable external{}


    modifier checkWalletBalanceModifier(uint _amount){
        checkWalletBalance(_amount);
        _;
    }

    function deposit() external payable{

    }

    function checkWalletBalance(uint _amount) private view {
        require(_amount <= address(this).balance, "Wallet not have enough balance try with less amount");
    }

    function findApproversByTransactionId(uint transactionId) private view returns(PendingTransaction storage) {

        uint loop = 0;

        while(loop < pendingTransactions.length) {
            if(pendingTransactions[loop].id == transactionId){
                return pendingTransactions[loop];
            }
            loop++;
        }

        revert("Transaction not found");
    }

    function existApprover(address[] storage approvers) private view returns(bool) {

        uint loop = 0;

        while(  loop < approvers.length) {
            if(approvers[loop] == msg.sender){
                return true;
            }
            loop++;
        }
        return false;
    }


    function transfer(address _to, uint _amount) isOwner checkWalletBalanceModifier(_amount) external {
        require(address(this).balance >= _amount);
        address[] memory approvers;
        pendingTransactions.push(
            PendingTransaction(msg.sender, _to, _amount, pendingTransactions.length, approvers, false)
        );
    }

    // Check if transaction id exist
    // Check if who is approving is different than requestor (who created the transaction)
    // Check if transaction was completed
    // Check if the owner has an approval
    function approveTransfer(uint transactionId) isOwner external{
        PendingTransaction storage pendingTransaction = findApproversByTransactionId(transactionId);
        ownerDifferentThanRequestor(pendingTransaction.requestor);
        bool oldApprover = existApprover(pendingTransaction.approvers);

        if(pendingTransaction.completed){
            revert("Transaction had been completed already");
        }

        if(oldApprover){
            revert("You had approved already");
        }

        checkWalletBalance(pendingTransaction.amount);

        pendingTransaction.approvers.push(msg.sender);

        if(pendingTransaction.approvers.length == limitApprovals){
            pendingTransaction.completed = true;
            payable(pendingTransaction.to).transfer(pendingTransaction.amount);
        }
    }
}

Doubt

  1. Which would it be the best approach to deal with transactions that already were completed? is it better to remove them when the transfer is completed? or is better to storage them?
#846

Here’s my code, forgot to add the events.

pragma solidity >=0.5.0 <0.9.0;

contract Multisig {
    mapping(address => bool) owner;
    uint8 limitApproval;
    mapping(uint => Transfer) public transfers;
    uint public transfersCount = 0;
    mapping(address => mapping(uint => bool)) signers;

    enum TransferStatus { Sent, Pending }

    struct Transfer {
        address payable to;
        uint amount;
        TransferStatus status;
        uint approvals;
        uint id;
    }

    modifier onlyOwner {
        require(owner[msg.sender], "You're not an owner");
        _;
    }

    constructor(address[] memory _owners, uint8 _limitApproval) {
        for(uint i = 0; i < _owners.length; i++) {
            owner[_owners[i]] = true;
        }
        limitApproval = _limitApproval;
    }

    function deposit() public payable {}

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

    function createTransfer(address payable _to, uint _amount) public onlyOwner {
        uint txId = transfersCount;
        transfers[txId] = Transfer(_to, _amount, TransferStatus.Pending, 0, txId);
        approveTransfer(txId);
        transfersCount++;
    }

    function approveTransfer(uint _id) public onlyOwner {
        require(signers[msg.sender][_id] == false, "Already approved");
        Transfer storage transfer = transfers[_id];
        signers[msg.sender][_id] = true;
        transfer.approvals++;
        
        if(transfer.approvals >= limitApproval ) {
            _executeTransfer(transfer);
        }
    }

    function _executeTransfer(Transfer storage _transfer) private {
        _transfer.to.transfer(_transfer.amount);
        _transfer.status = TransferStatus.Sent;
    }
}