OpenZeppelin / ethernaut

Web3/Solidity based wargame
MIT License
1.96k stars 656 forks source link

Fix the imports from OpenZeppelin in the sample code so Remix can compile #454

Closed hcheng826 closed 1 year ago

hcheng826 commented 1 year ago

Hi Ethernaut contributors,

Thanks for the great project! I've learned a lot from it! One experience I have is that for some levels, the Solidity code showing in the level instruction cannot directly compile, since it's importing the OpenZeppelin code from a relative path.

Some examples: Fallout imports SafeMath, Dex imports ERC20, PuzzleWallet imports UpgradableProxy, etc. The user needs to find the correct version of the package to import, in order to make it compile on Remix.

I can open a PR and try to fix the path and make it compile for Factory and Remix. Let me know if you want me to work on this. Thanks!

xaler5 commented 1 year ago

Hey thanks for raising this up. Happy to review any PR you might create out of it, how do you think to solve this ? @hcheng826

hcheng826 commented 1 year ago

Thanks for the reply!

My thought is to replace the relative path to the actual npm package path.

Example for the import in Fallout below (link)

import 'openzeppelin-contracts-06/math/SafeMath.sol';

maybe change it to the below according to the remapping in package.json (link)

import 'npm:@openzeppelin/contracts@3.4.2/math/SafeMath.sol';

Try if I can leverage the functionality on Remix for importing from npm registry: https://remix-ide.readthedocs.io/en/latest/import.html#import-from-npm

Does it sound like a plan?

xaler5 commented 1 year ago

It sounds good but probably it would be better to have a "Try it on Remix" button or similar like the Solidity docs ? https://twitter.com/solidity_lang/status/1456233507538227207

DAVEALLCAPS commented 1 year ago

The replacement code from @hcheng826: import 'npm:@openzeppelin/contracts@3.4.2/math/SafeMath.sol'; still failed to compile. I found success without the 'npm:' like so: import '@openzeppelin/contracts@3.4.2/math/SafeMath.sol';