agrhhhh it’s always the small things that get you , thanks Carlos
1 Like
@thecil, Hello, I would appreciate your feedback on my code. I know it need improvements but I have not looked the other videos so that was what I thought it is right to do? thanks, vilior
1 Like
Hi @thecil, I’m curious about the check that we make for availability of funds before attempting a transfer. Do we need to account for fees? Is it actually the case that we can spend every wei that a contract owns?
The relevant part of my payment function:
require(m_approvable_payments[a_transaction_id].amount <= m_contract_balance_wei, "Insufficient contract funds to attempt payment."); // account for fees?
1 Like
Glady sir!
There is an unused variable here:
function addOwner(address from) public {
owners.push(Owner(msg.sender, false));
}
the argument from is not in use, instead you are pushing the msg.sender and every one could add any amount of owners that they like, althoug it will add the function caller (msg.sender). Maybe you want to add a modifier and change msg.sender to from.
You should add the keyword view in all functions that should return a value that you want to check
function getOwner (uint _index) public returns (address, bool){
return (owners[_index].from, owners[_index].signed);
}
In this function I advice you to always use the pattern “check, effects, interactions”.
For check, all require goes first, to validate the inputed data. I remember filip explained quite detail in one of the videos.
function transfer(address recipient, uint amount) public {
uint ownersApproved = 0;
for (uint i=0; i<owners.length; i++){
if (owners[i].signed == true){
ownersApproved += ownersApproved;
}
}
require(ownersApproved >= 2, "Not all the owners approved the trnasaction");
require(balance[msg.sender] >= amount, "Balance not sufficient");
require(msg.sender != recipient, "Don't transfer money to yourself");
uint previousSenderBalance = balance[msg.sender];
_transfer(msg.sender, recipient, amount);
assert(balance[msg.sender] == previousSenderBalance - amount);
}
Overall you have made a good try, but taking in consideration that everyone can approve, add owners or what ever, because you are not using any limitation on who can do what, even an account that is non-owner, can add himself and then approve others transactions. 
Carlos Z
The accounts fees are deducted when the transactions is created by the user (in this case the wallet manager like metamask), the execution of the function will have a fee since the caller (msg.sender) will execute the function and he must pay the fees to execute those instructions.
So the funds of the contract does not pay for the fees, is the caller (msg.sender).
Carlos Z
1 Like
Thank you @thecil. Your comments on my below submission are very welcome.
I’ve completed the assignment and have uploaded the code to https://github.com/yeleva/blockchain-solidity-multi-sig-wallet
For convenience I’ve also included the code below. Some development and usage notes:
-
This is original work. I didn’t look ahead at the additional videos until completing the code as I was curious to see how my solution compared.
-
The contract accepts payments into an aggregated pool and responds to requests to withdraw by addresses configured at deployment only. See
m_approver_addresses. -
Withdrawals are only actioned when a minimum number of withdraw requests (also known as approvals) for a particular (indexed) withdrawal is reached. See
m_approver_addresses. -
Each withdrawal request is represented by the
ApprovablePaymentstruct. -
All withdrawal requests tracked by the contract are stored in the
m_approvable_paymentsmapping. -
Deposit funds to contract by calling the
depositfunction. Depositor addresses are not tracked and funds are stored in aggregate. -
Withdraw funds by calling the
request_and_approve_paymentfunction with an index representing the unique transaction. -
Each unique call to
request_and_approve_paymentby an address inm_approver_addresseswill increment the number of approvals for the parametised index. -
A withdrawal request is also an approval for that withdrawal.
-
Approvals cannot be made when the withdrawal request amount is more than the contract balance.
-
Once the approval threshold number is reached, the withdrawal is actioned and the withdrawal request flagged as such; meaning that index can not be resent.
-
All payment amounts are in units of wei.
-
There are usage instructions in the comments at the top of the source file.
-
By design, the owner may not necessarily be an approver as this allows the contract to be deployed for others to use.
-
In terms of style, I used m_ for contract member variables, a throwback to my c++ days that makes it easy to distinguish from function parameters. I’ve tried to keep formatting and style consistent as this is important to convey a sense of structure and quality.
-
Each call to
request_and_approve_paymentchecks that the specified transaction ID matches what we have recorded for that ID in terms of payment amount and destination address to catch cases where callers may input the wrong details by mistake and aren’t approving the transaction they think they are. -
I use events extensively to illustrate how the contract is executing. This could be wound back for production deployment.
-
Originally I wanted to avoid using externally managed transaction IDs so I experimented with hashing the withdrawal request details but came to the conclusion that I wasn’t satisfied with the uniqueness of this. This is because identical transaction details are a valid scenario (two identical payments to the same address) and hashing would not accurately distinguish this. I hence require that payments are externally identified by an index. I do check that the index supplied matches the payment details I have in storage (ie each time the contract sees an approval request for a particular index, it verifies that the destination and amount are the same as other calls made to approve that transaction index).
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.4;
// Description:
// Multi Signature Wallet.
// A contract that accepts payments into an aggregated pool and responds to requests to withdraw by addresses configured at deployment only. See m_approver_addresses.
// Withdrawals are only actioned when a minimum number of withdraw requests (also known as approvals) for a particular (indexed) withdrawal is reached. See m_approver_addresses.
// Each withdrawal request is represented by the ApprovablePayment struct.
// All withdrawal requests tracked by the contract are stored in the m_approvable_payments mapping.
//
// Usage:
// Deposit funds to contract by calling the deposit function. Depositor addresses are not tracked and funds are stored in aggregate.
// Withdraw funds by calling the request_and_approve_payment function with an index representing the unique transaction.
// Each unique call to request_and_approve_payment by an address in m_approver_addresses will increment the number of approvals for the parametised index.
// A withdrawal request is also an approval for that withdrawal.
// Approvals cannot be made when the withdrawal request amount is more than the contract balance.
// Once the approval threshold number is reached, the withdrawal is actioned and the withdrawal request flagged so; meaning that index can not be resent.
// All payment amounts are in units of wei.
//
// Enhancements:
// A function to check the sent / approval state of a given withdrawal request index could be handy. It could return "x of y approvals" and whether the transfer has been made.
contract MultiSigWallet {
// **VARIABLES**
// Only these addresses may approve payments. By design the owner may not be an approver as this allows the contract to be deployed for others to use.
address[] private m_approver_addresses;
// Configured when deployed. when this many approvals for a withdrawal request index is reached then transfer funds.
uint private m_min_approval_count_to_send;
// The current contract balance in wei.
uint public m_contract_balance_wei;
// Each withdrawal request contains payment details, transaction state and a mapping of which approver has approved the payment in response to request_and_approve_payment calls
struct ApprovablePayment {
address payable address_destination;
uint amount;
mapping(address => bool) approvals;
bool payment_sent;
bool payment_initialised;
}
// **EVENTS**
// Emitted from deposit function when funds added to this contract.
event depositDone(uint _amount_wei, address indexed _depositedTo);
// Emitted when request_and_approve_payment iterates over the m_approver_addresses array informing of which approver has approved the parametised withdrawal index already.
event approvalIterationUpdate(uint loopCount, uint approvalCount, address indexed msgSender, address indexed checkingThisApprover, bool approvalFlag);
// Emitted each time an existing approval for withdrawal index is found when request_and_approve_payment iterates over the m_approver_addresses array. See approvalIterationUpdate.
event approvalFoundFor(address indexed approver);
// Emmitted when request_and_approve_payment finds the approval count for a withdrawal index has met or m_min_approval_count_to_send just prior to enacting the transfer.
event paymentApproved(uint approvalCount, address indexed sendingTo);
// The payment details for all withdrawal request indexes ever seen by request_and_approve_payment, both sent and unsent.
// A mapping is more efficient than an array with nested indexing as iteration over the growing array would consume increasing amounts of gas over time.
mapping(uint => ApprovablePayment) private m_approvable_payments;
// **MODIFIERS**
// Guards request_and_approve_payment from being called by addresses not configured as approver addresses upon contract deployment.
modifier onlyApprover {
uint approver_length = m_approver_addresses.length;
bool approver = false;
for (uint i=0; i<approver_length; ++i) {
if (msg.sender == m_approver_addresses[i]) {
approver = true;
break;
}
}
require(approver, "Function must be called from an approver address.");
_; // run the function
}
// **FUNCTIONS**
// Configure the list of addresses that can requests/approve withdrawals and how many unique approvals are required for each transfer. Count can't exceed list length.
constructor(address[] memory a_approver_addresses, uint a_min_approval_count_to_send) {
require(a_min_approval_count_to_send <= a_approver_addresses.length, "Required approval count cannot exceed number of approver addresses.");
m_approver_addresses = a_approver_addresses; // decided to make this flexible so that the deployer doesn't need to be an approver. Makes wallet useful to setup for others to use.
m_min_approval_count_to_send = a_min_approval_count_to_send;
}
// Send wei to this contract and have it added to m_contract_balance_wei
function deposit() public payable returns (uint) {
m_contract_balance_wei += msg.value;
emit depositDone(msg.value, msg.sender);
return m_contract_balance_wei;
}
// This is where both approvals and transfers are made.
// A call to this function will approve the parameterised payment for the calling address and if doing so reaches the required approver count, the transfer is also made.
// Note that for safety, this fuction implements the Checks Effects Interactions pattern described at https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html
function request_and_approve_payment(address payable a_destination_address, uint a_amount_wei, uint a_transaction_id) public payable onlyApprover returns (uint) {
// Check calling address is found in m_approver_addresses (already done by onlyApprover modifier)
// Check we have the balance to attempt this payment before consuming resources to check the approval status.
require(m_approvable_payments[a_transaction_id].amount <= m_contract_balance_wei, "Insufficient contract funds to attempt payment.");
// If a payment of this id has already been sent then nothing to to. Don't double send.
require(!m_approvable_payments[a_transaction_id].payment_sent, "The payment for this id has already been sent.");
// If initialised, the requested destination address must match the details we have for this payment id in storage.
if(m_approvable_payments[a_transaction_id].payment_initialised) {
require(a_destination_address == m_approvable_payments[a_transaction_id].address_destination, "The specified destination address doesn't match the destination address for this payment ID.");
}
// If initialised, the requested transfer amount must match the details we have for this payment id in storage.
if(m_approvable_payments[a_transaction_id].payment_initialised) {
require(a_amount_wei == m_approvable_payments[a_transaction_id].amount, "The specified payment amount doesn't match the payment for this payment ID.");
}
// Step one of two: record our own approval for this transaction id.
m_approvable_payments[a_transaction_id].address_destination = a_destination_address;
m_approvable_payments[a_transaction_id].amount = a_amount_wei;
m_approvable_payments[a_transaction_id].approvals[msg.sender] = true;
m_approvable_payments[a_transaction_id].payment_initialised = true;
// Step two of two: determine whether our approval has met the threshold required to exercise the transfer - use Checks Effects Interactions pattern here.
uint approver_length = m_approver_addresses.length;
uint approval_count = 0;
for(uint i=0; i<approver_length; ++i) {
emit approvalIterationUpdate(i, approval_count, msg.sender, m_approver_addresses[i], m_approvable_payments[a_transaction_id].approvals[m_approver_addresses[i]]);
if( m_approvable_payments[a_transaction_id].approvals[m_approver_addresses[i]]) {
emit approvalFoundFor(m_approver_addresses[i]);
if(++approval_count >= m_min_approval_count_to_send) {
emit paymentApproved(approval_count, m_approvable_payments[a_transaction_id].address_destination);
m_contract_balance_wei -= m_approvable_payments[a_transaction_id].amount;
m_approvable_payments[a_transaction_id].payment_sent = true; // confirm approval in the storage struct
m_approvable_payments[a_transaction_id].address_destination.transfer(m_approvable_payments[a_transaction_id].amount); // unlike send, this will throw if unsuccessful so we get more information
break;
}
}
}
return m_contract_balance_wei;
}
}
1 Like
Hello thecil,
I have answered your question regarding what I’m trying to achieve with the loop iterator and would very much appreciate receiving your answer.
I’m “adding the owners addresses to the owners array -”
Thank you -
1 Like
I really appreciate your feedback. I will be working over the weekend to make the code more suitable. Hope you have a great weekend!
Vilior
1 Like
Yeah, sorry i forgot, to add the array address[] memory _owners from the constructor to the owners array of the contract, just simply go into a direct addition
owners = _owners
Carlos Z
Hi Carlos,
Hope you had a nice weekend! I tried to improve my code with a bit of help from Filip.
Thanks again,
Vilior
pragma solidity 0.7.5;
pragma abicoder v2;
contract MultisigWallet {
mapping (address => uint) balance;
address owner1 = msg.sender;
address owner2 = 0x1aE0EA34a72D944a8C7603FfB3eC30a6669E454C;
address owner3 = 0x5c6B0f7Bf3E7ce046039Bd8FABdfD3f9F5021678;
address[] public owners = [owner1,owner2,owner3];
uint public limit = 2;
event depositDone(uint amount, address indexed depositedTo);
struct Transfer {
address payable to;
uint amount;
uint txId;
uint counter;
bool signed;
}
Transfer[] transfer;
mapping (address => mapping (uint => bool)) approvals;
modifier OnlyOwner () {
bool owner = false;
for (uint i=0; i<owners.length; i++){
if (owners[i] == msg.sender){
owner = true;
}
}
require (owner == true);
_;
}
function deposit () public payable{}
function addTransfer(address payable _to, uint _amount, uint _txId) public OnlyOwner{
transfer.push(Transfer(_to, _amount, _txId,0,false));
}
function getTransfer (uint _index) public view returns (address, uint, uint, uint, bool){
return (transfer[_index].to, transfer[_index].amount, transfer[_index].txId, transfer[_index].counter, transfer[_index].signed);
}
function signedCompleted (uint _index) public OnlyOwner{
require (approvals[msg.sender][_index] == false);
require (transfer[_index].signed == false);
approvals[msg.sender][_index] = true;
transfer[_index].counter++;
}
function SendMoney(uint _index) public OnlyOwner {
require(transfer[_index].counter >= limit, "Not all the owners approved the trnasaction");
require(balance[msg.sender] >= transfer[_index].amount, "Balance not sufficient");
require(msg.sender != transfer[_index].to, "Don't transfer money to yourself");
uint previousSenderBalance = balance[msg.sender];
_SendMoney(msg.sender, transfer[_index].to, transfer[_index].amount);
assert(balance[msg.sender] == previousSenderBalance - transfer[_index].amount);
}
function _SendMoney(address from, address to, uint amount) private {
balance[from] -= amount;
balance[to] += amount;
}
}
1 Like
Hi!
I think I managed to do everything requested. But I had some trouble trying to split the Ownable contract. Does it happen because I have to define “owners[]” and “requiredApprovals” in the Ownable contract?
address[] internal owners;
uint internal requiredApprovals;
mapping(address => bool) internal cOwner;
constructor(address[] memory _owners, uint _requiredApprovals) {
owners = _owners;
requiredApprovals = _requiredApprovals;
for(uint i; i < owners.length; i++) {
cOwner[owners[i]] = true;
}
}
modifier onlyOwners {
require(cOwner[msg.sender], "Only owners!");
_;
}
When I imported this part from a different file, it required me to change the MultSig contract to “abstract”.
Full contract:
pragma solidity 0.8.1;
pragma abicoder v2;
contract MultSig {
address[] internal owners;
uint internal requiredApprovals;
mapping(address => bool) internal cOwner;
constructor(address[] memory _owners, uint _requiredApprovals) {
owners = _owners;
requiredApprovals = _requiredApprovals;
for(uint i; i < owners.length; i++) {
cOwner[owners[i]] = true;
}
}
modifier onlyOwners {
require(cOwner[msg.sender], "Only owners!");
_;
}
struct Request {
bool exists;
string description;
address to;
uint value;
bool completed;
uint approvalCount;
mapping(address => bool) approval;
}
mapping(uint => Request) requests;
Request[] requestLog;
uint requestId;
function deposit() public payable returns(uint){
return msg.value;
}
function checkBalance() public view returns(uint){
return address(this).balance;
}
function createRequest(string memory _description, address _to, uint _value) public onlyOwners returns(string memory){
Request storage nR = requests[requestId++];
nR.description = _description;
nR.to = _to;
nR.value = _value;
nR.completed = false;
nR.approvalCount = 0;
nR.exists = true;
return "Tx created!";
}
function approve(uint txId) public onlyOwners returns(string memory){
require(requests[txId].exists, "Tx does not exist."); //check if tx exists
require(requests[txId].approval[msg.sender] == false, "Tx already approved."); // check if tx is already approved
requests[txId].approval[msg.sender] = true;
requests[txId].approvalCount++;
return "Tx approved!";
}
function withdraw(uint txId) public returns(string memory){
require(requests[txId].exists, "Tx does not exist."); //check if tx exists
require(requests[txId].completed == false, "Tx already completed."); // check if tx is already completed
require(requests[txId].approvalCount >= requiredApprovals, "Tx missing approvals."); // check if approval threshold is reached
payable(requests[txId].to).transfer(requests[txId].value);
requests[txId].completed = true;
return "Tx completed!";
}
}
1 Like
I am storing the number of approvals inside the array that stores the tx requests.
This is probably not beautiful. But works.
This array could become enormous with # of calls increasing.
I’ll research how I can implement that as a stack or pipe…
pragma solidity 0.7.5;
import "./Destroyable.sol"; //Destroyable inherits Ownable
contract MultiSigWallet is Destroyable{
address[] private authorizedUsers;
uint public approvalsNecessary;
struct txRequest{
uint id;
address toAd;
uint amount;
uint approvals;
bool sent;
}
constructor(uint _numAprovals){
authorizedUsers.push(msg.sender);
approvalsNecessary = _numAprovals;
}
txRequest[] private txRequests;
mapping(address => mapping(uint => bool)) authRequests;
event depositDone (uint amount, address indexed toAddress);
event transferDone (uint amount, address indexed toAddress);
event transactionRequest(uint id, address sender, address to, uint amount);
function setNecessaryApprovals (uint _numApprovals) public onlyOwner returns(uint){
approvalsNecessary = _numApprovals;
return approvalsNecessary;
}
function addAuthUser(address _newAuthUser) public onlyOwner returns(bool){
bool userAlreadyAdded = false;
for(uint i=0; i < authorizedUsers.length; i++){
if(authorizedUsers[i] == _newAuthUser) { userAlreadyAdded = true; }
}
require(!userAlreadyAdded, "User already Added! Coan't add 'em twice!");
authorizedUsers.push(_newAuthUser);
return true;
}
function requestTx(address _to, uint _amount) public returns (uint){
require(address(this).balance >= _amount, "Insufficient balance");
bool txAuthorized = false;
for(uint i=0; i < authorizedUsers.length; i++){
if (authorizedUsers[i] == msg.sender) { txAuthorized = true; }
}
require(txAuthorized, "Unauthorized! Not a contract owner!");
uint newId = txRequests.length;
txRequests.push(txRequest(newId, _to, _amount, 1, false));
authRequests[msg.sender][newId] = true;
emit transactionRequest(newId, msg.sender, _to, _amount);
return newId;
}
function approveTx(uint _id) public returns (bool){
require(address(this).balance >= txRequests[_id].amount, "Insufficient balance");
require(!authRequests[msg.sender][_id], "Tx already approved! Cannot be done twice! Nice try!");
txRequests[_id].approvals++;
if(txRequests[_id].approvals >= approvalsNecessary && !txRequests[_id].sent){
_transfer(txRequests[_id].toAd, txRequests[_id].amount);
}
txRequests[_id].sent = true;
authRequests[msg.sender][_id] = true;
return true;
}
function _transfer(address _to, uint _amount) private {
payable(_to).transfer(_amount);
emit transferDone(_amount, _to);
}
function deposit() public payable{
emit depositDone(msg.value, msg.sender);
}
}
1 Like
Its looks better, but i could advice you to use a constructore to initialize properly the owners, check other students contract to give you an idea of it.
Carlos Z
Your MultiSig contract is not inheriting the ownable contract, also you can only have 1 constructor per contract (or is multisig the one with the constructor or ownable, but not both), abstract contracts is a suggestion for those that are not initialized with a constructor, is just a suggestion message, so nothing to worry about.
Carlos Z
Thank you Carlos 
for getting back and proposing a simpler solution - I will now finish the code including events and post it once more after assuring that it works as expected.
1 Like
Hello Carlos,
Thank you to you and Jon for your presistent help with the code of this assignment!
Below is the code including the events which I’ve tested on remix and (from my perspective) it seems to work ok.
A couple of comments / questions:
1 - Question: in the solution code of Filip the “emit TransferRequestCreated”-event is coded before actually pushing the request into the array. Is this for a specific reason or can the code for this event be moved after updating the array?
2 - Comment: I actually had to replace my require statements checking before actually making the transfer and bundle them into the if-function as with the former solution the code continued to run and fail on the require check for sufficient signatories therefore triggering an error as long as apprpovals whare below the limit. With the if loop this has been resolved
3 - Question: why is Filip limiting his TransferApproved-Event to the _id only? Wouldn’t it be convenient to include the _recipient and _amount of the transfer for more transparency?
Multisig Wallet Contract
pragma solidity 0.7.5;
pragma abicoder v2;
contract MultisigWallet {
address[] public owners;
uint limit;
struct Transfer {
address payable to;
uint amount;
uint totalSignatures;
bool transferComplete;
uint id;
}
Transfer[] public transferRequests;
mapping(address => mapping(uint => bool)) approvals;
mapping(address => bool) isOwner;
event newTransferRequest(uint _id, address _initiator, address indexed _to, uint _amount);
event newApproval(uint _id, uint _totalSignatures, address _approver);
event transferSuccessful(uint _id, address _to, uint _amount);
modifier onlyOwners(){
bool owner = false;
for(uint i=0; i<owners.length; i++){
if(owners[i] == msg.sender){
owner = true;
}
}
require(owner == true, "not an owner");
_;
}
constructor(address[] memory _owners, uint _limit) {
require(_owners.length > 0, "no owners registered");
require(_limit > 0 && _limit <= _owners.length, "limit must be greater than 0 and no larger than total amount of signatories");
owners = _owners;
limit = _limit;
}
function createTransfer(address payable _to, uint _amount) public onlyOwners {
transferRequests.push(
Transfer(_to, _amount, 0, false, transferRequests.length));
emit newTransferRequest(transferRequests.length, msg.sender, _to, _amount);
}
function approve(uint _id) public onlyOwners {
require(_id < transferRequests.length, "no Transfers exists to date"); // a transfer needs to exist
require(approvals[msg.sender][_id] != true, "can't approve same transfer twice"); // an single owner can not approve the same transfer twice
approvals[msg.sender][_id] = true; // update the mapping by recording the owner's approval for this txID
transferRequests[_id].totalSignatures += 1; // update amount of signatures of this trfRequest
emit newApproval(_id, transferRequests[_id].totalSignatures, msg.sender);
if(limit >= limit){
transferRequests[_id].transferComplete = true;
transferRequests[_id].to.transfer(transferRequests[_id].amount);
emit transferSuccessful(_id, transferRequests[_id].to, transferRequests[_id].amount);
}
}
function getTransferRequests() public view returns (Transfer[] memory){
return transferRequests;
}
function deposit() public payable {
}
}
I’d much appreciate further comments of yours where necessary.
Thanks again.
Andreas
1 Like
Appreciate your suggestion.
Cheers,
Vilior
1 Like
Hello (to whomever it is that is aiding the learners here)!
I have made progress using a different tutorial. I’ve come across some issues with my code. Could I please get a hand here?
I understand that for starters, the nested struct is not supported in the newer version of Solidity - at least not in the 0.7.5 version. Could I please get some pointers? Thanks in advance.
pragma solidity 0.7.5;
contract MultiSigWallet{
address [] public owners;
mapping(address => bool) public isOwner;
uint public numConfirmationsRequired;
struct Transaction {
address to;
uint value;
bytes data;
bool executed;
mapping(address => bool) isConfirmed;
uint numConfirmations;
}
Transaction[] public transactions;
constructor(address [] memory _owners, uint _numConfirmationsRequired) public {
require(_owners.length > 0, "Owners Required");
require(
_numConfirmationsRequired > 0 && _numConfirmationsRequired <= _owners.length);
for (uint i = 0; i < _owners.length; i++) {
address owner = _owners[i];
require(owner != address(0), "Invalid Owner");
require(!isOwner[owner], "Owner Not Unique");
isOwner[owner] = true;
owners.push(owner);
}
numConfirmationsRequired = numConfirmationsRequired;
}
event Deposit(address indexed sender, uint amount, uint balance);
event transactionSubmitted(
address indexed owner,
uint indexed txIndex,
address indexed to,
uint value,
bytes data
);
event transactionConfirmed(address indexed owner, uint indexed txIndexed);
event confirmationRevoked(address indexed owner, uint indexed txIndexed);
event transactionExecuted(address indexed owner, uint indexed txIndexed);
modifier onlyOwner(){
require(isOwner[msg.sender], "Not Owner");
_;
}
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 transactionSubmitted(msg.sender, txIndex, _to, _value, _data);
}
modifier txExists(uint _txIndex) {
require(_txIndex < transactions.length, "Transaction does not exist");
_;
}
modifier notExecuted(uint _txIndex)
{
require(!transactions[_txIndex].executed, "Transaction already executed");
_;
}
modifier notConfirmed(uint _txIndex) {
require(!transactions[_txIndex] .isConfirmed [msg.sender], "Transaction already confirmed");
_;
}
function confirmTransaction(uint _txIndex)
public
onlyOwner
txExists(_txIndex)
notExecuted(_txIndex)
notConfirmed(_txIndex)
{
Transaction storage transaction = transactions[_txIndex];
transaction.numConfirmations += 1;
emit transactionConfirmed(msg.sender, _txIndex);
}
function executeTransaction(uint _txIndex)
public
onlyOwner
txExists(_txIndex)
notExecuted(_txIndex)
{
Transaction storage transaction = transactions[_txIndex];
require(transaction.numConfirmations >= numConfirmationsRequired,
"Can not execute transaction"
);
transaction.executed = true;
(bool success, ) = transaction.to.call.value(transaction.value) (transaction.data);
require(success, "Transaction failed");
emit transactionExecuted(msg.sender, _txIndex);
}
function revokeConfirmation () {
}
1 Like
Hey there, something is wrong with my submitTransaction function, its not working 
Honestly I’m horribly confused
/*
Arrays of accounts that are owners = ["0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2", "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4", "0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db"]
Sending ether to this account = "0x617F2E2fD72FD9D5503197092aC168c91465E7f2"
amount = 1000000000000000000
*/
pragma solidity 0.7.5;
pragma abicoder v2;
import "./Ownable.sol";
contract MultisigWallet is Ownable {
/*
*
Events
*
*/
event depositDone(address indexed sender, uint amount, uint balance);
event ownersDone(address indexed sender);
event submitTransactionDone(address indexed sender, uint indexed txId, address indexed to, uint amount);
event approved(address indexed sender, uint indexed txId);
event executeTransferDone(address indexed sender, uint amount);
address[] public owners;
uint limit;
mapping(address => mapping(uint => bool)) approvals;
mapping(address => bool) isOwner;
struct Transfer {
uint amount;
address payable receiver;
uint approvals;
bool hasBeenSent;
uint txId;
}
Transfer[] transfer;
constructor(address[] memory _owners, uint _limit) {
require(_owners.length > 1, "More addresses required");
require(_limit > 0, "More signatures required");
for(uint i = 0; i < _owners.length; i++) {
address owner = _owners[i];
require(owner != address(0), "Invalid owner!");
require(!isOwner[owner], "Owner is not unique!");
isOwner[owner] = true;
owners.push(owner);
}
}
/*
*
Modifiers
*
*/
modifier onlyOwners() {
for(uint i = 0; i < owners.length; i++) {
address owner = owners[i];
require(msg.sender == owner);
}
_;
}
modifier txExist(uint _id) {
require(_id < transfer.length, "Transaction does not exist");
_;
}
modifier notExecuted(uint _id) {
require(!transfer[_id].hasBeenSent, "Transaction already confirmed");
_;
}
/*
*
Functions
*
*/
function deposit() payable external {
emit depositDone(msg.sender, msg.value, address(this).balance);
}
function getBalance() public returns(uint) {
return (address(this).balance);
}
function setOwner(address _owner) public onlyOwner {
owners.push(_owner);
emit ownersDone(_owner);
}
function submitTransaction(address payable _to, uint _amount) public onlyOwners {
uint txIndex = transfer.length;
transfer.push(Transfer({
amount: _amount,
receiver: _to,
approvals: 0,
hasBeenSent: false,
txId: txIndex
}));
emit submitTransactionDone(msg.sender, txIndex, _to, _amount);
}
function approve(uint _id) public onlyOwners txExist(_id) notExecuted(_id) {
approvals[msg.sender][_id] = true;
transfer[_id].approvals += 1;
emit approved(msg.sender, _id);
}
function executeTransfer(uint _id ) public onlyOwners txExist(_id) notExecuted(_id) {
if(transfer[_id].approvals >= limit) {
transfer[_id].hasBeenSent = true;
transfer[_id].receiver.transfer(transfer[_id].amount);
}
emit executeTransferDone(msg.sender, _id);
}
}
1 Like
When I make a separate contract for Ownable I get this error:
But if I copy the exact same instructions to the MultSig contract it works fine.
contract Ownable {
address[] internal owners;
uint internal requiredApprovals;
mapping(address => bool) internal cOwner;
constructor(address[] memory _owners, uint _requiredApprovals) {
require(_owners.length > 0, "Owners required");
require(_requiredApprovals > 0 && _requiredApprovals <= _owners.length, "Invalid Approvals Number");
owners = _owners;
requiredApprovals = _requiredApprovals;
for(uint i; i < owners.length; i++) {
cOwner[owners[i]] = true;
}
}
modifier onlyOwners {
require(cOwner[msg.sender], "Only owners!");
_;
}
}
1 Like