Skip to content

[ETHEREUM-CONTRACTS] make openzeppelin import path version specific #2058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from

Conversation

d10r
Copy link
Collaborator

@d10r d10r commented Mar 11, 2025

openzeppelin-contracts stable major version has been v5 for a while.
If a Solidity project using openzeppelin v5 also wants to use the Superfluid protocol, that can result in a conflict.
In order to make that less likely, and to prepare the migration to openzeppelin v5, we change the import path to explicitly contain v4.

Copy link

github-actions bot commented Mar 11, 2025

Changelog Reminder

Reminder to update the CHANGELOG.md for any of the modified packages in this PR.

  • CHANGELOG.md modified
  • Double check before merge

@d10r
Copy link
Collaborator Author

d10r commented Mar 12, 2025

strange failure in autowrap package:

Error (9582): Member "safeTransfer" not found or not visible after argument-dependent lookup in contract ISuperToken.
  --> packages/automation-contracts/autowrap/contracts/strategies/WrapStrategy.sol:65:9:
   |
65 |         superToken.safeTransfer(user, adjustedAmount);

Relevant code in WrapStrategy.sol:

import { SafeERC20 } from  "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
    using SafeERC20 for IERC20Mod;
    using SafeERC20 for ISuperToken;
...
  underlyingToken.safeTransferFrom(
            user,
            address(this),
            underlyingAmount
        );
...
  superToken.safeTransfer(user, adjustedAmount);

Note how the previous safeTransferFrom works.

The compile error could be solved by changing the paths to @openzeppelin/contracts-v4 too here.
But it should work as is too.

@d10r
Copy link
Collaborator Author

d10r commented Mar 12, 2025

finding:

superToken.safeTransfer(user, adjustedAmount);

fails because from the perspective of the compiler, ISuperToken is not type compatible with IERC20 in the context of this package.
That's because the IERC20 imports by ISuperToken and SafeERC20 resolve to different files now (even though there's no difference between them).

Ways to avoid this:

  • in autowrap remap @openzeppelin/contracts to node_modules/@openzeppelin/contracts-v4/
  • change import paths in autowrap to @openzeppelin/contracts-v4 too. What to do with the dependency in package.json in this case? Omit or use custom path too?

For projects using OZ v5, this type incompatibility cannot be avoided (regardless of this change happening or not). Possible workaround: cast to a type imported from v5.

@d10r d10r marked this pull request as ready for review March 12, 2025 19:12
@d10r d10r requested a review from hellwolf as a code owner March 12, 2025 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant