hey @Kacper. Ok so there is a few things here. I have fixed your code but there is still a lot you should do and change. I will leave that up to you but i will give you some notes or tips on the things you can improve on.
First lets look at why your code above did not work. It had nothing to do with you only owners modifier. The way you could have known this before hand is because you were able to successfully call the createTransfer function which also was dependant on this modifier. The problem with your logic lied with the first line of code in your apprive function. Namely
function approve(uint _id) public onlyOwners {
` approvals[msg.sender][_id] == true;`
.........
........
This is not an assinment but rather an assertion or check. And if you did want to assert this you would need to either use an if statment or the assert() function. So you should change this to
approvals[msg.sender][_id] = true;
However i do not like this approach for handling approvals it is a bit counter intuitive, especially since you set the needed approvals to 0 in your transfer struct and then deincrement it each time in your original code? For an outside reader Its a bit hard to understand. So i changed this attribute in your transfer struct to be equal to the approval limit which you define in the constructor.
I increment its value each time the approve function is called and then i increment the approvals attribute for your tansfer
approvals[msg.sender][_id] += 1;
transferRequests[_id].approvalsNeed++; //changed from transferRequests[_id].approvalsNeed--;
The last change i amde was in your if statement. I check if the transfer approval count is equal to the limit if it is we execute the transfer
if(transferRequests[_id].approvalsNeed == approvalCount){
transferRequests[_id].receiver.transfer(transferRequests[_id].amount);
emit transferDone(_id, transferRequests[_id].amount, msg.sender, transferRequests[_id].receiver);
//address(this).balance -= _amount;
}
Your code now works. But there is many things you should fix. Firstly in your createTransfer Function you should require that
require(balance[msg.sender] >= _amount, "Balance not sufficient");
and not
require(address(this).balance >= _amount, "Balance not sufficient");
This is because the contract balance stores the total funds deposited into the smart contract from every and any user who uses it. Where as our balance mapping stores individual user balances.
Another thing is in your constructor. You should not pass in the limit but rather dynamically calculate it based on the length of your user array. The way you currently have it is that you pass it in. However theere is nothing stopping you from passing in a limit that is greater than the length of your owners array which in combination with your only owner modifier restriction would break your code. In your constructor smething like this would suffice
Approvlimit = owners.length -1;
There are a few other things you could change to really make this a kickass contract. I suggest reading up a good bitin the forums and see what other people are doing the forums are really great resources to learn.
Other than that really well done man this is a great attempt and it really looks like your own solution and not the one from the vide so hats off because i rememberd struggling when doing this assignemnt. Anyways congrats for finishing the course and see u in the next one.
Happy coding,
Evan
also here is the snippet of code i edited iin remix to get it working. I only made as litle changes to get it working rather than grealty improving it
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.4;
pragma abicoder v2;
//["0x5B38Da6a701c568545dCfcB03FcB875f56beddC4", "0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2", "0xCA35b7d915458EF540aDe6068dFe2F44E8fa733c"], 2
contract MultiSignWallet {
address[] public owners;
mapping(address => uint) balance;
uint public id = 0;
uint Approvlimit;
struct Transfer{
uint id;
uint amount;
address payable receiver;
uint approvals;
bool sent;
}
Transfer[] transferRequests;
mapping(address => mapping(uint => uint)) approvals;
event transferRequeststDone(uint id, uint amount, address initiator, address indexed requestedTo);
event approvalDone(uint id, uint approvalsNeed, address approver);
event transferDone(uint id, uint amount,address lastApprover, address senTo);
//make sure only owner - addresse in owners array will call a certain function
modifier onlyOwners{
bool addressallowed = false;
uint len = owners.length;
//check if msg.sender is an owner.
for(uint i=0; i<len; i++){
if (owners[i] == msg.sender) {
addressallowed = true;
}
}
require(addressallowed = true, "You are not an owner of this wallet");
_;
}
modifier differentOwners{
uint len = owners.length;
bool ownersNotSame = true;
for(uint i=0; i<len; i++){
for(uint j=i+1; j<len; j++){
if (owners[i] == owners[j]) ownersNotSame = false;
}
}
require(ownersNotSame = true);
_;
}
constructor(address[] memory _owners,) differentOwners {
uint len = owners.length;
bool ownersNotSame = true;
for(uint i=0; i<len; i++){
for(uint j=i+1; j<len; j++){
if (owners[i] == owners[j]) ownersNotSame = false;
}
}
require(ownersNotSame = true);
Approvlimit = owners.length -1;
owners = _owners;
}
function deposit() public payable {
balance[msg.sender] += msg.value;
}
function getLimit() public view returns (uint) {
return Approvlimit;
}
function getBalance() public view returns(uint) {
return balance[msg.sender];
}
function getContractBalance() public view returns(uint) {
return address(this).balance;
}
function getOwners() public view returns(address[] memory) {
return owners;
}
function createTransfer(uint _amount, address payable _receiver) public onlyOwners {
require(address(this).balance >= _amount, "Balance not sufficient");
transferRequests.push(Transfer(id, _amount, _receiver, 0, false));
id++;
}
function approve(uint _id) public onlyOwners {
approvals[msg.sender][_id] += 1;
transferRequests[_id].approvals++;
emit approvalDone(_id, transferRequests[_id].approvals, msg.sender);
if(transferRequests[_id].approvals == 2){
transferRequests[_id].receiver.transfer(transferRequests[_id].amount);
emit transferDone(_id, transferRequests[_id].amount, msg.sender, transferRequests[_id].receiver);
//address(this).balance -= _amount;
}
}
function getTransferRequests() public view returns(Transfer[] memory){
return transferRequests;
}
}
