Develop#5
Conversation
| function fetchDayTokens() onlyOwner public { | ||
| dayToken.transfer(owner, dayToken.balanceOf(this)); | ||
| } | ||
|
|
There was a problem hiding this comment.
can we add the fallback function, just to ensure that ether are not transferred to the address?
There was a problem hiding this comment.
by default fallback is not payable so TX with ETH will be reverted
There was a problem hiding this comment.
Please add tests for deployment code.
Inline ERC20 and MintableToken minimal functionality. Fixes #8
| address user = msg.sender; | ||
|
|
||
| if(dayToken.transferFrom(user, this, dayTokenFees)){ | ||
| DebtToken newDebtToken = new DebtToken(_tokenName, _tokenSymbol, _initialAmount, _exchangeRate, |
There was a problem hiding this comment.
When calling new DebtToken from DeployDebtToken the msg.sender seen by DebtToken will be the address of DeployDebtToken contract.
To fix this we shall modify the contract so it accepts 'borrower' as an input.
There was a problem hiding this comment.
Great Observation, we would create a different DeployableDebtToken with that feature, so we can separate development process of the two.
Following the pattern used in the DayToken
There was a problem hiding this comment.
I really like the way DayToken has resolved it. In case of debt contract, I think it has too much of overhead. If we "rename" owner to "borrower" there is no need for 3rd party like owner.
My proposal is to
- add explicit argument to ctor
borrower - remove
ownervariable and semantics and replace withborrower(remove ownable inheritance)
kosecki123
left a comment
kosecki123
left a comment
There was a problem hiding this comment.
Modify the DebtToken contract so it get's the borrower as an input to the constructor.
|
Closing in favour of #14 |
3 participants
debt Token deployer contract added.