Project - Multisig Wallet

Elekko · January 21, 2021, 10:47pm

Thanks alot thecil!
I got it at last - project is done!

Elekko · January 21, 2021, 10:50pm

Here is my code, not the perfect one, but I tested it and it works… tried several transactions - in addition for this particular one “isSent” variable is not actually needed:

pragma solidity 0.7.5;
pragma abicoder v2;

contract MultiSigWallet {

    address[] public owners;
    uint signlimit;

    struct Transfer {
        uint amount;
        address payable recipiant;
        uint noApprovals;
        bool isSent;
    }

    Transfer[] transferRequests;

    constructor(address[] memory _owners, uint _signlimit) {
        owners = _owners;
        signlimit = _signlimit;
    }
    

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

    function getApprovals(uint _transferID) public returns(bool) {
        return approvals[msg.sender][_transferID];
    }

    function approve(uint _transferID) public onlyOwners {
        
        require(approvals[msg.sender][_transferID] == false, "You cannot approve same transaction twice.");
        
        approvals[msg.sender][_transferID] = true;
        transferRequests[_transferID].noApprovals+=1;            
        
        if(transferRequests[_transferID].noApprovals == signlimit) {
            transferRequests[_transferID].isSent = true;
            transferRequests[_transferID].recipiant.transfer(transferRequests[_transferID].amount); 
        }
            
        
    }
    
    function getTransfers() public view returns(Transfer[] memory) {
        return transferRequests;
    }
    
    
    function transfer(uint _amount, address payable _recipiant) public {
        
        Transfer memory transfer;
        transfer.amount = _amount;
        transfer.recipiant = _recipiant;
        transfer.noApprovals = 0;
        transfer.isSent = false;
        transferRequests.push(transfer);

    }

    modifier onlyOwners {
        
        bool isOwner = false;
        for(uint i = 0; i < owners.length; i++) {
            if(owners[i] == msg.sender) {
                isOwner = true;
            }
        }
        require(isOwner == true, "You don't have permission to execute onlyOwner functions.");
        _;
    }

    function deposit() public payable {
    }

}

thecil · January 21, 2021, 11:50pm

Amazing @Elekko, good job!

Now I have run your contract and it works perfect.

I deployed 3 addresses, signlimit of 2.
Deposit funds on the contract.
Created a transfer proposal.
Approve with 2 owners and the transaction goes valid right after approving it with the 2nd owner.
Check recipient balance, so your contract can deposit, approve and transfer funds.

Some Suggestions:

You could have a function to check the contract balance. So it will be easier to track the funds of the contract.

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

Also, in your transfer function, you are declaring a transfer object which contain the same name of the function, then remix shows this error which suggest to change the name of the variables to something different than the function name.

So just by renaming the object to another variable name does the trick.

    function transfer(uint _amount, address payable _recipiant) public {
        
        Transfer memory newTransfer;
        newTransfer.amount = _amount;
        newTransfer.recipiant = _recipiant;
        newTransfer.noApprovals = 0;
        newTransfer.isSent = false;
        transferRequests.push(newTransfer);

    }

Overall, amazing job! Congratulations!

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

Carlos Z.

Andro · January 22, 2021, 12:05am

Yes, sure. As shown in Filip’s video, I paste the addresses and the limit number (I used 2 like Filip did) and then deploy.
This is literally what I entered now and it works:

[“0x5B38Da6a701c568545dCfcB03FcB875f56beddC4”, “0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2”, “0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db”], 2

Screenshot:

thecil · January 22, 2021, 7:47pm

Yep, I manage to deploy it, was my fault sorry, Now i have tested it and its looks very well commented and also working as spected.

I deployed 3 addresses, signlimit of 2.
Deposit funds on the contract.
Created a transfer proposal.
Approve with 2 owners and the transaction goes valid right after approving it with the 2nd owner.
Check recipient balance, so your contract can deposit, approve and transfer funds.

Overall very great job!

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

Carlos Z.

thecil · January 24, 2021, 3:55pm

Here is mine, keep it simple so others can understand it easily .

//"SPDX-License-Identifier: MIT"
pragma solidity 0.7.5;
pragma abicoder v2;
contract Ownable{
  address payable private owner;

  modifier onlyOwner{
    require(msg.sender == owner,"Only contract owner allowed");
    _;
  }
  constructor(){
    owner = msg.sender;
  }
}

