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

Adding support for multiple contracts in the same file to be handled correctly #86

Closed
wants to merge 1 commit into from

Conversation

jgostylo
Copy link

@jgostylo jgostylo commented Feb 4, 2016

Here is the argument against "Just put all your contracts in separate files".

  1. I have a hub contract and a spoke contract.
  2. I have set them up so that the hub has calls into the spoke and the spoke has calls back into the hub.
  3. If they are in separate files I have to include an import from each contract into the other and solc complains that the contract already exists.
  4. If I remove the import from either file solc complains about type not found.
  5. The only way to keep solc happy is to put both contracts in the same file.

The crux of this change is to introduce config.contracts.filenames (which I think is more accurate for what it does) and then to parse the contract files at config creation time and use regex to extract the names of the contracts and put those into config.contracts.classes. contracts.es6 is updated to accommodate that change.

@jgostylo
Copy link
Author

jgostylo commented Feb 4, 2016

This is a pull request for the following issue: #83

@tcoulter
Copy link
Contributor

Hi @jgostylo. I'm trying to wrap my head around your context. Question:

Does the Hub have functions that create a new instance of the Spoke AND does the spoke have functions that create a new instance of the Hub? i.e., does the Hub call new Spoke() AND the spoke call new Hub()?

My guess is the latter is false: the Spoke likely doesn't ever create a new version of the Hub. In that case, I'd recommend creating a folder called abstract within your contracts directory, and create a new file called AbstractHub.sol. Make AbstractHub.sol look like this:

contract AbstractHub {
  function hubFunctionOne() returns (uint);
  function hubFunctionTwo();
  // etc.
}

The functions in AbstractHub.sol must match Hub.sol exactly, including any return types. Notice that AbstractHub.sol doesn't have any function definitions -- it only has the function signatures. You can use this to tell solc which functions exist at an address without having to include the whole code, and in your case stop you from having to manage two copies.

Finally, to make this all work, include AbstractHub.sol in Spoke.sol, like this:

import "./abstract/AbstractHub";

// Now, I'm just assuming your spoke looks something like this:
contract Spoke {
  AbstractHub hub; // this is actually an address

  function setHub(address addr) {
    hub = AbstractHub(addr);
  }

  function spokeFunctionOne() {
    // Since your Hub contract at the address set in `hub` actually contains 
    // a function called "hubFunctionOne", this will work just fine.
    hub.hubFunctionOne();
  }
}

And viola! No copying or circular dependencies.

Let me know if this works for you. I suspect it will, in which case I think your PR is likely unneeded.

@tcoulter
Copy link
Contributor

More info about abstract contracts here: http://solidity.readthedocs.org/en/latest/contracts.html#abstract-contracts

@jgostylo
Copy link
Author

@tcoulter you are correct, the spoke does not create new instances of the hub. I actually attended a local meetup recently and asked about this situation and someone reminded me of abstract contracts.

I would then agree with you that this pull request is not needed.

I have not yet tried it because I am just running with my change locally. I assume that if the function signatures between AbstractHub and Hub ever differ I would only be catching it at runtime if the function call matched the AbstractHub function signature. Basically it provides limited compile time validation while forcing you to sync information in two places.

Because of that and in the interest of compiling and deploying validly written contracts I think I would still prefer this change. My biggest problem with my PR is the regex is kind of hacky. Is there any concern with implementing the intention of my idea?

@tcoulter
Copy link
Contributor

This PR is now out of date. However, I think the feature is still a valid one to support. See new issue created, #204.

@tcoulter tcoulter closed this Jul 22, 2016
nakajo2011 pushed a commit to nakajo2011/truffle that referenced this pull request Aug 7, 2018
nakajo2011 pushed a commit to nakajo2011/truffle that referenced this pull request Aug 7, 2018
Remove solcjs upgrade logic and test example
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants