feat: [v0.8-develop, experimental]: sig validation in interface [2/N]

[adamegyed]

Motivation

In preparation of multi-validation support, we need the ability to avoid installing execution functions defined by a validation plugin, otherwise there would be a clash when installing multiple instances of the same validation plugin (see #52).

Currently, "ownership" plugins define isValidSignature as an execution function. However, logically it makes more sense to put this out in the IValidation interface, as identified by this standard improvement proposal: erc6900/resources#20

Solution

Add a validateSignature method to IValidation that performs signature validation.

Add a definition of isValidSignature to UpgradeableModularAccount that switches between installed signature validation plugins.

Temp changes

To get this change working at this point in the change stack, this PR creates a new field within AccountStorage called signatureValidations, and a new field in the manifest declaring these functions. Eventually, it would make sense to merge this with multi-validation support, and have some flag in the install flow and/or plugin manifest to indicate signature validation capabilities.

This PR also adds commented-out stubs for adding hooks to the signature validation flow. Because the entire pre-validation hook installation logic needs a revamp (see #52, #62), it isn't introduced here, and is instead punted for a later PR.

Also, it is not yet immediately obvious to me whether or not a functionId is strictly necessary for this workflow. It appears to be necessary to support composable validation (erc6900/validator-experiments) and for any sort of tiered auth (#41 (comment)), but we may be able to remove this field in the future.

[huaweigu]

lgtm

[huaweigu]

Curious to understand the typical use cases for preHooks in signature validations.

[adamegyed]

I think the simple ones are performing checks like limiting who can validate something - i.e. a session key that is allowed to sign permit messages, but only for USDC and only to a known spender.

[howydev]

Hmm, when the account gets a 1271 call, we only receive a bytes32 hash of all the input parameters, we'll need to get the struct/type + rebuild it to be able to check against the struct parameters

[howydev]

There are 3 main dimensions of association to consider here - the validation plugin, the typehash, and the address of the app

[jaypaik]

Also, it is not yet immediately obvious to me whether or not a functionId is strictly necessary for this workflow.

Slight preference to remove this until we deem it necessary to introduce. It seems that currently for validator-experiments, we're deriving the switching flag from the signature.

We're now expecting accounts (and not plugins) to (optionally) implement ERC-5267, and handle whatever ERC-1271 solution we land on to do initial validation on the signature right?

[jaypaik]

If we're assuming we don't need functionId, perhaps we can check if manifest.validationFunctions.length > 0, and if so, always allow validateSignature calls to that plugin.

[adamegyed]

Yeah, really it should just be part of how validations are installed (a third option alongside user op and runtime validation), but that will have to be part of a later PR addressing user-supplied install configs: erc6900/resources#9

[adamegyed]

We're now expecting accounts (and not plugins) to (optionally) implement ERC-5267, and handle whatever ERC-1271 solution we land on to do initial validation on the signature right?

I'm not sure - won't the interpretation of "what is a signature" and the logic of "how to generate a signature" still be plugin-dependent? I.e. if an account has multiple validation plugins installed on it, then each one may have a different 712 wrapper (or not use 712 altogether), so the 5267 domain retrieval function should be a part of the plugin still?

[adamegyed]

Slight preference to remove [function id]

I think one short-term benefit we get from keeping it, is that all of the functions in IValidation can be identified with one FunctionReference - combination of address and function id. If we remove it here, then 1 function gets a different calling scheme.

[jaypaik]

I'm not sure - won't the interpretation of "what is a signature" and the logic of "how to generate a signature" still be plugin-dependent? I.e. if an account has multiple validation plugins installed on it, then each one may have a different 712 wrapper (or not use 712 altogether), so the 5267 domain retrieval function should be a part of the plugin still?

Hmm in that case we'd need similar treatment for 5267, unless we expect callers to execute into those plugins instead. But I'd prefer the 712 domain be isolated to the account if possible. Similar to how in the alpha version of light-account 2.0, we had the base account do the 1271 parsing and "trimming", and let the different account implementations handle the raw signature checking. Makes plugins easier to write, and we can probably detect 712 vs. 191. I could be missing some valid use cases where plugins might want to use their own domain though.

I think one short-term benefit we get from keeping it, is that all of the functions in IValidation can be identified with one FunctionReference - combination of address and function id. If we remove it here, then 1 function gets a different calling scheme.

Yeah that's valid, let's keep it in.

[howydev]

Since IValidation.validateSignature returns MAGIC_VALUE or INVALID, we could just do return (IValidation(plugin).validateSignature(functionId, msg.sender, hash, signature[21:]) right?