matter-labs / hardhat-zksync

Apache License 2.0
279 stars 227 forks source link

npm warn ERESOLVE overriding peer dependency if your project requires `@openzeppelin/contracts` less than `v4.6.0` #1227

Closed mmv08 closed 1 week ago

mmv08 commented 4 months ago

🐛 Bug Report for hardaht-zksync plugins

💥 Plugin name

@matterlabs/hardhat-zksync

📝 Description

I'm trying to follow https://docs.zksync.io/build/tooling/hardhat/migrating-to-zksync#install-dependencies. After doing the first two steps, my project can't be compiled anymore (because the peer dependency won't be installed):

~/p/sa/safe-contracts v1.5.0-zksync !3 ❯ npm run build                           17:15:29

> @safe-global/safe-smart-account@1.4.1-build.0 build
> hardhat compile

Error HH402: File @openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol doesn't exist.

🔄 Reproduction Steps

  1. mkdir npm-hardhat-zksync-bug-reproduction && cd npm-hardhat-zksync-bug-reproduction && npm init -y && npm i @matterlabs/hardhat-zksync @openzeppelin/contracts@4.5.0

🤔 Expected Behavior

Our project depends on an @openzeppelin/contracts version v3.9.2 due to compiler specifics, and we also don't want our resulting bytecode to be changed since the contract has already been deployed to many EVM networks. Installing a hardhat plugin shouldn't force me to use a specific version of the library that my contracts can depend on. If a specific version is required for the plugin to work, it should be a dependency of the plugin and not a peer dependency.

😯 Current Behavior

1) peer dependency warnings 2) compilation failures

🖥️ Environment

📎 Log Output

npm warn ERESOLVE overriding peer dependency
npm warn While resolving: @matterlabs/zksync-contracts@0.6.1
npm warn Found: @openzeppelin/contracts@4.9.6
npm warn node_modules/@matterlabs/hardhat-zksync/node_modules/@openzeppelin/contracts
npm warn   @openzeppelin/contracts@"^4.9.2" from @matterlabs/hardhat-zksync@1.1.0
npm warn   node_modules/@matterlabs/hardhat-zksync
npm warn     @matterlabs/hardhat-zksync@"*" from the root project
npm warn
npm warn Could not resolve dependency:
npm warn peer @openzeppelin/contracts@"4.6.0" from @matterlabs/zksync-contracts@0.6.1
npm warn node_modules/@matterlabs/hardhat-zksync/node_modules/@matterlabs/zksync-contracts
npm warn   @matterlabs/zksync-contracts@"^0.6.1" from @matterlabs/hardhat-zksync@1.1.0
npm warn   node_modules/@matterlabs/hardhat-zksync
npm warn
npm warn Conflicting peer dependency: @openzeppelin/contracts@4.6.0
npm warn node_modules/@openzeppelin/contracts
npm warn   peer @openzeppelin/contracts@"4.6.0" from @matterlabs/zksync-contracts@0.6.1
npm warn   node_modules/@matterlabs/hardhat-zksync/node_modules/@matterlabs/zksync-contracts
npm warn     @matterlabs/zksync-contracts@"^0.6.1" from @matterlabs/hardhat-zksync@1.1.0
npm warn     node_modules/@matterlabs/hardhat-zksync
npm warn ERESOLVE overriding peer dependency
npm warn While resolving: @matterlabs/zksync-contracts@0.6.1
npm warn Found: @openzeppelin/contracts-upgradeable@4.9.6
npm warn node_modules/@openzeppelin/contracts-upgradeable
npm warn   @openzeppelin/contracts-upgradeable@"^4.9.2" from @matterlabs/hardhat-zksync@1.1.0
npm warn   node_modules/@matterlabs/hardhat-zksync
npm warn     @matterlabs/hardhat-zksync@"*" from the root project
npm warn   1 more (@matterlabs/hardhat-zksync-upgradable)
npm warn
npm warn Could not resolve dependency:
npm warn peer @openzeppelin/contracts-upgradeable@"4.6.0" from @matterlabs/zksync-contracts@0.6.1
npm warn node_modules/@matterlabs/hardhat-zksync/node_modules/@matterlabs/zksync-contracts
npm warn   @matterlabs/zksync-contracts@"^0.6.1" from @matterlabs/hardhat-zksync@1.1.0
npm warn   node_modules/@matterlabs/hardhat-zksync
npm warn
npm warn Conflicting peer dependency: @openzeppelin/contracts-upgradeable@4.6.0
npm warn node_modules/@openzeppelin/contracts-upgradeable
npm warn   peer @openzeppelin/contracts-upgradeable@"4.6.0" from @matterlabs/zksync-contracts@0.6.1
npm warn   node_modules/@matterlabs/hardhat-zksync/node_modules/@matterlabs/zksync-contracts
npm warn     @matterlabs/zksync-contracts@"^0.6.1" from @matterlabs/hardhat-zksync@1.1.0
npm warn     node_modules/@matterlabs/hardhat-zksync
npm warn deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
kiriyaga-txfusion commented 4 months ago

Hello @mmv08 , The hardhat-zksync-upgradable plugin requires OpenZeppelin Contracts v4 to work properly, so it is not a bug. I noticed your project at safe-smart-account and, since you won't be using hardhat-zksync-upgradable, you can import all the necessary plugins as you did in this branch but install hardhat-zksync as an umbrella plugin.

mmv08 commented 4 months ago

Hello @mmv08 , The hardhat-zksync-upgradable plugin requires OpenZeppelin Contracts v4 to work properly, so it is not a bug. I noticed your project at safe-smart-account and, since you won't be using hardhat-zksync-upgradable, you can import all the necessary plugins as you did in this branch but install hardhat-zksync as an umbrella plugin.

The problem is that it requires a dependency to function properly, but for some reason, it expects the dependency supplied by the host project. @openzeppelin/contracts is a very common dependency for Solidity projects and requiring the host project to use a specific version is suboptimal, as your versions may not align (use our case as an example). IMO, it should be a runtime dependency of the hardhat-zksync-upgradable plugin and not a peer dependency.

mmv08 commented 4 months ago

The problem we're having can be easily solved by switching @openzeppelin/contracts to a dependency.

kiriyaga-txfusion commented 4 months ago

Yes, I understand your point. hardhat-zksync is our umbrella plugin designed to gather all libraries useful for ZKSync-specific processes. One of the packages listed as a dependency is @matterlabs/zksync-contracts, which provides all ZKSync-specific contracts that users are using and where the OpenZeppelin version matters for them to work correctly. This package has a peer dependency on @openzeppelin/contracts version 4.6.0, causing these warnings. I would suggest for you to, in this case, is for you to avoid using the umbrella plugin and instead install only the specific plugins you need, thereby avoiding the installation of @matterlabs/zksync-contracts. I believe you've already done this on the branch I mentioned before, which should help you avoid the dependency conflicts you're currently facing. Additionally, hardhat-zksync-upgradable cannot be used because it requires OpenZeppelin Contracts v4 to function properly.

mmv08 commented 4 months ago

Additionally, hardhat-zksync-upgradable cannot be used because it requires OpenZeppelin Contracts v4 to function properly.

Could you please explain why it requires the dependency to be provided by the host project if it is required for the plugin to function? Why is it not a dependency if it is required?

What if I want to use a different openzeppelin contracts library version in my project and use the plugin as well?

mmv08 commented 4 months ago

No hardhat plugin should dictate which smart contract library the project should use. It may require a specific hardhat version since it's a hardhat plugin, this is all right. But if the plugin requires a specific smart contract library version, the smart contract library should be a dependency of the plugin.

kiriyaga-txfusion commented 4 months ago

Thanks for pointing out that the hardhat-zksync-upgradable plugin is missing the OpenZeppelin contracts dependency, which is essential for tasks such as compile, deploy, and upgrade processes. Older versions, like OpenZeppelin contracts 4.5.0, lack some necessary contracts, so it's important to specify a specific version to ensure proper functionality. We will fix this and include it in the next release.

mmv08 commented 4 months ago

Thank you very much @kiriyaga-txfusion I believe this results in a much more positive developer experience.

kiriyaga-txfusion commented 1 week ago

Hello @mmv08,

This issue was resolved starting from plugin version 1.5.2, as the plugin now uses aliasing for its supported version of the contracts.