Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Artifact is overwritten if there are two contracts with the same name #1087

Open
1 task
libertylocked opened this issue Jul 5, 2018 · 22 comments
Open
1 task

Comments

@libertylocked
Copy link

libertylocked commented Jul 5, 2018


Issue

If there are multiple sol files with the same name but under different folders in contracts, for example if there's 3 sol files in contracts

  • contracts/Game.sol
  • contracts/Game1/Game.sol
  • contracts/Game2/Game.sol

Only one artifact will be generated when running truffle compile, because it's overwritten by the last contract compiled, typically by the one with greatest lexicographic order in full path

Steps to Reproduce

Make 3 contracts in contracts folder, with any code, as long as the 3 contracts are different

  • contracts/Game.sol
  • contracts/Game1/Game.sol
  • contracts/Game2/Game.sol

Run truffle compile, only 1 artifact is created in build/contracts

Expected Behavior

The artifacts should be placed in folders too and not just overwritten. So there should be

  • build/contracts/Game.json
  • build/contracts/Game1/Game.json
  • build/contracts/Game2/Game.json

Which can be required by artifacts.require("./Game1/Game.json") and such

Actual Results

Only one artifact is generated, build/contracts/Game.json, which is the artifact of the last contract compiled, in this case contracts/Game2/Game.sol

Environment

  • Operating System: Linux 4.17.3-1-ARCH
  • Ethereum client:Ganache CLI v6.1.4 (ganache-core: 2.1.3)
  • Truffle version (truffle version): Truffle v4.1.13 (core: 4.1.13)
  • node version (node --version): v8.11.3
  • npm version (npm --version): 6.1.0
@gnidan
Copy link
Contributor

gnidan commented Jul 6, 2018

Thanks for raising this @libertylocked. Unfortunately this is a limitation of the way Truffle handles artifact files right now. We have this work scheduled for 5.1, as it's a substantial effort.

@anxolin
Copy link
Contributor

anxolin commented Sep 27, 2018

I find this issue important for projects that have a dependencies on other projects with collisions in the name of the contracts.

General names as StandardToken, Token, etc. can appear in your project dependencies easily, so I guess it would be a good idea to have a namespace functionality in place.

In any case, if it's scheduled to v5.1, wouldn't be better to re-open the issue, so we can keep track of it?

montyly added a commit to montyly/set-protocol-contracts that referenced this issue Nov 26, 2018
@stale
Copy link

stale bot commented Nov 26, 2018

Thank you for raising this issue! It has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you would like to keep this issue open, please respond with information about the current state of this problem.

@stale stale bot added the stale label Nov 26, 2018
@anxolin
Copy link
Contributor

anxolin commented Nov 26, 2018

Hi @gnidan ! Do you still plan to include this in 5.1? Do you know if there any progress regarding this issue?

Thanks!

@stale
Copy link

stale bot commented Nov 26, 2018

Thanks for your response! This issue is no longer considered stale and someone from the Truffle team will try to respond as soon as they can.

@stale stale bot removed the stale label Nov 26, 2018
@gnidan
Copy link
Contributor

gnidan commented Nov 26, 2018

@anxolin This is still on our radar. There hasn't been much progress since all the 5.0 work, but my initial requirements gathering from earlier this year is captured here.

@anxolin
Copy link
Contributor

anxolin commented Nov 29, 2018

@gnidan that's a really nice document!

I'm not sure I understood correctly, the idea is to use as part of the new schema new abstractions to differentiate between "Contract types" and "Contract instances" of it?

I think this would solve this issue, but would it handle the problem of the "name collisions", I mean, if you depend on two projects that happen to use 2 contracts with the same name (i.e. Token), truffle will take those, compile them and put it in the build/contracts. So there we have a collision.

Does this solution solve this other issue?

@stale
Copy link

stale bot commented Jan 28, 2019

Thank you for raising this issue! It has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you would like to keep this issue open, please respond with information about the current state of this problem.

@stale stale bot added the stale label Jan 28, 2019
@stale
Copy link

stale bot commented Feb 4, 2019

There has been no new activity on this issue since it was marked as stale 7 days ago, so it is being automatically closed. If you'd like help with this or a different problem, please open a new issue. Thanks!

@stale stale bot closed this as completed Feb 4, 2019
@gnidan gnidan reopened this Feb 4, 2019
@gnidan
Copy link
Contributor

gnidan commented Feb 4, 2019

@anxolin Sorry I lost track of this. Yes, name collisions are being accounted for in the design! Also, that document has probably changed since you saw it.

@anxolin
Copy link
Contributor

anxolin commented Feb 6, 2019

Thanks for the info @gnidan! looking forward to see this new version already!

Is there a rough non-binding estimate time? (just curious)

@gnidan
Copy link
Contributor

gnidan commented Feb 15, 2019

@anxolin is "not soon enough" a good answer?

Can't say more now because we don't even have a complete picture of requirements.

@stale
Copy link

stale bot commented Feb 15, 2019

Thanks for your response! This issue is no longer considered stale and someone from the Truffle team will try to respond as soon as they can.

@anxolin
Copy link
Contributor

anxolin commented Feb 16, 2019

Hehehhe, I understand, I hope you guys find the time to close the scope, :), maybe there’s something smaller that can be done and set things up in the right direction, i.e. something like making a new version, with the new format, but not implementing any of the new features but offers backwards compatibility

@Nanolucas
Copy link

It took me a long time to track down that this was the same issue I was having.

I had a setup with:

contracts/[name].sol
interfaces/[name].sol

and I could not figure out why I kept having my deployments fail with messages like "Contracts that inherit an abstraction must implement all its method signatures exactly." even though I knew all my methods were implemented.

Turns out the artifact for the interface was overwriting the contract artifact (due to folder alphabetical ordering meaning the contract artifact was generated first, then the interface one was generated and overwrote it) so when it got to the deployment stage it was trying to deploy the interface instead.

Renaming the interfaces (e.g. to i[name].sol) fixed the problem but it's an ugly workaround.

Hopefully this issue can be solved, but if not, would it be possible to generate a warning or error during compilation if multiple artifacts with the same name are generated during a single compilation?

@montyly
Copy link

montyly commented Apr 1, 2020

Hi,

As @Nanolucas mentioned, it would be awesome if Truffle could emit a warning if the json artifacts are corrupted because of a contract's name reused.

We had many issues in Slither when it is the case. We had to develop a specific detector to know what contracts names are reused, but I think that a warning from truffle would help the users.

Thanks

@gnidan
Copy link
Contributor

gnidan commented Apr 13, 2020

@montyly if you or anyone else would like to help, it'd be great to have this warning! Would definitely welcome PRs for it.

EDIT: Looks like there's a separate issue #387 specifically related to this warning

@mudgen
Copy link

mudgen commented Apr 14, 2020

This issue also affects interfaces that have the same name as a contract. More info here: #2951

@fainashalts
Copy link
Contributor

Note: this is still blocked, because it requires Truffle to properly read from DB, which is not yet the case.

@pahor167
Copy link

Hi,

Could you please share whether there is any timeline when to address this issue ?

@Raunaque97
Copy link

Is renaming the contracts the only solution for now?

@eggplantzzz
Copy link
Contributor

Sorry for the delayed response @Raunaque97. Yes, currently the only solution for this is to re-name your contracts.

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

No branches or pull requests