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

Add message_info and deprecate mock_info #2160

Merged
merged 7 commits into from
May 31, 2024
Merged

Conversation

webmaster128
Copy link
Member

@webmaster128 webmaster128 commented May 30, 2024

3 reasons for this:

  1. The function is a constructor that does not mock anything
  2. We have owned Addr instances and don't want to call .as_str() all the time
  3. mock_info is unsafe as it internally uses Addr::unchecked

TODO:

  • Implementation
  • CHANGELOG entry
  • MIGRATING entry for CosmWasm 1 -> 2

Copy link
Collaborator

@chipshort chipshort left a comment

Choose a reason for hiding this comment

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

Overall looks good, just two minor things.

packages/std/src/testing/message_info.rs Outdated Show resolved Hide resolved
packages/std/src/testing/mock.rs Show resolved Hide resolved
@chipshort
Copy link
Collaborator

IMHO, this looks good now, just changelog and migrating entry.
Also, this should get a backport to release/2.0, then I can release it with the other fix.

@webmaster128
Copy link
Member Author

@mergify backport release/2.0

Copy link
Contributor

mergify bot commented May 31, 2024

backport release/2.0

✅ Backports have been created

@webmaster128
Copy link
Member Author

I guess the text is good enough for merge now. We can improve it later when I updates the upgrade PR in the nois-contracts repo.

@webmaster128 webmaster128 merged commit 76479b3 into main May 31, 2024
31 checks passed
@webmaster128 webmaster128 deleted the add-message_info branch May 31, 2024 15:15
chipshort added a commit that referenced this pull request Jun 3, 2024
Add message_info and deprecate mock_info (backport #2160)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants