Open thedarkjester opened 2 years ago
Hello,
thanks a lot for this feedback - I am pushing some amends in a few minutes. The first point about the references is my oversight as there are copies of each of those imported contracts in the project repository and I neglected to update the references before submitting. Fixed in next commit. Also now see how the escapeHatch function and gas optimisation comments make a lot of sense.
Could you possibly explain the final comment about a smaller erc20 interface in a little more detail?
Thanks again!
My comment on the IERC20.
If you see: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/IERC20.sol
There are a few methods you don't use, so don't really need to use the entire interface.
If you declare your own IERC20.sol with just the approve and transfer functions, it should bring down the gas costs. Also, you get to remove the floating pragma on the interface.
The feedback is more for future and your knowledge - don't need to do them now :)
Brilliant, thanks. Yes, np I just like to do these things while they are fresh!
At the outset I would like to say that this is an interesting and ambitious project and you have done well to get to where you are.
There are a few things to note:
.sol
references in your main contract reference local directories, so pulling it down doesn't quite work.0.8.0
EscapeHatch
doesn't quite count as a circuit breaker, as other functions are still executable - aPause
on other methods is the general practiveEscapeHatch
LINK send is to the caller vs owner - still is valid and the same address, just inconsistent in the codeuint16
s firstI can see where the project is headed and I wish you well.