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

feat: copy callee's event manager's info to caller before callee is destructed #89

Closed

Conversation

loloicci
Copy link
Contributor

@loloicci loloicci commented Feb 17, 2023

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

  • Bug fix (changes which fixes an issue)
  • New feature (changes which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • ETC (build, ci, docs, perf, refactor, style, test)

Checklist

@loloicci loloicci added the dynamic_link relate the dynamic link call feature label Feb 17, 2023
@loloicci loloicci self-assigned this Feb 17, 2023
@loloicci loloicci force-pushed the dynamic-event-manager branch 2 times, most recently from d64bbff to 95c6f43 Compare February 17, 2023 11:14
@loloicci loloicci force-pushed the dynamic-event-manager branch 4 times, most recently from 3656ec5 to f89317c Compare February 20, 2023 09:38
@loloicci loloicci force-pushed the dynamic-event-manager branch 3 times, most recently from 5099e00 to 5904127 Compare March 1, 2023 04:41
@loloicci
Copy link
Contributor Author

loloicci commented Mar 1, 2023

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 cGetContractEnv. And we will solve this error in another PR.

@loloicci loloicci marked this pull request as ready for review March 1, 2023 04:46
@loloicci loloicci requested a review from dudong2 March 2, 2023 06:09
@@ -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() {
Copy link
Member

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?

Copy link
Contributor Author

@loloicci loloicci Mar 6, 2023

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

understood, thank you.

@da1suk8 da1suk8 mentioned this pull request Mar 16, 2023
8 tasks
@loloicci
Copy link
Contributor Author

closes because after Finschia/cosmwasm#273, it is not needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dynamic_link relate the dynamic link call feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants