Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add elaboration on interface use #17

Closed
wants to merge 1 commit into from
Closed

Add elaboration on interface use #17

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,15 @@ If a function or set of functions could reasonably be defined as its own contrac

Note this _does not_ mean that we should avoid inheritance, in general. Inheritance is useful at times, most especially when building on existing, trusted contracts. For example, _do not_ reimplement `Ownable` functionality to avoid inheritance. Inherit `Ownable` from a trusted vendor, such as [OpenZeppelin](https://github.com/OpenZeppelin/openzeppelin-contracts/) or [Solady](https://github.com/Vectorized/solady).

#### 7. Avoid writing interfaces.
#### 7. Interfaces

Interfaces separate NatSpec from contract logic, requiring readers to do more work to understand the code. For this reason, they should be avoided.
##### A. Use interfaces over manual abi encodings

When calling external contracts, wrapping an address in an interface to call its functions will provide type-safety and compiler help. Other methods like `abi.encodeWithSelector` or `abi.encodeWithSignature`, while functional, require manual definition without IDE support.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need an interface for this, right? You can just do ERC20(address).transfer are you assuming here there is some pragma conflict?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just do Contract(address).function, but then you bring the entire bytecode of Contract into your deployed contract, boosting the size. In extreme cases, you could overflow the 24kb limit without noticing. If you use IContract, then you are typically bringing much less bytecode into your contract, saving on deploy costs and also leaving more room for the 24kb limit if needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a contract or interface here is a compiler level thing and doesn't impact bytecode. These will print them exact same thing


contract T is Test {
    function test() public {
        console2.logBytes(type(UseContract).creationCode);
        console2.logBytes(type(UseInterface).creationCode);
    }
}

contract MyContract {
    function call() external {

    }
}

interface MyInterface {
    function call() external;
}

contract UseContract {
    function x() external {
        MyContract(address(1)).call();
    }
}

contract UseInterface {
    function x() external {
        MyInterface(address(1)).call();
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would only be included in bytecode if you did like new MyContract()


##### B. Write interfaces for contracts expecting calls from other contracts

The convenience of A is only possible when developers write interfaces. Writing and using an interface in your contracts can provide other developers this benefit. If you expect your contract to only be called by end-users or offchain bots, interfaces are less helpful and can even become an impediment by separating NatSpec documentation from contract logic.

#### 8. Avoid unnecessary version Pragma constraints.

Expand Down Expand Up @@ -270,9 +276,9 @@ NO:

```solidity
function test_transferFrom_works() {
// debits correctly
// debits correctly
// credits correctly
// emits correctly
// emits correctly
// reverts correctly
}
```
Expand All @@ -284,7 +290,7 @@ function test_transferFrom_debitsFrom() {
...
}

function test_transferFrom_creditsTo() {
function test_transferFrom_creditsTo() {
...
}

Expand Down Expand Up @@ -399,7 +405,7 @@ Structs can be documented with a `@notice` above and, if desired, `@dev` for eac
struct Position {
/// @dev The unix timestamp (seconds) of the block when the position was created.
uint created;
/// @dev The amount of ETH in the position
/// @dev The amount of ETH in the position
uint amount;
}
```
Expand All @@ -426,10 +432,10 @@ YES:
///
/// @dev ...
/// @dev ...
///
///
/// @param ...
/// @param ...
///
///
/// @return
```

Expand Down
Loading