Add a second param to isModuleInstalled() to support complex and conditional modules

[yaonam]

[Here module refers to validator, executor, and hooks.]

Idea

Change
function isModuleInstalled(address) external returns (bool)
to
function isModuleInstalled(address, bytes calldata) external returns (bool).

This second param can be very useful for implementations that require additional data to locate/check the module. Whether or not a module is installed might be dependent on conditions such as caller, validation method, userOp, etc. These conditions can be specified during install/uninstall in their bytes param, but currently not in the check function. Adding a bytes param to the check function will make it more uniform and flexible.

Examples

An approve+disapprove hook that runs only for transfer() userOps. The conditional hooks might be stored in a mapping indexed by the function selector.

Discussion

There's an argument for the fact that conditionally installed modules should still be considered installed. However, being able to check if a module is installed for any conditions and also for certain conditions will require separate storage usage. (ex. a mapping for each) Additionally, how useful is it knowing that a module is installed for a non-zero set of conditions, but the caller still has no easy way of knowing if it is installed for them?

Should this be moved into a separate function? Will implementations be forced to support the "any conditions" checker?

[kopy-kat]

Thanks for the detailed write up. Imo the point you’re making as a whole makes a lot of sense. As a quick summary from my understanding, here’s the pros and cons:

pros:

• allows accounts to conditionally use modules without needing to store extra data

cons:

• additional param needs to be passed for all checks, although since these will most likely be rpc calls this is fine
• Additional param could lead to confusion for devs that do not know about conditional usage of modules

On a whole I think it would make sense to include this in order to cater to more use cases (eg using function selectors to look up modules). The additional complexity should be explained by account builders (ie with an inline comment saying what the second parameter is and how it should be used for that particular account) and it can be abstracted away by frontend sdks (eg having a function isInstalledModule and isInstalledModuleWithData

[zeroknots]

I see the point.

one thing that is tricky with this, is that the caller of this view function then needs knowledge about how to encode this bytes parameter.

Lets take the case of a Fallback Module that requires a specific Hook to be installed on the account. so it would check during onInstall if the Hook is installed. It would make it quite difficult to encode the relevant data, pass it to the install call of the fallback etc.

(edited) frontends wouldn't really have to rely on this feature anyways, since the install functions emit the module install events.

[yaonam]

We already have those issues during install/uninstall if the account encodes additional data in the bytes param.

Also, did you mean "frontends wouldn't really have..."?

[zeroknots]

yea. sorry. corrected it 👍🏼

[yaonam]

zeroknots/ERC-minimalisticMSA#3
Has been implemented in updated interfaces