Allowing crowdsale to have multiple stages that are limited by weiRaised #1072
Conversation
…sed and every stage has it's own rate
|
the latest commit, 9d17a1d should be reverted since it adds the DS_Store, a mac-os specific file that you should generally avoid putting in version control :) giving the code a review now |
This reverts commit 9d17a1d.
… package.json according to review
|
We're currently super busy getting OZ v2.0 out the door, but we'll be taking a look at this branch early next week. thanks for your patience! |
|
Hello @tadejoremuz and sorry. I said I was going to review this last week but completely forgot about it. I will review it this week, though. |
|
Thanks a lot for your contribution @tadejoremuz. Let me first say I'm sorry, because we have been focusing on the 2.0 release so much that we had to drop any other work that would delay it. And second let me say sorry again, because many of the changes for 2.0 will cause conflicts with this branch, and we have changed a few style guidelines that will require changes here too. We can give you a hand with these. Now, I get the feeling that this is a crowdsale for a case that's a lot more specific than the ones we have in the repo. Our goal is to make a framework with building blocks, and let people connect and extend those blocks on their specific projects. With that in mind, I have two proposals: move it to the examples directory as a nice demonstration of how a capped crowdsale can be extended; or move it to the nursery (which is a concept that we are still trying to define, but anyway...) with the idea to give it to the community to test and to provide feedback before making them part of the framework. What do you think about this @tadejoremuz @nventuro @shrugs @frangio? What I would like to see is some encapsulation of a linear increase, like the one we have on TimedCrowdSale, and another for a staged increase like the one you are proposing here. And that we can combine that function with with either time or wei raised, for 4 possible combinations. Then this could be extended for fancier curves, like the one in #827. I know this is a lot to ask, specially taking into account the limitations of solidity, gas costs, and that the ethereum community doesn't seem to agree if the rate should be set on the token or on the crowdsale. But well, we are starting a new development cycle, we can dream big. I know it must be tiring for you to keep the PR open for such a long time, and deal with our slow feedback in stages (:man_facepalming: see what I did there? Bad joke, I'm sorry(x3)). I'm available on our slack channel if you would like to have a chat. This is some nice code and a nice idea, so we hope to keep you as a contributor for new PRs :) |
| Stage[] public stages; | ||
|
|
||
| /** | ||
| * @param _initialRate Number of tokens a buyer gets per wei - default parameter for crowdsale contract |
There was a problem hiding this comment.
This is never used, so there should be a comment saying so.
This might be a shortcoming of our crowdsales design. But let's see if we can give it a twist. What if we use the initialRate as the rate for the first stage. Every crowdsale must have at least one stage, and this way we could see normal crowdsales as having only one stage. This extension allows normal crowdsales to have more stages. If you add more stages, what you pass is the lower limit, the thing that triggers that stage instead of the one that ends it. This sounds more like a building block to me, as I was saying on my previous comment. But I'm just throwing an idea, feel free to say it's bad.
There was a problem hiding this comment.
Hi,
i'm thinking how would this work as this contract is limited by wei raised for every stage and default crowdsale contract does not include any limits. User will still be forced to define first stage of 'CapStagedCrowdsale' with "rate" and "stageLimit" as every stage requires to have an upper limit. Maybe i can make this contract to work like this: Every stage is defined like now: stage limit and rate, but we would also have hardCap variable in this contract which is the upper limit of how much contract can raise. If for example: hardCap is set to 100000wei and last stage limit is 50000, then contract would take _initialRate when weiRaised is over last stage limit and still under hardCap? Am i complicating too much?
| rate: _stageRates[i] | ||
| })); | ||
| } else { | ||
| revert(); |
There was a problem hiding this comment.
I find this confusing. It's not clear what conditions will trigger a revert.
I would prefer to do all the validations first. Have a statement that checks that the limits array is ordered by increasing value.
Allowing Crowdsale to have multiple stages that are limited by weiRaised
Fixes #1327
🚀 Description
npm run lint:all:fix).