-
Notifications
You must be signed in to change notification settings - Fork 4
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
[Improvement] Validation and pre-validation merge #8
Comments
Related: #4 |
The following diagrams illustrate the problems and the end state we may want to get to. Current stateThis is the current state, where each execution function (e.g., Merging pre-validation hooks with validation functionsThis is what it may look like if we merge the two components into one. Multiple validation functions are assignable to each execution function, and they are all serially run and the results ANDed to return a final result. The problem to address here is pointed out in Allowing different validation function groups to be calledWith #4, we want to allow multiple validation functions to be applied to a given execution function, and be able to specify which one to run for a given call. For example, one might want to call And with the merging of pre-validation hooks and validation functions, each of those validation functions may also have additional validation functions stacked on top within its "group". How do we install and appropriately group these validation functions? This is the problem that is pointed out in |
Some additional thoughts... Adding in the concept of validation function "groups" definitely increases complexity. Instead of associating "secondary" validation functions (currently known as pre validation hooks) to each "primary" validation function to form these groups, how about associating them to the execution function, so that they are executed no matter which "primary" validation function is used for the call? Something like this: "Secondary" validation functions apply over all "primary" validation functionsThere may be convincing use cases where certain logic is only applicable when routed via certain validation functions, so please comment here if you have concerns with this design. What you might also note is that now we need to distinguish between "primary" and "secondary" validation functions:
Secondary validation functions are looking more and more like what we had defined pre validation hooks to be. Because this design warrants a difference between primary and secondary validation functions anyway, could it be that it may be simpler just to keep pre validation hooks as is? Bringing back pre validation hooks into the design |
Adding one additional benefit of not merging the two interfaces: with the naming distinction between pre-validation and validation, it is more obvious to the reader that they serve different roles. To generalize, pre-validation hooks often tend to apply permissions or restrictions on what may be done with a specific validation function, while the validation function itself provides the actual "authorization" of an action - such as a signature check or caller check ( If these two concepts are merged, then yes we have one fewer interface, but responsibility now shifts onto the user (or developer, library, SDK, etc) to manage these two different categories of validation functions. If, for instance, the permission-enforcing functions are added but not the "authorization" check, then anyone could perform the actions within the permission bounds. The two categories of pre-validation & validation provide some context & connotation behind what they do that is useful. |
+1 to keeping them separate. I was in the opposite camp, but the logical separation between validation and authorization makes conceptual sense to me. |
Some background on why we might want the concept of pre validation hooks in the first place (as opposed to just having pre/post execution hooks): https://ethereum-magicians.org/t/erc-6900-modular-smart-contract-accounts-and-plugins/13885/31
The text was updated successfully, but these errors were encountered: