feat: add interface checks for validations and hooks

[adamegyed]

Motivation

We've been discussing how to do sanity-checking on addresses provided to be used as modules, and decided to encourage, but not enforce, ERC-165 interface checking in the standard.

Solution

This PR adds behavior to the reference implementation to check interface support for the addresses provided to be used as validation modules, validation hooks, and execution hooks. The account already performed this check for execution modules.

Also update any modules used in tests that were missing the appropriate interfaceId support.

This PR does not yet update the spec document - will follow up with a second PR for the spec update.

[adamegyed]

Flagging one potential downside of this setup: it will no longer be possible to add an EOA as a validation, with "direct call validation" access, due to it not implementing the ERC-165 interface for IValidationModule. Is that sufficient for us to reconsider this check?

[howydev]

Hmm, there's a discrepancy in the installValidation and installExecution flow, for the first we don't wrap the onInstall error but for the second we do. Would love if we could wrap the error in both cases so they could both use the new _onInstall, is there a reason why we wrap 1 of them but not the other?

[howydev]

Flagging one potential downside of this setup: it will no longer be possible to add an EOA as a validation, with "direct call validation" access, due to it not implementing the ERC-165 interface for IValidationModule. Is that sufficient for us to reconsider this check?

Thats interesting, can u elaborate on the "using EOA as a validation" case? Is that equivalent to a "pass through" validator that allows all calls?

[adamegyed]

Hmm, there's a discrepancy in the installValidation and installExecution flow, for the first we don't wrap the onInstall error but for the second we do. Would love if we could wrap the error in both cases so they could both use the new _onInstall, is there a reason why we wrap 1 of them but not the other?

I don't think so, we should probably do this wrapping. Will update unless anyone else has a reason to keep separate.

Thats interesting, can u elaborate on the "using EOA as a validation" case? Is that equivalent to a "pass through" validator that allows all calls?

Basically, since #90, you can install a validation with the magic value 0xffffffff for entityId, and it will not only be able to validate calls using validateUserOp/validateRuntime + singatures using validateSignature, but it will be able to do a new thing: directly call the account from that address, and this counts for validation.

This was intended to be used by modules that needed call access to the account, and it was more efficient to do this than to have the modules call executeWithAuthorization(...) and encode themselves as the validation module to use.

But, this workflow could be extended to allowing EOAs to directly call functions on the account, without using executeWithAuthorization. It does not pass through all calls, and any validation other than direct call validation will fail because the EOA has no code.

[howydev]

I see, thanks for explaining! Agree that its a workflow that we should preserve

[Zer0dot]

Wdyt if we check the interface regardless of data length?

[adamegyed]

I thought of this because the usual case for not running onInstall is if it's been already initialized, which usually only happens on re-install (i.e. if you're using the same hook id across two different validations), in which case the previous install with a call to onInstall would have checked this. Probably should either include that rationale in a comment, or update this to always check. Wdyt?

[Zer0dot]

I'm not that partial to one way or another, but I think the assumption we'd always call onInstall the first time we install may not necessarily be a safe one. I could imagine modules which inherently don't need to be directly initialized probs.

For that reason always checking makes a bit more sense to me, but it could end up being redundant. I leave this one up to your call!

[adamegyed]

One neat side effect of this, is that this actually lets us still do the "install EOA for direct call validation" - just added a test for this here: b858ccb

Given that one use case, I'll keep the interface check behind the initialization data for now.

[Zer0dot]

Only some small comments, lgtm otherwise!