-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: copy callee's event manager's info to caller before callee is destructed #89
Conversation
d64bbff
to
95c6f43
Compare
3656ec5
to
f89317c
Compare
5099e00
to
5904127
Compare
CI fails with a race error and this error is caused even without this PR. A test added with this PR catch the race error of the |
@@ -312,6 +312,19 @@ impl BackendApi for GoApi { | |||
}; | |||
gas_info.cost += callee_instance.create_gas_report().used_internally; | |||
|
|||
// copy callee event manager's info to caller instance | |||
if !callee_instance.env.is_storage_readonly() { |
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.
Why does the callee instance only copy to the caller when it has read-write permission?
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.
This is because events can be issued by instances having write permission.
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.
understood, thank you.
closes because after Finschia/cosmwasm#273, it is not needed |
Description
Add a hook before destructing callee instance to copy callee's event manager's information to caller's. This enables callee contract issues events/attributes via its event manager API's.
This PR solves the 3rd part of Finschia/cosmwasm#265.
Now, this PR is a draft because it uses loloicci's repository, and after Finschia/cosmwasm#269 is merged, it will be replaced.
Types of changes
Checklist