-
Notifications
You must be signed in to change notification settings - Fork 426
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
Support modifiers defined by user #807
Comments
I am not against this feature if we can make sure that we can deliver an implementation for ink! that is actually useful and does not introduce new footguns for users. Still, I would like you to make suggestions about design and we will see how far we will eventually go along this road. |
In Solidity, the user must define a modifier. It means that the user also must do that in case of ink!. We can require him to define the private modifier(function) in impl section. It means that we can call this function We created an example of an independent macro, which will paste Also maybe we need to think about cases when modifier returns a result with error and paste |
But it is an example of an independent modifier and it has low functionality. We can't mark where we want to paste the body of the function(In solidity you can use '_;' construction to indicate that). Suggested solutionWe can require the user to define the modifier in the body of the contract and mark it like It is an example of how ReentrancyGuard can be implemented with this feature. #[ink(modifier)]
fn non_reentrant(&mut self) {
assert_eq!(self._status, _NOT_ENTERED, "ReentrancyGuard: reentrant call");
self._set_status(_ENTERED);
#[ink(body_of_function)];
self._set_status(_NOT_ENTERED);
}
#[ink(message)]
#[ink(modifier(non_reentrant))]
fn swap(&mut self) {
...
} |
We implemented the described modifiers in the library. I hope the comments are clear enough to understand how to use them. It is the implementation of ReentrancyGuard modifier and example of usage. If you are okey with this concept, we can adapt the change for ink! and create a PR. |
On the other hand, a modifier is just a function that asserts certain state, and panics if it's not an expected one. Given that characteristic, why can't we just have an explicit function call? fn non_reentrant(&mut self) {
assert_eq!(self._status, _NOT_ENTERED, "ReentrancyGuard: reentrant call");
self._set_status(_ENTERED);
#[ink(body_of_function)];
self._set_status(_NOT_ENTERED);
}
#[ink(message)]
fn swap(&mut self) {
self.non_reentrant();
...
} In case there's a need for enforcing calling a modifier function with The only problem here is that ink! does not support trait functions carrying their body. I did try implementing the Ownable trait, and that was an issue I faced: I did want certain function (a.k.a. a modifier) to be called before another function, but I couldn't do that, as ink! would not allow it. Here's a code example: What do you think of having this approach (trait-level default methods requiring using some modifier function first)? |
We can do an explicit function call, but the problem is how you will pass the body of your main function inside of the modifier? The first simple suggestion about modifiers in this issue was about adding of explicit call at the beginning of the function. #[ink(message)]
#[ink(modifiers(non_reentrant))]
fn swap(&mut self) {
...
} Expanded into: #[ink(message)]
fn swap(&mut self) {
self.non_reentrant();
...
} But feature to specify the place where we want to call the code of the main function is very important. So the initial approach is not good as a final solution.
We redesigned the modifiers and now they are independed and we will do explicit call of modifiers after code expantion. Also you can pass arguments into modifier.
Yes, ink! doesn't support default implementation inside of traits at the moment. But we hope that they will fix it in a new version of trait definition. We also want to use a default implementation inside of traits and we are tracking it in issue. As a workaround, we added support of default implementation in OpenBrush's macros. But we found some problems with the current implementation and we will re-implement that next week. But usage of our default trait definition will require you to use our version of ink!(and macros), because it is using this feature. |
Hey. Can someone post an update on the issue, please? |
It will be cool to have some macro attribute in
#[ink(message)]
declaration which will automatically paste the code which will do some restriction check like in modifiers in Solidity.We can discuss here how it can be implemented=)
The text was updated successfully, but these errors were encountered: