add hookable modifier for all external functions in core-module

[0xxlegolas]

Issue

1. Core contract functionalities cannot be access controlled or modified if there is no hookable modifier.

function registerEntityType(uint8 entityTypeId, bytes32 entityType) external validEntityId(entityTypeId) {

IF we leave this function open for anyone to use then uint8 is probably not a large enough space. However, we may want to consider either namespacing this function so that only the namespace owner and others granted access can create entities, OR making it hookable so that we can configure it with some RBAC.

On this note we should consider making all external functions here hookable() even if we don't use it to permission things.. we (or others) may want to trigger functionality on when entities or entityTypes are registered/associated.
ref

2. There is no hookable modifier for multiple entities

 modifier hookable(
    uint256 entityId,

noticed that there are places where we have more than one entityId being passed to a function.. in these cases it is unclear which entityId we want hookable() to act upon.

As such we could solve this in this way (if we want to solve this):
Rather than accepting a unit256 entityId we could instead accept a bytes entityIds which is abi.encoded(uint256[], [entityId1, enityId2, ...]) and then decode and execute hooks for each entityId

I know we just discussed this today but now that I see it in written form it actually makes a lot of sense to do it that way, even though we would be dipping a toe in nested loops territory.

Perhaps have two different hookable modifiers, one for arrays and one for single entities, so that the syntax doesnt suffer too much from it. This should be the scope of another PR, though

3. Do we need a overloaded hookable() modifier without entity ?

Suggested Solution

• Modify all external functions in smart-object-framework to enforce hookable() modifier.
• Modify existing hookable() modifier to support multiple entities
• Added overloaded modifiers

Discussions

1. Do we need overloaded modifier for single entities and multiple entities separately ?
2. Do we need a overloaded modifier without entity ?

[0xxlegolas]

@MerkleBoy @ccp-tezza lets discuss here

[0xxlegolas]

@ccp-tezza
There are certain functions where there is no entity. But the hookable modifiers are for entities
eg:

function registerEntityType(uint8 entityTypeId, bytes32 entityType) external validEntityId(entityTypeId) {

So in this case

1. Do we need a hookable modifier without entity ? or
2. Do we use the same hookable modifer and instead of associating with a entity should we associate with any key ?

Then while execution, we can check if its a entity then check its registered if not no.

Note: Make sure entities are hookable only by owners.

[0xxlegolas]

@ccp-tezza There are certain functions where there is no entity. But the hookable modifiers are for entities eg:

function registerEntityType(uint8 entityTypeId, bytes32 entityType) external validEntityId(entityTypeId) {

So in this case

1. Do we need a hookable modifier without entity ? or
2. Do we use the same hookable modifer and instead of associating with a entity should we associate with any key ?

Then while execution, we can check if its a entity then check its registered if not no.

Note: Make sure entities are hookable only by owners.

Solution : Use System Hooks instead of a modifier without a entity.

[MerkleBoy]

Using MUD System Hooks is an interesting idea that is worth exploring;

on the other side I did a little bit of research and it turns out modifier overloading is not something that is allowed in Solidity: ethereum/solidity#12332

It's not the first time this week I end up finding that Solidity is lacking when it comes to overloading stuff, so we need to keep that in mind. A bit of a bummer, really