Add a function to convert an ACL Param to uint256

[leftab]

Currently, ACL parameters are created with the Param struct:

struct Param {
    uint8 id;
    uint8 op;
    uint240 value;
}

but the ACL grantPermissionP function takes a uint256[] for its parameters:

function grantPermissionP(address _entity, address _app, bytes32 _role, uint256[] _params)

It could be interesting to have a helper function to convert a Param struct to uint256. Something like this:

function encodeParam(uint8 id, uint8 op, uint240 value) internal pure returns (uint256) {
    return uint256(id) << 248 | uint256(op) << 240 | value;
}

[leftab]

I will submit a pull request if you think it's a good idea

[bingen]

Hey @leftab , yes I think it's a good idea. Besides I think it would good also to:

• expose struct Param somewhere where can be used from outside (maybe ACLHelpers and then inherit it when needed?)
• expose also Op enum and the *_ID constants
• add a version of that new function that takes Param as argument, instead of those integers
• add a version of that function that takes an array of Param instead of only one. Maybe we could try to use some assembly conversion to make it more efficient

Anyway I would wait to hear @izqui or @sohkai opinion about it.

[sohkai]

On top of @bingen's list, I think there are a number of ways we could restructure the ACL so that the "scripting language" we're defining and evaluating with the params is logically decoupled from the ACL itself.

Users should be able to include a library (there's a few technical problems with this) or inherit from a contract to get access to the definitions needed to construct and massage params, and the evaluator could be its own contract so any subclass could potentially have the ability to evaluate these params.

[Quazia]

I think restructuring to address the decoupling and adding the more customizable approach as detailed by @sohkai can be addressed at a later date but the additional helper function and changes detailed by @bingen seem safe enough to try. @leftab are you still interested in making these changes?

[leftab]

Sorry for the delay we were a little bit busy completing our first milestone hehe. I agree with @Quazia that maybe it would be a good idea to split this issue in two. I can certainly work on a pull request for @bingen's changes. It should be ready next week :)

[leftab]

@sohkai @bingen I was about to add some tests for the new functions but I just noticed that the ACLSyntaxSugar.sol is in the skip list of .solcover.js

Do you know the reason why?

[sohkai]

@leftab Mostly because it's a sugar library whose tests would be fairly academic, but there would be value in adding tests for it, especially the ACLHelpers :).

[leftab]

Got it! Thanks for the explaination @sohkai ! ;)