-
Notifications
You must be signed in to change notification settings - Fork 2
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
Partial crowdfund support #51
Closed
Closed
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
10e179a
Move monthly interest rate into a calculated rather than stored field
dan-menlo 1510404
Add principalDisbursed as a field
dan-menlo 8b56f7f
Add principalDisbursed
dan-menlo a3bc525
Lint solidity{
dan-menlo b1a306a
Revert getExpectedRepayment to previous
dan-menlo 1bc2535
Add getLoanPeriod() as a method
dan-menlo d30a0b7
Add overloaded getExpectedRepaymentValue
dan-menlo b4738bc
refactor onlyStatus into modifier functions, use principalDisbursed
dan-menlo 24224db
Allow for partial principalDisbursed, tests"
dan-menlo ff9b344
Lint contracts
dan-menlo 5686a29
Remove ScheduledPayments and paymentTable
dan-menlo da435b4
Add getRequestedScheduledPayment and getScheduledPayment
dan-menlo ebdb38e
Add comments
dan-menlo d95d6e2
Add comments
dan-menlo 73f7a5e
Errors in Crowdloan due to vm reverts, will do deep dive tomorrow
dan-menlo 9d12f5b
Implement partial crowdfunding, partial principalDisbursed. Still pen…
dan-menlo ad5a0ee
Lint
dan-menlo 0941954
Refactor crowdloan tests to test for partial crowdloan and partial pr…
dan-menlo 3b96275
Finish test suite for partialCrowdfund, test events and access control
dan-menlo f1aba44
Remove totalCrowdfunded variable for getBalance. Implement overfunded…
dan-menlo 6b586e0
Linting
dan-menlo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 should be
termsContract.startRepaymentCycle(_getPrincipalToken().balanceOf(address(this)))
,which simply takes the balance at this point in time of the crowdloan contract.
This also removes the need to track
totalCrowdfunded
within the fund and refund functions.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.
@adibas03 Ah yes I thought about that. That approach might create a few problems:
Someone does a native ERC20 transfer to the smart contract, such that
_getPrincipalToken().balanceOf(address(this))
is > thanprincipalRequested
In TermsContract, we have a
require
that checks thatprincipalDisbursed
is not more thanprincipalRequested
If that happens, the funds are locked and can't be withdrawn
I think I can resolve it by removing
totalCrowdfunded
, and instead just passing in the smaller ofbalanceOf(address(this))
andprincipalRequested
into thestartRepaymentCycle
functionThere 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.
Changes made in f1aba44
Tied to issue #52
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.
If we are to use a ceiling, it means we have to implement a cap for the withdrawal function as well, which makes withdrawing in bits more annoying, and would be better we have a simple withdraw function that takes the lesser of
principalRequested
andbalanceOf(address(this))
and transfers the said amount, after which it locks withdrawalThere 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.
Ah just saw your message in the other post - makes sense. I'll implement the donations functionality today
#52