Invalid and inconsistent types in ABIs for library functions
- Dominant language
- C++
- Stars
- 25.7k
- Forks
- 6.2k
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 21
Description
## Description
While most library functions don't go in the ABI, `pure` and `view` functions do. Of course, actually using these is [infamously](https://github.com/ethereum/solidity/issues/6420) [difficult](https://github.com/ethereum/solidity/issues/7530), due to the fact that library selectors are computed differently from other selectors, but they're there. The specific types that differ are structs, enums, and contracts, which in the signature use the internal name of the type, rather than the usual conversion to an ABI type.
The problem is this: In nearly every version of Solidity, including 0.6.10, the ABI parameter entries for enums and contracts put this internal name of the type in the `type` field, rather than the name of the corresponding ABI type.
Now, this does have the advantage that it allows the selector to be computed correctly by the naive method, without specifically knowing that you're dealing with a library! (As the ABI still lacks anything to indicate the contract/library distinction.) But it has the disadvantage that it screws up any more sophisticated attempt to process the ABI, that is expecting something that conforms to the spec. (Like: You can't actually *encode* with these types! Without other information, there's nothing to tell us whether a `Blank` is the name of a contract type or the name of a globally-declared enum type. Web3 and Ethers sure can't handle it, and I'm not about to make Truffle try.)
I suspect this is simply a bug. Further evidence for this is that this only seems to happen to enums and contracts -- structs are unaffected. Even if it's deliberate... well, it should be changed, and the library problem should be handled a different way. (Having `internalType` goes a long way here, even if we still need something in the ABI to distinguish contracts from libraries.)
Now, on noticing this, I had to check which versions are affected... it turns out that this behavior goes all the way back to **0.1.5**, a mere two versions after libraries were introduced in 0.1.3. Given how long this has been around, one might be tempted to say, this isn't a bug, this is just how it works, at least for now. But I'm still calling it a bug: it breaks the spec; it breaks any processing more sophisticated than computing the selector, such as encoding or decoding; and structs don't work this way and have never worked this way, not since 0.4.17 when it first became possible to pass a struct externally.
## Environment
- Compiler version: 0.1.5 -- 0.6.10
- Target EVM version (as per compiler settings): default
- Framework/IDE (e.g. Truffle or Remix): Truffle (for 0.4.12 and later); Remix (for 0.4.11 and earlier)
## Steps to Reproduce
Try compiling the following:
```solidity
pragma experimental ABIEncoderV2;
library ABITest {
struct Pair {
uint x;
uint y;
}
function structy(Pair calldata) external view { //works fine
}
enum Ternary {
Yes, No, MaybeSo
}
function enumy(Ternary) external view { //problem!
}
function contracty(Blank) external view { //problem!
}
}
contract Blank {
}
```
(comment things out as appropriate to get it to compile with earlier versions; changing `calldata` to `memory` and `external` to `public` may also be necessary; also changing `view` to `constant` (or just omitting that, since everything went in the ABI prior to 0.5.6))
Contributor guide
Research direction
Start by compiling the Solidity example in the issue and inspect the ABI entries generated for the enum and contract parameters, comparing them with the struct entry and ABI specification. Trace the compiler's ABI generation for library view and pure functions. Done means the affected parameter types use the corresponding ABI types without breaking selector handling or existing library ABI behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, solidity
- Domain
- blockchain, compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100