FOLIO: A global pay-it-forward chain#7
Conversation
…types into pay-it-forward
ssolit
left a comment
There was a problem hiding this comment.
This is great! code is super well organized. Most of my comments are nitpicks.
Remember to add the tests when the are ready.
pay-it-forward/src/CompManager.sol
Outdated
| import "./IStoreFront.sol"; | ||
|
|
||
| contract CompManager { | ||
| // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~at deploy~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
There was a problem hiding this comment.
I would call this section constants and modifiers and move the constructor to its own sections after
pay-it-forward/src/Chain.sol
Outdated
|
|
||
| // Solution to check if chain exists: https://ethereum.stackexchange.com/questions/13021/how-can-i-figure-out-if-a-certain-key-exists-in-a-mapping-struct-defined-inside | ||
|
|
||
| contract ChainContract { |
There was a problem hiding this comment.
Nitpick: this name is a bit awkward. Maybe call it ChainTracker?
it would also be great it the name of the contract matched the file name, i.e. ChainTracker.sol
pay-it-forward/src/CompManager.sol
Outdated
| CompetitionPhases public competition = CompetitionPhases.PRE; | ||
|
|
||
| // constructor function that run at deployment | ||
| constructor(address charAddr) { |
There was a problem hiding this comment.
nitpick: char usually means character. maybe call this charityAddr for clarity
pay-it-forward/src/CompManager.sol
Outdated
| // functions to set up the competition prior to starting it | ||
|
|
||
| // determines the charity we will be donating to | ||
| function selectCharity(address charityAddr) public managerOnly atStage(CompetitionPhases.PRE) { |
There was a problem hiding this comment.
why not do this in the constructor?
pay-it-forward/src/CompManager.sol
Outdated
|
|
||
| //don't update last PIF because it is the same | ||
| } | ||
| endCompetition(); //checks every transaction if the competition should be ended |
There was a problem hiding this comment.
this is not standard. usually endCompetition is something people call. you don't want to have to purchase something to end the competition is the time has already passed.
I think it's reasonable to assume the organizer or someone else would just set a bot up to call endCompetition at the right time. if you want it to be fully automatic, you could call endCompetition at the start of every makeCompTransaction call and it will only actually end it if its the correct time
pay-it-forward/src/CompManager.sol
Outdated
| // check if charity address if valid | ||
| require(charity != address(0x0), "Invalid address."); | ||
| // payout charity | ||
| (bool paid,) = charity.call{value: donation}(""); |
There was a problem hiding this comment.
don't send money automatically. make the charity make its own transaction to collect the money.
If the charity messes up some how and setUpPayout reverts, all of the money would be stuck in the CompManager contract forever
pay-it-forward/src/CompManager.sol
Outdated
| } | ||
|
|
||
| // reset's the competition if everyone has been paid out and the wallet balance is empty | ||
| function resetCompetition() internal atStage(CompetitionPhases.POST) { |
There was a problem hiding this comment.
this is unnecessary. people can just deploy a new contract to start a new competition.
There was a problem hiding this comment.
a bit unsure why this file is getting updated
Pay-It-Forward Chain Competition is a competition wherein a group of people compete by paying it forward to their fellow man. The goal of the competition is to see who can create the longest chain involving the most unique businesses. This really fosters a sense of community with upside for the participants themselves. There will be a global prize pot (GPP) that is contributed to by the participants as they pay it forward. The GPP will be paid out to the participants of the winning chain, the businesses involved in the chain, and a charity pre-selected at the start of the competition. This aligns with our goal of wholesome community driven action.
Our project introduces the following contracts: