Everything Lauri said.
As you've discovered, unbounded for
loops are deadly. https://blog.b9lab.com/getting-loopy-with-solidity-1d51794622ad
Even if runs (less than 30,000) it might get too expensive to be practical or popular. To resolve that, you need a way to complete the operation in a single move. To accomplish that, you need to organize storage so the information you need is readily available without iteration.
This issue, and countless others like it is a recurring theme. I've been experimenting with a re-usable library and decided to see how it can help here. In this case, you can do without the delete
capability so some optimization is possible.
This implementation uses the library for two concerns:
- The set of campaigns that have been started.
- The set of funders that have contributed to each campaign.
It does the accounting of campaign funder contributions on-the-fly so it isn't necessary to iterate the funders to get the funds raised for a campaign, or iterate the contributions to get the total funds sent (to all campaigns) by a funder. I also dropped the amount
from contribute()
as I expect what you really want is a payable
function that accepts ether and accounts for it.
Other than that, I tried to accomodate the functions I see in your code.
pragma solidity 0.5.1;
// https://github.com/rob-Hitchens/UnorderedKeySet
import "./HitchensUnorderedKeySet.sol";
contract CrowdFunding {
using HitchensUnorderedKeySetLib for HitchensUnorderedKeySetLib.Set;
mapping(address => uint) public funderTotalContributions;
struct Campaign {
uint totalRaised;
address beneficiary;
mapping(address => uint) funderContributions;
HitchensUnorderedKeySetLib.Set funderSet;
}
mapping(bytes32 => Campaign) campaigns;
HitchensUnorderedKeySetLib.Set campaignSet;
function newCampaign() public {
bytes32 campaignId = genCampaignId();
campaignSet.insert(campaignId);
campaigns[campaignId].beneficiary = msg.sender;
}
function contribute(bytes32 campaignId) public payable {
require(campaignSet.exists(campaignId), "Campaign does not exist.");
Campaign storage c = campaigns[campaignId];
if(!c.funderSet.exists(addressToBytes32(msg.sender))) c.funderSet.insert(addressToBytes32(msg.sender));
c.funderContributions[msg.sender] += msg.value;
funderTotalContributions[msg.sender] += msg.value;
c.totalRaised += msg.value;
}
function getCampaignFundsByAddress(address funder, bytes32 campaignId) public view returns(uint) {
return campaigns[campaignId].funderContributions[funder];
}
function getCampaignInfo(bytes32 campaignId) public view returns(uint, address, uint) {
require(campaignSet.exists(campaignId), "Not a campaign.");
Campaign storage c = campaigns[campaignId];
return (c.totalRaised, c.beneficiary, c.funderSet.count());
}
function getCampaignAtIndex(uint index) public view returns(bytes32) {
return campaignSet.keyAtIndex(index);
}
function getCampaignFunderAtIndex(bytes32 campaignId, uint index) public view returns(address) {
require(campaignSet.exists(campaignId));
return bytes32ToAddress(campaigns[campaignId].funderSet.keyAtIndex(index));
}
// unique campaignIds
function genCampaignId() private view returns(bytes32 campaignId) {
return keccak256(abi.encodePacked(this, msg.sender, campaignSet.count()));
}
// type conversion
function addressToBytes32(address a) private pure returns(bytes32) {
return bytes32(uint(uint160(a)));
}
function bytes32ToAddress(bytes32 b) private pure returns(address) {
return address(uint160(uint(b)));
}
}
In case the library
function isn't clear. It basically sets up a new Set
type with methods to manage lists conveniently. Explainer: https://medium.com/robhitchens/solidity-crud-epilogue-e563e794fde.
No testing. No warranty. ;-)
The type conversions are there because the out-of-the-box library uses bytes32
for keys. You could refactor it to use address
keys with the caveat that they aren't as versitile.
Hope it helps.