Asset Pool Underwriter Token (COV)

[pdyraga]

Underwriter tokens represent an ownership share in the underlying
collateral of the asset-specific pool. Underwriter tokens are minted
when a user deposits ERC20 tokens into asset-specific pool and they are
burned when a user exits the position. Underwriter tokens natively
support meta transactions. Users can authorize a transfer of their
underwriter tokens with a signature conforming EIP712 standard,
rather than an on-chain transaction from their address just like in the
case of Uniswap LP or DAI token. Anyone can submit this signature
on the user's behalf by calling the permit function, as specified in
EIP2612 standard, paying gas fees, and possibly performing other
actions in the same transaction.

The implementation mostly mirrors UniswapV2ERC20 contract adding
some tweaks from OpenZeppelin ERC20 implementation such as
meaningful revert messages and Approval event emitted on calls to
transferFrom. This allows applications to reconstruct the allowance
for all accounts just by listening to said events. Other
implementations of the EIP may not emit these events, as it isn't
required by the specification and UniswapV2ERC20 is one of them.

Unit tests were based on OpenZeppelin ERC20 unit tests with our own
tests implementation for EIP2612 and EIP712 functions.

Last but not least, this token contract can be cloned using EIP1167
mechanism.

[pdyraga]

Open questions (things I do not want to address in this PR):

There is a problem with ethlint and override keyword. When I try to lint UnderwriterToken contract, linter reports syntax error on fields with override. ethlint does not look like an actively maintained project and we might consider switching to solhint. Please compare:
https://github.com/duaraghav8/Ethlint/releases
https://github.com/protofire/solhint/releases

There is an open question on how Asset-specific pools and their corresponding COV tokens will be created. The implementation proposed in this PR allows for EIP1167 cloning but we may want to use CREATE2 opcode. Asset-specific pool creation may be more expensive with CREATE2 but calls to the token and to the pool may be less expensive later as they would not require a DELEGATECALL. Something to decide about later. FWIW, Uniswap uses CREATE2 to create a new pair.

[mhluongo]

Big question I have here — why not make use of the OpenZeppelin ERC-20 helpers?

[pdyraga]

Big question I have here — why not make use of the OpenZeppelin ERC-20 helpers?

I should make it more clear in the PR description, sorry. I wanted to add support for EIP2612 permit function based on EIP712 signed approvals. Permit function is supported in Dai and in Uniswap LP and allows to, for example, pay for transactions in Dai, see https://github.com/makerdao/developerguides/blob/master/dai/how-to-use-permit-function/how-to-use-permit-function.md.

I do not think permit will be used from day one after we deploy coverage pools but it can improve UX of all dApps that will be integrating with coverage pools in the future and enable new possibilities we are not yet aware of.

OpenZeppelin have quite recently added permit to their ERC20 but this code is in a draft state, was released in the most recent v4.0.0, and to my understanding was not audited yet, and can have breaking changes. On the contrary, Uniswap implementation was audited and is even referenced from the EIP as an implementation example.

I was on the fence and finally decided to copy (audited) Uniswap implementation and do two changes in other (non-permit) functionalities: add revert messages for SafeMath reverts and zero addresses just like they are done in OpenZeppelin, plus add a way to create a token with clone factory (waived in the next PR as a premature optimization). I was not happy with Uniswap tests for the token so decided to port OpenZeppelin tests to satisfy my internal need for paranoid test coverage and implemented more test cases for EIP2612 permit as Uniswap has only one test for this functionality. For what is worth, I was also trying to extend v3 OpenZeppelin ERC20 and add permit functionality copy-pasting it from Uniswap implementation but it does not feel right given that I had to overwrite some OpenZeppelin functions to provide support for uint256(-1) permanent approval mentioned in EIP2612 but not supported by ERC20 approve.

tl;dr; This is an audited, EIP2612-referenced, version with better revert messages and much better test coverage.

[dimpar]

No more comments other than testing structure might change based on this thread. But also we can refactor it later.

[mhluongo]

👍 on your rationale @pdyraga, permit() is a big UX win. Custom tokens just trigger my audit spidey senses a bit. Something to remember when we bring in reviewers