argotorg / argotorg/solidity

Calls to attached functions in a library do not always match equivalent direct calls in terms of being internal/external

Open
#13,908 2 comments 0 reactions 0 assignees View on GitHub
bug :bug: low impact medium effort
Dominant language
C++
Stars
25.7k
Forks
6.2k
Avg merge
2d 19h
Merged PRs (30d)
29

Description

### Description
In Solidity we distinguish between internal and external library calls by whether the function is qualified with a library name or not. Generally, a qualified call (`L.f()`) is external while an unqualified one (`f()`) is internal. The only exception are internal functions - in that case `L.f()` is an internal call.

This behavior should be preserved when the function is attached via `using for`. Whether the call is internal or external should depend on whether `using {L.f}` or `using {f}` was used. Currently this is not the case - there is no difference between these two directives.

### How to reproduce

#### Public functions
Attached public functions are currently called externally (which is not allowed in libraries), even if unqualified in `using for`:
```solidity
library L {
using {publicFunction} for uint;

function publicFunction(uint) public pure {}

function test(uint x) public pure {
x.publicFunction(); // Error: Libraries cannot call their own functions externally.
publicFunction(x); // internal call
}
}
```
`x.publicFunction()` here should be an internal call as well and compile without errors.

#### Private functions
Attached private functions are always called internally, even if qualified in `using for`:
```solidity
library L {
using {L.privateFunction} for uint;

function privateFunction(uint) private pure {}

function test(uint x) public pure {
x.privateFunction(); // internal call
L.privateFunction(x); // Error: Member "privateFunction" not found or not visible after argument-dependent lookup in type(library L).
}
}
```
`x.privateFunction()` here should be an external call and produce a compilation error.

#### External functions
External library functions are currently not usable with `using for` (#13765) but after that's fixed (#13855), the following example is going to be inconsistent:

```solidity
library L {
using {externalFunction} for uint;

function externalFunction(uint) external pure {}

function test(uint x) public pure {
x.externalFunction(); // external call
externalFunction(x); // Error: Undeclared identifier. "externalFunction" is not (or not yet) visible at this point.
}
}
```
`x.externalFunction()` here should be an external call and produce a compilation error.

### Specification
- Given `using {f}`, `x.f()` should be equivalent to `f()`.
- Given `using {L.f}`, `x.f()` should be equivalent to `L.f()`.
- Make sure the fix does not break functions qualified in different ways. For example `M.f()`, where `M` is a module and `f()` is a free function should always be an internal call.
- Calling the function in presence of conflicting directives (i.e. both `using {f}` and `using {L.f}` for the same type) should be an error.
- Unless the function is internal. Then it is allowed.
- To avoid introducing non-backwards-compatible changes, the error should be issued at the point of call. If the function is not actually called, it's not an error.
- Note that the conflict may exist only in some scopes, not everywhere. E.g. if there is a file-level `using {L.f}` and one library has `using {L}` while another does not.
- For the external case it would be fine to produce an error directly in the directive because such use currently an error (though this will change when #13765 is fixed).

Contributor guide

Open the contributing guide

Research direction

Start with the public, private, and external Solidity reproducers and the specification, comparing qualified and unqualified attached-function calls. The work is done when the specified internal/external behavior and conflicting-directive errors are consistent without breaking differently qualified functions.

Written by the indexing model from the issue text.

Assessment

Tech stack
solidity
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.