Closed moodysalem closed 4 years ago
I think this is just stylistic, I prefer the former just because it's more what I'm used to from package.json
fair enough, i'm fine with it
In that case I think we should just use the openzepplin-solidity version and remove the IERC20.sol from this repo
as tempting as this is, for such a trivial import i'm not in favor of introducing the open-zeppelin dependency, if for no other reason than it doesn't fit with the DIY approach we've taken in the rest of the codebase
What are the gotchas? On the surface it seems like it would make sense to make IUniswapV2ERC20 inherit from IERC20 and remove those duplicate declarations
specifically, the way UniswapV2Pair.sol
is defined would not be kosher in 0.6.6
, because it's inheriting the ERC20 methods from both IUniswapV2Pair.sol
and IUniswapV2ERC20.sol
via UniswapV2ERC20.sol
. the specific compiler error is:
"Derived contract must override function "{foo}". Two or more base classes define function with same name and parameter types."
in 0.5.16
interfaces can't inherit from each other so IUniswapV2ERC20.sol
can't inherit from IERC20.sol
. at the moment my preference is probably a minimal IERC20.sol
with an =0.5.16
pragma, and perhaps no interface at all for UniswapV2ERC20.sol
the changes i suggested would be consistent with https://github.com/Uniswap/uniswap-v2-periphery/pull/10
I think this is just stylistic, I prefer the former just because it's more what I'm used to from package.json
In that case I think we should just use the openzepplin-solidity version and remove the IERC20.sol from this repo
What are the gotchas? On the surface it seems like it would make sense to make IUniswapV2ERC20 inherit from IERC20 and remove those duplicate declarations