WOW!! Thanks a lot for your feedback mate, I means a lot. I’ve watched Filip’s solutions and gone ahead and improved the code, with some features he did that I liked better. Can’t wait to start the 201 course. I still can’t believe I’ve managed to do my first simple project on solidity. Once again thanks a ton for the help, it really means a lot! 
1 Like
Hi !
Even though I felt like I wouldn’t be able to do it all, I’ve been trying to tackle this project on my own without any help int the past week:
first attempt:
pragma solidity 0.7.5;
pragma abicoder v2;
contract Wallet {
//mapping
mapping(address => uint) public balance; //(unknown situation with mapping)
//constructor - The contract creator should be able to input (1): the addresses of the owners and (2):
//the numbers of approvals required for a transfer, in the constructor. For example, input 3 addresses and set
//the approval limit to 2.
constructor(){
address[3] owners = [0x5B38Da6a701c568545dCfcB03FcB875f56beddC4, 0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2, 0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db];
address = msg.sender; //?
}
//onlyOwner
modifier onlyOwner {
require(msg.sender == owner);
_;
}
//Add balance to wallet
function addBalance() public payable returns (uint) {
balance[msg.sender] += msg.value;
//emit depositDone(msg.value, msg.sender); //event?
return balance[owners];
//add to struct ?
//Get Balance
function getBalance() public view returns (uint) {
return balance;
}
//Transfer function
function Transfer(address recipient, uint amount) public {
balance[msg.sender] -= amount;
balance[recipient] += amount;
require(balance[msg.sender] >= amount, "Balance not sufficient");
require(msg.sender != recipient, "Don't transfer money to yourself");
//require signature event
require(sign = true)
//require(sign function )
}
//Sign function or event ?
function sign(string) public returns (bool) {
string
if(string "yes"){
bool = true
}
else(string "no"){
bool = false
}
}
}
Second attempt after watching the hint video:
pragma solidity 0.7.5;
pragma abicoder v2;
contract Wallet {
//owners of wallet
address[] public owners = [0x5B38Da6a701c568545dCfcB03FcB875f56beddC4, 0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2, 0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db];
//transactions struct
struct transactions {
address to;
uint value;
uint numConfirmations;
}
//Signature confirmations
uint confirmations
address owner
constructor(){
owner = msg.sender;
}
//onlyOwner
modifier onlyOwner {
require(msg.sender == owner);
_;
}
//event: deposit
event deposit(uint date, uint amount, address from, address to);
//event transfer request submitted
event transferRequestReceived(address indexed owner, address indexed to, uint amount);
//event Signature
event signature(bool);
//deposit function
function deposit() public payable returns (uint) {
balance[msg.sender] += msg.value;
emit deposit(msg.value, msg.sender);
return balance[msg.sender];
}
//transfer request function
function requestTransfer(address _to, uint _amount) public onlyOwner {
require(balance[msg.sender] >= amount, "Balance not sufficient");
require(msg.sender != recipient, "Don't transfer money to yourself");
transactions.push(transactions({_to, _value, _numConfirmations = 0}));
emit transferRequestReceived(msg.sender, _to, _amount);
}
//sign request function
function signTransfer(string) public {
if(string "true"){
emit signature(true);
}
else(string "false"){
emit signature(false);
}
}
//require(_confirmations > 2, 'not enough signatures'); //to execute transaction ? put in function ?
//transfer function
function transfer(address _to, uint amount) public payable {
require(balance[msg.sender] >= amount, "Balance not sufficient");
require(msg.sender != recipient, "Don't transfer money to yourself");
_transfer(msg.sender, recipient, amount);
assert(balance[msg.sender] == previousSenderBalance - amount);
}
}
It’s been a little bit frustrating because it feels a little overwhelmign at this point, but pushing through.
I am now going to watch the third video, and I’ve been helping myself with this one I found on YouTube: https://www.youtube.com/watch?v=Dh7r6Ze-0Bs which is really good and helpful, and since I can’t store it in my memory for very long I am still forced to think about it deeply when doing it on my own 
(YouTube video code):
pragma solidity 0.7.5;
pragma abicoder v2;
contract Wallet {
//events
event Deposit(address indexed sender, uint amount, uint balance);
event SubmitTransaction(
address indexed owner,
uint indexed txIndex,
address indexed to,
uint value,
bytes data
);
event ConfirmTransaction(address indexed owner, uint indexed txIndex);
event ExecuteTransaction(address indexed owner, uint indexed tkIndex);
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 transactons;
constructor(address[] memory _owners, uint _numConfirmationsRequired) public {
require(_owners.length > 0, 'owners required');
require(_numConfirmationsRequired > 0 && _numConfirmationsRequired <= _owners.length, 'not enough 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
}
function submitTransaction(){
}
function confirmTransaction(){
}
function executeTransaction(){
}
}
1 Like
What a great exercise! Simple yet complicated and covered quite a bit of the course content. Here’s my solution for the final project. I threw in my tests, not perfectly explained I know, they did help me stay on track.
pragma solidity 0.8.1;
contract MultiSigWalletRequests{
/////////////////////////////////////////////
//STATE VARIABLES
/////////////////////////////////////////////
////Request object(s)
uint seq_requestId; //Handles unique id for requests
struct Request{
//uint request_id;
address payable requestor;
uint approval_status; // 0=unapproved, >1 equals approved.
uint amount;
}
mapping (uint => Request) maprequests;
////Approval object(s)
mapping(address => mapping(uint => bool)) mapapproved;
//Initialize approver settings
address[] oktoApprove;
uint requiredApprovalCount;
/////////////////////////////////////////////
//MODIFIERS
////////////////////////////////////////////
modifier onlyApprovers() {
bool approver = false;
//Check the approvers for a match
for(uint i = 0;i < oktoApprove.length;i++){
if(oktoApprove[i] == msg.sender){
approver = true;
}
}
require(approver == true);
_; //continue execution
}
/////////////////////////////////////////////
//FUNCTIONS
/////////////////////////////////////////////
constructor (address [] memory _Owners, uint _ReqApprovals){
//Ensure we have more approvers provided than the required number of approvals.
assert(_ReqApprovals <= _Owners.length);
//Set the contract required number of signatories for full approval.
requiredApprovalCount = _ReqApprovals;
//Add the contract creator as an approver
oktoApprove.push(msg.sender);
//Create the addresses of approvers
for(uint i=0;i<_Owners.length;i++){
oktoApprove.push(_Owners[i]);
}
}
function deposit() public payable{}
//Enables the contract to be funded.
function enterRequest (uint _amount) public payable returns(Request memory) {
//Enables the funding requests to be submitted
uint myRequestId = getNewRequestId();
Request memory myRequest = Request(payable(msg.sender),0,_amount);
//Add this new request to the mapping
maprequests[myRequestId] = myRequest;
//Returns the submitted request details.
return myRequest;
}
function approveRequest(uint _reqId) public payable onlyApprovers returns(bool) {
//set return value hook in case of transmit actions later
bool boolTransmit = false;
//Check for prior approval
require(mapapproved[msg.sender][_reqId] == false);
//Approve actions
mapapproved[msg.sender][_reqId] = true;
maprequests[_reqId].approval_status += 1; //update the request to +1. THIS IS WORKING
//Check for all required approvals require transfer
if(maprequests[_reqId].approval_status >= requiredApprovalCount) {
transfer(_reqId);
boolTransmit = true;
}
return boolTransmit;
}
function transfer(uint _reqId) private onlyApprovers {
//Sends the ether for a given
address payable sendTo = maprequests[_reqId].requestor;
uint sendAmount = maprequests[_reqId].amount;
//Send it
sendTo.transfer(sendAmount);
}
/////////////////////////////////////////////
//HELPER FUNCTIONS
//helper function for manging request ids
/////////////////////////////////////////////
function getNewRequestId() public returns(uint) {
seq_requestId = seq_requestId + 1;
return seq_requestId;
}
/////////////////////////////////////////////
//TEST CASES
/////////////////////////////////////////////
function testContractBal(address _contractAddy) public view returns(uint){
return _contractAddy.balance;
}
function testmodifersList() public view returns(bool) {
//Test Case: Checks if the current address is an meets modifier for approver
bool approver = false;
//Check the approvers for a match
for(uint i = 0;i < oktoApprove.length;i++){
if(oktoApprove[i] == msg.sender){
approver = true;
}
}
return approver;
}
function testTransferReqMap(uint _reqId) public view returns(bool){
//Test Case: Checks if current address has approved or not
return mapapproved[msg.sender][_reqId];
}
function testApproversConstructor() public view returns(address [] memory){
//Test Case: Allows verifying the approvers input at deployment of the contract are correct.
return oktoApprove;
}
function testRequestQ(uint _reqId) public view returns(Request memory){
return maprequests[_reqId];
}
function testCheckLastRequestId() public view returns(uint){
return seq_requestId;
}
function testRequiredAppCount() public view returns(uint){
return requiredApprovalCount;
}
function testReqAddress(uint _reqId) public view returns(address){
return maprequests[_reqId].requestor;
}
}
1 Like
Hi here is my proposed solution to the Multisig Wallet Project. Took me a bit of scraping and starting from scratch but I’m proud to have come to a solution on my own.
//SPDX-License-Identifier:SPDX-License
pragma solidity 0.8.4;
pragma abicoder v2;
contract msWallet
{
address owner;
address owner1;
address owner2;
address owner3;
uint limit;
constructor (address _owner1, address _owner2, address _owner3, uint _limit)
{
owner = msg.sender;
owner1 = _owner1;
owner2 = _owner2;
owner3 = _owner3;
limit = _limit;
setOwner(_owner1, _owner2, _owner3, 2);
}
modifier onlyOwner
{
require(msg.sender == owner, "Only contract creator can use feature");
_;
}
modifier onlyOwners
{
require(msg.sender == _owners[0].ownerx || msg.sender == _owners[1].ownerx || msg.sender == _owners[2].ownerx, "only authorized users can use feature ");
_;
}
modifier hasntSigned
{
require( ((msg.sender == _owners[0].ownerx) && (_owners[0].sign == 1)) || ((msg.sender == _owners[1].ownerx) && (_owners[1].sign == 1)) || ((msg.sender == _owners[2].ownerx) && (_owners[2].sign == 1)), "available to users who have not already signed transaction!" );
_;
}
modifier moreThanNothing
{
require(msg.value > 0, "You must input a value greater than zero to make deposit!");
_;
}
modifier hasNoPendingWithdraw
{
require(hasPendingWithdraw[msg.sender] == false, "you can not init a withdraw unitl finalization of previous withdraw request!");
_;
}
modifier hasNoPendingTransaction
{
require(hasPendingTransaction[msg.sender] == false, "you can not init a transaction unitl finalization of previous transaction!");
_;
}
mapping(address => uint)balance;
mapping(address => mapping(uint => bool))approvedIt;
mapping(address => mapping(uint => mapping(uint => bool)))approvedWithdraw;
mapping(address => bool)hasPendingWithdraw;
mapping(address => bool)hasPendingTransaction;
struct Transaction
{
uint txId;
address from;
address too;
uint amount;
uint signatures;
}
struct HistoricalTransactions
{
uint HtxId;
address Hfrom;
address Htoo;
uint amountSent;
uint signatures;
}
struct Withdrawal
{
uint withdrawId;
address user;
uint amount;
uint signature;
}
struct HistoricalWithdraws
{
uint Hwithdraws;
address Haddress;
uint Hamount;
uint Hsignatures;
}
struct Owners
{
uint id;
address ownerx;
uint sign;
}
Transaction[] _pendingTx;
HistoricalTransactions[] _history;
Withdrawal[] _withdrawRequest;
HistoricalWithdraws[] _Whistory;
Owners[] _owners;
event withdrawalInit(string msg, uint wId, address initUser);
event transferInit(string msg, uint txId, address initUser);
event transferSuccess(uint txId, address from, address too, uint amount);
event withdrawSuccessful(uint wId, address receiver, uint amount);
//contract owner functions
function setOwner(address _owner1, address _owner2, address _owner3, uint _limit ) internal
{
owner1 = _owner1;
owner2 = _owner2;
owner3 = _owner3;
limit = _limit;
Owners memory ownerData1 = Owners(_owners.length, _owner1, 1);
_owners.push(ownerData1);
Owners memory ownerData2 = Owners(_owners.length, _owner2, 1);
_owners.push(ownerData2);
Owners memory ownerData3 = Owners(_owners.length, _owner3, 1);
_owners.push(ownerData3);
}
function getOwner(uint id) public view onlyOwner returns (uint Id, address user)
{
return (_owners[id].id, _owners[id].ownerx);
}
//public functions
function deposit() public payable moreThanNothing
{
balance[msg.sender] += msg.value;
}
function myBalance() public view returns(uint)
{
return balance[msg.sender];
}
function contractBalance() public view returns (uint)
{
return (address(this).balance);
}
//Transaction init
function initTransfer(address _too, uint _amount) public onlyOwners hasNoPendingTransaction
{
require(balance[msg.sender] >= _amount, "Not enough funds to send!");
require(msg.sender != _too, "Cannot send funds to yourself!");
Transaction memory _pendingTxApprovals = Transaction(_pendingTx.length, msg.sender, _too, _amount, 0);
_pendingTx.push(_pendingTxApprovals);
hasPendingTransaction[msg.sender] = true;
emit transferInit("Transaction submitted! Awaiting approval", _pendingTx.length, msg.sender);
}
function withdrawRequest(uint amount) public payable onlyOwners hasNoPendingWithdraw
{
require(balance[msg.sender] >= amount, "balance insufficient!");
Withdrawal memory _Request = Withdrawal(_withdrawRequest.length, msg.sender, amount, 0);
_withdrawRequest.push(_Request);
hasPendingWithdraw[msg.sender] = true;
emit withdrawalInit("Withdraw submitted! Awaiting approval", _withdrawRequest.length, msg.sender);
}
//Transaction data history view
function pendingTransactions(uint _txId) public view returns (uint txId, address from, address too, uint amount, uint signatures)
{
return (_pendingTx[_txId].txId, _pendingTx[_txId].from, _pendingTx[_txId].too, _pendingTx[_txId].amount, _pendingTx[_txId].signatures);
}
function pendingWithdraws(uint _id) public view onlyOwners returns(uint withdawId, address receiver, uint amount, uint signatures)
{
return (_withdrawRequest[_id].withdrawId, _withdrawRequest[_id].user, _withdrawRequest[_id].amount, _withdrawRequest[_id].signature);
}
function TxHistory (uint _HtxId) public view returns (uint HistroicTXid,address from, address too, uint amount, uint signatures)
{
return (_history[_HtxId].HtxId, _history[_HtxId].Hfrom, _history[_HtxId].Htoo, _history[_HtxId].amountSent, _history[_HtxId].signatures);
}
function withdrawHistroy(uint _wTxId) public view returns(uint HistoricWid, address too, uint amount, uint signatures)
{
return (_Whistory[_wTxId].Hwithdraws, _Whistory[_wTxId].Haddress, _Whistory[_wTxId].Hamount, _Whistory[_wTxId].Hsignatures);
}
//Approval functions
function approveTransfer(uint _txId, uint _Id) public
{
require(msg.sender != _pendingTx[_txId].from, "You cannot approve your own request!");
require(msg.sender == _owners[_Id].ownerx, "You may only enter the user id assigned to your wallet!");
require(approvedIt[msg.sender][_txId] == false);
_pendingTx[_txId].signatures += 1;
approvedIt[msg.sender][_txId] = true;
if(_pendingTx[_txId].signatures == limit)
{
address transferRequestTracker = _pendingTx[_txId].from;
_approveTransfer(_txId);
hasPendingTransaction[transferRequestTracker] = false;
emit transferSuccess(_pendingTx[_txId].txId, _pendingTx[_txId].from, _pendingTx[_txId].too, _pendingTx[_txId].amount);
}
}
function _approveTransfer(uint _txId) private
{
uint previousBalance = balance[_pendingTx[_txId].from];
balance[_pendingTx[_txId].from] -= _pendingTx[_txId].amount;
balance[_pendingTx[_txId].too] += _pendingTx[_txId].amount;
assert(balance[_pendingTx[_txId].from] == previousBalance - _pendingTx[_txId].amount);
emit transferSuccess(_pendingTx[_txId].txId, _pendingTx[_txId].from, _pendingTx[_txId].too, _pendingTx[_txId].amount);
HistoricalTransactions memory _Htx = HistoricalTransactions(_pendingTx[_txId].txId, _pendingTx[_txId].from, _pendingTx[_txId].too, _pendingTx[_txId].amount, _pendingTx[_txId].signatures);
_history.push(_Htx);
delete _pendingTx[_txId];
}
function approveWithdraw(uint withdrawId, uint _id, uint enterWithdrawIdAgain) public onlyOwners
{
require(msg.sender != _withdrawRequest[withdrawId].user, "You cannot approve your own submitted request!");
require(msg.sender == _owners[_id].ownerx, "You can only use your user ID to sign withdrawals");
require(approvedWithdraw[msg.sender][withdrawId][enterWithdrawIdAgain] == false, "you have already approved this withdraw");
_withdrawRequest[withdrawId].signature += 1;
approvedWithdraw[msg.sender][withdrawId][enterWithdrawIdAgain] = true;
if(_withdrawRequest[withdrawId].signature == limit)
{
address withdrawRequestTracker = _withdrawRequest[withdrawId].user;
uint wrID = _withdrawRequest[withdrawId].withdrawId;
_withdraw(_withdrawRequest[withdrawId].amount, _withdrawRequest[withdrawId].user, wrID);
hasPendingWithdraw[withdrawRequestTracker] = false;
}
}
function _withdraw(uint amount, address _id, uint wrID) private
{
address payable userAddress = payable(_id);
uint previousBalance = balance[userAddress];
balance[_id] -= amount;
userAddress.transfer(amount);
assert(balance[_id] == previousBalance - amount);
emit withdrawSuccessful(_withdrawRequest[wrID].withdrawId, _withdrawRequest[wrID].user, _withdrawRequest[wrID].amount);
HistoricalWithdraws memory _WtxH = HistoricalWithdraws(_withdrawRequest[wrID].withdrawId, _withdrawRequest[wrID].user, _withdrawRequest[wrID].amount, _withdrawRequest[wrID].signature);
_Whistory.push(_WtxH);
delete _withdrawRequest[wrID];
}
}
1 Like
Hello everybody,
Here is my solution to the multisig wallet project … after struggling with it more than a month. I’m sure it still has a lot of flaws. Much appreciation for any advice on improvement!
Eric
Solidity codes begin –
pragma solidity 0.8.4;
// This contract is a multi-signature wallet for ether.
// Users need to specify at contract deployment the maximum number of owners.
// Users also need to specify at deployment the minimum number of owners required for approving a transfer.
// Any ethereum address can deposit ether into the contract; it is not limited to owners.
contract wallet {
mapping(address => uint) balance; // To enable getting of contract balances
mapping(address => uint) ownerIndex; // For checking the owners registry quickly
mapping(uint => Transfer) transfers; // A registry of all transfer requests created
mapping(uint => mapping(address => uint)) approvalIndex; // For quickly checking number of approvals given for a transfer
event ownerAdded(address owner);
event transferCreated (address recipient, uint amount);
event transferDone (address recipient, uint amount);
uint numOfOwners;
uint numOfApprovals;
uint pendingTransfers;
uint transferID;
address[] owners;
uint[] transferList;
struct Transfer {
uint id;
address transferTo;
uint transferAmt;
address[] ownersApproval;
bool approved;
}
// Create contract by specifying the maximum number of owners and number of approvals needed for transfers
constructor (uint _persons, uint _approveRequired) {
require (_persons > 0 && _approveRequired > 0, "Number of owners and number of approvals must be greater than one.");
require (_approveRequired <= _persons, "Required number of approvals exceeds number of owners.");
numOfOwners = _persons;
numOfApprovals = _approveRequired;
transferID = 0;
pendingTransfers = 0;
}
function addOwner(address newOwner) public {
// Ensure owner and approval requirements are already specified
require(numOfOwners != 0 || numOfApprovals !=0, "Please first specify number of owners and number of approvals needed.");
// Ensure address to be added is not already in contract
require (!_ownerExists(newOwner), "Owner already exists in contract.");
// Ensure number of owners does not exceed limit set for contract
require(owners.length <= numOfOwners-1, "Number of owners exceeds limit set for contract");
uint ownerID;
owners.push(newOwner);
ownerID = owners.length;
ownerIndex[newOwner] = ownerID;
emit ownerAdded (newOwner);
}
// Public function for anyone to deposit ether into the contract
function deposit() public payable {
balance[msg.sender] += msg.value;
}
// Public function for checking nominal contract balance (excluding pending transfers)
function getContractBalance() public view returns(uint){
return address(this).balance;
}
// Public function for checking total amount in pending transfers
function getTotalPendingTransfers() public view returns(uint){
return pendingTransfers;
}
// Public function for checking the list of owners
function getOwners() public view returns(address[] memory) {
return owners;
}
// Public function for an owner to create a transfer request
// Each new transfer is identified by a transferID
function transferRequest(address recipient, uint amount) public {
require (_ownerExists(msg.sender), "Only owners can make transfer requests."); //Ensure only an owner can initiate a transfer request
require (address(this).balance - pendingTransfers >= amount, "Not enough fund in balance."); //Ensure the contract has more liquid funds for the transfer
transferID ++;
transferList.push(transferID);
transfers[transferID].id = transferID;
transfers[transferID].transferTo = recipient;
transfers[transferID].transferAmt = amount;
transfers[transferID].ownersApproval.push(msg.sender);
transfers[transferID].approved = false;
approvalIndex[transferID][msg.sender] = 1;
pendingTransfers += amount; //Increase the balance of total pending transfers
emit transferCreated (recipient, amount);
// To approve the transfer if only one approver is specified in contract deployment
_execTransfer(transferID);
}
// Public function to get all pending transfers not yet approved; zeros in fields returned indicate a transfer has already been executed
function getPendingTransfers () public view returns (uint[] memory, address[] memory, uint[] memory) {
uint[] memory tempID = new uint[](transferList.length);
address[] memory to = new address[](transferList.length);
uint[] memory amount = new uint[](transferList.length);
for (uint i=1; i<=transferList.length; i++){
if (transfers[i].approved != true) {
tempID[i-1] = transfers[i].id;
to[i-1] = transfers[i].transferTo;
amount[i-1] = transfers[i].transferAmt;
}
}
return (tempID, to, amount);
}
// Public function for owners to approve transfers
// Approving owner is required to provide the transferID of a pending transaction
function approveTransfer (uint _transferID) public {
// Require that only owner can approve transfer
require (_ownerExists(msg.sender), "Only owners can approve transfer request.");
// Require that no owner can approve the same transfer more than once
require (!_ownerAlreadyApproved(_transferID), "This owner already approved the transfer.");
// To avoid executing transfers already approved previously
require (transfers[_transferID].approved != true, "This transfer has already been processed.");
// Ensure _transferID is in the list of pending transactions
require (_transferID != 0 && _transferID <= transferList.length, "This transfer ID does not exist.");
transfers[_transferID].ownersApproval.push(msg.sender);
approvalIndex[_transferID][msg.sender] = transfers[_transferID].ownersApproval.length;
_execTransfer(_transferID);
}
// Private function to check existence of owner address in contract
function _ownerExists(address _owner) private returns (bool) {
return (ownerIndex[_owner] != 0);
}
//Private function to check existence of an owner's approval to a specific transfer
function _ownerAlreadyApproved(uint _transferID) private returns(bool) {
return (approvalIndex[_transferID][msg.sender] != 0);
}
// Private function to execute transfer
function _execTransfer(uint _transferID) private {
if (transfers[_transferID].ownersApproval.length == numOfApprovals) {
address _recipient = transfers[_transferID].transferTo;
uint _amount = transfers[_transferID].transferAmt;
payable(_recipient).transfer(_amount);
pendingTransfers -= _amount; // Decrease the balance of total pending transfers
transfers[_transferID].approved = true;
emit transferDone(_recipient, _amount);
}
}
}
2 Likes
hey @Eric_Toronto,
this is a very good piece of code especially since this is you first solidity course. I really like the way that you did not hard code in your approvers array. The fact that you created a function to dynamically create your owners array makes your program much more versatile. One thing that you could do to improve upon this is to create a deleteUser() function that allows you to delete users from this array. You could do this by making a require that the user you want to delete is not in the array already. Then what you could do delete by address or delete by user index. You should loop through the array and if the approvers index (or address depending on which way you want to delete by) is equal to the address of the approver you want to delete you could move that index to the end of the array and use the .pop() method to get rid of it
Second thing im curious about is in your transfer struct you have an approvers attribute thats an array. And then you pass in a value into your constructor which defines the number of approvals you need (on the contract creation) to be able to send a transfer. This is definitely a unique approach so well done. I suppose it all comes down to personal opinion but i could suggest another approach you could choose to improve upon this and save you hard coding a value fro the number of approver sin the constructor. You could change the struct atribute to have a approvals attribute that is a uint not an array. Then what you could do is not to define the number of approvals required in the constructor but rather in you addUser() function define the number of required approvals for a transfer instance to be say 2/3’'s of the approver set or even something easier to be 1 minus the len of the owners set. So in your addUser() function write something akin to
function addOwner(address newOwner) public {
........
.......
.......
numApprovals = owners.length - 1
}
note that if you do a delete user function remeber to redefine this variable after you pop off the owner. What you could then do is to create another attribute in your transfer struct called approved that is a boolean and set this equal to false initially and then when the number of approvals is satified set it equal to true and then in your actual transfer function instead of checking if the length of you approvals array is equal to the number of approvals defined on the contract creation, just check is the Approed attribute is equal to true.
But again amazing work man i really like your contract. continue on take solidity 201 new and old learn even more.
Evan
1 Like
Hi, @jon_m
Quick question that I’m struggling to find the answer for!
Can a struct have a constructor in solidity? If so what is the syntax please
Thanks
Ismail
1 Like
Hi @Random,
The struct itself cannot contain a constructor. However, if you want to create struct instances on deployment, then you could do that with a constructor as follows:
struct Person {
string name;
uint age;
}
mapping(address => Person) people;
constructor(
string memory name1, uint age1,
string memory name2, uint age2, address person2
) {
require(
msg.sender != person2,
"Address of 2nd person must be different to deployer's address"
);
people[msg.sender] = Person(name1, age1);
people[person2] = Person(name2, age2);
}
Is that the kind of thing you want to do? Just let me know if you have any further questions about this.
I wanted to make a project a bit more real so I added a bit beyond what was required to allow revocation of approvals and cancellation of transfers that failed.
While was designing the project I was wondering about the efficiency of the code:
- What is a better way of storing transfers, mappings or arrays?
- Should I delete objects that are no longer needed?
The first question is simple. The use of either is fine for this project as we do value lookups by a known index. Should we use more complex transfer ids then mapping would be the more reasonable choice.
The answer to the second question was interesting. I found that if I delete a Transfer object once it is no longer needed the function consumes less gas. In my case the final call to approveTransfer was reduced from 90642 to 49665 wei. However not all deletions are beneficial in terms of gas. I tried to delete the double mapping with approvals. For this I had to iterate over the array of owners in order to access their approvals and delete them one by one. Having added the deletion of approvals I found that the gas went up from 49665 to 69251. So it was better be left forever growing up until the length of 2^256. It seems like it is a matter of how much space you free vs how much effort is needed in order to access that memory. Iteration over array some consumes gas, although not a lot for an array with 3 entries. And the deletion of a bool value doesn’t seem to have much of an impact on how much gas the caller is reimbursed.
These have been my observations so far. Very interesting. Continuing to study…
Here’s the code:
pragma solidity ^0.8.4;
contract Wallet {
// constants
uint8 constant public MAX_OWNERS = 10;
// enums
enum TransferState {
Deleted, // This state should be only reached when the Transfer item is deleted
Pending,
Ready, // Transient state
Completed,
Cancelled, // Transient state
Failed
}
// state variables
uint public approvalCount;
uint public id;
address[MAX_OWNERS] private owners_array; // Used to cleanup approvals (not worth it in termas of gas)
mapping(address => bool) private owners; // Used for onlyOwner modifier
mapping(uint => Transfer) public transfers; // Infinitely growing
mapping(uint => mapping(address => bool)) public approvals; // Infinitely growing
// structs
struct Transfer {
address payable recepient;
uint amount;
uint creationTime;
uint approvalCount;
TransferState state;
}
// events
event ReportTransferState(
uint indexed id,
address indexed recepient,
uint amount,
TransferState indexed state
);
// modifiers
modifier onlyOwner() {
require(owners[msg.sender] == true, "Owner-only operation");
_;
}
modifier transferExists(uint _id) {
require(_id <= id, "Transaction doen't exist yet");
_;
}
// constructor
constructor(uint _approvalCount, address[] memory _owners) {
require(_owners.length <= MAX_OWNERS, "Too many owners");
require(_owners.length >= _approvalCount, "Approval count cannot exceed the number of owners");
for(uint i = 0; i < _owners.length; i++) {
owners[_owners[i]] = true;
owners_array[i] = _owners[i];
}
approvalCount = _approvalCount;
id = 0;
}
//public
function deposit () public payable {}
function withdraw (uint amount) public payable onlyOwner {
payable(msg.sender).transfer(amount);
}
function getBalance () public view returns (uint) {
return address(this).balance;
}
// Should create a new Transfer object
// Only owner is allowed to create TransferState
// Do not create transfers if the wallet balance is lower than the specified amount
function createTransfer (address payable recepient, uint amount) public onlyOwner returns (uint){
require(address(this).balance >= amount, "Insufficient balance in Wallet");
// Create a new transfer object
// approvalCount is set to 1 because the creator is assumed to have approved the transfer he has created
Transfer memory new_transfer = Transfer(
recepient,
amount,
block.timestamp,
1,
TransferState.Pending
);
//Register the new transfer
transfers[id] = new_transfer;
approvals[id][msg.sender] = true;
processState(id);
id++;
return id - 1;
}
// Approves existing transfers which are in Pending state
// Only owner can approve transfers
// Each address can approve transation only once
function approveTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint){
require(transfers[_id].state == TransferState.Pending, "Only pending transfer can be approved");
require(approvals[_id][msg.sender] == false, "This address already approved this transaction");
// Change transfer states
transfers[_id].approvalCount++;
approvals[_id][msg.sender] = true;
assert(
transfers[_id].approvalCount <= approvalCount &&
transfers[_id].approvalCount > 1
);
processState(_id);
return _id;
}
// Revokes approval from existing transfers which are in Pending state
// Only owner can revoke transfers
// Each address can revoke transation only if it has previously approved it
function revokeTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint){
require(transfers[_id].state == TransferState.Pending, "Only pending transfer can be revoked");
require(approvals[_id][msg.sender] == true, "This address didn't approved this transaction");
// Change transfer states
transfers[_id].approvalCount--;
approvals[_id][msg.sender] = false;
assert(
transfers[_id].approvalCount < approvalCount &&
transfers[_id].approvalCount >= 0
);
processState(_id);
return _id;
}
// Execute transfer that is in Ready state
function executeTransfer (uint _id) public payable onlyOwner transferExists(_id) returns (uint) {
require(
transfers[_id].state == TransferState.Ready ||
transfers[_id].state == TransferState.Failed,
"Only Ready or Failed states are allowed"
);
if (transfers[_id].recepient.send(transfers[_id].amount))
transfers[_id].state = TransferState.Completed;
else
transfers[_id].state = TransferState.Failed;
processState(_id);
return _id;
}
// Cancels transfer that is in Failed state
function cancelFailedTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint) {
require(transfers[_id].state == TransferState.Failed, "Only Failed transfer can be cancelled");
transfers[_id].state == TransferState.Cancelled;
processState(_id);
return _id;
}
//private
function processState(uint _id) private {
Transfer storage t = transfers[_id];
if (t.state == TransferState.Pending) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
if (t.approvalCount == approvalCount)
t.state = TransferState.Ready;
else if (t.approvalCount == 0)
t.state = TransferState.Cancelled;
}
if (t.state == TransferState.Ready) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
executeTransfer(_id);
}
if (t.state == TransferState.Failed) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
}
if (t.state == TransferState.Cancelled || t.state == TransferState.Completed) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
// This delete reduces gas fee from 90642 to 49665
delete transfers[_id];
// Further deletion of approvals via iteration through owners_array
// only increases the gas fee to 69251
}
}
}
1 Like
hey @SergeyG wow great solution. Really love the way you included block.timestamp to get the time at which a transfer was created. and then also the use of enumerate to create a variety of different transfer “states” thats actually something i think is very elegant incredible. and lastly great to see your using events
Great to also see that your concerned with gas.You really know what you are doing man well done. Its an intresting question. See arrays are nice because you can always have an entire histroy of all transfers made but the larger the array gets the worse this is for gas costs especially if you want to do some lookup and the index you are looking for is towards the end of the array. I always liked using arrays for these types of structs because you can have the entire histroy. But i suppose what could do to be more efficent is use more events. So you could store your transfer struct in mappings where the address of the person who created the transfer maps to the transfer struct itself. Then what you can do is create a number of events, like an event on trasnfer created, and event on approval, and event and transfer complettion all of these things like you have done above with you reportTransferState event. That way you can delete your mapping at the end and all of the data is still avaialble for quereing through alll of the fired events. I always hated deleting things (which imo is a bad habbit to not delete thing ssometimes) because im a fan of being able to query information (even though it sometimes may not be nessecary lol)
However i would say it comes down to both personal preferance but also comes down to the specific needs of your application. Like if your making some front end and you have some array of structs contain valuable information on something and you want to have the option to click a button and it displays the entire histroy, i.e the entire array then maybe you would want to store everything in the array over mapings even if thus will cost increasingly more gas as the size increases.
I really like your reflection it has made me think you have definietly taught me a thing or two as well. Any more intresting thoughts dont be afraid to share. Everybody can learn something from posts like these its great to see. Keep going my man.
Evan
1 Like
hey @SergeyG i had a play with your code in remix because i was curious about your gas insights when deleting the approvals. I managed to reduce the gas cost of executing a transfer but only by around 10000 gas which isnt brilliant. the way you can get aroun this is to get rid of your approvals maping and to just add an extra attrivute to your transfer struct which is an array of address who have already approved the transfer. Something like this
struct Transfer {
address payable recepient;
uint amount;
uint creationTime;
uint approvalCount;
address[] approvers;
TransferState state;
}
and then in your createTransfer function it now looks like
function createTransfer (address payable recepient, uint amount) public onlyOwner returns (uint){
require(address(this).balance >= amount, "Insufficient balance in Wallet");
// Create a new transfer object
// approvalCount is set to 1 because the creator is assumed to have approved the transfer he has created
Transfer memory new_transfer = Transfer(
recepient,
amount,
block.timestamp,
1,
new address[](0),
TransferState.Pending
);
//Register the new transfer
transfers[id] = new_transfer;
processState(id);
id++;
return id - 1;
by doing this each time an approval get its gets pushed into this approvers array in the transfer struct. This saves you from having to do two deletes because when you go to approve again you can just check that the address wgo is approving is not already in the approvers address array in the transfer struct. so your approve functions requirment now looks like this
function approveTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint){
require(transfers[_id].state == TransferState.Pending, "Only pending transfer can be approved");
// require(approvals[_id][msg.sender] == false, "This address already approved this transaction");
for (uint i = 0; i < transfers[_id].approvers.length; i++) {
if(transfers[_id].approvers[i] == msg.sender) revert();
}
........
........
........
this method does switch to loops which changes the complexity to no longer be constant time (but only in the two requirement statements in the approve and revoke transfer functions so it actually doesnt affect things much at all) as with mappings we are doing direct look ups but the even with that fact for my test case i had an arrray with 5 addresses and 4 approvals and the cost of the execute trade function was around 58000 - 60000 gas over the few times i tried it. However in solidity you cannot really delete mappings, when you call the delete transfers[_id] it actually just sets all of the attributes to zero. You can check this by creating a get transfer function and running it
function getTransfers(uint _id) public view returns (Transfer memory) {
return transfers[_id];
}
So what i sought to do what to change you transfer mapping to instead create an array of transfer strcts. Doing this i thought that deleting would be more efficient because what you can do is to move the index you want to delete to then last element of the array and call .pop(). However after i implemented the change in datastructures, although i prefer the .pop() method for deleting data the gas actually rose to about 100000, so it was not as efficient as the mappings themselves.
I have attached the changes to your code i made below. Note that i only made changes to play around with the gas there was a few security things you could look into in terms of the requirments of trasfers and what is and is not allowed, for example you could change it so that the transfer creater is not allowed to approve, this would mean setting approvalCount == 0 in the struct instance in the createTransfer function instead of approvalCount == 1. and also perhaps you could include a requirement that if one of the oweners is sending to another owner then the recipient in this specific case is not allowed to approve, or perhaps you could make it that the owners are not allowed to transfer amoungst themselves it really comes down to your preferances. and lastly you can make and addUser and deleteUser() functions that allow you to dynamically allocate and delete the wallet owners, i did not implement this in your code but here but an example would be
//note using this method you need to change your onlyOwner modifier to the following
modifier onlyOwner() {
bool owner = false;
for(uint i=0; i<owners_array.length;i++){
if(owners_array[i] == msg.sender){
owner = true;
}
}
require(owner == true);
_;
}
function addUsers(address _owners) public
{
for (uint user = 0; user < owners_array.length; user++)
{
require(owners_array[user] != _owners, "Already registered");
}
owners_array.push(_owners);
//from the current array calculate the value of minimum consensus
approvalCount = owners_array.length - 1;
}
//remove user require the address we pass in is the address were removing
function removeUser(address _user) public
{
uint user_index;
for(uint user = 0; user < owners_array.length; user++)
{
if (owners_array[user] == _user)
{
user_index = user;
require(owners_array[user] == _user);
}
}
owners_array[user_index] = owners_array[owners_array.length - 1];
owners_array.pop();
//owners;
}
//gets wallet users
function getUsers() public view returns(address[] memory)
{
return owners_array;
}
but i hope maybe this example of different approaches will inspire you to go on even further because you have a great program here. Anyway incase your curious here is my edited version to get reduced gas (even if the reduction is smaller than i had hoped (10000) gas or so). Onto 201 next my dude and 201 [OLD] great projects in both
pragma solidity ^0.8.4;
pragma abicoder v2;
contract Wallet {
// constants
uint8 constant public MAX_OWNERS = 10;
// enums
enum TransferState {
Deleted, // This state should be only reached when the Transfer item is deleted
Pending,
Ready, // Transient state
Completed,
Cancelled, // Transient state
Failed
}
// state variables
uint public approvalCount;
uint public id;
address[MAX_OWNERS] private owners_array; // Used to cleanup approvals (not worth it in termas of gas)
mapping(address => bool) private owners; // Used for onlyOwner modifier
mapping(uint => Transfer) public transfers; // Infinitely growing
// structs
struct Transfer {
address payable recepient;
uint amount;
uint creationTime;
uint approvalCount;
address[] approvers;
TransferState state;
}
// events
event ReportTransferState(
uint indexed id,
address indexed recepient,
uint amount,
TransferState indexed state
);
// modifiers
modifier onlyOwner() {
require(owners[msg.sender] == true, "Owner-only operation");
_;
}
modifier transferExists(uint _id) {
require(_id <= id, "Transaction doen't exist yet");
_;
}
// constructor
constructor(uint _approvalCount, address[] memory _owners) {
require(_owners.length <= MAX_OWNERS, "Too many owners");
require(_owners.length >= _approvalCount, "Approval count cannot exceed the number of owners");
for(uint i = 0; i < _owners.length; i++) {
owners[_owners[i]] = true;
owners_array[i] = _owners[i];
}
approvalCount = _approvalCount;
id = 0;
}
//public
function deposit () public payable {}
function getTransfers(uint _id) public view returns (Transfer memory) {
return transfers[_id];
}
function withdraw (uint amount) public payable onlyOwner {
payable(msg.sender).transfer(amount);
}
function getBalance () public view returns (uint) {
return address(this).balance;
}
// Should create a new Transfer object
// Only owner is allowed to create TransferState
// Do not create transfers if the wallet balance is lower than the specified amount
function createTransfer (address payable recepient, uint amount) public onlyOwner returns (uint){
require(address(this).balance >= amount, "Insufficient balance in Wallet");
// Create a new transfer object
// approvalCount is set to 1 because the creator is assumed to have approved the transfer he has created
Transfer memory new_transfer = Transfer(
recepient,
amount,
block.timestamp,
1,
new address[](0),
TransferState.Pending
);
//Register the new transfer
transfers[id] = new_transfer;
processState(id);
id++;
return id - 1;
}
// Approves existing transfers which are in Pending state
// Only owner can approve transfers
// Each address can approve transation only once
function approveTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint){
require(transfers[_id].state == TransferState.Pending, "Only pending transfer can be approved");
// require(approvals[_id][msg.sender] == false, "This address already approved this transaction");
for (uint i = 0; i < transfers[_id].approvers.length; i++) {
if(transfers[_id].approvers[i] == msg.sender) revert();
}
// Change transfer states
transfers[_id].approvalCount++;
transfers[_id].approvers.push(msg.sender);
assert(
transfers[_id].approvalCount <= approvalCount &&
transfers[_id].approvalCount > 1
);
processState(_id);
return _id;
}
// Revokes approval from existing transfers which are in Pending state
// Only owner can revoke transfers
// Each address can revoke transation only if it has previously approved it
function revokeTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint){
require(transfers[_id].state == TransferState.Pending, "Only pending transfer can be revoked");
for (uint i = 0; i < transfers[_id].approvers.length; i++) {
require(transfers[_id].approvers[i] == msg.sender);
}
// Change transfer states
transfers[_id].approvalCount--;
assert(
transfers[_id].approvalCount < approvalCount &&
transfers[_id].approvalCount >= 0
);
processState(_id);
return _id;
}
// Execute transfer that is in Ready state
function executeTransfer (uint _id) public payable onlyOwner transferExists(_id) returns (uint) {
require(
transfers[_id].state == TransferState.Ready ||
transfers[_id].state == TransferState.Failed,
"Only Ready or Failed states are allowed"
);
if (transfers[_id].recepient.send(transfers[_id].amount))
transfers[_id].state = TransferState.Completed;
else
transfers[_id].state = TransferState.Failed;
processState(_id);
return _id;
}
// Cancels transfer that is in Failed state
function cancelFailedTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint) {
require(transfers[_id].state == TransferState.Failed, "Only Failed transfer can be cancelled");
transfers[_id].state == TransferState.Cancelled;
processState(_id);
return _id;
}
//private
function processState(uint _id) private {
Transfer storage t = transfers[_id];
if (t.state == TransferState.Pending) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
if (t.approvalCount == approvalCount)
t.state = TransferState.Ready;
else if (t.approvalCount == 0)
t.state = TransferState.Cancelled;
}
if (t.state == TransferState.Ready) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
executeTransfer(_id);
}
if (t.state == TransferState.Failed) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
}
if (t.state == TransferState.Cancelled || t.state == TransferState.Completed) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
// This delete reduces gas fee from 90642 to 49665
delete transfers[_id];
// Further deletion of approvals via iteration through owners_array
// only increases the gas fee to 69251
}
}
}
1 Like
Hey @mcgrane5 thank you for the great feedback. I appreciate the time you’ve spent to write so much of an insight and even provide the modified code snippets. This is probably the best ever feedback I’ve had in my life.
In the meantime I had started the 201 course yesterday. Filip went over the patterns that are meant to provide the best of the two worlds, the mappings and the arrays.
I made an alternate implementation where I used an array instead of mapping for Transfer object. This allowed me to delete the Transfer object and place the last element to the position of the deleted object in the array. The problem I’ve got however was that I couldn’t do the same type of remapping in the approvals mapping. Here’s the code that threw me an error:
if (t.state == TransferState.Cancelled || t.state == TransferState.Completed) {
if (t.state == TransferState.Completed)
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
if (transfers.length == 1) {
delete transfers;
}
else {
delete transfers[_id];
transfers[_id] = transfers[transfers.length - 1];
approvals[_id] = approvals[transfers.length - 1]; // Types in storage containing (nested) mappings cannot be assigned to
transfers[_id].id = _id;
transfers.pop();
}
}
So I couldn’t evaluate the gas consumption for this implementation yet.
I’m gonna head and try the changes you’ve outlined in your post and try to poke around with the code a bit more. I’ll get back with the results.
PS
Infinitely growing arrays doesn’t sound right to me lol. I agree with you on the use of the events. In my code I had only one event with the state that conveys the current status of the transfer. For the sake of readability and ease of use, it might be better to define dedicated events for each of the transfer state, e.g., TransferCreated, TransferCancelled, TransferCompleted, TransferFailed, etc. This would make the code cleaner. Nevertheless I also do not discredit the use of arrays as long as there is a proper housekeeping in place.
Sergey
1 Like
Here’s the updated solution. Gas for the final approval that invokes executeTransfer is lowered from 49665 to 39259. @mcgrane5 similar to what you have achieved. I used a different approach for maintaining the approvals by encoding them via the uint256. The limitation here is that you cannot have more than 255 users which is probably fine. The beauty is though that regardless of the amount of users the gas cost should remain the same for sending approvals. Here’s the code:
pragma solidity ^0.8.4;
contract Wallet {
// constants
uint8 constant public MAX_OWNERS = 10;
// enums
enum TransferState {
Deleted, // This state should be only reached when the Transfer item is deleted
Pending,
Ready, // Transient state
Completed,
Cancelled, // Transient state
Failed,
Expired // Transient state
}
// state variables
uint public approvalCount;
uint256 public bitmask = 1; // Can accommodate up to 255 owners
address[] private owners_array; // Used to cleanup approvals (not worth it in termas of gas)
mapping(address => uint256) public owners; // Used for onlyOwner modifier
Transfer[] public transfers; // Infinitely growing
// structs
struct Transfer {
address payable recepient;
uint amount;
uint creationTime;
uint approvalCount;
TransferState state;
uint256 approvalBitmap;
}
// events
event ReportTransferState(
uint indexed id,
address indexed recepient,
uint amount,
TransferState indexed state
);
// modifiers
modifier onlyOwner() {
require(owners[msg.sender] > 0, "Owner-only operation");
_;
}
modifier transferExists(uint _id) {
require(transfers.length > _id, "Transaction doen't exist");
_;
}
// constructor
/*
constructor(uint _approvalCount, address[] memory _owners) {
require(_owners.length <= MAX_OWNERS, "Too many owners");
require(_owners.length >= _approvalCount, "Approval count cannot exceed the number of owners");
// Initialize bitmask and encode owners via the bits
for(uint i = 0; i < _owners.length; i++) {
owners_array.push(_owners[i]);
if (i == 0) {
owners[_owners[i]] = 1;
}
else {
owners[_owners[i]] = owners[_owners[i-1]] << 1;
bitmask <<= 1;
bitmask |= 1;
}
}
}
*/
constructor() {
address[10] memory _owners = [
0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,
0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2,
0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db,
0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB,
0x617F2E2fD72FD9D5503197092aC168c91465E7f2,
0x17F6AD8Ef982297579C203069C1DbfFE4348c372,
0x5c6B0f7Bf3E7ce046039Bd8FABdfD3f9F5021678,
0x03C6FcED478cBbC9a4FAB34eF9f40767739D1Ff7,
0x1aE0EA34a72D944a8C7603FfB3eC30a6669E454C,
0x0A098Eda01Ce92ff4A4CCb7A4fFFb5A43EBC70DC
];
// Initialize bitmask and encode owners via the bits
for(uint i = 0; i < _owners.length; i++) {
owners_array.push(_owners[i]);
if (i == 0) {
owners[_owners[i]] = 1;
}
else {
owners[_owners[i]] = owners[_owners[i-1]] << 1;
bitmask <<= 1;
bitmask |= 1;
}
}
approvalCount = 3;
}
//public
function deposit () public payable {}
function withdraw (uint amount) public payable onlyOwner {
payable(msg.sender).transfer(amount);
}
function getBalance () public view returns (uint) {
return address(this).balance;
}
function getOwners() public view returns (uint256[] memory){
uint256[] memory _addr = new uint256[](owners_array.length);
for (uint i = 0; i < owners_array.length; i++) {
_addr[i] = owners[owners_array[i]];
}
return _addr;
}
// Should create a new Transfer object
// Only owner is allowed to create TransferState
// Do not create transfers if the wallet balance is lower than the specified amount
// gas: 189193
function createTransfer (address payable recepient, uint amount) public onlyOwner returns (uint){
require(address(this).balance >= amount, "Insufficient balance in Wallet");
// Create a new transfer object
// approvalCount is set to 1 because the creator is assumed to have approved the transfer he has created
Transfer memory new_transfer = Transfer(
recepient,
amount,
block.timestamp,
1,
TransferState.Pending,
bitmask ^ owners[msg.sender]
);
//Register the new transfer
transfers.push(new_transfer);
uint index = transfers.length - 1;
processState(index);
return index;
}
// Approves existing transfers which are in Pending state
// Only owner can approve transfers
// Each address can approve transation only once
// gas: 44471
function approveTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint){
require(transfers[_id].state == TransferState.Pending, "Only pending transfer can be approved");
require(transfers[_id].approvalBitmap & owners[msg.sender] != 0, "This address already approved this transfer");
// Change transfer states
transfers[_id].approvalCount++;
transfers[_id].approvalBitmap ^= owners[msg.sender];
assert(
transfers[_id].approvalCount <= approvalCount &&
transfers[_id].approvalCount > 1
);
processState(_id);
return _id;
}
// Revokes approval from existing transfers which are in Pending state
// Only owner can revoke transfers
// Each address can revoke transation only if it has previously approved it
function revokeTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint){
require(transfers[_id].state == TransferState.Pending, "Only pending transfer can be revoked");
require(transfers[_id].approvalBitmap & owners[msg.sender] == 0, "This address didn't approved this transfer");
// Change transfer states
transfers[_id].approvalCount--;
transfers[_id].approvalBitmap ^= owners[msg.sender];
assert(
transfers[_id].approvalCount < approvalCount &&
transfers[_id].approvalCount >= 0
);
processState(_id);
return _id;
}
// Execute transfer that is in Ready state
function executeTransfer (uint _id) public payable onlyOwner transferExists(_id) returns (uint) {
require(
transfers[_id].state == TransferState.Ready ||
transfers[_id].state == TransferState.Failed,
"Only Ready or Failed states are allowed"
);
if (transfers[_id].recepient.send(transfers[_id].amount))
transfers[_id].state = TransferState.Completed;
else
transfers[_id].state = TransferState.Failed;
processState(_id);
return _id;
}
// Cancels transfer that is in Failed state
function cancelFailedTransfer (uint _id) public onlyOwner transferExists(_id) returns (uint) {
require(transfers[_id].state == TransferState.Failed, "Only Failed transfer can be cancelled");
transfers[_id].state == TransferState.Cancelled;
processState(_id);
return _id;
}
//private
function processState(uint _id) private {
Transfer storage t = transfers[_id];
if (t.state == TransferState.Pending) {
// emit event only if the transfer has just been created
if (t.approvalCount == 1)
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
if (t.approvalCount == approvalCount)
t.state = TransferState.Ready;
else if (t.approvalCount == 0)
t.state = TransferState.Cancelled;
}
if (t.state == TransferState.Ready) {
executeTransfer(_id);
}
if (t.state == TransferState.Failed) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
}
if (t.state == TransferState.Cancelled || t.state == TransferState.Completed) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
// with deletion: 39259 for the final approval
// w/o deletion: 54601 for the final approval
delete transfers[_id];
}
}
}
I commented out the constructor that requires parameters to be passed and created one without for the ease of deployment. The owner’s addresses would need to change for ones particular remix environment.
The array of transfers is still infinitely growing. Maintaining that aspect would consume more gas though as after each transaction completion or cancellation, the remapping of the transfers array would have to take place.
I’m wondering how it is laid out in memory. I assume that each index is a memory address. So forever-growing array == memory leak. Also the contract in the current state can handle as many as 2^256 transfers, which is a lot and may result in a significant memory footprint of 2^256 * (the size of memory address). Some kind of limits should probably be imposed to make it safer to operate over long periods of time.
1 Like
hey @SergeyG no worries at all g. Nicee one brilliant man. Thats a great reduction. great work around getting rid of the approvals mapping. Hhahaha yeah definitely i would imagine a real multisig wallet would only have like 5 - 10 owners max, so you would not have to worry. Cool solution with bitmask i actually dont have experince with this i must look it up. Much more elegant than my approach of using an array of addresses it seems great work. i really like your solution you will do well in the next courses.
Evan
2 Likes
Here’s my solution for a multi-signature wallet. After tinkering with it for a few days, I’ve finally written a solution without going through hint videos.
I have partitioned the code into two files for better structuring using inheritance. Owners.sol is associated with owners only. The main functionality I’ve implemented in Wallet.sol.
I’ve segregated the action of the transfer function into 3 other functions -
createTxn() - For creating a transfer request.
approveTxn() - Using this function, other owners can approve the transaction.
runTxn() - The creator of the transfer request can run the transaction after getting the required number of approvals.
Highly appreciative of any advice on improvement. 
Owners.sol
pragma solidity 0.7.5;
contract Owners{
address [] allOwners;
uint numberOfApprovals;
mapping (address => bool) isOwner;
modifier onlyOwners {
require(isOwner[msg.sender],'Accessible by owners only');
_;
}
function getOwners() public view onlyOwners returns(address[]memory){
return allOwners;
}
}
Wallet.sol
pragma solidity 0.7.5;
pragma abicoder v2;
import './Owners.sol';
contract MultisigWallet is Owners{
struct Transaction{
address transmitter;
address payable receiver;
uint amount;
uint confirmationCount;
uint txnId;
}
Transaction[] transactions;
// mapping of an address to its balance.
mapping(address => uint)balance;
//mapping of a transaction id to an array of owner addresses those who've approved.
mapping(uint => address[])checkApprovals;
//mapping of a transaction id to a transaction itself.
mapping(uint => Transaction)transactionLog;
event AmountDeposited(address indexed depositor, uint amount);
event TransactionCreated(address sender, address receiver, uint amount,uint txnId);
event ViewTransaction(address sender, address receiver, uint amount,uint txnId, uint approvalCount, uint approvalsNeeded);
//takes input as an array of owners and the number of approvals needed for a transfer request
constructor(address[] memory _allOwners , uint _approvals) {
//approvals must be less than number of owners. As an owner can't approve his own request
require(_approvals < _allOwners.length && _approvals > 0 , 'Invalid number of approvals');
allOwners = _allOwners;
numberOfApprovals = _approvals;
for(uint i = 0 ; i < _allOwners.length ; i++){
isOwner[allOwners[i]] = true;
}
}
// anybody can deposit funds in the wallet
function deposit()public payable returns(uint){
balance[msg.sender] += msg.value;
emit AmountDeposited(msg.sender , msg.value);
return balance[msg.sender];
}
// a function for creating a transfer request, takes input as a recipient address and an amount to tranfer
function createTxn(address payable _recipient,uint _amount) public onlyOwners{
require(balance[msg.sender] >= _amount , 'The amount is beyond deposited.');
require(msg.sender != _recipient,"You can't transfer to yourself");
Transaction memory txn = Transaction(msg.sender,_recipient,_amount,0,transactions.length);
transactionLog[transactions.length] = txn;
emit TransactionCreated(msg.sender,_recipient, _amount,transactions.length);
transactions.push(txn);
}
/*
a function for approving transfer requests.
a creator of a transfer request can not approve his own request.
an owner can not approve a request more than once.
*/
function approveTxn(uint _txnId) public onlyOwners {
require(transactionLog[_txnId].transmitter != msg.sender,"You can't approve your own transaction");
for(uint i = 0 ; i < checkApprovals[_txnId].length ; i++){
require(msg.sender != checkApprovals[_txnId][i],"You've already approved the transaction");
}
checkApprovals[_txnId].push(msg.sender);
transactionLog[_txnId].confirmationCount += 1;
}
//once a request has the required number of approvals, it can be executed by the request creator.
function runTxn(uint _txnId) public onlyOwners{
Transaction memory txn = transactionLog[_txnId];
require(txn.transmitter == msg.sender,'Please create a transaction first.');
require(txn.confirmationCount >= numberOfApprovals,'Please get your transaction approved');
uint oldBalance = balance[msg.sender];
balance[msg.sender] -= txn.amount;
txn.receiver.transfer(txn.amount);
assert(balance[msg.sender] == oldBalance - txn.amount);
}
// an owner can withdraw his own funds.
function withdrawMyFunds(uint amount) public onlyOwners returns(uint){
require(balance[msg.sender] >= amount,'Withdrawal is more than a deposit');
uint oldBlance = balance[msg.sender];
balance[msg.sender] -= amount;
msg.sender.transfer(amount);
assert(balance[msg.sender] == oldBlance - amount);
return balance[msg.sender];
}
//an owner can check his balance
function myBalance() public view onlyOwners returns(uint){
return balance[msg.sender];
}
//anybody can check the wallet Balance
function walletBalance() public view returns(uint256){
return address(this).balance;
}
//anybody can view transaction
function viewTxn(uint _txnId) public returns(Transaction memory){
Transaction memory txn = transactionLog[_txnId];
emit ViewTransaction(txn.transmitter,txn.receiver,txn.amount,_txnId,txn.confirmationCount,numberOfApprovals);
return txn ;
}
}
2 Likes
Ok here is mine, took me a good 4 hours…pfiouuu
// SPDX-License-Identifier: GPL-3.0
pragma solidity >=0.7.0 <0.9.0;
pragma abicoder v2;
abstract contract Ownable {
address[] internal owners;
uint internal numberOfApprovals;
modifier onwersAccessOnly{
bool ownerFound=false;
for (uint j = 0; j < owners.length; j++) {
if (msg.sender==owners[j]){
ownerFound=true;
}
}
require(ownerFound,"You are not a declared owner for this contract");
_;
}
constructor(address[] memory _owners,uint _numberOfApprovals ) {
owners=_owners;
numberOfApprovals=_numberOfApprovals;
}
}
//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;
event moneyDeposited(address,uint);
//transfer request id,requester,transfer recipient, amount
event transferRequested(uint,address,address,uint);
//transfer request id,transfer recipient, amount
event transferAccepted(uint,address,uint);
//example ["0x1aE0EA34a72D944a8C7603FfB3eC30a6669E454C","0x0A098Eda01Ce92ff4A4CCb7A4fFFb5A43EBC70DC","0xCA35b7d915458EF540aDe6068dFe2F44E8fa733c"],2
constructor(address[] memory _owners,uint _numberOfApprovals) Ownable(_owners,_numberOfApprovals){
owners=_owners;
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");
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++;
}
require(address(this).balance-totalRequested>=_amount,"Too much money already pledged to accept the transfer request");
TransferRequest memory _transferRequest=TransferRequest(size,_recipient,new address[](0),_amount,false);
transferRequests[size]=_transferRequest;
transferRequests[size].acceptedBy.push(msg.sender);
emit transferRequested(_transferRequest.id,msg.sender,_transferRequest.recipient,_transferRequest.amount);
_attemptApproval(size);
return size;
}
//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 {
_transfertRequestExists;
TransferRequest memory _transferRequest=transferRequests[_transfertRequestId];
if ( _transferRequest.acceptedBy.length==numberOfApprovals){
_acceptTransfer(_transferRequest.id);
}
}
function _acceptTransfer (uint _transfertRequestId) private {
_transfertRequestExists;
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);
emit transferAccepted(_transferRequest.id,_transferRequest.recipient,_transferRequest.amount);
transferRequests[_transfertRequestId].paid=true;
}
function _transfertRequestExists (uint _transfertRequestId) private view {
//way of checking if key exists
require(transferRequests[_transfertRequestId].amount>0);
}
/*
To know the contract total deposited amount
*/
function getWalletBalance() public view returns(uint) {
return address(this).balance;
}
}
1 Like
I like the owners mapping, very useful! I believe you don’t need to store the array of owners received in the constructor then.
If I understand your code properly, once someone has approved a transaction request, he actually approves all transactions requests. My understanding of the requirements is that each transaction request should have its own lifecycle.
JP
2 Likes
Thanks, @jeanphi for your input.
Once someone has approved a transaction request, he approves that particular request only associated with a specific txnId.
I don’t think the code indicates that one owner can approve all requests in a single go.
Tanu
1 Like
Oh yeah that’s right, I read your code too quickly sorry.
Hey @mcgrane5. I poked around for a bit more and I got a solution with fixed array for transfers. There is indexQueue that is a dynamic array of freed indices which is populated with the index of deleted transfer. Next time a transfer created the index is popped of the indexQueue and reused for the new transfer object.
Drawbacks:
- Transfer IDs are reused.
- The gas price went up on the final execution to from 39k to 53k compared to the previous solution.
Conclusion:
Unless there is an implication of infinitely growing arrays this solution is a no go.
Code:
pragma solidity ^0.8.4;
contract Wallet {
// constants
uint8 constant public MAX_OWNERS = 255;
uint8 constant public MAX_TRANSFERS = 3;
// enums
enum TransferState {
Deleted, // This state should be only reached when the Transfer item is deleted
Pending,
Ready, // Transient state
Completed,
Cancelled, // Transient state
Failed,
Expired // Transient state
}
// state variables
uint id;
uint public approvalCount;
uint256 public bitmask = 1; // Can accommodate up to 255 owners
uint[] public indexQueue; // Holds available transfers' positions
address[] private owners_array; // Used to cleanup approvals (not worth it in termas of gas)
mapping(address => uint256) public owners; // Used for onlyOwner modifier
Transfer[MAX_TRANSFERS] public transfers; // Infinitely growing
// structs
struct Transfer {
uint id;
address payable recepient;
uint amount;
uint creationTime;
uint approvalCount;
TransferState state;
uint256 approvalBitmap;
}
// events
event ReportTransferState(
uint indexed id,
address indexed recepient,
uint amount,
TransferState indexed state
);
// modifiers
modifier onlyOwner() {
require(owners[msg.sender] > 0, "Owner-only operation");
_;
}
// constructor
/*
constructor(uint _approvalCount, address[] memory _owners) {
require(_owners.length <= MAX_OWNERS, "Too many owners");
require(_owners.length >= _approvalCount, "Approval count cannot exceed the number of owners");
// Initialize bitmask and encode owners via the bits
for(uint i = 0; i < _owners.length; i++) {
owners_array.push(_owners[i]);
if (i == 0) {
owners[_owners[i]] = 1;
}
else {
owners[_owners[i]] = owners[_owners[i-1]] << 1;
bitmask <<= 1;
bitmask |= 1;
}
}
id = 0;
}
*/
constructor() {
address[10] memory _owners = [
0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,
0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2,
0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db,
0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB,
0x617F2E2fD72FD9D5503197092aC168c91465E7f2,
0x17F6AD8Ef982297579C203069C1DbfFE4348c372,
0x5c6B0f7Bf3E7ce046039Bd8FABdfD3f9F5021678,
0x03C6FcED478cBbC9a4FAB34eF9f40767739D1Ff7,
0x1aE0EA34a72D944a8C7603FfB3eC30a6669E454C,
0x0A098Eda01Ce92ff4A4CCb7A4fFFb5A43EBC70DC
];
// Initialize bitmask and encode owners via the bits
for(uint i = 0; i < _owners.length; i++) {
owners_array.push(_owners[i]);
if (i == 0) {
owners[_owners[i]] = 1;
}
else {
owners[_owners[i]] = owners[_owners[i-1]] << 1;
bitmask <<= 1;
bitmask |= 1;
}
}
approvalCount = 3;
id = 0;
}
//public
function deposit () public payable {}
function withdraw (uint amount) public payable onlyOwner {
payable(msg.sender).transfer(amount);
}
function getBalance () public view returns (uint) {
return address(this).balance;
}
function getOwners() public view returns (uint256[] memory){
uint256[] memory _addr = new uint256[](owners_array.length);
for (uint i = 0; i < owners_array.length; i++) {
_addr[i] = owners[owners_array[i]];
}
return _addr;
}
function getAvailableIndices() public view returns (uint[] memory) {
return indexQueue;
}
// Should create a new Transfer object
// Only owner is allowed to create TransferState
// Do not create transfers if the wallet balance is lower than the specified amount
// gas: 193435 or 186681
function createTransfer (address payable recepient, uint amount) public onlyOwner returns (uint){
require(address(this).balance >= amount, "Insufficient balance in Wallet");
require(id < MAX_TRANSFERS || indexQueue.length > 0, "Transfer queue is full");
// Get free index
uint index;
if (indexQueue.length > 0) {
//gas: 186681
// Get index that was freed
index = indexQueue[indexQueue.length - 1];
indexQueue.pop();
}
else {
// gas: 193435
// Write sequentially
index = id;
id++; // cannot be more that MAX_TRANSFERS
}
// Create a new transfer object
// approvalCount is set to 1 because the creator is assumed to have approved the transfer he has created
Transfer memory new_transfer = Transfer(
index,
recepient,
amount,
block.timestamp,
1,
TransferState.Pending,
bitmask ^ owners[msg.sender]
);
//Register the new transfer
transfers[index] = new_transfer;
processState(index);
return index;
}
// Approves existing transfers which are in Pending state
// Only owner can approve transfers
// Each address can approve transation only once
// gas: 41279
function approveTransfer (uint _id) public onlyOwner returns (uint){
require(transfers[_id].state == TransferState.Pending, "Only pending transfer can be approved");
require(transfers[_id].approvalBitmap & owners[msg.sender] != 0, "This address already approved this transfer");
// Change transfer states
transfers[_id].approvalCount++;
transfers[_id].approvalBitmap ^= owners[msg.sender];
assert(
transfers[_id].approvalCount <= approvalCount &&
transfers[_id].approvalCount > 1
);
processState(_id);
return _id;
}
// Revokes approval from existing transfers which are in Pending state
// Only owner can revoke transfers
// Each address can revoke transation only if it has previously approved it
function revokeTransfer (uint _id) public onlyOwner returns (uint){
require(transfers[_id].state == TransferState.Pending, "Only pending transfer can be revoked");
require(transfers[_id].approvalBitmap & owners[msg.sender] == 0, "This address didn't approved this transfer");
// Change transfer states
transfers[_id].approvalCount--;
transfers[_id].approvalBitmap ^= owners[msg.sender];
assert(
transfers[_id].approvalCount < approvalCount &&
transfers[_id].approvalCount >= 0
);
processState(_id);
return _id;
}
// Execute transfer that is in Ready state
function executeTransfer (uint _id) public payable onlyOwner returns (uint) {
require(
transfers[_id].state == TransferState.Ready ||
transfers[_id].state == TransferState.Failed,
"Only Ready or Failed states are allowed"
);
if (transfers[_id].recepient.send(transfers[_id].amount))
transfers[_id].state = TransferState.Completed;
else
transfers[_id].state = TransferState.Failed;
processState(_id);
return _id;
}
// Cancels transfer that is in Failed state
function cancelFailedTransfer (uint _id) public onlyOwner returns (uint) {
require(transfers[_id].state == TransferState.Failed, "Only Failed transfer can be cancelled");
transfers[_id].state == TransferState.Cancelled;
processState(_id);
return _id;
}
//private
function processState(uint _id) private {
Transfer storage t = transfers[_id];
if (t.state == TransferState.Pending) {
// emit event only if the transfer has just been created
if (t.approvalCount == 1)
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
if (t.approvalCount == approvalCount)
t.state = TransferState.Ready;
else if (t.approvalCount == 0)
t.state = TransferState.Cancelled;
}
if (t.state == TransferState.Ready) {
executeTransfer(_id);
}
if (t.state == TransferState.Failed) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
}
if (t.state == TransferState.Cancelled || t.state == TransferState.Completed) {
emit ReportTransferState(_id, t.recepient, t.amount, t.state);
// with deletion: 53258 for the final approval
// w/o deletion: 92449 for the final approval
delete transfers[_id];
indexQueue.push(_id);
}
}
}
Again man, thank you for inspiration to explore different implementations. I spent quite a time working on this project and learned a lot more than I expected. Moving on 
1 Like