feat: start updating spec for v0.8 #150
Conversation
|
added some suggestions! |
| The modular smart contract accounts themselves are trusted components. Installed plugins are trusted to varying degrees, as plugins can interact with an arbitrarily large or small set of resources on an account. For example, a wide-reaching malicious plugin could add reverting hooks to native function selectors, bricking the account, or add execution functions that may drain the funds of the account. However, it is also possible to install a plugin with a very narrow domain, and depend on the correctness of the account behavior to enforce its limited access. Users should therefore be careful in what plugins to add to their account. | ||
|
|
||
| Users should perform careful due diligence before installing a plugin and should be mindful of the fact that plugins are potentially dangerous. The plugin's manifest can give users an understanding of the domain of the plugin, i.e., the requested permissions to install certain validation functions and/or hooks on certain execution selectors. Generally, plugins that include native function selectors in their domain, e.g., plugins that add a validation hook to the native `uninstallPlugin()` function, can introduce significantly more harm than plugins that simply add validation hooks to function selectors that the plugin itself is adding to the account. | ||
| The modular smart contract accounts themselves are trusted components. Installed modules are trusted to varying degrees, as modules can interact with an arbitrarily large or small set of resources on an account. For example, a wide-reaching malicious module could add reverting hooks to native function selectors, bricking the account, or add execution functions that may drain the funds of the account. However, it is also possible to install a module with a very narrow domain, and depend on the correctness of the account behavior to enforce its limited access. Users should therefore be careful in what modules to add to their account. |
There was a problem hiding this comment.
Do you think it makes sense to mention that users have the purview to dictate permissions for installed modules at a granular level, separate from the module itself.
What I mean is, you could install a module with a standard set of hooks you use for every module (with varying configurations), so even a malicious module has to go through hooks it can't do anything about. This is a super strong security model-- especially if the wallet service itself appends these hooks separate from the applications users connect to.
There was a problem hiding this comment.
Yes, this is worth mentioning. Do you want to take a stab at writing up this consideration?
There was a problem hiding this comment.
Actually, could we do this on a stacked PR? To be able to close this & work off of develop?
standard/ERCs/erc-6900.md
Outdated
| @@ -13,33 +13,26 @@ requires: 165, 4337 | |||
|
|
|||
| ## Abstract | |||
|
|
|||
| This proposal standardizes smart contract accounts and account modules, which are smart contract interfaces that allow for composable logic within smart contract accounts. This proposal is compliant with [ERC-4337](./eip-4337.md), and takes inspiration from [ERC-2535](./eip-2535.md) when defining interfaces for updating and querying modular function implementations. | |||
| This proposal standardizes smart contract accounts and account modules, which are smart contract interfaces that allow for composable logic within smart contract accounts. This proposal is compliant with [ERC-4337](./eip-4337.md). This standard emphasizes secure permissioning of modules, and maximal interoperability between all spec compliant accounts and modules. | |||
There was a problem hiding this comment.
Nit: spec-compliant
A thought: I wonder if we should change the naming of IAccountLoupe to something else, now that we're getting rid of the reference to 2535.
There was a problem hiding this comment.
We should consider renaming it. Maybe IAccountView?
standard/ERCs/erc-6900.md
Outdated
| - A module **manifest** is responsible for describing the execution functions, validation functions, and hooks that will be configured on the MSCA during installation, as well as the module’s metadata, dependency requirements, and permissions. | ||
| There are two types of hooks: | ||
| - **Validation Hook** functions run before a validation function. These can enforce permissions on actions authorized by a validation function. | ||
| - **Execution Hook** functions run can run before and/or after an execution function. The pre execution hook may optionally return data to be consumed by a post execution hook functions. |
There was a problem hiding this comment.
Nit: consistency in hyphen usage in pre-execution and post-execution. See line 32.
There was a problem hiding this comment.
Shifting to no hyphen for "pre" and "post" modifiers, as that occurs in more places now than the hyphenated version.
| - A **native function** refers to a function implemented natively by the modular account, as opposed to a function added by a module. | ||
| - A **module** is a deployed smart contract that hosts any amount of the above three kinds of modular functions: execution functions, validation functions, or hooks. | ||
| - A module **manifest** is responsible for describing the execution functions, validation functions, and hooks that will be configured on the MSCA during installation, as well as the module’s metadata, dependency requirements, and permissions. | ||
| There are two types of hooks: |
There was a problem hiding this comment.
Do we also need to call out permission hooks here? Or is that what @howydev is working on? Could be confusing to mention "permissions" in line 48.
There was a problem hiding this comment.
I really think we should stop calling them "permission hooks". They're just validation-associated "execution hooks", and we already have well defined terminology for execution hooks.
Even in just the RI, there are many things that are logically "permissions" that aren't enforced via "permission hooks", such as the allowlist module.
standard/ERCs/erc-6900.md
Outdated
| @@ -82,16 +67,17 @@ Each step is modular, supporting different implementations, that allows for open | |||
|
|
|||
| - `IAccount.sol` from [ERC-4337](./eip-4337.md). | |||
| - `IAccountExecute.sol` from [ERC-4337](./eip-4337.md). | |||
| - `IModuleManager.sol` to support installing and uninstalling modules. | |||
| - `IStandardExecutor.sol` to support open-ended execution. **Calls to modules through this SHOULD revert.** | |||
| - `IModularAccount.sol` to support installing and uninstalling modules. | |||
There was a problem hiding this comment.
This now also has accountId. We want this to be required and not optional, which is why that view function is not in IAccountLoupe.
adamegyed
force-pushed
the
adam/spec-update
branch
from
4604f07 to
0fbd371
Compare
Motivation
We want to start work on updating the 6900 spec for v0.8.
Additionally, there is a lot of historical context in the current contents of the spec, including details that are not relevant to the standard itself anymore.
Solution
Remove unneeded historical context from the spec.
Update the spec interfaces for the IStandardExecutor + IModuleManager merge.
Update term definitions to be more concise.