-
Notifications
You must be signed in to change notification settings - Fork 859
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
Correctly returning data from tx execution through module #728
Correctly returning data from tx execution through module #728
Conversation
Pull Request Test Coverage Report for Build 7503212266
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Let's also wait for @nlordell and @akshay-ap reviews, as I'd deem this a security-critical change and the more eyes, the better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just left one OCD nit 😛.
The
execTransactionFromModuleReturnData(...)
returned data from guard execution, if a guard was enabled. It was rectified by assigning thereturnData
to the return value from theexecute(...)
and then running the post-tx checks in the guard. As these pre and post-checks through guards were shared between allexecTransactionFromModule*(...)
, internal functions were written for the same.Closes #708