contract multiSig is Ownable{
    address[] private owners;
    uint limit;

    struct Transfer{
        uint amount;
        address payable receiver;
        uint approvals;
        bool hasBeenSent;
        uint id;
    }
    event depositDone(address indexed _from, uint _amount);
    event transferDone(address indexed _from, address indexed _to, uint _amount);
    event transferRequested(address indexed _from, uint _amount);
    event transferApproved(uint _id, address indexed _from, uint _amount);

    Transfer[] internal transferRequests;

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

    //Should only allow people in the owners list to continue the execution.
    modifier onlyOwners(){
      bool isOwner = false;
      for(uint _index = 0; _index < owners.length; _index++){
        if(owners[_index] == msg.sender){
          isOwner = true;
        }
      }
      require(isOwner == true, "Only multisig owners allow");
      _;
    }

    //Should initialize the owners list and the limit
    constructor(address _owner1, address _owner2, address _owner3, uint _limit) {
      require(_owner1 != _owner2 && _owner1 != _owner3 && _owner2 != _owner3, "Signatures should be different");
      owners = [_owner1, _owner2, _owner3];
      limit = _limit;
    }

    //Empty function
    function deposit() public payable {
      require(msg.value > 0, "Value should be above zero");
      emit depositDone(msg.sender, msg.value);
    }

    //Create an instance of the Transfer struct and add it to the transferRequests array
    function createTransfer(uint _amount, address payable _receiver) public onlyOwners {
      require(address(this).balance > _amount, "Insuficient Contract Funds");
      transferRequests.push(Transfer(_amount, _receiver, 0, false, transferRequests.length));
      emit transferRequested(_receiver, _amount);
    }

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

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

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

Carlos Z.

gentledepp · January 24, 2021, 2:46pm

I tried to keep it as dynamic as possible.
One could potentially enhance this contract to allow afterwards changing the minimum approvers count and add/remove owners on-the-fly. To allow for this, the minimum number of approvers is stored on each transaction when it is created.

Also, I opted to use an array to store the transactions as that way, I could remove those that have successfully been approved to reduce the memory footprint.
Instead, I emit an event that contains the most important data of any transaction as I recall events are cheaper then normal smart contract state? (is that actually true)

Wallet.sol

pragma solidity 0.7.5;
pragma abicoder v2;

import "./Ownable.sol";


contract Wallet is Ownable {
    
    uint minVotes = 1;
    address[] signers;
    mapping(address => bool) owners;
    
    // some operations are only valid for wallet owners
    modifier onlyWalletOwner {
        require(owners[msg.sender] == true, "must be wallet owner");
        _;
    }
    
    // adds owner to signers list
    // so even if the owner forgets to add his address to the list of signers, it is added automagically
    constructor(address[] memory _signers, uint _minVotes) {
        
        signers = _signers;
        
        bool ownerFound = false;
        for(uint i = 0; i < signers.length; i++){
            ownerFound = signers[i] == owner;
            owners[signers[i]] = true; // add to owners mapping
        }
        if(!ownerFound)
            signers.push(owner);
            
        // setup mapping of owners that is used by "onlyWalletOwner" modifier
        owners[owner] = true;
            
        require(signers.length >= minVotes, "there must be enough signers to reach minimum votes");
        minVotes = _minVotes;
    }
    
    // allow anyone to deposit
    function deposit() public payable returns (uint) {
        return address(this).balance;
    }
    
    // get all addresses that can sign
    function getWalletOwners() public view onlyWalletOwner returns (address[] memory){
        return signers;
    }
    
    struct Transfer{
        uint id;
        address to;
        uint amount;
        // rememver "minVotes" count. That way we could opt to allow to change that count for new transactions later
        uint minVotes;
        address[] approvers;
    }
    uint transferId;
    Transfer[] transfers;
    uint pendingTransferSum = 0;
    

    function requestTransfer(address _to, uint _amount) public onlyWalletOwner {
        require(_amount <= address(this).balance, "cannot spend more than you have...");
        require(pendingTransferSum + _amount <= address(this).balance, "too many creadits waiting to be sent. Cannot add transfer request");
        require(_to != address(this), "do not send yourself money. This is a waste of time...");
    
        address[] memory approvers = new address[](1);
        approvers[0] = msg.sender;
        Transfer memory t = Transfer(transferId++,_to,_amount, minVotes, approvers);
        
        transfers.push(t);
        
        // add to pending transaction sum
        pendingTransferSum += _amount;
    }
    
    function approveTransfer(uint _transferId) public onlyWalletOwner {
    
        bool transferFound = false;
        Transfer storage transfer;
        uint index = 0;
        for(uint i = 0; i < transfers.length; i++){
            if(transfers[i].id == _transferId){
                transferFound = true;
                transfer = transfers[i];
                index = i;
            }
        }
        
        require(transferFound, "not a valid transfer id");
        require(!containsAddress(transfer.approvers, msg.sender), "you already approved that transfer");
        
        transfer.approvers.push(msg.sender);
        
        if(isApproved(transfer)){
            // perform transaction
            _transfer(address(this), transfer.to, transfer.amount);    
            
            // reduce transfer amount
            pendingTransferSum -= transfer.amount;
            
            // remove transfer to save space and money 🤑
            delete transfers[index];
            
            // emit event
            emit Transferred(transfer.to, transfer.amount, transfer.approvers, transfer.minVotes);
        }
    }
    
    function getPendingTranfserAmount() public view onlyWalletOwner returns (uint){
        return pendingTransferSum;
    }
    
    // returns all pending transfers
    function getPendingTransfers() public view onlyWalletOwner returns (Transfer[] memory) {
        return transfers;
    }
    
    // returns all pending transfers THIS user can still signers
    function getSignableTransfers() public view onlyWalletOwner returns (Transfer[] memory){
        
        // first count number of signable tranfsers so we can actually create a bounded array
        uint signableCount = 0;
        for(uint i = 0; i < transfers.length; i++){
            // get all transfers that the msg.sender did not yet approve
            if(!containsAddress(transfers[i].approvers, msg.sender)){
                signableCount ++;
            }
        }
        
        // create bounded array
        Transfer[] memory pending = new Transfer[](signableCount);

        // second run: fill the array with signable transfers        
        uint addedIndex = 0;
        for(uint i = 0; i < transfers.length; i++){
            // get all transfers that the msg.sender did not yet approve
            if(!containsAddress(transfers[i].approvers, msg.sender)){
                pending[addedIndex] = transfers[i];
                addedIndex ++;
            }
        }
        
        return pending;
    }
    
    function isApproved(Transfer storage _t) private view returns (bool){
        return _t.approvers.length >= _t.minVotes;
    }
    
    function containsAddress(address[] memory _list, address _address) private pure returns (bool){
        for(uint i = 0; i < _list.length; i++){
            if(_list[i] == _address)
                return true;
        }
        return false;
    }
    
    event Transferred(address indexed to, uint amount, address[] approvers, uint minApprovers);
    
    function _transfer(address _from, address _to, uint _amount) private {
    
        uint _oldFrom = address(this).balance;
        
        require(_from != _to, "don't send stuff to yourself");
        require(_oldFrom >= _amount, "insufficient funds"); 
        require(_amount > 0, "cannot send nothing"); 
        require(_oldFrom - _amount < _oldFrom, "avoid negative overflow"); 
    
        // actually transfer the amount
        address payable _recipient = payable(_to);
        _recipient.transfer(_amount);        
    }
}

Ownable.sol:

pragma solidity 0.7.5;

contract Ownable{
  address payable internal owner;
    
  modifier onlyOwner {
      require(msg.sender == owner);
      _;
  }
  constructor(){
      owner = msg.sender;
  }
}

Mark_Dalton · January 25, 2021, 5:13am

Sigh… I struggled to long for a ‘dumb’ mistake… And I went through and redid the script a few times. The thing I missed making it so the transfers failed/reverted was there was no deposit done first… so requests were insufficient… I added the event/emit to ensure the balance was sufficient before adding a new transfer request.

One question in ‘best practices’ it said not to use transfer, but use ‘call’ instead, do you agree?
https://consensys.github.io/smart-contract-best-practices/recommendations/
Or is this out of date or perhaps you were just being nice to us to make it easier.

Anyway… my wallet is here…

// Marks MultiSig wallet - Ethereum 101
pragma solidity 0.7.5;
pragma abicoder v2;

contract MyWallet{
    address[] public approvers;
    uint approvalsRequired;
    
    mapping(address => mapping(uint => bool)) approveTransaction;
    
    modifier onlyOwners(){
        bool approver = false;
        for(uint i=0; i<approvers.length;i++){
            if(approvers[i] == msg.sender){
                approver = true;
            }
        }
        require(approver == true);
        _;
    }
    
    constructor(address[] memory _approvers, uint _approvalsRequired) {
        approvers = _approvers;
        approvalsRequired = _approvalsRequired;
    }

    event depositDone(uint amount, address indexed depositedFrom);
    event newPendingTransfer(uint _id, address _requester, address _recipient, uint _amount, string _comment);
    event ApprovalReceived(uint _id, uint _approveCount, address _approver, string _comment);
    event sentTransfer(uint _id);

    struct Transfer{
        uint amount;
        address payable recipient;
        uint approveCount;
        bool sent;
        uint id;
    }
 
    Transfer[] requestedTransfers;
 
    function deposit() public payable {
        emit depositDone(msg.value, msg.sender);
    }
 
    function requestedTransfer(address payable _recipient, uint _amount, string memory _comment) public onlyOwners {
        // Confirm there is sufficient balance.. sigh it bit me.. I was wondering why my transfer failed..
        require(address(this).balance > _amount, "Contract balance is insufficient to meet request.");
        emit newPendingTransfer(requestedTransfers.length, msg.sender, _recipient, _amount, _comment);
        requestedTransfers.push(
             Transfer(_amount, _recipient, 0, false, requestedTransfers.length)
        );
     }

     function approveRequest(uint _id, string memory _comment) public onlyOwners {
          // Do not allow two approvals by same address
          require(approveTransaction[msg.sender][_id] == false, "Cannot approve twice from same address");
          // Do not approve if already sent
          require(requestedTransfers[_id].sent == false, "Transfer was already sent");
          
          approveTransaction[msg.sender][_id] = true;              // Set that the sender voted/approved
          requestedTransfers[_id].approveCount++;                  // Increment the number of approvals
          
          // I was wanting the comment - thinking the dapp it would be useful to know what/why
          emit ApprovalReceived(_id, requestedTransfers[_id].approveCount, msg.sender, _comment);

          if(requestedTransfers[_id].approveCount == approvalsRequired) {   // Check if enough approvals to start the transfer
              requestedTransfers[_id].sent = true;
              requestedTransfers[_id].recipient.transfer(requestedTransfers[_id].amount); // Transfer
              emit sentTransfer(_id);
          }
      }

    // list the Requested Transfers (both pending and approved/sent requests)
    function listRequestedTransfers() public view returns (Transfer[] memory){
        return requestedTransfers;
    }
}

Sil · January 25, 2021, 8:58am

So, I found it pretty hard to make the multisig wallet on my own. Maybe it’s because programming is a whole new language for me, but eventually I wanted to add some extra features to my multisig wallet. The problem is that every time I want to create a transaction and send it, it fails doing so. It keeps saying the same thing that’s

“transact to Wallet.createTransfer errored: VM error: revert. revert The transaction has been reverted to the initial state. Note: The called function should be payable if you send value and the value you send should be less than your current balance. Debug the transaction to get more information.”

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;
    }
    
    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;
    }
    
    event transferRequestAdded(uint _id, address RequestedBy, address recipient, uint amount);
    event ApprovalReceived(uint _id, uint _approvals, address _approver);
    event TransferSent (uint _id);
    
    //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 {
        require(address(this).balance >= _amount, "You can't send more than the balance.");
          emit transferRequestAdded(transferRequests.length, msg.sender, _receiver, _amount);
          transferRequests.push(
              Transfer(_amount, _receiver, 0, false, transferRequests.length)
              );
    }
    
    //EVENTS AFTER transferRequests
    //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(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 TransferSent(_id);
        }
    }
    
    //Should return all transfer requests
    function getTransferRequests() public view returns (Transfer[] memory){
        return transferRequests;
    }
    
    function getBalance() public view returns(uint){
        return address(this).balance;
    }
    
    
}

