Thanks Carlos,
Sorry for the delay, I was full on my new startup.
I understand your points.
Regarding your first point, I guess I see how to fix it but I’m not sure.
Here is the new version of my contract with some other extra improvements, I’m keeping track of the totalRequested as a state variable so that I don’t have to calculate it every time. Does it fix the problem or is it introducing new concerns?
pragma solidity >=0.7.0 <0.9.0;
pragma abicoder v2;
abstract contract Ownable {
uint internal numberOfApprovals;
mapping (address => bool) owners;
modifier onwersAccessOnly{
require(owners[msg.sender],'Accessible by owners only');
_;
}
}
//If it was in another file Owner.sol at the same level
//import "./Ownable.sol";
abstract contract Destroyable is Ownable {
function destroy() public onwersAccessOnly {
selfdestruct(payable(msg.sender));
}
}
contract Multisig is Destroyable{
struct TransferRequest {
uint id;
address recipient;
address[] acceptedBy;
uint amount;
//bool paid;
}
mapping (uint => TransferRequest) transferRequests;
uint totalRequested;
uint lastId;
event moneyDeposited(address _owner,uint _amount);
//transfer request id,requester,transfer recipient, amount
event transferRequested(uint _transferRequestId,address _requester,address _recipient,uint _amount);
//transfer request id,transfer recipient, amount
event transferAccepted(uint __transferRequestId,address _requester,uint _amount);
//example ["0x1aE0EA34a72D944a8C7603FfB3eC30a6669E454C","0x0A098Eda01Ce92ff4A4CCb7A4fFFb5A43EBC70DC","0xCA35b7d915458EF540aDe6068dFe2F44E8fa733c"],2
constructor(address[] memory _owners,uint _numberOfApprovals ) {
require(_numberOfApprovals < _owners.length && _numberOfApprovals > 0 , 'Invalid number of approvals');
for(uint i = 0 ; i < _owners.length ; i++){
owners[_owners[i]] = true;
}
numberOfApprovals=_numberOfApprovals;
}
function deposit () public onwersAccessOnly payable returns (uint){
emit moneyDeposited(msg.sender,msg.value);
return address(this).balance;
}
//example "0x14723A09ACff6D2A60DcdF7aA4AFf308FDDC160C",2000000000000000000
//example "0x583031D1113aD414F02576BD6afaBfb302140225",4000000000000000000
//amount int wei
function requestTransfer (address _recipient, uint _amount) public onwersAccessOnly returns (uint) {
require(_amount!=0,"Cannot request a transfer with 0 amount");
require(address(this).balance>=_amount,"Not enough money in the wallet to accept the transfer request");
require(address(this).balance-totalRequested>=_amount,"Too much money already pledged to accept the transfer request");
/* uint totalRequested=0;
uint size=0;
//to check if this key exist
while (transferRequests[size].amount!=0){
if (!transferRequests[size].paid){
totalRequested+=transferRequests[size].amount;
}
size++;
}*/
TransferRequest memory _transferRequest=TransferRequest(lastId,_recipient,new address[](0),_amount/*,false*/);
transferRequests[lastId]=_transferRequest;
transferRequests[lastId].acceptedBy.push(msg.sender);
emit transferRequested(_transferRequest.id,msg.sender,_transferRequest.recipient,_transferRequest.amount);
_attemptApproval(lastId);
totalRequested+=_amount;
lastId++;
return _transferRequest.id;
}
//recipient, amount, #approvals
function getRequestTransferDetails(uint _transfertRequestId) public view onwersAccessOnly returns(TransferRequest memory) {
_transfertRequestExists;
return transferRequests[_transfertRequestId];
}
function getRequiredNumberOfApprovals() public view onwersAccessOnly returns(uint) {
return numberOfApprovals;
}
function approve(uint _transfertRequestId) public onwersAccessOnly {
_transfertRequestExists;
bool alreadyApproved;
for (uint i=0;i< transferRequests[_transfertRequestId].acceptedBy.length;i++){
if (transferRequests[_transfertRequestId].acceptedBy[i]==msg.sender){
alreadyApproved=true;
break;
}
}
if (!alreadyApproved){
transferRequests[_transfertRequestId].acceptedBy.push(msg.sender);
_attemptApproval(_transfertRequestId);
}
}
function _attemptApproval (uint _transfertRequestId) private {
require(_transfertRequestExists(_transfertRequestId), "operator query for nonexistent transaction");
TransferRequest memory _transferRequest=transferRequests[_transfertRequestId];
if ( _transferRequest.acceptedBy.length==numberOfApprovals){
_acceptTransfer(_transferRequest.id);
}
}
function _acceptTransfer (uint _transfertRequestId) private {
require(_transfertRequestExists(_transfertRequestId), "operator query for nonexistent transaction");
TransferRequest memory _transferRequest=transferRequests[_transfertRequestId];
require(address(this).balance>=_transferRequest.amount,"Not enough money in the wallet to accept the transfer");
payable(_transferRequest.recipient).transfer(_transferRequest.amount);
totalRequested-=_transferRequest.amount;
emit transferAccepted(_transferRequest.id,_transferRequest.recipient,_transferRequest.amount);
//transferRequests[_transfertRequestId].paid=true;
delete transferRequests[_transfertRequestId];
}
function _transfertRequestExists (uint _transfertRequestId) private view returns(bool) {
//way of checking if key exists
//require(transferRequests[_transfertRequestId].amount>0);
return transferRequests[_transfertRequestId].recipient != address(0);
}
/*
To know the contract total deposited amount
*/
function getWalletBalance() public view returns(uint256) {
return address(this).balance;
}
}```
? is it perfect?
