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

Let compiler handle cyclic dependencies #135

Closed
jeremyellis opened this issue Apr 14, 2016 · 18 comments
Closed

Let compiler handle cyclic dependencies #135

jeremyellis opened this issue Apr 14, 2016 · 18 comments

Comments

@jeremyellis
Copy link

jeremyellis commented Apr 14, 2016

If I have two contracts like so

import "./pong.sol"
contract ping
{
    pong p;
}

and

import "./ping.sol"
contract pong
{
    ping p;
}

I get a cyclic dependency error when compiling. I was under the impression this shouldn't happen as they only have each other as members in the contracts, not inherit from each other.

@markledev
Copy link

markledev commented Jun 6, 2016

I have the same error. @tcoulter Could you explain why we should avoid this cyclic dependency AT ALL COST while designing smart contracts?

@redsquirrel
Copy link
Contributor

redsquirrel commented Jun 6, 2016

Cyclic dependencies are impossible in [certain situations](If a contract wants to create another contract, the source code %28and the binary%29 of the created contract has to be known to the creator.):

If a contract wants to create another contract, the source code (and the binary) of the created contract has to be known to the creator.

But it does seem like they should be allowed when the contracts are members of each other.

@tcoulter
Copy link
Contributor

tcoulter commented Jun 6, 2016

@redsquirrel For some reason your link is 404'ing for me. That said, @jeremyellis and @phuongvietle, you solve cyclic dependencies using abstract contracts: http://solidity.readthedocs.io/en/latest/contracts.html?highlight=abstract#abstract-contracts

Imagine a scenario where contract A will create a new contract B (A depends on B). And B will use the contract A. Here, there's a cyclic dependency. You can resolve that by creating an abstract version of A and using that within B. Example:

A.sol

import "B.sol";
contract A {
  B b;
  function someFunction() {
    b = new B();
  }
}

AbstractA.sol

contract AbstractA {
  function someFunction(); // No implementation!
}

B.sol

import "AbstractA.sol";

contract B {
  AbstractA a;

  function setA(AbstractA new_a) {
    a = new_a;
  }

  function useA() {
    a.someFunction();
  }
}

Obviously this isn't a great example because B doesn't do anything useful, but you get around the cyclic dependencies using the abstract version of A. Note: You can't create new instances of an abstract contract (i.e., new AbstractA() will throw an error). If you need both contracts to create a new version of A, you should create a third contract called "AFactory" whose sole job it is to create A's.

Closing as this isn't a truffle issue.

@tcoulter tcoulter closed this as completed Jun 6, 2016
@jeremyellis
Copy link
Author

jeremyellis commented Jun 6, 2016

@tcoulter But solc happily compiles the ping and pong contracts, is there a reason truffle doesn't allow it?

@redsquirrel
Copy link
Contributor

Sorry for the bad link. Here's where I was trying to point: http://solidity.readthedocs.io/en/latest/contracts.html#creating-contracts

@tcoulter I do think this is actually a Truffle issue. contracts.js looks for cycles in contract dependencies and errors if they are detected.

Cyclic dependencies don't appear to be a problem in Solidity as long as A and B don't need to create each other. If C did the creating and connected A and B to each other, then everything should technically work (but Truffle would still error). Only cyclic creation dependencies are impossible.

@tcoulter
Copy link
Contributor

tcoulter commented Jun 6, 2016

Ah, perhaps I misunderstood. Reopening.

@tcoulter tcoulter reopened this Jun 6, 2016
@tcoulter
Copy link
Contributor

tcoulter commented Jun 6, 2016

Truffle creates its own dependency graph in order to determine which files need to be compiled when a specific solidity file has been edited. Perhaps it's overzealous about cyclic dependencies, and should allow the compiler to handle that.

@markledev
Copy link

@tcoulter I am looking forward to new version of truffle! When will the new version be released?

@tcoulter
Copy link
Contributor

tcoulter commented Jun 6, 2016

@phuongvietle I'm working on some documentation right now on a PR that needs to be merge. That PR as well as other small fixes will make up the 3.0 release. Follow this: #143 . I'll remove the WIP header and have documentation there today, likely within the next hour.

@markledev
Copy link

@tcoulter Got it! Thank you for saving us huge amount of time figuring out a sound workaround for this issue.

@tcoulter
Copy link
Contributor

tcoulter commented Jun 6, 2016

👍 :)

PS: docs might be a couple hours, rather than an hour. :)

@tcoulter tcoulter changed the title Cyclic dependencies Let compiler handle cyclic dependencies Jul 22, 2016
@D-Nice
Copy link

D-Nice commented Mar 6, 2017

Ran into same issue, and hopefully looking to have solc handle this.

@cryptophonic
Copy link

+1 same issue. the check doesn't seem to be working correctly either. For example, I'm getting this cyclic dependency error from a contract that has a sole import of an abstract contract that (a) no other contract imports and (b) doesn't import anything else. Looking into how to disable this check..

@tcoulter
Copy link
Contributor

This has been fixed in truffle-compile, v1.1.1. You should be able to get this update if you uninstall and reinstall Truffle, if on Truffle 3.x. Thanks for the reports!

@cryptophonic
Copy link

Using this new version of Truffle, I have a simple contract that imports another contract (not a library). When the imported contract is changed, it's not rebuilding the contract that imports it. Using 'truffle compile --compile-all' resolves the problem by forcing a complete rebuild. Hopefully this isn't an either / or situation where we need the cyclic dependency check in order to have dependencies built automatically.

@tcoulter
Copy link
Contributor

@mjackson001 The issue you reported sounds very different to the one reported here. I'd recommend you create a new ticket instead.

When doing so, it's very helpful to provide code and show us your directory structure, and tell us where you're importing from, say from npm, or ethpm, etc. If from npm or ethpm, that's a known issue.

@cryptophonic
Copy link

ok, let me try to create a simple test case and submit a separate issue...

@cryptophonic
Copy link

Test case submitted: #384

nakajo2011 pushed a commit to nakajo2011/truffle that referenced this issue Aug 7, 2018
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

6 participants