I hope someone knows what I’m doing wrong

gkassaras · January 25, 2021, 11:12pm

pragma solidity 0.7.5;

pragma abicoder v2;

contract MyWallet {
    
    address[] public walletOwners; // Wallet owner addresses 
    uint txtLimit; //Transaction signature limit
    
    struct txtTransfer{ // Transaction transfer struct 
        
        uint amount;
        address payable theReceiver;
        uint txtVotes;
        bool wasSend;
        uint txtId;
    }
    
    // Logging events
    event TransferRequestCreated(uint _id, uint _amount, address _initiator, address _receiver);
    event ApprovalReceived(uint _id, uint _approvals, address _approver);
    event TransferApproved(uint _id);
    
    txtTransfer[] txtRequests;
    
    mapping(address => mapping(uint => bool)) txtVotes;
    
    
    modifier onlyOwners(){ //Check if owner is within the walletOwners array
        
        bool _walletOwner = false;
        uint _counter = 0;
        
        while (_walletOwner != true) { // Rotate throught he walletOwners array to see if the sender is among them
        
            if(walletOwners[_counter] == msg.sender) {
                
                _walletOwner = true;
            }
            
            else {
                
                _counter++;
            }
        }
            
        require(_walletOwner == true);
        _; // Continue execution 
    }
    
    /////////////////////////////////////////////////////////////////
    //Initialize txtLimit and walletOwners on contract startup
    /////////////////////////////////////////////////////////////////    
    
    constructor(address[] memory _walletOwners, uint _txtLimit) {
        walletOwners = _walletOwners;
        txtLimit = _txtLimit;
        
    }
    
    function deposit() public payable {// Deposit function
        // Empty body function
    }
    
    function createTransfer(uint _amountToSend, address payable _receiver) public onlyOwners {//Initialize the txtTransfer struct and add it to the transferRequests array
        emit TransferRequestCreated(txtRequests.length, _amountToSend, msg.sender, _receiver);
        txtRequests.push(txtTransfer(_amountToSend,_receiver,0,false,txtRequests.length));
    }
    
    function checkVotes(uint _id) public onlyOwners {
        require(txtVotes[msg.sender][_id] == false);
        require(txtRequests[_id].wasSend == false);
        
        txtVotes[msg.sender][_id] = true;
        txtRequests[_id].txtVotes++;
        
        emit ApprovalReceived(_id, txtRequests[_id].txtVotes, msg.sender);
        
        if(txtRequests[_id].txtVotes >= txtLimit){
            txtRequests[_id].wasSend = true;
            txtRequests[_id].theReceiver.transfer(txtRequests[_id].amount);
            emit TransferApproved(_id);
        }
    }
}

