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.

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

Normalize address vs. interface usage #454

Closed
pegahcarter opened this issue May 18, 2024 · 1 comment
Closed

Normalize address vs. interface usage #454

pegahcarter opened this issue May 18, 2024 · 1 comment

Comments

@pegahcarter
Copy link

It can be a confusing developer experience when using a library that references different contracts as interfaces vs. addresses. For example, these are both contracts:

// https://etherscan.io/address/0x5300A1a15135EA4dc7aD5a167152C01EFc9b192A
address internal constant ACL_ADMIN = 0x5300A1a15135EA4dc7aD5a167152C01EFc9b192A;
// https://etherscan.io/address/0x464C71f6c2F760DdA6093dCB91C24c39e5d6e18c
ICollector internal constant COLLECTOR = ICollector(0x464C71f6c2F760DdA6093dCB91C24c39e5d6e18c);

This could be a problem when importing a library to use within a contract to reference. For example, this compile would succeed:

import {AaveV3Ethereum} from "aave-address-book/AaveV3Ethereum.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

contract MyContract is Ownable {
    constructor() Ownable(AaveV3Ethereum.ACL_ADMIN)
    ...
}

But this compile would fail:

import {AaveV3Ethereum} from "aave-address-book/AaveV3Ethereum.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

contract MyContract is Ownable {
    constructor() Ownable(AaveV3Ethereum.CONTROLLER)
    ...
}

My opinion is to have every contract referenced as an address and provide an additional library variable prepended with I to reference the same contract as an interface. It would add some bloat to the libraries but given they are auto-generated and lightweight I don't see it as an issue.

@sakulstra
Copy link
Collaborator

@pegahcarter, i have no to strong opinion on this.

We added interfaces for addresses that in governance-proposals are usually used as Interface and omitted them on addresses that are usually just used as contract (if at all).

Changing this now would be a huge breaking change, bloat contract verification (as all these contract will end up on etherscan even when only a single address/interface is used). Leaning towards thinking it's not worth it, given you can simply fix your example by casting to address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants