Add ERC3156 extension of ERC20 (flash minting and lending)

[Amxx]

Fixes #2540

PR Checklist

• Tests
• Documentation
• Changelog entry

[frangio]

This is good with some minor changes.

[alcueca]

Two corrections in comments, plus a question of the reason for one require in the borrower mock.

[alcueca]

This check is useless if you don't verify that you trust msg.sender.

[Amxx]

This is just a mock, its not designed to be used outside our testing suit anyway.

The argument used to be that doing a functionCall with arbitrary data to an arbitrary address (line 38) is bad, and we should not show that. Someone could use this endpoint with an arbitrary token address, 0 amount, 0 fee, and an arbitrary payload, which could transfer token held by the receiver.

I "fixed" that by saying that (within our testsuite) only the token should advertize onFlashLoan ... which is not how the ERC might me used in the broader sens, but corresponds to the usage we push in this example.

TLDR: I disagree, it prevents line 38 being used to do arbitrary calls to arbitrary address ... anyone using this method to do anything nasty will only be able to do it to themselves.

[alcueca]

What I'm concerned about is that coding flash borrowers that are safe is not trivial. Coders using the OZ flash lending implementation might come into the OZ repo to see how you did it, and they will find an unsafe implementation of a flash borrower.

Going that extra mile and making sure that your flash borrower is a decent example on how to code one would be useful for the community.

My mock flash borrower is not great itself, but it could quickly be polished up, and I was able to fully test the reference implementation with it.

[frangio]

I share these concerns. I think the mock should be either absurdly wrong or have a huge warning, or it should be correct.

[Amxx]

I hear your concern, and I do agree we should have extensive documentation in this source code saying this is just for testing, and should not be considered a good example of how to do FlashBorrower.

That being said, I'm having a hard time to see which security issues are not covered by the require(msg.sender == token) check. Do you have anything in mind ?

[alcueca]

Actually, you might be right. I can't think of any scenario where require (token == msg.sender) let's a malicious lender through. It might be protection enough for a borrower that only accepts loans from flash minters, and not from generic flash lenders.

A bit of weird scenario to discriminate lenders on their internal implementation, but maybe not unsafe.

[frangio]

Looks good!