Widen the pragma

[moodysalem]

No description provided.

[NoahZinsmeister]

3 comments:

1. is there a reason to prefer >=0.5.0 over >=0.5?
2. imo interfaces that aren't reasonably public can/should have fixed pragmas. in this case i'm thinking of IERC20.sol, which is used for internal reasons, and should be readily accessible to others via their own dev environment. i also think we should consider removing all functions from this interface that we don't actually rely on in the code, for the sake of simplicity.
3. after playing around with solc@0.6.6 in periphery, there's some gotchas around duplicating/extending interfaces. i'm increasingly feeling that we should remove IUniswapV2ERC20.sol in favor of IUniswapV2Pair.sol (which already includes all IUniswapV2ERC20.sol declarations).

[moodysalem]

• is there a reason to prefer >=0.5.0 over >=0.5?

I think this is just stylistic, I prefer the former just because it's more what I'm used to from package.json

imo interfaces that aren't reasonably public can/should have fixed pragmas. in this case i'm thinking of IERC20.sol, which is used for internal reasons, and should be readily accessible to others via their own dev environment. i also think we should consider removing all functions from this interface that we don't actually rely on in the code, for the sake of simplicity.

In that case I think we should just use the openzepplin-solidity version and remove the IERC20.sol from this repo

after playing around with solc@0.6.6 in periphery, there's some gotchas around duplicating/extending interfaces. i'm increasingly feeling that we should remove IUniswapV2ERC20.sol in favor of IUniswapV2Pair.sol (which already includes all IUniswapV2ERC20.sol declarations).

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

[NoahZinsmeister]

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

[NoahZinsmeister]

the changes i suggested would be consistent with Uniswap/v2-periphery#10