Deprecate dappels Linking?

[mhhf]

I'm currently in the process of refactoring the build/linking process and classes.json file. Currently dapple has a complicated linking process including recopying all files to a flat structure and renaming imports with unique hash identifier. I'm aware that the linker was written in a time, when solc didn't support import statements.

Is there any reason to keep this linking process or should I deprecate it. @nmushegian @ryepdx

[nmushegian]

Import statements did exist, but namespaces didn't. So "importing a subpackage" required changing import paths if you wanted it in a subdirectory. I think the native import/linking is still rather limited, but I haven't explored it fully.

I would love for this to be a native solc feature, obviously. I agree the linker is very complex. On the other hand if we have the only working thing, we maybe should go through with it. Hard to say.

[nmushegian]

The current library linker is also almost the same as the CONSTANT macro which is another one of our linker's features

[mhhf]

Solidity has come a long way in terms of linking and namespaces: http://solidity.readthedocs.io/en/latest/layout-of-source-files.html#importing-other-source-files

If our only requirement is to support linking packages correctly, I think its fine to go with the official solution.

[nmushegian]

if that's enough to make it work then let's do it

[ghost]

Not entirely sure how much simplification the new solc options allow for, given the requirement of allowing subpackages to have their own versions of any given package. Have either of you dug into this any deeper?

[nmushegian]

within a file distinct versions have different names. The aliases should be defined in the dappfile.

[mhhf]

I most looking on this: http://solidity.readthedocs.io/en/latest/layout-of-source-files.html#use-in-actual-compilers Currently the linking is done by copying a bunch of files and substituting the import statement with a regex. Linking and Building are components, I didn't spend much time on, yet. But we absolutely need to think about something different:

1. export linking & building in its own dapple-module to clean up the overall code
2. outsource the linking complexity to solidity as this file copying thing can be done by just pointing solidity to package dirs:

solc module1:github.com/ethereum/dapp-bin/=/usr/local/dapp-bin/ \
     module2:github.com/ethereum/dapp-bin/=/usr/local/dapp-bin_old/ \
     source.sol

1. we need rethink the classes.json format (#263 #212) and probably with this, how we do packaging
2. additionally on my wish list is to generate for every contract a "minimal recompile object" a tree of .sol file beginning with the file the contract is defined in and recursively all import files. This would make verification easier and also human readable diff views of 2. contracts/ addresses (for dapphub)

[ghost]

There's no way to get around the file copying at the moment because we have to mutate the file contents in order to protect against contract name collisions between packages. It also seems that solc's package directory aliasing scheme isn't sufficient for our needs at the moment. It does not appear to allow multi-tiered aliasing (i.e., aliasing references in a sub-dependency of a dependency). So we're stuck with the linker's current methods, unfortunately.

We can probably break the linker out into its own plugin, but we might have to re-think how plugins work. It needs something like a Controller, but with the source code stream instead of the build pipeline (which it's part of). This could mean breaking the build piece out into its own plugin with a dependency on the linker plugin. I think this is our best option.

What are your present thoughts on how we might change the classes.json format and our packaging system?

I like the "minimal recompile object" idea. I think it would also be good to allow outputting a flat file suitable for handing off to a contract verification service.

[mhhf]

It does not appear to allow multi-tiered aliasing

Do we need this actually? We can settle down with a flat structure together with solidity's module assignments context:prefix=target: Let A, B, C are packages and A depend on B which depends on C (A -> B -> C): pkg dir looks like this:

.
./A/a.sol
./B/b.sol
./C/c.sol

and solc executed as: solc A:B=./pkg/B B:C=./pkg/C Note, the context in those module links.

which allows for:

// a.sol
import "b/b.sol";
...
// b.sol
import "c/c.sol";

Is there anything I don't see in this example?

Also, can you explain how the current model solves contract name collisions? contract name collisions are a serious problem and I want to push solidity to support it, but before that I want to understand the problem in depth.

[nmushegian]

honestly I'm ok with stepping back, using a flat package dir structure, and avoiding the namespace problem until Solidity guys address it

[mhhf]

At least I can describe this problem in a github issue to put their attention to it.

Also a flat dir structure is future proven and with this we would outsource the linking problem completely to solidity.

[nmushegian]

@mhhf I suggest trying to migrate maker-core to 0.8 as a case study

[nmushegian]

there's a few collisions of each type due in part to multiple dappsys versions

[ghost]

IIRC, name collisions in the current code are resolved by keeping track of contract names as they're encountered during the linking process, with each contract name being replaced by a hash of its fully-qualified name (i.e., file content hash plus contract name), with each reference in importing packages also being replaced so as to keep the package compilable. Then a second pass is made and hashes of any non-occluded contracts are replaced with their original names, under the assumption that the user is less interested in the deeply nested contracts. Could definitely be rewritten to be done in a single pass if the contract files are sorted by depth first, of course. Allowed myself a naive implementation for the first version as linking shouldn't be a very expensive operation anyway.

TBH, the method we're currently using comes very close to a "flat directory" approach as it is. All package code is reorganized into content-addressed sibling files prior to compilation. At first blush, one might think we could use solc's aliasing to map those content hashes to their original locations, saving us the need to write anything to the hard drive. But we also need to update the references in the importing source code before we can compile and solc doesn't allow mapping paths to other paths, so that doesn't actually help us at all. We end up having to copy code anyway since any file that imports anything will need those imports changed. Doesn't matter if we continue using content-addressing or if we switch to "package name + semver" addressing, we still have to face that problem. On top of that, the latter introduces the need to either halt or make an educated guess and warn the user when packages purporting to have the same name and version end up having different contents. (To be fair, we should probably at least be emitting a warning for that case as it is. I don't think it should be disallowed entirely though, as sometimes, in the course of debugging, one needs to log something in the guts of a particular dependency.)

We also don't get to avoid doing our contract name depth analysis and replacement because naming collisions are still a problem in Solidity, which also necessitates copying files around on the hard drive in order to rename colliding contracts.

In short, I'm not sure we can keep namespaces as we know them now and also lean on solc's aliasing flags to at all simplify our code. We'll end up having to do all the things we're doing now and also add code to make sure solc gets the right flags on top of it.

That said, I do think the linker could be better. I think the contract name linking could be done in one pass instead of two if we were cleverer about it. More importantly, I hate how extensively the linker uses regexes. Would be nice to leverage https://www.npmjs.com/package/solidity-parser and get away from that sort of thing.

[ghost]

FWIW, I'd love to be wrong on this. That's just how I see it at the moment.

[mhhf]

Thanks for shining some light on it. I know understand how dapple resolves name collisions. However apart from name collisions I don't understand this fully:

But we also need to update the references in the importing source code before we can compile and solc doesn't allow mapping paths to other paths, so that doesn't actually help us at all.

Suppose we don't have to care about name collisions. Can you please provide me a minimal failing example for this?

[ghost]

Sorry again for the late reply. I've been consumed by cPay and overwhelmed with Github emails, so I missed your reply.

If we stop caring about contract name collisions, that might allow us to realize some LOC savings by leaning on solc's aliasing. But this also means we'll likely be restricted to one instance of each package in the dependency tree, since two different versions of the same package will almost certainly cause a name collision.

[mhhf]

name collision will be solved soon in solidity by implementing a new in and output format. See the current draft: https://pad.riseup.net/p/7x3G896a3NLA (Contract names will now be prefixed by the path of its definition file) So we will have to refactor a lot in order to adapt to the new version so moving the responsibility away seems like a good thing to do.

[mhhf]

Also with this name collision is solvable:

pragma solidity ^0.4.0;

import {A as AA} from "./a1/a.sol";
import {A as AB} from "./a2/a.sol";

contract ASD is AA {
  bool val;
  function ASD() {
    val = true;
  }
}

So we can go and refactor linking, although refactoring the code would be necessary as well...

[nmushegian]

+1 prefer to move on this as soon as possible, I am happy to refactor all our solidity to not be left out of the ecosystem