Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

Different Compiler Versions #25

Open
woodydeck opened this issue Dec 1, 2020 · 3 comments
Open

Different Compiler Versions #25

woodydeck opened this issue Dec 1, 2020 · 3 comments

Comments

@woodydeck
Copy link

woodydeck commented Dec 1, 2020

The dependencies for these contracts all have different compiler versions. I'm not sure if Truffle accounts for that automatically, but it would be nice to a dist folder of contracts without dependencies referenced. Just a wall of contracts would be helpful.

/*
DESIGN NOTES:

  • We assume Class 0 is common!
  • Because this is a library we use a state struct rather than member
    variables. This struct is passes as the first argument to any functions that
    need it. This can make some function signatures look strange.
  • Because this is a library we cannot call owner(). We could include an owner
    field in the state struct, but this would add maintenance overhead for
    users of this library who have to make sure they change that field when
    changing the owner() of the contract that uses this library. We therefore
    append an _owner parameter to the argument list of functions that need to
    access owner(), which makes some function signatures (particularly _mint)
    look weird but is better than hiding a dependency on an easily broken
    state field.
  • We also cannot call onlyOwner or whenNotPaused. Users of this library should
    not expose any of the methods in this library, and should wrap any code that
    uses methods that set, reset, or open anything in onlyOwner().
    Code that calls _mint should also be wrapped in nonReentrant() and should
    ensure perform the equivalent checks to _canMint() in
    CreatureAccessoryFactory.
    */

I think this thinking is part of the problem. It is much better to use member variables here not just for other people developing, but end users reading the contract. Too many contracts all over the place doing god knows what.

Edit: Yikes, what a mess Truffle is. I'm just going to rewrite. I would recommend not using Truffle. It's an anti-pattern for open source development. You have way too many dependencies with too many points of failure using it. It makes it very difficult to pickup where you left off, and maintain things in the future.

There are dependencies of dependencies in this project, and all have different compiler versions. When you factor everything for one compiler version then you make things simple in the future, and you don't get caught off guard by other projects progressing for the sake of progressing. I think the intent here is to make it easy for others to come onboard, but Truffle is a giant roadblock. I know it makes testing and development slightly quicker when you are the only person doing it.

Edit 2:

import "openzeppelin-solidity/contracts/ownership/Ownable.sol";doesn't comply with the state Open-Zeppelin recommendations on safe development. See: https://forum.openzeppelin.com/t/importing-openzeppelin-contracts-in-remix/1420

In addition, this repositories URL has been redirected. Just another reason to not use Truffle. Now I'm hunting down a compatible version of this, and since OpenSea didn't publish it, I can't be sure it's not opening a security flaw. Polymorphism is unavoidable sometimes but smart contracts should not turn into node projects where nobody understand the dependencies they inherit.

@woodydeck
Copy link
Author

woodydeck commented Dec 1, 2020

Solution: The contract is no longer compilable without the project being updated with the correct version of the contract Ownable.sol. Open-Zeppelin deleted the compatible contract.

Please include all dependencies in the project without references to other repos. I did find the repo here: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v2.4.0/contracts/ownership/Ownable.sol

I have no idea though if that's the version intended to be used.

Edit: You can flatten the contract with truffle-flattener ./contracts/ERC1155Tradable.sol > ./contracts/dist/ERC1155Tradable.sol

This is what I was looking for, but holy crap...Install x-code CLI>Install Truffle>Install nvm>Install Yarn>Install correct Yarn version>Install correct node version>Install truffle-flattener. All at the expense of not being able to design contracts as securely as possible, and not being able to refactor easily to new compiler version.

Please pretty please add the flattened contract to each release!

@eakdogan
Copy link

Can you please share the flattened contract if you could get it working on Rinkeby?

@woodydeck
Copy link
Author

Can you please share the flattened contract if you could get it working on Rinkeby?

You are better off just using OpenZeppelin boiler plates and working in Remix, though unit testing is easier with truffle. Just include all the dependencies like so:
"https://raw.githubusercontent.com/smartcontractkit/chainlink/master/evm-contracts/src/v0.6/VRFConsumerBase.sol"

Keep in mind that each dependency might have its own dependencies and they need to be added in order depending on what options you include in the contract.

You can try looking at a completed implementation and seeing the order like here: https://etherscan.io/address/0xff3559412c4618af7c6e6f166c74252ff6364456#code

Truffle and remix both have bugs in their flatteners!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants