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

Interfaces only expose a small subset of the public/external contract functions #458

Closed
ewingrj opened this issue Nov 14, 2018 · 4 comments

Comments

@ewingrj
Copy link

ewingrj commented Nov 14, 2018

It would be great if the interfaces exposed all public/external contract functions.

In the current state, users must either implement their own interface in order to expose the necessary functions, or import the implementation contract itself.

Importing the impl contract is not ideal b/c these contracts are typically pinned, thus forcing any contract importing the impl contract to use the same solidity version.

@sohkai
Copy link
Contributor

sohkai commented Nov 14, 2018

We generally try to keep the interfaces minimal, and only include the most important parts of an interface.

The current Kernel and ACL have a number of interfaces which are "non-standard", but we could probably provide an "interop" interface contract for them.

Are there any interfaces in particular you'd like to expose?

@ewingrj
Copy link
Author

ewingrj commented Nov 14, 2018

A non-exhaustive list of functions that I've run into so far in trying to decouple our contracts from the aragonOS solidity version that aren't exposed on the interface:

ACL:

    function createPermission(address _entity, address _app, bytes32 _role, address _manager) external;
    function grantPermission(address _entity, address _app, bytes32 _role) external;
    function revokePermission(address _entity, address _app, bytes32 _role) external;
    function setPermissionManager(address _newManager, address _app, bytes32 _role) external;
    function function grantPermissionP(address _entity, address _app, bytes32 _role, uint256[] _params) public;

Kernel:

function initialize(IACL _baseAcl, address _permissionsCreator) external;
function APP_ADDR_NAMESPACE() external pure returns (bytes32);
function newAppInstance(bytes32 _appId, address _appBase) external returns (ERCProxy appProxy);
function newAppInstance(bytes32 _appId, address _appBase, bytes _initializePayload, bool _setDefault) external returns (ERCProxy appProxy);
function recoveryVaultAppId() external pure returns (bytes32);
function acl() external view returns (IACL);

I'm sure there are more...

We are using aragonOS a bit differently than probably designed for. Our contracts right now are not being integrated into the aragon app ecosystem. We are using aragonOS b/c of the acl and proxy functionality included.

However I feel developers using aragonOS to build an AragonApp will still run into these issues. To deploy the app, the current practice is to use a Kit (correct me if I'm wrong). These kits often need to create/grant permissions on the ACL, but these functions aren't exposed via the interface. The kit will need to import ACL instead of IACL thus forcing the use of the aragonOS pinned solidity version.

I understand the reason behind pinning the solidity version as mentioned in #341. But not providing appropriate interfaces seems a bit restrictive on app developers.

@sohkai
Copy link
Contributor

sohkai commented Nov 14, 2018

To deploy the app, the current practice is to use a Kit (correct me if I'm wrong). These kits often need to create/grant permissions on the ACL, but these functions aren't exposed via the interface. The kit will need to import ACL instead of IACL thus forcing the use of the aragonOS pinned solidity version.

Hmm, yes, this is a definite area of improvement. Being able to smoothly use kits with the current implementation of the Kernel and ACL is quite important (might live elsewhere, like beside kits-base).

@sohkai
Copy link
Contributor

sohkai commented Mar 18, 2020

Closing for general interface improvements in #577.

@sohkai sohkai closed this as completed Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants