Track missing features from components

• Trigger TermsContract to do a loanStatus update after each payment/ withdrawal.
• Optimize payee variable within the contract: Either remove completely or optimize to be usable.

Why

• Our coding styles are very different
• We have a lot of "hackathon" code (i.e. abandoned imports, unused variables)

Clean up

• Install codebase-wide checker?

Javascript

• Install eslint
• Install prettier / formatter

Solidity

• Install solidity linter
• Is there a solidity formatter? (Yes: prettier-plugin-solidity).
  https://prettier.io/docs/en/cli.html
  Use prettier --write to autoformat solidity files
  https://github.com/prettier-solidity/prettier-plugin-solidity

Monitoring

• Setup Discord channel for tracking CI failures

Git Hooks?

• Git hook to run linter prior to commit?

We hope to deploy the contracts to the kovan testnet, by the 15 - 16 July 2019.
To begin actual network testnets. Will also be good to deploy to Ropsten , as it is the closest testnet to the mainnet

• Simple: We can leave the repayment function open such that anyone can repay (and they can continue in case of lost keys)

• We could suggest the borrower set up a multisig and use that for their borrower address, with a 1 of n scheme such that multiple accounts they own can act as athe borrower.

• The borrower could also set up a system such as this once it's ready IdentityContract

Ultimately, we don't want to get into the key / identity management business - but we want to make it easy for users to use better practices. Key management and opsec is on the user.

This is related to the inherent issue of ERC20 tokens that there is no way to know if someone has sent them to a contract on-chain (no hooks) and no way to react when this happens.

(Aside: Maybe some sort of event log + proof could exist to solve this... Hopefully tokens with hooks become the standard)

Situation 1: Borrower sends payment tokens natively to RepaymentRouter

• We don't want to allow the user to withdraw tokens because this contract stores tokens for others! Otherwise we could have an onlyBorrower() method that allows approval + transfer from the contract.

Situation 2: Borrower sends payment tokens natively to incorrect contract

• If they use the UI, this won't happen

Situation 3: Lenders make native token transfers to Crowdsale

• If they use the UI, this won't happen
• Funds are often lost this way IRL

Situation 4: Someone sends Eth somewhere

• All contracts will have fallback functions that revert() to prevent accidental Eth sends

https://medium.com/edgefund/testing-and-code-coverage-of-solidity-smart-contracts-660cb6291701

• surya
• solidity-coverage
• eth-gas-reporter
• solhint
• solidity-docgen
• Use MythX? Mithril?

Surya for graphing out contract dependencies
https://github.com/ConsenSys/surya

https://ethereum.stackexchange.com/questions/47188/what-continuous-integration-services-support-running-truffle-solidity-tests/59176

https://blog.coinfabrik.com/test-solidity-smart-contracts-using-travis-ci/

https://ethereum.stackexchange.com/questions/47188/what-continuous-integration-services-support-running-truffle-solidity-tests/59176

https://forum.zeppelin.solutions/t/test-smart-contracts-like-a-rockstar/1001

The current payess on Repayment Manager contract is a private state; thus, it's difficult to retrieve from the front end.

See here

High-level spec:

• To simplify the smart contracts, we are not going to allow for refunds (at least for Ines.fund) during crowdfunding phase
• We can explore starting loan status once contract is deployed, to reduce the attack surface there as well
• We may still need refunds for the scenario where funding is done, but borrower wants to cancel the loan

I've created a mindmap of our current contracts architecture:

https://www.mindmeister.com/1292941609/enable-contracts

Closing this issue

Issue

Tracking what lenders are entitled to withdraw what repayments at what time requires overhead. We want to minimize cost and keep interfaces as familiar as possible.

Solutions

MiniMe Style ERC-20

[+] Can handle ERC-20 payment tokens if sent though a function, like repay()

[-] O(logn) loops on transfer and withdraw
[-] Large storage overhead

Conclusion: Likely won't use due to gas costs

ERC-721

[+] Can handle ERC-20 payment tokens if sent though a function, like repay()
[+] No loops, withdraws simple to calculate

[-] Can't split or combine stake without extra logic
[-] Can't withdraw multiple stakes at once without a batch function (which we could totally do)

ERC-1843: Claims Token Standard

[+] Can keep ERC-20 ownership tokens, looks efficient (still evaluating)

[-] Requires payment token to be ERC-223 compatible for the tokenFallback hook.
* We could potentially "wrap" the payment token before sending, it'd be the same number
of calls as approve() + function() anyways.

ERC-1726: Dividend-Paying Token Standard

[-] Only supports Ether payments

Conclusion: Won't use as ERC-20 payments are critical. Can learn from though.

• Spoofed site at another domain
• Domain hack

Should we have an on-chain registry of all Enable "approved" crowdloans (e.g. deployed with an official factory)?

