trufflesuite / truffle

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
14.02k stars 2.31k forks source link

Allow artifacts loading from outside contracts and test folders #3436

Open elenadimitrova opened 3 years ago

elenadimitrova commented 3 years ago

Issue

Current support for artifacts loading only extends to contracts, test and node_modules as Chris helpfully described in https://github.com/trufflesuite/truffle/issues/730#issuecomment-376914718 and more flexibility is required to load from other folders.

Steps to Reproduce

We have the following folder structure

contracts
contracts-legacy
contracts-test
lib

where contracts-legacy contains duplicate contracts to contracts folder and cannot be compiled to the same build location. We are therefore compiling contracts-legacy contracts to build-legacy folder (instead of the default build folder where all remaining contract artifacts live). When trying to load from there for integration testing we get a Could not find artifacts error for either of the options below:

const LegacyTransferManager = artifacts.require("../contracts-legacy/v1.6.0/contracts/modules/TransferManager.sol"); or const LegacyTransferManager = artifacts.require("../build-legacy/v1.6.0/TransferManager");

Expected Behavior

Loading contract artifacts from folders outside contracts to be successful.

Actual Results

Compiling your contracts...
===========================
> Compiling ./contracts/Migrations.sol
> Artifacts written to /var/folders/06/dn1zc46956n6bjwv3ygdhs6h0000gn/T/test--4346-9fMGYXOP7j6H
> Compiled successfully using:
   - solc: 0.6.12+commit.27d51765.Linux.g++

0x1a35526c
Error: Could not find artifacts for ../build-legacy/v1.6.0/TokenPriceProvider from any sources

Environment

haltman-at commented 3 years ago

Hm. Can you perhaps link us the repo? It would help in figuring out what we can do about this. A possible workaround in the meantime might be to publish contracts-legacy as an NPM module and import it that way. But we'll have to figure out what we want to do about this. Thanks!

elenadimitrova commented 3 years ago

@haltman-at The repo is the Argent wallet here https://github.com/argentlabs/argent-contracts

gnidan commented 3 years ago

Thanks @elenadimitrova! Getting this into our backlog.

elenadimitrova commented 3 years ago

Thanks @gnidan I really want to move the Argent repo to use truffle but this is blocking me atm. Any workaround you can suggest?

eggplantzzz commented 3 years ago

@gnidan We could do something like provide a way for the user to provide extra source locations in the config perhaps? We could add another resolver source that would check in these directories - it would probably extend the FS source.

gnidan commented 3 years ago

What causes this to fail right now @eggplantzzz? Like, what part of the code gets in the way?

I'm into your idea for allowing extra locations. Seems like a reasonable stopgap, at least! Care to look into that?

elenadimitrova commented 3 years ago

Isn't the contracts_build_directory in config already that? Can you source artefacts from those build directories? We are compiling with 9 different truffle configs, see https://github.com/argentlabs/argent-contracts/pull/146 for details and using a handful of different contracts_build_directorys.

gnidan commented 3 years ago

@elenadimitrova the problem with contracts_build_directory is that Truffle expects .json files in that directory itself. So you might want to do contracts_build_directory: <something-with-subdirs-with-artifacts>, but Truffle won't look into those subdirectories to find the artifacts.

I think what @eggplantzzz is getting at is to add some kind of field contracts_build_directories, maybe, that takes an array. This might get tricky, however, since there are likely places in the codebase where we expect a single directory, and so we'd have to update that to handle the existence of multiple. Does not seem insurmountable, though.

In the meantime, the workaround would be to publish individual NPM packages, since Truffle already knows how to require artifacts that live in local node_modules. But definitely not ideal.

Now, @elenadimitrova, question for you - why do you need 9 different truffle-configs in the first place? Is it just to handle different versions of solc? Or do these configs differ in other ways as well? I took a quick look and it seems like the only difference is in solc version. If that's the case, then we're planning to release #3417 next week, adding the configuration option version: "pragma", which tells Truffle to detect solc version on a per-source basis and should obviate the need for separate configs altogether. (@eggplantzzz maybe we should publish a dist-tag of that branch ahead of release?)