I tried to deploy the project and get this error:
creation of MyWallet errored: Error encoding arguments: Error: expected array value (argument=null, value="", code=INVALID_ARGUMENT, version=abi/5.0.7)

MrSeven · January 26, 2021, 3:18am

Hello, here is my code. I did my best without looking at any of the next videos for hints or help. It seems to work according to Fillip’s instructions.

pragma solidity 0.7.5;
pragma abicoder v2;

contract MultiSegWallet {

address owner1;
address owner2;
address owner3;
uint numberApprovals;
uint approvalCounter;
address firstApprover;
address secondApprover;

constructor(address _owner1, address _owner2, address _owner3, uint _approvals)  {
    owner1 = _owner1;
    owner2 = _owner2;
    owner3 = _owner3;
    numberApprovals = _approvals;
    require(numberApprovals > 1, "You need to enter a number greater than 1.");
}

struct Request {
    address originator;
    address from;
    address to;
    uint amount;
    address secondApprover;
    bool transferCompleted;
}

struct Transfer  {
    address from;
    address to;
    uint amount;
}

modifier onlyOwner {
    require(msg.sender == owner1 || msg.sender == owner2 || msg.sender == owner3, "Only a contract owner can make this request!");
    _;
}

mapping(address => uint) private balance;
uint contractBalance;

mapping(address => Request) private requestLedger;
mapping(address => Transfer) private transferLedger;


function deposit(uint amount) public payable returns (uint) {
    balance[msg.sender] += amount;
    contractBalance += amount;
    return balance[msg.sender];
}

function getBalance(address account) public view returns (uint) {
    return balance[account];
}

function getContractBalance() public view returns (uint) {
    return contractBalance;
}

function createRequest(address _from, address _to, uint _amount) public onlyOwner {
    Request memory newRequest;
    newRequest.originator = msg.sender;
    newRequest.from = _from;
    newRequest.to = _to;
    newRequest.amount = _amount;
    newRequest.secondApprover;
    requestLedger[msg.sender] = newRequest;
    firstApprover = msg.sender;
    approvalCounter += 1;
    if (approvalCounter < numberApprovals)  {
        newRequest.transferCompleted = false;
    }
    requestLedger[msg.sender] = newRequest;
}

function getTransferRequest() public view returns (Request memory)    {
    return requestLedger[msg.sender];
}

function getApproval() public onlyOwner {
    require(msg.sender != firstApprover);
    secondApprover = msg.sender;
    approvalCounter +=1;
}

function transferFunds(address originator) public returns (uint) {
    if (approvalCounter >= numberApprovals && requestLedger[originator].transferCompleted == false) {
        Transfer memory transaction;
        transaction.from = requestLedger[originator].from;
        transaction.to = requestLedger[originator].to;
        transaction.amount = requestLedger[originator].amount;
        transferLedger[msg.sender] = transaction;
        balance[transaction.from] -= transaction.amount;
        balance[transaction.to] += transaction.amount;
        requestLedger[originator].transferCompleted = true;
        approvalCounter = 0;
    }
}

}

thecil · January 26, 2021, 6:25pm

Hi @Mark_Dalton, hope you are great.

I have tested your code and it works perfectly, congratulations!

Now about the best practices, the article that you mention is way outdated, some of those suggestions or security improvements has been fixed by programmers team like OpenZepellin, also in every new solidity version they improve a lot of points.

If you are interested to get more indeep of solidity programming I can suggest you to follow “Eat The Blocks” in youtube and check OpenZepellin contracts

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

Carlos Z.

thecil · January 26, 2021, 6:32pm

basically the console said that you have an error in the createTransfer function, the error point to a payable method for the function to execute, so lets check your createTransfer function.

Sil:

//Create an instance of the Transfer struct and add it to the transferRequests array
    function createTransfer(uint _amount, address payable _receiver) public onlyOwners {
        require(address(this).balance >= _amount, "You can't send more than the balance.");
          emit transferRequestAdded(transferRequests.length, msg.sender, _receiver, _amount);
          transferRequests.push(
              Transfer(_amount, _receiver, 0, false, transferRequests.length)
              );
    }

The requested argument is address payable _receiver, since you are declaring this argument as payable is expected to receive a value (msg.value), but you dont need to declare as payable because in the function body there is nothing that will use msg.vale or transfer funds, you are just adding to the array a transferRequest struct that will later be used to approved by the other multisig owners.

