hats-finance / Possum-Labs--Portals--0xed8965d49b8aeca763447d56e6da7f4e0506b2d3

GNU General Public License v2.0
0 stars 2 forks source link

Codebase won't compile because a mismatch in openzeppelin and project solidity version #38

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @0xfuje Twitter username: 0xfuje Submission hash (on-chain): 0x9d69fab0be3135cef24731650fbe7948fe972ec798a5bdf4e414b4dabc6aea3a Severity: low

Description:

Description

The project can't be compiled, since a mismatch in openzeppelin and project pragma. While the project uses a fixed =0.8.19 solidity version (which is Arbitrum compatible) the openzeppelin library used by the project (v5.0.0) uses a floating ^0.8.20 version: which will cause the compiler to fail due to the incompatible versions.

The impact is that the project can't be deployed in it's current state, unless fixes have been made first.

Recommendation

Consider downgrading to openzeppelin v4.9.0 and make some small modifications to be compatible with v4.9.0:

src/Possum.sol

- import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
+ import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";

src/MintBurnToken.sol

- ) ERC20(name, symbol) Ownable(initialOwner) ERC20Permit(name) {}
+ ) ERC20(name, symbol) Ownable() ERC20Permit(name) {}
PossumLabsCrypto commented 1 year ago

We did compile the project without problems simply by changing the solidity version to 0.8.19 in all dependencies manually.

Is there a reason not to do this?

0xfuje commented 1 year ago

Oh I didn't know that, thanks. That could work and probably won't cause any problems with a small codebase.

Although I think it would be better practice in the future to use versions of dependencies that are compatible with the codebase's solidity version.

Openzeppelin recommends against modifying contracts:

To keep your system secure, you should always use the installed code as-is, and neither copy-paste it from online sources, nor modify it yourself.

One small potential issue I can think of is: a new feature is used by the dependency that the old compiler does not support: e.g. solidity version 0.8.22 introduced event definitions at the file level, if OZ would use this pattern in a future version: codebase won't compile unless you modify OZ contracts.

I leave you to decide whether it's something that's worth changing.

PossumLabsCrypto commented 1 year ago

Thanks a lot for the helpful comment!

Our policy right now is to only touch the contracts where absolutely necessary to minimize the risk of introducing new bugs shortly before launch. Though, I will certainly keep this in mind and consider in future development, thank you. 🙂