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.32k forks source link

Resolver.resolve doesn't make any sense #2599

Open gnidan opened 4 years ago

gnidan commented 4 years ago

Issue

@truffle/resolver provides two main methods for external use:

Now, the resolver's purpose is to load build artifacts, not sources, and so, when you do artifacts.require("MetaCoin"), it reads it from build/contracts/MetaCoin.json, not the Solidity. This is true even if you do artifacts.require("MetaCoin.sol"): Truffle finds the JSON file whose source was MetaCoin.sol, and that's what it loads.

And so you'd think, that the resolve() function is just a lightweight require() that returns the artifact path instead of the fully loaded contract abstraction from that path.

But that's not what resolve() does! Look: the one place we use this function inside Truffle (the profiler). The profiler uses it to figure out Solidity import statements. And it looks like it expects resolve to give the path and the loaded contents.

So, why can't we contain this functionality entirely within the profiler itself? Well, we need to provide a means to inform the profiler of additional sources at runtime, e.g. with Solidity tests' import "truffle/DeployedAddresses.sol". That source is generated from a template and never lives on disk. Truffle hooks this up to the profiler by means of the TestSource, injected into the TestResolver, so that the profiler can include that in compilation.

There is a package associated with sources, @truffle/contract-sources, but currently it only lists available sources, and incompletely so. My first thought is that this is a better home for this functionality, although there are various limitations in the way:

Blech. This is sort of related to #2469, and it'd probably be worth solving both this issue and that one at the same time.

gnidan commented 4 years ago

Some scratch-paper notes that @eggplantzzz and I came up with: https://peerpad.net/#/r/markdown/2FjSfHhW5bT18dBCqshwj9ZmvZrBMdi7TsyxLbnN5CTF/4XTTM7a4mmELQnm5eGAYobAbBcyGeW85a8K1PDLfh2WpgzG3N