So just by deleting the payable keyword, should fix the issue (because you dont need to specify payable for this case).

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

Carlos Z.

thecil · January 26, 2021, 6:58pm

Hey @gkassaras, hope you are great.

It could be that you are setting the array variables incorrectly.
It goes something like this:

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

Meaning open brackets to sent the values for the array with [], then each item should be set inside "" and a comma for each item ,.

I try it myself in your contract and it deploys very well!

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

Carlos Z.

Javier_Flores · January 26, 2021, 9:52pm

Hello,

I have a question regarding project, hopefully you guys can help me.

I am building a completely new contract or adding code to the bank.sol?
If so will it be inherited process?

Thank you.
JF

Andro · January 27, 2021, 3:20am

Thanks for the solution Carlos.
Could you explain how we can create a function that tracks balances of every owner’s address?
For example if we deposit 10 ETH to Owner1 address, after we request a transfer and send 7 ETH to another address Owner1’s balance should now be 3 ETH.

Sil · January 27, 2021, 10:39am

Thanks a lot, really appreciate your help!! It only says that _receiver must be a payable address because the address (receiver) in the struct is payable. That doesn’t mean that “receiver” in the struct has to be changed right? That would only make the address not compatible with transferring ETH as it’s not have a payable functionality. Or am I wrong? Kinda a rookie right now

thecil · January 27, 2021, 4:43pm

Hi @Javier_Flores, hope you are ok.

I do not understand quite well this question, for the multisig project you should create a new contract, but if there is any code that can be used from the bank contract, off course is valid to just copy/paste some of the functions.

Maybe you have not understand the Inheritance concept that well.
Inheritance allows you to made more modular contracts, for example the Ownable contract, you could have all its logic on the bank contract for example, but in order to create a more clear structure on what each contract does, you use inheritance.
Example:

pragma solidity 0.7.5;

contract A {
    //declare a INTERNAL variable
    uint internal _number = 10;
}

contract B is A{
    //call the Contract A internal variable.
    function getNumber() public view returns(uint){
        return _number;
    }
}

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

Carlos Z.

thecil · January 27, 2021, 4:44pm

That will be easy with a mapping for balance, like the one is used in the bank contract from filips lessons

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

Carlos Z.

thecil · January 27, 2021, 4:45pm

correct, you dont need to change anything in your contract, rather than just delete the payable keyword from the arguments of createTransfer function.

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

Carlos Z.