elenadimitrova commented 3 years ago

@gnidan are you saying that artefacts loading already happens from the contracts_build_directory? If so are the multiple configs' contracts_build_directorys supported or just the default config?

On the 9 different configs topic which was previously discussed here https://github.com/trufflesuite/truffle/issues/2021#issuecomment-664883908 in addition to the solc version there are different optimizer settings as well as different target build directories (to avoid overwriting legacy versions of the same contract we use for regression testing). Also what do you do with "glue" contracts that have pragma like pragma solidity >=0.5.4 <0.7.0;?

gnidan commented 3 years ago

@gnidan are you saying that artefacts loading already happens from the contracts_build_directory? If so are the multiple configs' contracts_build_directorys supported or just the default config?

Yep - when you do artifacts.require() it looks inside contracts_build_directory. Each config only supports one such directory, naturally, though. Does this make sense?

On the 9 different configs topic which was previously discussed here #2021 (comment) in addition to the solc version there are different optimizer settings as well as different target build directories (to avoid overwriting legacy versions of the same contract we use for regression testing).

Ah, this will pose a slight problem, but maybe that's something we can look into as follow-on work to #3417.

Also what do you do with "glue" contracts that have pragma like pragma solidity >=0.5.4 <0.7.0;?

The new feature takes all sources in each "import tree" and finds a Solidity version to satisfy all pragmas. So glue contracts will get passed to solc multiple times as-needed.

gnidan commented 3 years ago

So it seems like there's definitely stuff actionable here - @eggplantzzz maybe we expand the resolver to support multiple contracts_build_directorys somehow. Seems like this would be fairly straightforward. Still needs requirements though.

elenadimitrova commented 3 years ago

Personally I'd prefer more control over compilation rather than automating decision making based on pragma set deductions. I imagine it will also be simpler to implement and maintain. In my ideal world I'd have only one truffle config which allows an array of contracts set objects (there's probably a better name for this) such as:

contracts:
   {
       contracts_set: {
            `contracts_set_id`,
            `contracts_directory`, 
            `contracts_build_directory`,
            ` compiler`
       },
       ....
   },
 networks:
  {
        development: {
                ...
        },
        ropsten: {
                ....
        },
        ....
  }

Each contracts set can be compiled for different networks such via truffle compile --contracts_set some-set-of-contracts --network development, and not passing a contracts set compiles all sets.

In the Argent wallet case which runs out of the following contract structure and compilation requirements:

contracts
   infrastructure
   infrastructure_0.5
   modules
   wallet
contracts-legacy
contracts-test
lib

where contracts\infrastructure_0.5, contracts-legacy and lib are compiled with 0.5.4 while the rest with 0.6.12 . Optimiser settings are 200 for lib and contracts-test and 999 for the rest of the contracts. We'd have the following config:

contracts: {
    core_wallet: {
      contracts_directory: ["contracts/infrastructure", "contracts/modules", "contracts/wallet"],
      contracts_build_directory: "build",
      compiler: {
        solc: {
          version: "0.6.12",
          docker: true,
          settings: {
            optimizer: {
              enabled: true,
              runs: 999
            }
          }
        }
      }
    },
    contracts_infrastructure_0_5: {
      contracts_directory: ["contracts/infrastructure_0.5"],
      contracts_build_directory: "build",
      compiler: {
        solc: {
          version: "0.5.4",
          docker: true,
          settings: {
            optimizer: {
              enabled: true,
              runs: 999
            }
          }
        }
      }
    },
    contracts_lib: {
      contracts_directory: ["lib"],
      contracts_build_directory: "build",
      compiler: {
        solc: {
          version: "0.5.4",
          docker: true,
          settings: {
            optimizer: {
              enabled: true,
              runs: 200
            }
          }
        }
      }
    },    
    contracts_legacy_1_3: {
      contracts_directory: ["contracts-legacy/v1.3.0"],
      contracts_build_directory: "build-legacy/v1.3.0",
      compiler: {
        solc: {
          version: "0.5.4",
          docker: true,
          settings: {
            optimizer: {
              enabled: true,
              runs: 999
            }
          }
        }
      }
    },
    contracts_legacy_1_6: {
      contracts_directory: ["contracts-legacy/v1.6.0"],
      contracts_build_directory: "build-legacy/v1.6.0",
      compiler: {
        solc: {
          version: "0.5.4",
          docker: true,
          settings: {
            optimizer: {
              enabled: true,
              runs: 999
            }
          }
        }
      }
    },
    contracts_test: {
      contracts_directory: ["contracts-test"],
      contracts_build_directory: "build",
      compiler: {
        solc: {
          version: "0.6.12",
          docker: true,
          settings: {
            optimizer: {
              enabled: true,
              runs: 200
            }
          }
        }
      }
    },
  },

  networks: {
    development: {
      ...
    },

To the above we need to be able to say where an artifact.require will source its json as there can be duplicates e.g artifacts.require("contracts_legacy_1_3/WalletFactory"); vs artifacts.require("core_wallet/WalletFactory");

gnidan commented 3 years ago

@elenadimitrova I really like this idea, but I'm concerned that implementing this would introduce a lot of complexity in various places through Truffle.

I see your point about the upcoming version: "pragma". I'm not sure it's harder to maintain, but it's definitely intended as a convenience feature, rather than a catch-all for all use cases.

Anyway, I want to propose an alternative approach to your suggestion that I think will be easier to get out the door quickly, while still covering your use case:

To get into some specifics, I currently favor this approach for the type of that config field:

  interface RelatedProjects {
    [projectName: string]: string; // relative (and/or absolute?) path to folder containing a truffle-config
  }

This way, you could name projects something unique and avoid relying on directory structure. For instance, one of your truffle-configs might look like:

module.exports = {
  // ... rest of truffle-config ...
  related_projects: {
    legacy: "../legacy",
    test: "../test",
    // ...
  }
}

And then in migrations/tests, you'd do:

const LegacyWalletFactory = artifacts.require("legacy:WalletFactory");

I can think of a couple trade-offs with this approach. My main apprehension would be that this pushes Truffle into the domain of monorepos without due consideration, but my feeling right now is that this is outweighed by how easy this approach would be to implement.

What do you think? How strongly do you prefer having only a single truffle-config? Do you agree with my reasoning that related projects should be listed by name? Are you, too, worried at all that this would forever be a wart on Truffle's requirements?

edit definitely need to cc @eggplantzzz on this one because I bet you're gonna hate this buddy

elenadimitrova commented 3 years ago

How are "projects" defined @gnidan , is that done in config? I'm okay with multiple configs, didn't know of truffle-config.base.js so will try to abstract most of the configuration there. Also as a general note, since you allow contracts_build_directory to be specified, was a folder different from the default never supported for artifacts loading?

elenadimitrova commented 3 years ago

Meanwhile I am trying to work around this using @truffle/contract https://github.com/trufflesuite/truffle/issues/1735#issuecomment-729684884

rsodre commented 1 year ago

I up this request.

My use case is very simple, I have 2 distinct truffle projects. Project1 is deployed, and the new Project2 needs to access its contracts. I just need to read the other project deployed addresses.

// migration file
const Query = artifacts.require('Query');
const Minter = artifacts.require('../cards/Minter'); // does not work!

module.exports = async function (deployer, network) {
    await deployer.deploy(Query, Minter.address);
};

I used to create symbolic links but it does not work sometimes.

rsodre commented 1 year ago

I made a workaround for my use case...

// migration file
const Query = artifacts.require('Query');
const { resolveContract } = require('./resolver');
const Minter = resolveContract('Minter', '../cards/Minter');

module.exports = async function (deployer, network) {
    const _Minter = await Minter.deployed();
    await deployer.deploy(Query, _Minter.address);
};

Using build_directory from the truffle-config.js...

// resolver.js
const contract = require("@truffle/contract");

function resolveContract(contractName, sourceName = null) {
    const sourcePath = `${config.build_directory}/${sourceName ?? contractName}.json`;
    const artifact = require(sourcePath);
    const result = contract(artifact);
    result.setProvider(config.provider);
    result.detectNetwork();
    return result;
}

module.exports = {
    resolveContract,
};