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: allow current pkg access to bank #402

Closed
wants to merge 1 commit into from

Conversation

giansalex
Copy link
Contributor

Considering pkg:A calls pkg:B

B can't send its own coins

Copy link
Member

@moul moul left a comment

Choose a reason for hiding this comment

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

Can you add an example or a test showing your intention with this PR?

I see pros and cons to make the change.

@moul moul changed the title Allow current pkg access to bank feat: allow current pkg access to bank Nov 29, 2022
@moul moul mentioned this pull request Nov 29, 2022
@zivkovicmilos
Copy link
Member

Hey @giansalex,

Can you please update this branch to the latest master changes?

@moul What are some of the cons you see?

@moul
Copy link
Member

moul commented Jan 24, 2023

My concern is about #473, where we're here changing from one mode to another, and I'm looking for a finale solution that's flexible and with sane defaults.

@moul
Copy link
Member

moul commented Jan 24, 2023

Could someone add unit tests on this PR? one that shows the new usage and one that shows that the previous one does not work anymore, please?

@moul
Copy link
Member

moul commented Mar 31, 2023

Please share your thoughts on PR #683.

@thehowl
Copy link
Member

thehowl commented Jul 21, 2023

This PR is very old and needs changes to even address the issue in its current form (for instance, the same code should be replicated for BankerTypeOrigSend). I'm closing this and proposing to move discussion to #986.

@thehowl thehowl closed this Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants