Skip to content
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

Vendor openzeppelin 4.9.3 to use contracts in VRFCoordinatorV2_5 #11154

Closed
wants to merge 1 commit into from

Conversation

kidambisrinivas
Copy link
Collaborator

Context

  • Currently, we are not able to verify VRFCoordinatorV2_5 with forge because of using @openzeppelin/contracts/Ownable as a dependency.
  • If you use files that are outside the project through a remapping (like @remapped/=../remapped/ from node_modules for ex), and a file there does a relative import, then verification fails.
  • Foundry bug, which is open and not fixed yet
  • Import chain: VRFCoordinatorV2_5 --imports--> ChainSpecificUtil --imports--> ./vendor/../OVM_GasPriceOracle --imports--> @openzeppelin/Ownable (Outside project) -->imports--> '../utils/Context.sol' (Relative import)

Goal

  • Vendor @openzeppelin/[email protected], so that we can use Ownable in VRFCoordinatorV2_5

Copy link
Contributor

github-actions bot commented Nov 2, 2023

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@makramkd
Copy link
Contributor

makramkd commented Nov 2, 2023

Two questions (for other reviewers as well):

  • Should we vendor the entire repo or just what is needed?
  • Should we update the remappings.txt to point to this vendor folder instead of the node_modules folder? Should be backwards compatible and would fix the verification issue you're facing.

@RensR
Copy link
Contributor

RensR commented Nov 2, 2023

Please only vendor what is required

Copy link
Contributor

github-actions bot commented Jan 2, 2024

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jan 2, 2024
@github-actions github-actions bot closed this Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants