dapple
dapple copied to clipboard
dapphub
linker errors? "Invalid implicit conversion from contract DSTokenFrontend to contract DSTokenFrontend requested."
collisions from redundant dappsys includes:
/Users/nikolai/maker-core/dapple_packages/dappsys/contracts/factory/token_factory.sol:16:38: Error: Invalid type for argument in function call. Invalid implicit conversion from contract DSTokenFrontend to contract DSTokenFrontend requested.
ret = new DSTokenController( frontend, bal_db, appr_db );
^------^
/Users/nikolai/maker-core/dapple_packages/dappsys/contracts/factory/factory.sol:103:15: Error: Type contract DSEasyMultisig is not implicitly convertible to expected type contract DSEasyMultisig.
ret = _ms.buildDSEasyMultisig( n, m, expiration );
https://github.com/MakerDAO/maker-core/commit/d08d10333e5c1aed67e9c5a0befe95cac943fdfa
This seems too weird to be a linker error, how is the compiler emitting the same type name for something it thinks is 2 different types??
The problem stems from the linker calculating import hashes based on file contents before replacing the imports contained in the file. So in the case of two different versions of dappsys we end up with files occluding each other which import different versions of the same-named files. This leads to problems when the file that was excluded due to its importing file being occluded gets imported directly and treated as if it were the version preferred by the occluding file. Commenting out the line in the logger which replaces hashes with their original values makes things less confusing:
_545201f38a52745a0e02777cc465ecda0562660c673f8327ef356c5ceac23d89.sol:103:15: Error: Type contract _CT70690b5ec65753f1673e9e3abf4090dce8e5839b51175e475cd1741a433c22e6 is not implicitly convertible to expected type contract DSEasyMultisig.
ret = _ms.buildDSEasyMultisig( n, m, expiration )
Of course, since _CT70690b5ec65753f1673e9e3abf4090dce8e5839b51175e475cd1741a433c22e6 is an alias for the other version of DSEasyMultisig, the logger translates the message to:
/home/dev/maker-core/dapple_packages/dappsys/contracts/factory/factory.sol:103:15: Error: Type contract DSEasyMultisig is not implicitly convertible to expected type contract DSEasyMultisig.
ret = _ms.buildDSEasyMultisig( n, m, expiration );
On a related note, we definitely need to be emitting a warning when there are two different versions of the same package in play.
On a related note, we definitely need to be emitting a warning when there are two different versions of the same package in play.
How do we handle multiple versions of the same package? I think the right solution is to alias old versions as dappsys_v* ?
Old versions should already be getting aliased as a byproduct of the hash replacement in import statements. We could require versions as part of the import paths, though, to enforce explicitness and simplify the linker's duties a bit. But this doesn't really get around the problem of a subpackage returning an older version of a type which is to be passed to another subpackage which expects a newer version of the same type.
If we resolve the linking issue where files importing different versions of the same file collide, though, then we should also augment the logging output to include package version information to clarify things when there is such a type collision. (E.g., "Type contract DSEasyMultisig (from dappsys 0.2.0) is not implicitly convertible to expected type contract DSEasyMultisig (from dappsys 0.3.0)")
We could also disallow mixing package versions. I like this approach because it makes it more difficult to shoot oneself in the foot. It's a very conservative approach, and smart contract development should be exceedingly conservative, IMO. There should be as little ambiguity as possible. On the other hand, developers might find this excessively constraining and demand (and regularly use) an override.
On further reflection, I favor fixing the current functionality so it works as expected and augmenting the logger so it shows package names and versions along with types. It's almost certain that requiring version uniformity won't go over well and we'll find ourselves having to write that code anyway so people can override that behavior.
Might be good to add a --strict flag at some point, though, which does enforce version uniformity for those who want it.
Does this mean that this means you can't reliably use two different versions of a package in one file at all?
How does this relate to the alias key for dependencies? I imagine it doesn't help with type name collisions?
Yes, at the moment it's not possible to reference two different versions of a package in one file. Aliases don't help this at all because the import hashes are based on file contents. No part of the path involved is used. Fundamentally the current problem is that there are files whose contents are identical until their imports have been replaced by hashes, at which point one has occluded the other. This breaks referential integrity.
I don't think we're going to be able to properly allow two different versions of a package in one file until either Solidity introduces something akin to the "from x import y" statement from Python or we write something to interpret such statements ourselves. As it stands, once the linker bug is fixed to calculate hashpaths leaves-first, Dapple will allow such imports but type resolution behavior is currently undefined (though it is deterministic).
@ryepdx
Solidity already have a from x import y statement: http://solidity.readthedocs.io/en/latest/layout-of-source-files.html#importing-other-source-files
In my opinion we should rely as little as possible on own solutions and outsource linking to solidity which it does fairly well.