With the implementation of the ERC20 stable coin,
more than the principal can be sent to the smart contracts as described in #32 ,
This becomes a problem if a user natively sends tokens to the contract address during the fund process, which means the user can send any amount, and will not be recorded as a contributor to the crowdloan, and will get not repayment.
The issue to the borrower can be removed, by removing limitation of the amount of principalDisbursed been worked on by @onggunhao , so any amount can be disbured, and that would be used in calculating the repayment.
The problem to the sender of the said funds, is that they can not get the funds back, and it would be sent to the contributors as part of the repayment, which the sender will not partake of.

3. LoanManager

Conceptually, this keeps track of loan terms, and takes care of accounting and tracking whether the loan has been paid on time. In "real-life" terms this is the loan operations and accounting department, a back-office function.

1. Record loan terms: Principal, interest, term, etc
2. Keep track of Amortization Schedule
   1. Pre-loan disbursement: just shows schedule in terms of generic months (e.g. "Month 1", "Month 2")
   2. Post-loan disbursement: Amortization schedule will have repayment timestamps associated with each repayment tranche
3. Keeps track of repayments
   1. Keeps track of repayments (array)
   2. Keeps track of whether loan is in default or current

Follow up of enabledao/enable#12

This could be strait-forward, but there is no modern example.

Problem

We want to deploy a Factory which references the existing App contract for the current network, to find out where the implementations are so we can deploy proxy instances for them.

Solution

A 'deploy' script which will run in front of tests

The script will:

• Figure out what network we're on
• If there is no deployment (e.g. new local blockchain), create one (zos publish zos push)
• Allow the tests to get the current deployed addresses
• Create a factory instance using zos js (Creating a new logic contract + init is not the same test imo)
• Run all tests as per normal

I prefer the zos artifacts to the truffle ones, and will be using those in my tests. This is because you can use the zos cli 'create' command via js to deploy instances. The most notable difference is they use web3-style commands to send() and call().

This is a collection of Github comments + discussions that we KIV-ed:

Usability

• Understand ERC20 approve step, can it be done as part of "pay"? (for both lenders and borrowers)

Loan Statuses

