-
Notifications
You must be signed in to change notification settings - Fork 15
FOLIO: A global pay-it-forward chain #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…types into pay-it-forward
ssolit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is unnecessary. people can just deploy a new contract to start a new competition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: