[Improvement] Replace plugin operation

[adam-alchemy]

• Uninstalling and reinstalling plugins through executeBatch relies on some fragile assumptions for how the account should behave. If we introduce a standardized replacePlugin method, these issues can be avoided. The current blocker has been the handling of dependencies and injected hooks, which add significant overhead to what the replace plugin operation needs to do to prevent inconsistent account state. Addressing #9 could make a replace plugin operation feasible.

[jaypaik]

Related: https://github.com/erc6900/resources/issues/22, which proposes removal of injected hooks.

[dlim-circle]

@jaypaik Curious if we solve https://github.com/erc6900/resources/issues/4, would there still be installation conflicts on plugin?

Trying to see what other use cases would need replacePlugin beyond trying to address installation conflict.

[jaypaik]

@jaypaik Curious if we solve https://github.com/erc6900/resources/issues/4, would there still be installation conflicts on plugin?

Great question! Installation conflicts on validation functions will go away, but execution function conflicts will still result in a revert (unless we choose to address that too, which could be interesting). If what you were alluding to was how #4 by itself solves the problem of swapping ownership plugins, yes #4 will enable that by allowing the new ownership plugin to be installed prior to the removal of the old ownership plugin.

replacePlugin has other uses in being able to "upgrade" or "downgrade" a plugin that has dependents (i.e., other plugins that are installed after the plugin in question that depend on that plugin). Currently, if a plugin has dependents, those dependents must be uninstalled before being able to uninstall that plugin. replacePlugin will enable "hot swapping" the plugin while keeping those dependents intact.

[sm-stack]

replacePlugin will enable "hot swapping" the plugin while keeping those dependents intact.

A following question about this:

If during the hot swapping process, the removal and reinstallation of dependencies occur, and the data of dependents might be deleted due to the onUnInstall function. Is there any plan to create a feature that allows distinguishing between cases where the onUnInstall function is called by replacePlugin and cases where it's called only by uninstallPlugin(no reinstall), in order to decide whether to delete data or not? (e.g., adding isReplacePlugin flag at the first bytes of uninstallData)

[jaypaik]

If during the hot swapping process, the removal and reinstallation of dependencies occur, and the data of dependents might be deleted due to the onUnInstall function. Is there any plan to create a feature that allows distinguishing between cases where the onUnInstall function is called by replacePlugin and cases where it's called only by uninstallPlugin(no reinstall), in order to decide whether to delete data or not? (e.g., adding isReplacePlugin flag at the first bytes of uninstallData)

I need to do some more exploration for this, but ideally replacing a plugin leaves dependents untouched (bypassing the dependency checks within replacePlugin). However, the assumption with this is that the plugin's dependents' initialization does not depend on any changed aspect of the replaced plugin.

It could be worth thinking about something like what you've described, where you can conditionally "reinstall" dependencies to update their data.

[sm-stack]

One point that should be considered is the data that plugin stores in the replacement process. If the replacement happens, the data in the previous plugin will be removed. Therefore, users should be able to migrate some data to the new plugin.

One of the possible solutions is to introduce a new function like onReplacement, which is similar to onInstall function. In this case, user may pass bytes that contain the data to migrate and the instructions of migration. Or reusing the onInstall function can be considered, but it might make plugin developers bit tricky, since they have to consider the two different cases of installation and replacement when they write down their onInstall function.

[jaypaik]

One of the possible solutions is to introduce a new function like onReplacement, which is similar to onInstall function. In this case, user may pass bytes that contain the data to migrate and the instructions of migration.

I think something like this makes sense, though we should think about how the existing data will be supplied to the new plugin. We probably don't want to supply that data as a user defined argument to the replacePlugin function because there could be cases where the user is not allowed to change the plugin data arbitrarily. One idea is a view function that is called on the old plugin during the replacePlugin operation, the result of which is passed to onReplace on the new plugin. But perhaps there are better ways.

Also, while the migration of data makes sense in the context of upgrading or downgrading a plugin across different versions of it, it may not be appropriate when wanting to replace a plugin with a similar plugin from a different author (e.g., attempting to switch ownership plugins for the purposes of updating the validation functions used by its dependents).

Assuming #4 is solved, we should think about how this operation should work and whether replacePlugin should/can be generalized across both of these cases, or whether they should be treated separately.

[sm-stack]

Considering various cases that replacePlugin can work, I think one idea is to utilize version to decide whether to execute the replacement or not. The main reason for this is that replacePlugin should not change the interface of existing functions, which can invoke a serious inconsistency of state if they are related to dependencies.

One possible solution is to bring the idea of semantic versioning 2.0.0, which is a common solution for package managers. So we can divide plugin versions into three sections: Major / Minor / Patch version.

Major one means there are breaking changes like updating the main logics or interfaces of certain functions, so it must not be allowed in replacePlugin. Minor version change means adding new functions, and patch version change includes slight changes like bug fixes or gas optimization, both maintaining compatibility with other versions who have the same major version.

However, there can be an edge case: A malicious replacePlugin operation can pretend to be a patch version change, but actually it can include a serious logic update (which is a major version change). It is difficult to catch this on-chain. Thus, we can introduce a kind of Version Registry that registers only verified plugins and manages their versions. A committee dedicated for this job may be included. This registry will enhance the security and possibly reduce gas overhead by storing multiple sets of plugins that are compatible with each other, but will add extra complexity on the spec and not really a 'decentralized' way, since it will depend on a set of people in charge of verifying plugins' compatibility.

Wonder how you guys think about this, and please share your thoughts on this!

[sm-stack]

I made a pull request addressing this, but it was not enough to handle the full functionalities mentioned in this issue.

I'd like to share the ongoing issues / considerations and our thoughts in this comment:

1. Previous PR on replacePlugin, now closed since some significant changes are needed
2. Notes on implementation details and some considerations on replacePlugin
3. Comments on the replacePlugin PR by @jaypaik

I'd like to hear any thoughts on replacePlugin. Feel free to add comments and continue discussions on this issue!

[jaypaik]

I may spend some time exploring having the account store plugin-related storage instead of the plugin itself, similar to @yoavw 's idea of "per-plugin storage": https://ethereum-magicians.org/t/erc-6900-modular-smart-contract-accounts-and-plugins/13885/23#per-plugin-storage-8.

Reasons:

1. It'd greatly simplify the data migration related to plugin upgrades.
2. It'd also solve the problem of data inconsistency between accounts and plugins if an account is upgraded without the plugins being uninstalled ahead of time (i.e., plugin thinks it is installed on the account while the account does not know about it).