Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove RestrictedMethod permission type #4238

Open
rekmarks opened this issue May 1, 2024 · 0 comments
Open

Remove RestrictedMethod permission type #4238

rekmarks opened this issue May 1, 2024 · 0 comments
Labels
PermissionController Related to the PermissionController.

Comments

@rekmarks
Copy link
Member

rekmarks commented May 1, 2024

Requires: #4239

As elaborated in the permission controller's architecture documentation, its original purpose was to control access to RPC methods. To this end, we enshrined this responsibility within the current implementation of the permission controller in the form of the RestrictedMethod permission type. In the almost 2.5 years since we began using this implementation, we (special credit to @Gudahtt) have determined that the RestrictedMethod permission is misguided, and should be removed.

First, the RestrictedMethod permission type adds significant complexity to the implementation of the permission controller. /restrictedmethod/i is currently mentioned 46 times in the permission controller, and methods like executeRestrictedMethod and imports like decorateWithCaveats exist solely to support this permission type.

Second, and more importantly, although the RestrictedMethod permission type is supposed to encapsulate the enforcement of RPC method permissions within the permission controller, this goal has never been, and can never be, accomplished. For one thing, RPC method implementations still need to ask the permission controller if an action is permitted or not. Yet, the controller cannot and should not assume responsibility of the entire RPC pipeline. For another thing, we have been making ad hoc mutations of permissions since the days of rpc-cap, namely by editing the eth_accounts array according to our needs.

In other words, the RestrictedMethod permission type exists to uphold a lie at the cost of convoluting one of our most security-critical abstractions. The permission controller should have a single responsibility: at all times maintaining a valid permissions state. To that end, we should remove the RestrictedMethod permission type, replacing it entirely with the use-case agnostic Endowment permission type, and ultimately get rid of the notion of "permission types" entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PermissionController Related to the PermissionController.
Projects
None yet
Development

No branches or pull requests

1 participant