• Should refactor loan statuses to have different concepts for stage and outcomes
• (LoanStatusUpdated should have timestamp)[https://github.com//pull/54#discussion_r307490843]
• Remove LogAccess event
• Remove REPAYMENT_COMPLETE from loanStatuses

Crowdloan

• Should crowdfundStart and crowdfundEnd be params, instead of a struct (which cannot be changed by zos?
• fund should not allow funding beyond crowdfundEndTimestamp
• Should have an onlyBorrower modifier (for loan disbursement)
• Is there a way trackCrowdfundStatus can avoid calling updateCrowdfund multiple times?
• contributor should be a lender
• Can we have roles defined? e.g. borrower, lender
• expect statements should not have the unspecified flag
• Should have an onlyLender modifier (for refund)
• isBelowMaxSupply should be belowCrowdfundingGoal

Naming

• Rename interestRate to monthlyInterestRate
• controllerRole should be something other than minters
• Should have a different word for borrower who is having funds disbursed by the smart contract, and lender who is withdrawing repayments from the smart contract

RepaymentManager

• getRepaymentStatus should have both a now() version and a `(timestamp) version
• Event LenderWithdrawal should be PaymentReleased
• Payees should be lenders
• "Pay" should be "Repay"
• Remove the whole updateRepaymentStatus?
• Remove the pay function and just use ERC20 transfers to track off-chain for the status of the loan?
• Refactor borrower mint and approve function (I can't remember what I was thinking when I wrote this down)

Overall

• Implement loanStatusUpdated messages throughout the smart contract

Testing

• Implement more realistic test scenario generation
• Use a variable number of lenders, refactor split_MAX_CROWDFUND

From enabledao/enable#71

Issue migrated from: enabledao/enable#104

Crowdloan

• Crowdloan
• CrowdloanFactory

Debt contracts

• DebtManager
• RepaymentRouter
• TermsContract

Debt Tokens

• DebtToken
• DebtTokenFactory

For Terms Contracts, see time shifting util
https://kauri.io/article/f95f956261494090be1aaa8227464773/truffle:-testing-your-smart-contract

@adibas03 Can I get your help to reach out to a few people to see how much a smart contract audit would cost?

Track missing functions in crowdloan contract

• Implement crowdloanEnd (Time trigger to end crowdfund period)
• Set time to wait between crowdfund completion and withdrawal, after which lenders can refund their loans if the borrower did not start the loan (Prevents bad borrower holding lenders funds hostage, might be pointless)

Overview

@tspoff @adibas03

With T-9 days to Ines' fundraise, I'd like us to have a really productive weekend, and I've taken some time to spelunk through our codebase and write up clearer specifications for business logic.

I'll be focusing full-time and finally understand enough Solidity et al to be useful. I understand if you guys have other commitments, but I need your advice + experience to code review and point me in the right direction. I'm fairly new to this but learning very fast, and really appreciate your help.

I think we can break the work into 3 workstreams, and stub dependencies so we can work quickly in parallel. I've drawn it out below.

Questions I've had going through our codebase

Sorry if they're off-point or just a work-in-progress! A few things jumped out at me:

1. DebtManager vs DebtToken: After looking at our contracts, a lot of our logic on Debt is spread across debtToken, debtManager. Can we combine them?

2. DebtTokenFactory: Why don't we just create a DebtToken in the constructor of CrowdloanFactory?

3. DebtManager re-implementing functionality that is already available in the ERC721 library: DebtManager current implements addDebtValue. Is it possible that we just have a simple mapping(uint256 tokenId => uint256 debtValue) that we write to while extending the mint() from ERC721?

4. Which ERC721 libraries should we inherit from?: ERC721MetadataMintable instead of ERC721Mintable and ERC721Metadata?

Specifications

From the above questions, I thought we could re-organize our workstreams into the following 3 contracts:

(This might change, e.g. if @tspoff finds the PaymentSplitter is the easiest way for us to route repayments)

1. DaiCrowdsale

We should have a standalone DaiCrowdsale that has been extensively tested for the following functionality. We might consider inheriting the ERC20FundedCrowdsale that we found in the OpenZeppelin repo.

Conceptually, this functions very much like an ERC20Crowdsale, just that instead of minting a ERC20 token we replace it with calls to the DebtManager:mint().

1. Dai Contract integration: The Dai contracts should be deployed as part of our Truffle migrations, and we should also get it working on Kovan which is the Dai testnet
2. Hold "Crowdsale" Parameters: i.e. the Cap amount, start timestamp, end timestamp
3. Accept Dai Funding: (a.k.a. "makeLoan")
   1. Checks to make sure funding amount doesn't put us over the Cap amount (may have to use a stubbed method from DebtManager)
   2. Once amount has been transferred, call the stubbed method in DebtManager to mint a new debt token that represents the funding amount
4. Disburse Loan:
   1. The borrower will "start" the loan by disbursing the amount from the crowdsale to her address
   2. This will trigger the TermsContract method to write timestamps to the amortization table, officially starting the loan

2. DebtManager

Conceptually, this keeps track of the lenders, and takes care of withdrawals. In "real-life" terms this is the back-office executive who specializes in dealing with fund suppliers.

1. Keeps track of debt: i.e. the fundraising goal, the current total amount raised
2. Minting debt tokens: internal method to mint a debt token
   1. This is Potential Security Break Point 1 as minting unlimited debt tokens would break the entire repayments system
3. Keeping track of owner balances: each debt token would have a loan amount associated with it, and repayments are calculated accordingly
4. Withdrawal Manager:
   1. Tracks the amount the each lender address or debt token has already withdrawn
      1. Implementation method 1: withdraw by owner
      2. Implementation method 2: withdraw by debt token (i.e. if you've funded it 15 times, you have to withdraw it 15 times). I like this method because of its simplicity
   2. Calculate the amount available for withdrawal, by getting LoanManager:getTotalRepaid() and then dividing it by the fractionalShare that belongs to the lender or debt token. (we can use a stub for this method first)

3. Terms Contract

Conceptually, this keeps track of loan terms, and takes care of accounting and tracking whether the loan has been paid on time. In "real-life" terms this is the loan operations and accounting department, a back-office function.

1. Record loan terms: Principal, interest, term, etc
2. Keep track of Amortization Schedule
   1. Pre-loan disbursement: just shows schedule in terms of generic months (e.g. "Month 1", "Month 2")
   2. Post-loan disbursement: Amortization schedule will have repayment timestamps associated with each repayment tranche
3. Keeps track of repayments
   1. Keeps track of repayments (array)
   2. Keeps track of whether loan is in default or current

Right now, our terms contract assumes that the full amount is raised.

I'll be adding:

• principalRequested (initial funding goal)
• principalDisbursed (the actual amount that is disbursed, will be used to calculate repayments)

This will affect Terms Contract only.

Closed in PR #51

While setting up the e2e tests, I realized, that based on the size of the contribution to the loan, some contributors might not have a repayment due every month, but instead, every second month or more.
For instance: A contribution of 6 units out of 60,000 units (0.01%), will not be due for any repayment until 10,000 units have been repaid.
Do we leave this as is and educate, or do we set a minimum contribution, based on the expected repayment.
I think education might be sufficient to allow for smaller contribution, especially since the project is P2P and less corporate.
Note* Implementation of any decision should be against the next version

@onggunhao @tspoff thoughts