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

Allow public constants in interfaces #3411

Closed
fulldecent opened this issue Jan 19, 2018 · 10 comments
Closed

Allow public constants in interfaces #3411

fulldecent opened this issue Jan 19, 2018 · 10 comments

Comments

@fulldecent
Copy link
Contributor

fulldecent commented Jan 19, 2018

(Previously, this issue discussed ALL constants, now it only addresses public constants.)

Currently constants are not allowed in interfaces:

pragma solidity ^0.4.19;

interface Nameable {
    string public constant name;
}

Error is:

TypeError: Variables cannot be declared in interfaces.

string public constant name;
^-------------------------^

Public constants affect ABI and should be allowed in interfaces.


If this issue is implemented and also if interfaces gain the ability to bring default implementations, then this syntax will be even more useful:

pragma solidity ^0.4.19;

interface ERC165 {
    // Interface signature for ERC-165
    bytes4 constant INTERFACE_SIGNATURE_ERC165 = // 0x01ffc9a7
        bytes4(keccak256('supportsInterface(bytes4)'));

    function supportsInterface(bytes4 interfaceID) constant returns (bool);
}
@axic
Copy link
Member

axic commented Jan 20, 2018

This is a tricky one. Interfaces only allow declarations which are reflected in the external ABI and a constant isn't such a thing.

I think at this point this may cause more problems as it solves given the current messy handling of public modifiers on variable and inheritance.

@axic axic added the feature label Jan 20, 2018
@fulldecent fulldecent changed the title Allow constants in interfaces Allow public constants in interfaces Jan 21, 2018
@fulldecent
Copy link
Contributor Author

@axic Thank you, I have studied more how interfaces are implemented. For now I have reduced the scope of this issue to only address public constants without implementations (i.e. values).

@axic
Copy link
Member

axic commented Jan 21, 2018

Can you actually revert the change and post this new version as a comment? My comment makes little to no sense now.

@fulldecent
Copy link
Contributor Author

Sorry, I lost history, I thought GitHub saved it. For clarity, I did at a note to the top.

@axic
Copy link
Member

axic commented Jan 29, 2018

Having thought a bit more about this, I do not think constants belong to interfaces. Are you sure you do not need enums instead?

@fulldecent
Copy link
Contributor Author

fulldecent commented Jan 30, 2018

This issue only makes sense if interfaces are being removed/changed wholesale -- #3420 I'll let that discussion continue in #3420


If interfaces are truly just to track ABI of a contract on EVM (again, I hate the ABI word choice) then I agree and only function wording is needed, the constant is just an implementation detail of a function.


So either way, yes I agree this should be closed.

@axic
Copy link
Member

axic commented Jan 30, 2018

ABI (again, I hate this word choice)

You are using this in all issues. How would you call it if not the ABI?

The "JSON ABI" part can be misleading, there might be a better name for "the JSON description file of the particular entry points in a contract".

@fulldecent
Copy link
Contributor Author

The application binary interface is anything that touches EVM -- the function signature hash, function call data types, function return data types. Everything else is not ABI.

The JSON file includes implementation-specific details:

  • An unverifiable guarantee of function names (helpful)
  • An unverifiable guarantee of parameter names (helpful)
  • An unverifiable guarantee of enum names (helpful, but dangerous)
  • An unverifiable guarantee of event topic names (helpful)
  • An unverifiable guarantee of whether require(msg.value==0) boilerplate is at the top of the function
  • A recommendation of whether STATICCALL should always be used instead of CALL
  • Maybe something similar for PURECALL in the future

Maybe that's a "contract description file" or something else. But I need to keep reminding myself that it is NOT an ABI.

Thanks for listening.

@axic
Copy link
Member

axic commented Apr 9, 2020

Since we allow accessing constants through contract names (#1290) and enums in interfaces (#4087), it may make sense to reconsider this feature, at least for some healthy discussion.

I am still not sure if it is a good idea or not (it definitely can be useful from a users' perspective, but could be dangerous?)

The case I ran into is the following (ralexstokes/deposit-verifier#2):

interface DepositContract {
    uint constant PUBLIC_KEY_LENGTH = 48;
    uint constant SIGNATURE_LENGTH = 96;

    function deposit(
        bytes calldata publicKey,
        bytes32 withdrawalCredentials,
        bytes calldata signature,
        bytes32 depositDataRoot
    ) external payable;
}

In this example the interface uses basic types (bytes), but they have a limit enforced (the two constant above) so it would be useful from the caller to have require statements:

contract DepositProxy {
    function verifyAndDeposit(...) public {
        require(publicKey.length == DepositContract.PUBLIC_KEY_LENGTH, "incorrectly sized public key");
        require(signature.length == DepositContract.SIGNATURE_LENGTH, "incorrectly sized signature");

        // ...

        DepositContract(0x...).deposit(...);
    }
}

@axic
Copy link
Member

axic commented Apr 25, 2020

Oops, I think the original issue was about allowing "getter" functions to be defined. This has been solved by #3514.

My proposal was unrelated, hence moved to #8775.

@axic axic closed this as completed Apr 25, 2020
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