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

Cannot compile OpenZeppelin 5.x as a dependency #1817

Closed
miohtama opened this issue Jan 5, 2024 · 18 comments · Fixed by #1832
Closed

Cannot compile OpenZeppelin 5.x as a dependency #1817

miohtama opened this issue Jan 5, 2024 · 18 comments · Fixed by #1832
Labels
category: bug Something isn't working

Comments

@miohtama
Copy link
Contributor

miohtama commented Jan 5, 2024

Environment information

  • OS: macOS

  • Python Version: 3.10

  • ape and plugin versions: 0.7.3

  • Contents of your ape-config.yaml (NOTE: do not post anything private like RPC urls or secrets!):

name: Terms Of Service

dependencies:
  - name: OpenZeppelin
    github: OpenZeppelin/openzeppelin-contracts
    version: 5.0.1

solidity:
  import_remapping:
    - "@openzeppelin=OpenZeppelin/5.0.1"

plugins:
  - name: solidity
    version: 0.7.0

What went wrong?

Please include information like:

OpenZeppelin 5.x does not work with ape.

ape pm compile -v debug
DEBUG: 'tmpeosen690' is not an 'ApeProject', but attempting to process as one.
WARNING: Compiled manifest produced no contract types! Are you missing compiler plugins?
SUCCESS: Package 'OpenZeppelin@5.0.1' compiled.

# No files are compiled, scanned, or created

How can it be fixed?

Downgrade to OpenZeppelin 4.x.

@miohtama miohtama added the category: bug Something isn't working label Jan 5, 2024
Copy link

linear bot commented Jan 5, 2024

@antazoey
Copy link
Member

antazoey commented Jan 5, 2024

I am confused, it looks like it is work? The last line in your output is

SUCCESS: Package 'OpenZeppelin@5.0.1' compiled.

What is the part that makes this not working?

Edit: Ok I ran the code and I got vastly different output, with contracts available

@antazoey
Copy link
Member

antazoey commented Jan 5, 2024

For some reason I am not able to reproduce, it compiled fine for me.

INFO: Compiling 'mocks/ERC165/ERC165MissingData.sol'.
INFO: Compiling 'mocks/PausableMock.sol'.
INFO: Compiling 'token/ERC20/extensions/ERC20Permit.sol'.
DEBUG: Compiling using Solidity compiler '0.8.23+commit.f704f362'
INFO: Compiling using Solidity compiler '0.8.23+commit.f704f362'.
SUCCESS: Package 'OpenZeppelin@5.0.1' compiled.

@antazoey
Copy link
Member

antazoey commented Jan 5, 2024

Do you have ape-solidity installed?

@rafael-abuawad
Copy link

Encountered the same issue while trying to use solmate.

This is my ape-config.yaml

name: demo

plugins:
  - name: solidity
    version: 0.7.0

dependencies:
  - name: smartcontractkit
    github: smartcontractkit/chainlink-brownie-contracts
    version: 0.8.0
  - name: solmate
    github: transmissions11/solmate

solidity:
  version: 0.8.23
  import_remapping:
    - "@chainlink/contracts=smartcontractkit/0.8.0"
    - "@solmate=solmate"

Yeah, I didn't specify the version, but Ape created a folder for the latest v6. Even if I define the version using version: 6 or ref: "v6" it doesn't work

Made a repo with minimum code, that yields the same result https://github.com/rafael-abuawad/super-duper-journey

@rafael-abuawad
Copy link

I cannot downgrade solmate to v5 because there isn't a v5, I wasn't able to find a workaround for this issue yet.

@antazoey
Copy link
Member

antazoey commented Jan 5, 2024

I mean, why are specifying the solidity version (0.7) in your ape-config? That is what I am curious about. Arent the OZ contracts mostly on 0.8?

Either way, I havent been able to reproduce yet!
You shouldn't have to downgrade.

@antazoey
Copy link
Member

antazoey commented Jan 5, 2024

your config is missing some things actually, i will open a PR against your repo that i had to do in order to get it to compile

@rafael-abuawad
Copy link

@antazoey, I'm specifying the ape-solidity plugin version as 0.7.0

plugins:
  - name: solidity
    version: 0.7.0

But the Solidity version is the latest one.

@antazoey
Copy link
Member

antazoey commented Jan 5, 2024

But the Solidity version is the latest one.

Oops, sorry I was confused there.

Anyway, I opened a PR against your repro repo showing how the config needed to get that dependency to compile: rafael-abuawad/super-duper-journey#1 (review)

However I don't think it is the same reason you are having issues with OZ 5.0 as that one works out-of-the-gate for me.

@antazoey
Copy link
Member

antazoey commented Jan 5, 2024

I did find one other bug when working on the PR against the repro repo: #1819

@rafael-abuawad
Copy link

I'm still not able to compile solmate, I have the exact same config as the one your mentioned on your PR but is not working for me.

$ ape pm compile -f
ERROR: 1 validation error for Config
exclude
  extra fields not permitted (type=value_error.extra)

@rafael-abuawad
Copy link

Empty solmate/v6 folder, even after your changes. But it works for you? Thats what i'm not following

@antazoey
Copy link
Member

antazoey commented Jan 5, 2024

Empty solmate/v6 folder, even after your changes. But it works for you? Thats what i'm not following

Yes, it does work for me.

@antazoey
Copy link
Member

antazoey commented Jan 5, 2024

Perhaps solmate and OZ5 are different issues?

Here is a demo of using OpenZeppelin 5.0.1 in a project, like you are , and it just works for me, I am not sure why it doesn't for you but I am invested in finding out.

antazoey/ape-playground#4

@rafael-abuawad
Copy link

After deleting the cached stuff, it compiled and worked correctly!

rm -rf ~/.ape/packages/solmate
rm -rf ./.build
rm -rf contracts/.cache

I thought of this before but I didn't try to run the first command:

rm -rf ~/.ape/packages/solmate

@rafael-abuawad
Copy link

Maybe something is being wrongly cached in the ~/.ape/packages directory.

@antazoey
Copy link
Member

antazoey commented Jan 8, 2024

After a bunch of testing, I deduce the problem is just that it did not fail in anyway when the dependency had no sources.
Like - obviously compiling an empty manifest is not going to produce contract types -- so we should have failed or at least warned much before that point.

It makes sense to just fail and not even cache a dependency if it is missing sources because that is useless and will always require a re-install after fixing the config or whatever needs to be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants